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]>
288 lines
16 KiB
Markdown
288 lines
16 KiB
Markdown
# 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?
|