fix(board): refuse foreign Host and Origin on every control-board route (#1507)

After a DNS rebind, a web page could read /api/board and POST /api/reply, which pastes into a live seat pane. foreignRequest() now runs first and returns 403 for a non-loopback Host, a wrong port, userinfo or a path in Host, or any Origin other than http://<Host>. A missing Origin still passes, which covers the WebUI proxy. Dewey authored it; Rocko approved de9ff942 (review 5a12f08e) with one low wording finding, now fixed in the notes. Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-09-26 15:40:34 -05:00
parent 401cc850bb
commit d1629d610d
8 changed files with 461 additions and 6 deletions
@@ -0,0 +1,73 @@
+# Control-board Host/Origin guard review
+
+Verdict: **approve**. Rocko for Sage and Dewey, 2026-09-26.
+
+Reviewed against base `ef0020ad`. Verified candidate pins:
+
+- NOTES.md: `480c4577d27612b7b394c26dbffcdd5953424d522cbcb148b965a91436c05489`
+- candidate.patch: `de9ff9421eb388e4db71f04de0429dcf59d0d3b07bf03e7d0be5a4f875c3c87e`
+- packages/control-board/src/serve.mjs: `d0a9bbed4c427690ded16432a1db40829a3c2e3362160aded1ba603d218b7f94`
+- packages/control-board/tests/serve.test.mjs: `e01d7bd9e51fe48c3baf07b21e4ea27ecdc537c9422645f4376a327f52292f2c`
+
+No blocking findings. The guard runs before routing, scanning, JSON body
+handling and pane delivery. Foreign DNS names cannot acquire access merely
+by resolving to loopback. Missing Origin is an intentional allowance for
+nonbrowser clients, not authentication; local clients remain trusted.
+
+## 1. Low, nonblocking — packet overstates raw Host syntax rejection
+
+`new URL()` normalizes input before `plain` is tested. Direct HTTP requests
+with Host `127.0.0.1:PORT/`, `@127.0.0.1:PORT`, or
+`127.0.0.1:PORT?` pass without Origin. A nonempty `/x` path and nonempty
+credentials are refused. Empty userinfo and query delimiters disappear,
+and the explicit root slash is indistinguishable from an implicit one.
+
+These remain parsed loopback authorities on the listener's port. I found
+no foreign browser origin bypass from this; it does not block the security
+fix. Suggested fix: describe the check as validating the parsed authority,
+or reject raw userinfo/path/query/fragment delimiters before parsing if
+strict Host syntax is intended. Add boundary cases when tightening it.
+
+## Requested boundaries
+
+Executed raw HTTP requests against an ephemeral candidate board server's
+health route, without scanning or contacting live panes:
+
+| Input | Result |
+|---|---|
+| `[::1]:PORT`, expanded `[0:0:0:0:0:0:0:1]:PORT` | 200 |
+| `LOCALHOST:PORT`, Origin absent | 200 |
+| `LOCALHOST:PORT`, lowercase `http://localhost:PORT` Origin | 403 |
+| `localhost.:PORT` | 403 |
+| `127.0.0.1.:PORT` | 200, numeric URL canonicalization |
+| `[::ffff:127.0.0.1]:PORT` | 403 |
+| Empty or `null` Origin | 403 |
+| Foreign Host, nonempty Host path | 403 |
+
+Exact raw Origin comparison is deliberately stricter than semantic URL
+comparison: mixed-case raw Host plus canonical lowercase Origin is refused.
+That fails closed; ordinary browser URLs are canonicalized. Trailing-dot
+localhost is unsupported and fails closed. Expanded IPv6 works without
+Origin or with its exact matching Origin; the candidate suite also covers
+bracketed `::1` with matching Origin. These are Host-header tests over IPv4,
+not a claim that this host's IPv6 listener was exercised.
+
+The WebUI source creates its own fetch headers and does not forward the
+incoming Host or Origin. An executed WebUI-to-capture-server request with a
+`http://localhost:PORT` board URL produced `Host: 127.0.0.1:PORT`, no Origin,
+and matching socket.localPort. Its port check therefore works behind the
+actual proxy. The existing real-board proxy fixture also passed.
+
+Refused bodies are precisely `{error: "non-local Host refused"}` or
+`{error: "cross-origin request refused"}` (with JSON formatting/newline).
+No board data or attacker-controlled header is interpolated. No CORS
+allow-origin header is sent. Candidate tests assert both board/reply
+refusals leave the scan index and agent-send capture absent.
+
+## Verification
+
+`timeout 40s node --test --test-concurrency=1 packages/control-board/tests/serve.test.mjs packages/webui/tests/serve.test.mjs`
+
+Result: 39 tests passed, zero failures, exit 0. Additional in-memory HTTP
+boundary checks and proxy capture above passed. This report is the only
+repository write for the review. No commit, live restart or live reply.