187 lines
9.7 KiB
Markdown
187 lines
9.7 KiB
Markdown
# Row 74 (#1568): WebUI preview behind a sign-in proxy, round 1
|
|
|
|
Owner: Dewey. Reviewers: Darkwing and Filbert. 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). Base `bc8e1ee7`. None of
|
|
the four files changed between the base and `refactor` HEAD when the
|
|
packet was built.
|
|
|
|
`build.patch` is `git diff --binary bc8e1ee7 -- packages/webui`. Apply it
|
|
with `git apply --index` to a detached worktree of the base;
|
|
`candidate-manifest.sha256` then checks 4/4 OK.
|
|
|
|
## 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.
|
|
- 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). 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 code, tests or
|
|
README. The tests use `remote-user`/`jason` and, for the first
|
|
deployment's shape, `X-authentik-uid` with a 64-hex stand-in id. The
|
|
README example uses `X-Example-Uid`.
|
|
|
|
## 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 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 |
|
|
| `packages/webui/tests/preview.test.mjs` | new, 9 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 `{}`).
|
|
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 `/`.
|
|
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; wrong Host, `NAME:443` and `NAME.evil.example` get
|
|
403; wrong Origins get 403.
|
|
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. Without the flags NAME is refused and a POST still reaches the board.
|
|
|
|
## Mutants
|
|
|
|
22 single-site mutants in a separate worktree, run against
|
|
`preview.test.mjs` and `serve.test.mjs` (`mutants.py`, output in
|
|
`mutants.txt`). **22 of 22
|
|
killed**; the worktree was clean after the run.
|
|
|
|
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
|
|
|
|
`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)
|
|
|
|
All suites ran one after another in a detached worktree of `bc8e1ee7` with
|
|
`build.patch` applied, `TMPDIR` in my scratch and Docker on, from
|
|
2026-10-11T22:26:06Z to 22:35:49Z. The manifest checked 4/4 OK before the
|
|
run, and `core.hooksPath` stayed unset.
|
|
|
|
| Suite | Result |
|
|
|---|---|
|
|
| `packages/webui` | 49 pass, 0 fail (9 new), 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) |
|
|
|
|
## Questions for reviewers
|
|
|
|
1. Is refusing `NAME:443` (decision 2) right, or should the default port be
|
|
accepted in case the proxy adds it?
|
|
2. Anything in the README's proxy contract that ops-01 would need and
|
|
doesn't have?
|