Enforce allowed host and WebSocket origin checks on server ingress#1169
Enforce allowed host and WebSocket origin checks on server ingress#1169juliusmarminge wants to merge 2 commits intomainfrom
Conversation
- Add `--allowed-hosts` / `T3CODE_ALLOWED_HOSTS` config parsing and validation - Reject HTTP and WebSocket requests from disallowed hosts with 403 - Enforce production WebSocket origin matching (including proxy headers), with dev-url bypass - Expand CLI and server tests for host filtering and origin verification
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
github-actions
bot
added
size:L
- validate allowed hosts with `Schema.Trim` + `Schema.check` - normalize hosts via lowercase decode step - simplify CSV decode/encode by removing Effect-wrapped transforms
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| const invalidAllowedHostIssue = (input: string) => | ||
| new SchemaIssue.InvalidValue(Option.some(input), { | ||
| message: `Invalid host "${input}". Expected bare host[:port], for example "app.example" or "app.example:443".`, | ||
| }); | ||
|
|
||
| const AllowedHost = Schema.Trim.pipe( | ||
| Schema.check( | ||
| Schema.makeFilter( | ||
| (input) => { | ||
| if (input.length === 0 || input.includes("://")) { | ||
| return invalidAllowedHostIssue(input); | ||
| } | ||
|
|
||
| const candidateUrl = `http://${input}`; | ||
| if (!URL.canParse(candidateUrl)) { | ||
| return invalidAllowedHostIssue(input); | ||
| } | ||
|
|
||
| const parsed = new URL(candidateUrl); | ||
| return parsed.username || | ||
| parsed.password || | ||
| parsed.pathname !== "/" || | ||
| parsed.search || | ||
| parsed.hash | ||
| ? invalidAllowedHostIssue(input) | ||
| : true; | ||
| }, | ||
| { identifier: "AllowedHost" }, | ||
| ), |
There was a problem hiding this comment.
🟢 Low src/main.ts:47
The AllowedHost validation accepts port-only inputs like :8080. When parsed as new URL("http://:8080"), parsed.hostname is empty ("") but parsed.host is ":8080". The code returns parsed.host.toLowerCase() without checking that parsed.hostname is non-empty, so :8080 passes validation as a valid host.
const parsed = new URL(candidateUrl);
if (
parsed.username ||
parsed.password ||
+ parsed.hostname === "" ||
parsed.pathname !== "/" ||
parsed.search ||
parsed.hash
) {🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/main.ts around lines 47-75:
The `AllowedHost` validation accepts port-only inputs like `:8080`. When parsed as `new URL("http://:8080")`, `parsed.hostname` is empty (`""`) but `parsed.host` is `":8080"`. The code returns `parsed.host.toLowerCase()` without checking that `parsed.hostname` is non-empty, so `:8080` passes validation as a valid host.
Evidence trail:
- apps/server/src/main.ts lines 47-76 (at REVIEWED_COMMIT): Shows AllowedHost validation logic that checks username, password, pathname, search, hash but NOT hostname
- Node.js URL documentation (https://nodejs.org/api/url.html): Confirms that `new URL("http://:8080")` has `hostname = ""` and `host = ":8080"`, and `URL.canParse("http://:8080")` returns true
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Trailing slash passes AllowedHost validation but never matches
- Changed the AllowedHost schema transformation to store
new URL("http://" + input).hostinstead of the raw lowercased input, which strips trailing slashes and ensures consistency withnormalizeRequestHost.
- Changed the AllowedHost schema transformation to store
- ✅ Resolved by another fix: Default port stripped at lookup but not at storage
- The same fix that stores
parsed.hostinstead of the raw input also strips default ports (e.g.:80), since the WHATWG URL parser's.hostproperty omits default ports for the scheme.
- The same fix that stores
Or push these changes by commenting:
@cursor push ec7629b2b1
Preview (ec7629b2b1)
diff --git a/apps/server/src/main.ts b/apps/server/src/main.ts
--- a/apps/server/src/main.ts
+++ b/apps/server/src/main.ts
@@ -74,7 +74,13 @@
{ identifier: "AllowedHost" },
),
),
- Schema.decodeTo(Schema.String, SchemaTransformation.toLowerCase()),
+ Schema.decodeTo(
+ Schema.String,
+ SchemaTransformation.transform({
+ decode: (input) => new URL(`http://${input}`).host,
+ encode: (input) => input,
+ }),
+ ),
);
const AllowedHostsCsv = Schema.String.pipe(| ), | ||
| ), | ||
| Schema.decodeTo(Schema.String, SchemaTransformation.toLowerCase()), | ||
| ); |
There was a problem hiding this comment.
Trailing slash passes AllowedHost validation but never matches
Low Severity
The AllowedHost schema stores the lowercased original input string, so an entry like "app.example/" passes validation (because parsed.pathname === "/" satisfies the !== "/" guard) but is stored as "app.example/" with a trailing slash. Meanwhile, normalizeRequestHost returns parsed.host which is "app.example" without a slash. The allowedHostSet.has(requestHost) check therefore never matches, silently creating an allowlist entry that blocks all requests rather than permitting the intended host.
Additional Locations (1)
| ), | ||
| ), | ||
| Schema.decodeTo(Schema.String, SchemaTransformation.toLowerCase()), | ||
| ); |
There was a problem hiding this comment.
Default port stripped at lookup but not at storage
Low Severity
AllowedHost stores the lowercased original input string via SchemaTransformation.toLowerCase(), but normalizeRequestHost extracts parsed.host from a http:// URL, which strips the default HTTP port (80) per the WHATWG URL spec. This means an entry like "app.example:80" passes validation and is stored as "app.example:80", but incoming requests with Host: app.example:80 produce "app.example" (no port) from normalizeRequestHost, so allowedHostSet.has(requestHost) always returns false — silently blocking the host rather than allowing it.
Additional Locations (1)
|
Adding, at edit: or a |
|
the remote mode will be more of a first class citizen soon and we'll make sure things are super solid when we get there for sure! |
2 participants
Summary
allowedHoststo server config and wire it through CLI/env (--allowed-hosts,T3CODE_ALLOWED_HOSTS)host[:port]values (deduped, lowercased)403 ForbiddenTesting
apps/server/src/main.test.ts: verifies CLI + env parsing for allowed hosts, CLI precedence over env, and invalid--allowed-hostsrejection pathapps/server/src/wsServer.test.ts: verifies HTTP403 Forbidden hostfor disallowed hosts, HTTP allow path for allowed hosts, and WebSocket allow/deny cases for missing origin, mismatched origin, forwarded proxy headers, allowed hosts, and dev-url bypassbun fmtbun lintbun typecheckbun run testNote
Medium Risk
Introduces new ingress validation that can reject previously accepted HTTP/WebSocket traffic (host allowlist and required Origin matching), which may break deployments/clients if headers or config are mis-set.
Overview
Adds an ingress allowlist and stricter WebSocket verification. The server config now includes
allowedHosts, configurable via--allowed-hostsandT3CODE_ALLOWED_HOSTS, with validation/normalization (barehost[:port], lowercased, deduped) and CLI taking precedence over env.HTTP requests and WebSocket upgrades are now blocked with
403whenHost/X-Forwarded-Hostis not in the allowlist (empty list still allows all). In non-dev mode, WebSocket upgrades additionally require a matchingOrigin(includingX-Forwarded-Protohandling), with improved upgrade rejection status text, and expanded tests covering parsing, precedence, invalid inputs, and allow/deny scenarios.Written by Cursor Bugbot for commit 11091bf. This will update automatically on new commits. Configure here.
Note
Enforce allowed host and WebSocket origin checks on server ingress
--allowed-hostsCLI flag andT3CODE_ALLOWED_HOSTSenv var accepting comma-separatedhost[:port]values; invalid entries (with scheme, path, etc.) cause a parse failure.HostorX-Forwarded-Hostheader does not match the configured allowlist (when non-empty).Originheaders; origin checks are skipped whendevUrlis configured.X-Forwarded-Host,X-Forwarded-Proto) are honored during host and origin validation.allowedHostslist must ensure forwarded headers are set correctly, or all requests will be rejected.📊 Macroscope summarized 49dda48. 4 files reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues