Files
stack/agents/dewey/work/queue-74/build.md
T
jason.woltjeandClaude Opus 5.5 fce04c1556 review(dewey): row 74 round 2 packet, repeated-Host 400 and proxy-Host 403 (#1568)
Round 2 per Sage's notes revs 521 and 523: a repeated Host is 400 before
any check; HEADER with a Host other than NAME is 403 proxy Host refused;
HEAD, x.NAME and --auth-header tests; the README proxy contract lines.
Candidate manifest 17b6d7f6, base 17ea0dbc. 34 of 34 mutants killed;
gate green.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-10-11 18:23:26 -05:00

16 KiB

Row 74 (#1568): WebUI preview behind a sign-in proxy, round 2

Owner: Dewey. Reviewers: Darkwing and Filbert (Filbert re-reviews the delta). Brief: docs/plans/2026-10-11_webui-preview.md, blob 1fe76f32 (rev 487; the change from 59fd510f is the out-of-scope line saying Jason starts the preview, and nothing in the code depends on it). Round 2 scope: Sage's queue notes rev 521 (Filbert's findings, comment 27433) and rev 523 (Darkwing's findings). Base 17ea0dbc. None of the four files changed between bc8e1ee7 (round 1's base) and 17ea0dbc, or between 17ea0dbc and refactor HEAD when the packet was built.

build.patch is git diff --binary 17ea0dbc -- packages/webui, the whole row, not a delta on round 1. Apply it with git apply --index to a detached worktree of the base; candidate-manifest.sha256 then checks 4/4 OK. cli.mjs is unchanged from round 1 (same digest).

Round 2 changes

Round 1 was the packet in ea07e19c, candidate 67515db5…76a0, request comment 27428. Filbert approved it (27433); Darkwing sent findings instead of a verdict. The delta, all in serve.mjs, the README and the tests:

  1. A repeated Host is 400 repeated Host refused, before any other check, in preview mode and without it. Node keeps the first of repeated Host headers, so Filbert's Host: 127.0.0.1:PORT then Host: NAME with no auth header landed on the loopback path and got 200 with inbox rows. No edge proxy sends two, and nothing else in the Console has a use for them, so the check is not limited to preview mode. Test: Filbert's case exactly, the reverse order, the same Host twice, each with and without HEADER, with and without the flags.
  2. HEADER with any Host but NAME is 403 proxy Host refused, with message: 'the proxy must forward Host unchanged'. nginx's default proxy_set_header Host $proxy_host sends the upstream's Host, which is a loopback Host on the Console's port, so the request skipped the user check. Darkwing's case, Host: 127.0.0.1:PORT with a wrong user, got 200 on /api/config. The operator's browser never sends HEADER, so it still gets through. Test: 127.0.0.1, localhost and [::1] on the port, with a wrong user, the right one, the header in mixed case and an empty value, across a bus read, /api/config, HEAD and a POST; nothing reaches the bus or board. This check runs before the loopback check, so a wrong Host carrying HEADER now gets proxy Host refused rather than non-local Host refused; both are 403.
  3. Tests only (the code already did these): HEAD unsigned is 401 and signed is 200; HEAD is refused wherever GET is refused for Host and Origin; x.NAME and xNAME are refused; --auth-header refuses remote_user, _ and -remote-user. The comma-joined value Sage skipped as equivalent is not added. send() now leaves an empty HEAD body unparsed, and a raw-socket helper sends the two Host lines node:http won't.
  4. README: the general security line names the repeated-Host 400; the preview bullets name the loopback forms, the proxy-Host 403 and the repeated Host; the proxy contract says Host is forwarded unchanged, names nginx's proxy_set_header Host $host; (Caddy and Traefik keep it by default), says any loopback Host on the port is the operator path, localhost included, so a tunnel whose local end uses the same port number reaches it, and binds every hop on this host that reaches the loopback port, not only the edge.
  5. Kept: the NAME:443 refusal (both reviewers). The X-Authentik-Uid mixed-case test stays (Sage, rev 523).

Not in round 2, by Sage's scope: Filbert's N4 (--allow-user has no leading-character check) and the wrong "Delivery may be unknown" text on a 403 Reply, which row 76 fixes before the send route opens.

What ships

serve --public-host NAME --auth-header HEADER --allow-user ID, all three or none. Off by default, and without the flags the Console behaves as before (serve.test.mjs unchanged and green, plus a test that NAME is refused and a POST still reaches the board).

  • previewOptions (serve.mjs) validates and lowercases NAME and HEADER. NAME is a DNS name with at least one dot: no scheme, port, path or IP literal, and not localhost. HEADER is a header name, 1 to 64 of [a-z0-9-], not host or origin. ID is 1 to 256 visible ASCII characters without a comma. A partial set throws. startServer revalidates its preview option the same way, so a direct caller can't skip the check, and preview: {} refuses instead of serving without it.
  • The CLI collects the three flags and calls previewOptions before any config is read, so a partial or bad set exits 2 with refused: … and prints nothing on stdout. With all three it prints a Preview: line.
  • A request is a NAME request when its raw Host, lowercased, equals NAME exactly. Anything else goes through the existing loopback Host check unchanged, so NAME:443 and NAME.evil.example get the existing 403 (round 2: with HEADER they get proxy Host refused, also 403).
  • The Origin, when sent, must be https://NAME for a NAME request and the existing http://<Host> for a loopback one.
  • A NAME request must carry HEADER exactly once (req.headersDistinct), with a value equal to ID. Otherwise 401 {error: 'sign-in required'}, before routing, so no bus read or board call is made. Node lowercases header names and trims surrounding whitespace from values; the compare is otherwise exact.
  • In preview mode every method other than GET and HEAD answers 403 {error: 'read-only', message: 'the Console preview is read-only'}, whatever the Host, after the sign-in check. /api/seen and /api/reply never reach the board.
  • Loopback requests need no header and read as before. The GET routes are unchanged.

Order of checks for a NAME request: Host, Origin, header (401), method (403). Round 2 puts two checks ahead of all of these, for every request: a repeated Host (400), then HEADER with a Host other than NAME (403). An unsigned write gets 401, not 403, so nothing about the routes is answered before sign-in.

Decisions and tradeoffs

  1. Every non-GET/HEAD method gets 403, not only POST. The brief says "every POST". The Console's only write routes are POST today, and every other non-GET/HEAD method already gets 404 or 405 on the base, so the superset changes no allowed request. It means a write route added later is refused in preview mode until someone opens it on purpose (row 76 does, under --send).
  2. A port on NAME is refused. The README tells the proxy to preserve Host: NAME. A proxy that sends NAME:443 fails closed with a 403 rather than being matched loosely. If ops-01's proxy adds the port, the fix is in the proxy config, not here.
  3. Repeated header is 401. Two values could come from a client copy the proxy failed to strip plus the proxy's own. Refusing is the safe reading. A comma is refused in ID for the same reason: a proxy that joins values with a comma would make ID,ID look like one value.
  4. HEAD is allowed with GET. It reads nothing GET doesn't.
  5. Provider-neutral. No identity provider is named in the code or the README; the README example uses X-Example-Uid. The tests use remote-user/jason and, for the first deployment's shape as the brief asks, X-authentik-uid with a 64-hex stand-in id, so a provider's header name does appear in the tests and the demo. (Round 1 said "code, tests or README", which overstated it; Filbert's N3.)

Where the brief and the code differ (for Sage)

  • The superset in decision 1.
  • With row 60 landed, /api/bus/credentials is also readable through the preview by the signed-in user: credential states and dates, no tokens. /api/bus/decisions is readable today. Both are GET routes that "stay as they are"; I mention them because the brief lists inbox, tasks, agents and trails as the minimum.
  • Overlap with row 60: both change packages/webui/src/serve.mjs (row 60 only the BUS_VERBS line) and packages/webui/README.md. The two patches apply on top of each other in either order without conflict, but whichever lands second has different digests for those files, so its manifest needs a new round.
  • A refused Reply in the page shows read-only Delivery may be unknown; check the seat before sending again. The second sentence is wrong for a 403. It comes from src/public/app.js, which isn't in this row's files, and row 76 changes that path.
  • The brief's line references (serve.mjs 82, 92, 96) match the base.

Files

File Change
packages/webui/src/serve.mjs previewOptions, the preview option, the repeated-Host, proxy-Host, NAME, Origin, header and read-only checks
packages/webui/src/cli.mjs the three flags, checked before config; the Preview: line
packages/webui/README.md the preview section, the proxy contract, the local exposure; two lines of the old "do not expose" text replaced; the repeated Host in the general security line
packages/webui/tests/preview.test.mjs new, 11 tests

The README's bus-reads exposure paragraph is untouched, to keep the overlap with row 60 small.

Tests (preview.test.mjs)

  1. Options are all three or none, and each is checked (hosts, headers, users; startServer with a partial set or {}). Round 2 adds remote_user, _ and -remote-user to the refused headers.
  2. The CLI refuses a partial or invalid flag set at start: exit 2, refused:, empty stdout.
  3. The CLI with all three flags serves preview mode: the Preview: line, 401 unsigned, 200 signed in, 403 for a loopback POST.
  4. 401 with nothing read: no header, another user, a suffix, a different header, two values, across a bus read, a trail, the board and /, and a HEAD for each case (round 2).
  5. Header name case and exact value: X-authentik-uid on the flag with a 64-hex id; lower, mixed and upper names pass; an uppercased value, a truncated value, a suffix, or ID,ID gets 401.
  6. Signed-in reads of inbox, tasks, agents, a trail, the board and /; NAME in mixed case; a signed HEAD of / is 200. Wrong Host, NAME:443, NAME.evil.example, x.NAME and xNAME get 403 for GET and HEAD: non-local Host refused without HEADER, proxy Host refused with it. Wrong Origins get 403 for GET and HEAD.
  7. Every write gets 403 read-only from NAME and from loopback, with and without Origin, for both write routes, a bus route and /; PUT, DELETE, PATCH and OPTIONS too; an unsigned write gets 401; nothing reaches the board or bus.
  8. Loopback is unchanged in preview mode, and with the board down its route answers the existing 502 while the bus views answer.
  9. A proxy that rewrites Host to the loopback port gets 403 proxy Host refused (round 2, Darkwing's case and its variants, listed above).
  10. A repeated Host is 400 before any other check, with and without the flags (round 2, Filbert's case and its variants).
  11. Without the flags NAME is refused and a POST still reaches the board.

Without the round 2 serve.mjs change, tests 6, 9 and 10 fail and the other eight pass.

Mutants

Round 2: 34 single-site mutants in a separate worktree (17ea0dbc plus build.patch), run against preview.test.mjs and serve.test.mjs (mutants.py, output in mutants.txt). 34 of 34 killed; the worktree was clean after the run. They are round 1's 22 (one site pinned more narrowly, since the header lookup now appears twice) and 12 new:

Mutant Edit
repeated-host-allowed two Hosts pass; only three are refused
repeated-host-preview-only the repeated-Host check only with the flags
proxy-host-skipped the proxy-Host check removed
proxy-host-wrong-user-only refused only when the value isn't ID
proxy-host-nonempty-only an empty HEADER value passes
proxy-host-get-only the proxy-Host check only for GET
proxy-host-loopback-only the check only for a loopback-looking Host
proxy-host-raw-name the check looks HEADER up by the raw flag
header-check-skips-head Darkwing's: HEAD skips the sign-in check
head-refused Darkwing's: HEAD gets 403 read-only
header-leading-dash --auth-header accepts a leading -
header-underscore --auth-header accepts _

The last four edit code round 1 already had. Against round 1's tests all four survive (mutants-round1-tests.txt, mutants.py pointed at a worktree of bc8e1ee7 with round 1's patch); round 2's tests kill them.

Round 1's 22, for reference:

Partial set allowed; NAME not lowercased; HEADER not lowercased; IP allowed; host allowed as HEADER; comma allowed in ID; preview: {} served; NAME compared case-sensitively; NAME as a prefix; NAME with any port; http://NAME Origin allowed; https://NAME Origin allowed on loopback; header check skipped; any of several values accepted; value as a prefix; value compared without case; header looked up by the raw flag; read-only only for NAME; read-only only for POST; read-only without the flags; CLI drops the option; CLI never validates.

The first run found two faults in the tests, not the code, both fixed before the gate. setup() registered the fake board's cleanup after startServer, so a mutant that made startServer throw left the board open and the run hung instead of failing. And test 1 left open any server a mutant let start. Both now close what they open, and the mutants that had hung fail in about a second. Test 3 was added because no test started the CLI with all three flags, so a CLI that dropped the option would have passed.

Demonstration

Round 2 adds four refusals to demo.sh: a wrong Host without and with HEADER, a Host rewritten to 127.0.0.1:PORT with a wrong user and to localhost:PORT with the right one, and the two Hosts (over a raw socket, since curl sends one Host however many are given). demo.txt was run on the build worktree, whose four files match the manifest.

demo.sh starts the CLI in preview mode on loopback port 0 with no system config and a closed board port, then drives it with curl, showing each refusal and the allowed reads. demo.txt is its output, run on the gate worktree, with the server's start lines. Nothing leaves the host. With no bus configured the inbox read answers 503 not-configured, which shows it got past the sign-in check; tests 6 and 8 show the bus reads themselves.

Gate (my run on the candidate)

Round 2: all suites ran one after another in a detached worktree of 17ea0dbc with build.patch applied, TMPDIR in my scratch and Docker on, from 2026-10-11T23:15:06Z to 23:22:54Z. The manifest checked 4/4 OK before the run, and core.hooksPath stayed unset. No flake, no rerun.

Suite Result
packages/webui 61 pass, 0 fail (11 in preview.test.mjs), the browser tests included
packages/control-board 124 pass, 0 fail
packages/cli 112 pass, 0 fail
bus, business, conversation, discord, harness, ledger, mosaic, queue, runs, seat, tasks all 0 fail (80, 60, 182, 178, 57, 78, 69, 150, 41, 27, 71 pass)
build-tokens --check exit 0
test-auth, test-conductor, test-config, test-discord, test-extension-package, test-foundation, test-queue exit 0 (15, 17, 24, 66, 18, 44, 27 passed)
test-release 14 passed, 0 failed (Docker, nothing skipped)
test-task 98 passed, 0 failed (Docker, nothing skipped)

Round 1's gate ran on bc8e1ee7 from 22:26:06Z to 22:35:49Z, also all green (webui 49 then).

Questions for reviewers

Round 1's two are answered: keep refusing NAME:443, and the contract lines are added. One for round 2:

  1. The repeated-Host 400 applies without the flags too. Without them only the first Host is ever read, and it must pass the loopback check by itself, so the 400 closes nothing there. I kept it unconditional because a repeated Host has no legitimate sender and one rule is simpler to state. Would you rather it be preview-only?