diff --git a/docs/decisions.md b/docs/decisions.md index 1756757..09d80fc 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -290,3 +290,90 @@ session does not silently re-litigate them. Dismissal is keyed by **version**, not a boolean. A boolean would make the first dismissal permanent and the feature would quietly stop existing. + +17. **A same-origin gate on every write and on the `/ws` upgrade.** This is the + fix decision 12 said it was not. That decision required + `content-type: application/json` on the three settings routes, restoring the + CORS preflight, and scaled itself honestly: "the pre-existing action routes + are POST already and carry larger levers, so this is not new in kind. It is + a floor, not a fix." + + Two holes were open, and they composed into one attack. `/ws` upgraded + unconditionally — a WebSocket handshake is exempt from CORS entirely, so no + preflight and no browser rule stood in the way — while `hubWebSocket.open` + sends the whole snapshot on connect, so any page the operator visited could + read every agent's name, id and screen from `127.0.0.1`. And + `POST /api/agents/:id/text` types arbitrary text into a live coding agent + while reading its body with `jsonBody`, which never inspects the content + type: an `enctype="text/plain"` form posts syntactically valid JSON to it as + a CORS-*simple* request, no preflight, no same-origin check. Read the ids off + the socket, then type into the agent. On the loopback listener there is not + even an Access session to borrow — decision 3 gives that port no + authentication at all, which is correct for an operator on their own machine + and no defence whatsoever against their own browser. + + `src/server/origin.ts` holds the rule as pure predicates, called from exactly + two enforcement points: one Hono middleware in `routes.ts` that both + listeners inherit with the app, and the `/ws` interception in `ws/serve.ts`, + which is already the single shared definition of that upgrade. Neither is a + per-route check, because the guard belongs to the **verb**: a future write + route must be covered by existing, not by remembering to opt in. + + The allowlist itself is ONE thunk, built in `index.ts` and handed to every + consumer — both apps' middleware and both listeners' upgrade — rather than + derived where it is used. That is not tidiness: the first version derived it + twice, once in the middleware from `settings`/`tunnelUrl` and once at the + call site, and the gated listener's writes and its WebSocket upgrades + consequently answered to DIFFERENT allowlists. A gate with a seam in it is + the failure this repo keeps rediscovering, and `tests/origin-tunnel.test.ts` + is what found this instance. + + Three asymmetries are deliberate, and each one is load-bearing. + + **GET and HEAD are not guarded.** Browsers omit `Origin` on same-origin + GETs, so a guard there would have to accept a missing one and would gate + nothing; a cross-origin GET cannot read the response anyway, since paddock + sends no CORS headers. What guarding reads *would* achieve is breaking + `/sw.js` and the app shell — decision 3's exact failure reached from a new + direction. + + **A missing `Origin` passes a write but fails an upgrade.** Browsers always + send it on a POST, so its absence means a non-browser caller, which carries + no hostile page to act for; refusing would break every command-line use and + buy nothing. Browsers also always send it on a WebSocket handshake — + same-origin included, unlike a GET — so requiring it there costs a browser + nothing and shuts out a non-browser reader. That is worth having, because + herdr's control socket is a FILE whose permissions keep other local users + out, while paddock's port is TCP and every uid on the host can reach it. + + **The host allowlist is opportunistic, and empty means inactive.** Writes + require `Origin` to equal `Host`, which closes ordinary cross-site CSRF with + no configuration at all. It cannot close DNS rebinding, where the browser is + tricked into resolving `evil.example` to `127.0.0.1` so that `Host` and + `Origin` *agree*. Catching that needs to know the deployment's real + hostname, and paddock already does — `settings.publicUrl` plus a live + `paddock tunnel` URL — so `publicHostsFrom` derives the allowlist from those + and needs no new setting. But it is enforced only when non-empty. Making it + unconditional would mean a named-tunnel operator who never set `publicUrl` + loses the reply path, and `publicUrl` lives in the **Notifications** + section: a Telegram convenience would silently become the difference between + a working dashboard and a read-only one, for a reason no operator could + guess. So knowing a public hostname buys rebinding protection on top; + not knowing one costs nothing that already worked. Loopback is always + allowed even against a populated list, or setting a public URL would break + desk browsing and `make dev`. + + **This is not authentication and decision 3 still stands.** Nothing here + identifies anybody. It asks the one question a browser cannot lie about — + which page this request acts for — and no token is minted, held or checked. + + A refusal is reported on stderr, once per distinct `origin -> host` pair and + at most 20 pairs, because the likeliest cause is not an attack but a + misconfiguration. A dashboard that quietly stopped accepting replies would be + unexplainable; unbounded logging from a caller who chooses the origin would + be a flood. The message names the REMEDY, and `refusalReason` picks it, + because the two causes need opposite fixes: `Origin` and `Host` disagreeing + means a proxy is rewriting `Host`, while the pair AGREEING and being refused + anyway means `publicUrl` names a hostname this deployment is not reached on. + Telling the second operator to check their proxy would send them to a file + that is already correct. diff --git a/docs/gotchas.md b/docs/gotchas.md index 5a6c27d..253dd3e 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -310,6 +310,34 @@ one, recorded here so they are not reintroduced. notification was worth sending. A longer Access session duration lowers the frequency and removes nothing. +- **A proxy that rewrites `Host` breaks every write, and the browser reports + nothing useful.** Since decision 17, a state-changing request must carry an + `Origin` matching the request's `Host`. `cloudflared`'s ordinary HTTP ingress + forwards the browser's `Host` unchanged, which is what `docs/deploy-cloudflare.md` + describes and what makes the check hold — but a proxy configured to override + it (cloudflared's own `httpHostHeader`, or an nginx `proxy_set_header Host` + pointing at `localhost`) makes `Host` disagree with `Origin` on every request, + and paddock refuses all of them with a `403`. From the phone this reads as + replies silently failing while the dashboard still updates, because reads are + deliberately ungated. **The tell is on the host's stderr:** paddock logs + `refused a cross-origin write` once per distinct `origin -> host` pair, naming + both, precisely so this is diagnosable in seconds rather than guessed at. The + fix is to forward `Host` unchanged; adding the rewritten value to + `settings.publicUrl` would NOT help, because the mismatch is between the two + headers and not with any allowlist. + +- **A `publicUrl` naming a hostname you do not actually reach paddock on + refuses every write.** The other half of the entry above, and the opposite + fix. Once `settings.publicUrl` is set, its host becomes an allowlist (that is + what buys DNS-rebinding cover), so a stale value, a typo, or a SECOND + legitimate hostname for the same paddock is refused with a `403` even though + `Origin` and `Host` agree. Loopback is always exempt, so it fails from the + phone while the desk keeps working — which reads as "the tunnel broke". The + stderr line distinguishes it from a rewriting proxy: `this host is not the + public URL saved in settings`. Fix by correcting `publicUrl` to the hostname + in the browser's address bar, or by clearing it — clearing costs only the + rebinding cover and the Telegram deep link, never the reply path. + - **A verification request must come from a context holding no Access session.** An already-authenticated browser renders the dashboard whether or not the policy is correct, so it cannot tell a working gate from a missing diff --git a/src/server/index.ts b/src/server/index.ts index d0924dd..447e465 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -17,6 +17,7 @@ import { Supervisor } from "@server/supervisor"; import { shapeMessage, shapeSummary } from "@server/herdr/shape"; import { Hub } from "@server/ws/hub"; import { hubWebSocket, tryUpgradeWs, type WsData } from "@server/ws/serve"; +import { publicHostsFrom } from "@server/origin"; import { buildIdFrom } from "@server/build-id"; import { SettingsStore, defaultConfigDir, isConfigured } from "@server/settings/store"; import { recordState, removeState } from "@server/lifecycle/state"; @@ -496,8 +497,22 @@ if (DEMO) { // Named rather than inline so `paddock tunnel` can build a SECOND app from the // same dependencies plus the pairing gate — one description of the app, not two // that could drift. +/** + * The public hostnames this process answers on, for the same-origin gate. + * + * ONE definition, handed to every consumer: both apps' write middleware and + * both listeners' `/ws` upgrade. They must never be able to disagree about + * which origins are legitimate — a write accepted from an origin the socket + * refuses (or the reverse) is a gate with a seam in it. + * + * Empty until an operator saves a `publicUrl` or a tunnel run sets one, which + * is `origin.ts`'s documented inactive case rather than a weakening. + */ +const publicHosts = () => publicHostsFrom(settings.current().publicUrl, tunnelUrl); + const appDeps = { store, + publicHosts, hub, actions, settings, @@ -533,7 +548,11 @@ try { // handlers below are shared with the tunnel's gated listener rather than // written out here — `ws/serve.ts` says why. `null` means this request // is not the socket route, so it belongs to the app. - const ws = tryUpgradeWs(req, server); + // The hostnames a `/ws` upgrade may claim to come from. Read through a + // thunk rather than captured: `publicUrl` is editable from the settings UI + // while the process runs, and `tunnelUrl` is set mid-run by `paddock + // tunnel`. + const ws = tryUpgradeWs(req, server, publicHosts()); if (ws !== null) return ws; return app.fetch(req); }, @@ -724,6 +743,7 @@ if (command === "tunnel") { bin: tunnelBin, deadlineMs: tunnelDeadlineMs, setPublicUrl: (u) => { tunnelUrl = u; }, + publicHosts, registerShutdown: (fn) => { onShutdown = fn; }, }); } catch (err) { diff --git a/src/server/origin.ts b/src/server/origin.ts new file mode 100644 index 0000000..802fa14 --- /dev/null +++ b/src/server/origin.ts @@ -0,0 +1,219 @@ +/** + * The same-origin gate: two predicates and the allowlist they consult. + * + * WHY THIS EXISTS. `docs/decisions.md` decision 12 required + * `content-type: application/json` on the three settings writes, restoring the + * CORS preflight that is paddock's only CSRF control — and said plainly what it + * did not cover: "the pre-existing action routes are POST already and carry + * larger levers, so this is not new in kind. It is a floor, not a fix." + * + * This is the fix. Two holes were open. `POST /api/agents/:id/text` types + * arbitrary text into a live coding agent and reads its body with `jsonBody`, + * which never inspects the content type, so it was a CORS-SIMPLE request: an + * `enctype="text/plain"` form on any page the operator visited could post + * syntactically valid JSON to it with no preflight and no same-origin check. + * And `/ws` upgraded unconditionally — a WebSocket handshake is exempt from + * CORS entirely — while `hubWebSocket.open` sends the full snapshot on connect, + * so the same hostile page could read every agent's name, id and screen and + * then use those ids against the write route. + * + * WHY IT IS NOT AN AUTH TOKEN. Decision 3 stands: an application token would + * gate `/sw.js` and silently kill the service worker. Nothing here authenticates + * anybody. It asks one question a browser cannot lie about — which page is this + * request acting for — and refuses when the answer is "not paddock's own". + * + * PURE ON PURPOSE. No imports, no clock, no settings, no transport. Both + * enforcement points (`routes.ts`'s middleware and `ws/serve.ts`'s upgrade) call + * these, and `tests/origin.test.ts` calls them directly without booting + * `Bun.serve` — the shape `docs/roadmap.md` asks for where composition is + * otherwise untestable. + */ + +/** The three spellings of loopback, as a `Host` header carries them. */ +const LOOPBACK_HOSTNAMES: readonly string[] = ["127.0.0.1", "localhost", "[::1]", "::1"]; + +/** + * The hostname of a `Host` header value, port stripped, lowercased — or null if + * it does not parse. + * + * Parsed with `URL` rather than split on `:`, because `[::1]:8787` has two of + * those and a hand-rolled split gets it wrong. The `http://` prefix is a parsing + * scaffold and says nothing about the real scheme. + */ +function hostnameOf(host: string): string | null { + try { + const h = new URL(`http://${host}`).hostname; + return h === "" ? null : h.toLowerCase(); + } catch { + return null; + } +} + +/** + * The host this request was addressed to. + * + * The `Host` HEADER first, because that is what a browser sent and what a proxy + * may have rewritten — the value the check is actually about. The URL's host is + * the fallback, and in production the two are the same value: Bun builds + * `req.url` FROM the `Host` header, so neither can disagree with the other on a + * real request. The fallback exists because a `Request` constructed in a test + * carries no `Host` header at all — the header is added by the transport — and a + * predicate that only worked over a live socket could not be tested at the one + * layer worth testing it at. + */ +export function hostOf(req: Request): string { + const header = req.headers.get("host"); + if (header !== null && header !== "") return header; + try { + return new URL(req.url).host; + } catch { + return ""; + } +} + +/** + * Whether a `Host` header names this machine. + * + * Matched on the parsed hostname, never on a prefix: `127.0.0.1.evil.com` and + * `localhost.evil.com` are ordinary registrable names that resolve wherever + * their owner points them, and a `startsWith` check would hand both of them the + * allowlist bypass below. + */ +export function isLoopbackHost(host: string): boolean { + const name = hostnameOf(host); + return name !== null && LOOPBACK_HOSTNAMES.includes(name); +} + +/** The `host` of an origin string (`https://h:port` -> `h:port`), or null. */ +function hostOfOrigin(origin: string): string | null { + let url: URL; + try { + url = new URL(origin); + } catch { + // Includes the literal "null" a sandboxed iframe or a `data:` document + // sends — no scheme, so it parses as a URL nowhere. + return null; + } + if (url.protocol !== "http:" && url.protocol !== "https:") return null; + return url.host === "" ? null : url.host.toLowerCase(); +} + +/** + * The hostnames this deployment is legitimately reached on, or **empty when + * none is known**. + * + * Empty means INACTIVE, not "deny everything", and that distinction is the + * whole reason `publicUrl` can stay optional. It lives in the Notifications + * section — `docs/settings.md` calls it "what turns a bare 'docs-cleanup is + * blocked' into a tap-through link" — so an operator on a named tunnel who does + * not use Telegram has never had a reason to set it. Enforcing an allowlist + * built from it unconditionally would make a notification setting the + * difference between a working dashboard and a read-only one, for a reason no + * operator could guess. That is the failure mode `CLAUDE.md` bans, so: + * knowing a public hostname buys DNS-rebinding protection on top of the + * same-origin check; not knowing one costs nothing that was already working. + * + * A live tunnel URL is included as well as the saved one. A `paddock tunnel` + * run's hostname differs on every run and is deliberately never written to + * `settings.json` (see `docs/settings.md`), so the live value is the only place + * it exists. + */ +export function publicHostsFrom( + publicUrl: string | null, + tunnelUrl: string | null, +): readonly string[] { + const hosts: string[] = []; + for (const candidate of [publicUrl, tunnelUrl]) { + if (candidate === null || candidate.trim() === "") continue; + // An unparseable value is no knowledge at all, not a lockout: `publicUrl` + // is a free-text field, and a typo in it must never populate the allowlist + // with garbage that then refuses every write. + const host = hostOfOrigin(candidate.trim()); + if (host !== null && !hosts.includes(host)) hosts.push(host); + } + return hosts; +} + +/** + * Whether the request's `Host` is one this deployment answers on. + * + * Checked BEFORE the origin comparison and fail-closed, because it is the only + * one of the two that can see a rebinding attack: a browser tricked into + * resolving `evil.example` to `127.0.0.1` sends `Host` and `Origin` that AGREE, + * so the comparison below passes on its own. + */ +function hostAllowed(host: string, publicHosts: readonly string[]): boolean { + if (publicHosts.length === 0) return true; // inactive — see publicHostsFrom + if (isLoopbackHost(host)) return true; // the desk, and `make dev` + return publicHosts.includes(host.toLowerCase()); +} + +function sameOrigin(origin: string, host: string, publicHosts: readonly string[]): boolean { + if (!hostAllowed(host, publicHosts)) return false; + const from = hostOfOrigin(origin); + return from !== null && from === host.toLowerCase(); +} + +/** + * Why a request was refused — because the two causes need OPPOSITE fixes, and + * naming the wrong one sends an operator to the wrong file. + * + * `cross-origin`: `Origin` and `Host` disagree. Either a hostile page, or a + * proxy in front rewriting `Host` (cloudflared's `httpHostHeader`, nginx's + * `proxy_set_header Host`). The fix is in the proxy. + * + * `host-not-allowed`: they AGREE and the allowlist refused anyway, so this + * deployment is being reached on a hostname `publicUrl` does not name. The fix + * is in settings — and telling this operator to look at their proxy would be + * actively misleading, since their proxy is behaving correctly. + */ +export type RefusalReason = "no-origin" | "cross-origin" | "host-not-allowed"; + +export function refusalReason( + origin: string | null, + host: string, + publicHosts: readonly string[], +): RefusalReason { + if (origin === null) return "no-origin"; + if (!hostAllowed(host, publicHosts)) return "host-not-allowed"; + return "cross-origin"; +} + +/** + * Whether a state-changing request may proceed. + * + * A MISSING `Origin` is allowed, and that is not an oversight. Browsers always + * send it on a POST, so its absence means the caller is not a browser — curl, a + * script, a health check — and a non-browser caller carries no hostile page on + * whose behalf it could be acting. Refusing here would buy nothing and break + * every command-line use of the API. + */ +export function allowWrite( + origin: string | null, + host: string, + publicHosts: readonly string[], +): boolean { + if (origin === null) return true; + return sameOrigin(origin, host, publicHosts); +} + +/** + * Whether a `/ws` upgrade may proceed. + * + * Identical to `allowWrite` except that a missing `Origin` is REFUSED, which is + * the one asymmetry worth stating twice. Browsers send `Origin` on every + * WebSocket handshake, same-origin included — unlike a GET, where they omit it — + * so requiring it costs a browser nothing and shuts out a non-browser reader. + * That is worth having: herdr's control socket is a FILE, so its permissions + * keep other local users out, while paddock's port is TCP and every uid on the + * host can reach it. `/ws` sends the whole snapshot on connect, so it is the + * read worth closing. + */ +export function allowUpgrade( + origin: string | null, + host: string, + publicHosts: readonly string[], +): boolean { + if (origin === null) return false; + return sameOrigin(origin, host, publicHosts); +} diff --git a/src/server/routes.ts b/src/server/routes.ts index a5c3152..08947ac 100644 --- a/src/server/routes.ts +++ b/src/server/routes.ts @@ -16,6 +16,8 @@ import type { Hub } from "@server/ws/hub"; import { formatCode } from "@server/tunnel/pairing"; import { setCookie } from "@server/tunnel/gate"; import { EMBEDDED } from "@server/embedded"; +import { allowWrite, hostOf, refusalReason } from "@server/origin"; +import { warn } from "@server/term"; import { isNavKey, type NotifyTrigger, type SettingsPatch } from "@shared/types"; import { diffScreens, digestOf } from "@shared/screen"; @@ -161,6 +163,36 @@ function pruneScreens(liveIds: Set): void { for (const id of [...recentScreens.keys()]) if (!liveIds.has(id)) recentScreens.delete(id); } +/** + * How many distinct `origin -> host` pairs a refusal will be reported for. + * + * Bounded because the caller is hostile by definition: a page that varies its + * origin could otherwise flood the operator's terminal, and the Set would grow + * without limit. Bounded rather than silent because the most likely cause of a + * refusal is NOT an attack — it is a proxy that rewrites `Host` so it no longer + * matches the browser's `Origin`, and a dashboard that stopped accepting + * replies with nothing on stderr would be unexplainable. `docs/gotchas.md` + * records exactly that class of failure. + */ +const REFUSAL_LOG_LIMIT = 20; +const refusalsSeen = new Set(); + +function reportRefusal(origin: string | null, host: string, hosts: readonly string[]): void { + const key = `${origin ?? "(none)"} -> ${host}`; + if (refusalsSeen.has(key) || refusalsSeen.size >= REFUSAL_LOG_LIMIT) return; + refusalsSeen.add(key); + + warn(`paddock: refused a write — origin ${origin ?? "(none)"}, host \`${host}\``); + // The remedy, not just the fact. These two causes need opposite fixes, and + // sending an operator to their proxy config when their proxy is correct is + // worse than saying nothing. + if (refusalReason(origin, host, hosts) === "host-not-allowed") { + warn(" this host is not the public URL saved in settings — correct it, or clear it"); + } else { + warn(" if this is your own dashboard, the proxy in front is rewriting `Host`"); + } +} + /** * A request body as an object, whatever the client actually sent. * @@ -371,6 +403,13 @@ export interface AppDeps { }; /** The live tunnel URL, for the settings view. */ tunnelUrl?: () => string | null; + /** + * The hostnames this deployment is legitimately reached on, for the + * same-origin gate — see `origin.ts`. Omitted in tests and in any caller with + * no public hostname, which is the documented INACTIVE case: the origin/Host + * comparison still applies, only DNS-rebinding cover is absent. + */ + publicHosts?: () => readonly string[]; } export function createApp(deps: AppDeps) { @@ -392,6 +431,51 @@ export function createApp(deps: AppDeps) { */ app.use("*", compress()); + /** + * The same-origin gate on every state-changing request — the other half of + * decision 12, which restored the CORS preflight for the three settings + * routes and said what it did not cover: "the pre-existing action routes are + * POST already and carry larger levers ... It is a floor, not a fix." + * + * ONE middleware rather than a check per route, because the guard belongs to + * the VERB, not to a handler's dependencies: `/ack` is registered even in demo + * mode, `/text` only when `actions` is present, and a future write route must + * be covered by existing to be a write rather than by remembering to opt in. + * Both listeners share this app, so this covers the desk's 8787 and the + * tunnel's gated listener at once. + * + * GET and HEAD are deliberately NOT guarded. Browsers omit `Origin` on + * same-origin GETs, so a guard there would have to accept a missing one and + * would gate nothing; meanwhile a cross-origin GET cannot read the response, + * since paddock sends no CORS headers. What guarding reads WOULD achieve is + * breaking `/sw.js` and the app shell — decision 3's exact failure, arrived at + * from a new direction. `/ws` is not a route on this app at all: it is + * intercepted before `app.fetch`, and `ws/serve.ts` guards it there. + * + * This is still not authentication. Nothing here identifies anybody; it asks + * the one question a browser cannot lie about — which page this request acts + * for — so decision 3 stands untouched. + */ + app.use("*", async (c, next) => { + const method = c.req.method; + if (method === "GET" || method === "HEAD") return next(); + + const host = hostOf(c.req.raw); + const origin = c.req.header("origin") ?? null; + // Read through the thunk on every request, never cached: `publicUrl` is + // editable from the settings UI while the process runs, and a tunnel URL + // appears mid-run. Taken as a DEPENDENCY rather than derived here from + // `settings` and `tunnelUrl`, because `ws/serve.ts` needs the same list and + // two derivations of one fact is how they come to disagree — which is not + // hypothetical: the first version of this middleware derived its own, and + // the gated listener's HTTP writes and its WebSocket upgrades then answered + // to different allowlists. + if (allowWrite(origin, host, deps.publicHosts?.() ?? [])) return next(); + + reportRefusal(origin, host, deps.publicHosts?.() ?? []); + return c.json({ ok: false, detail: "cross-origin rejected" }, 403); + }); + // No authentication middleware. Cloudflare Access is the only gate — see // docs/decisions.md before adding one. app.get("/api/health", (c) => c.json(deps.health())); diff --git a/src/server/tunnel/run.ts b/src/server/tunnel/run.ts index 15f6e37..e12b5ef 100644 --- a/src/server/tunnel/run.ts +++ b/src/server/tunnel/run.ts @@ -35,6 +35,17 @@ export interface TunnelDeps { deadlineMs?: number | null; startTunnel?: typeof realStartTunnel; setPublicUrl?: (url: string | null) => void; + /** + * The origins a `/ws` upgrade on THIS listener may claim, for the same-origin + * gate in `ws/serve.ts`. A thunk because a tunnel run learns its own hostname + * partway through starting up. + * + * Optional, defaulting to none: empty is `origin.ts`'s documented "no public + * hostname is known" case, under which the origin/Host comparison still + * applies. A test that omits it is therefore testing the same rule an + * operator without a saved `publicUrl` runs under, not a relaxed one. + */ + publicHosts?: () => readonly string[]; /** * The environment and the terminal, injected for the same reason every clock * in this codebase is. Read from the real ones in production; a test that @@ -85,7 +96,7 @@ export function serveGated(deps: TunnelDeps): { port: number; stop(): void } { // Past the gate, this listener serves EXACTLY what the plain one serves, // from one definition — see `ws/serve.ts`. `null` means the request is // not the socket route and belongs to the app. - const ws = tryUpgradeWs(req, srv); + const ws = tryUpgradeWs(req, srv, deps.publicHosts?.() ?? []); if (ws !== null) return ws; return deps.app.fetch(req); }, diff --git a/src/server/ws/serve.ts b/src/server/ws/serve.ts index d92cc10..ebb3bc3 100644 --- a/src/server/ws/serve.ts +++ b/src/server/ws/serve.ts @@ -1,6 +1,7 @@ import type { Server, WebSocketHandler } from "bun"; import type { AgentStore } from "@server/state/store"; import type { Hub, HubClient } from "@server/ws/hub"; +import { allowUpgrade, hostOf } from "@server/origin"; /** * What a socket carries: the hub client it was added as, so `close` can remove @@ -30,9 +31,27 @@ export interface HubSocketDeps { * - `null` — not this route; the caller falls through to `app.fetch`. * - `undefined` — upgraded. Bun's own signal that the response IS the upgrade. * - a `Response` — the upgrade was refused. + * + * The ORIGIN CHECK is here, before `srv.upgrade`, and it has to be: a WebSocket + * handshake is exempt from CORS entirely, so no preflight and no browser rule + * stopped a hostile page opening this socket — and `open` below sends the whole + * snapshot, so a refusal that arrived any later would have already disclosed + * every agent's name, id and screen. `allowUpgrade` refuses a MISSING `Origin` + * as well as a mismatched one; `origin.ts` says why that is right here and wrong + * for a write. + * + * `publicHosts` defaults to empty, which is the correct value for "no public + * hostname is known" rather than a weakening — see `publicHostsFrom`. */ -export function tryUpgradeWs(req: Request, srv: Server): Response | undefined | null { +export function tryUpgradeWs( + req: Request, + srv: Server, + publicHosts: readonly string[] = [], +): Response | undefined | null { if (new URL(req.url).pathname !== "/ws") return null; + if (!allowUpgrade(req.headers.get("origin"), hostOf(req), publicHosts)) { + return new Response("cross-origin rejected", { status: 403 }); + } const upgraded = srv.upgrade(req, { data: {} }); return upgraded ? undefined : new Response("upgrade failed", { status: 400 }); } diff --git a/tests/origin-gate.test.ts b/tests/origin-gate.test.ts new file mode 100644 index 0000000..7950373 --- /dev/null +++ b/tests/origin-gate.test.ts @@ -0,0 +1,176 @@ +import { expect, test } from "bun:test"; +import { createApp } from "@server/routes"; +import { AgentStore } from "@server/state/store"; +import { Hub } from "@server/ws/hub"; +import { tryUpgradeWs, type WsData } from "@server/ws/serve"; +import type { Server } from "bun"; +import type { Agent } from "@shared/types"; + +/** + * The two ENFORCEMENT POINTS, as opposed to `tests/origin.test.ts`, which pins + * the rule. Both are the single shared definition for their transport — one + * Hono middleware that both listeners inherit with the app, and the one `/ws` + * interception `ws/serve.ts` exists so there is only one of — so a hole here is + * a hole on the desk's 8787 and on the tunnel's gated listener at once. + * + * `app.request()` builds a request for `http://localhost/…`, so `http://localhost` + * is the same-origin case throughout this file. + */ + +const NOW = 1_700_000_000_000; + +const health = () => ({ + ok: true, hostId: "dev-box", agents: 1, clients: 0, herdrConnected: true, + lastEventAt: NOW, lastNotifyError: null, version: "0.0.0-dev", latestKnown: null, + herdrProtocol: null, schemaWarning: null, +}); + +function agent(state: Agent["state"] = "blocked"): Agent { + return { + hostId: "dev-box", agentId: "w1:p1", name: "api-refactor", + task: "Extract auth middleware", state, workspaceId: "w1", + workspaceLabel: "api work", cwd: "/srv/project", + stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, + }; +} + +function harness(state: Agent["state"] = "blocked") { + const store = new AgentStore("dev-box"); + store.replaceAll([agent(state)], NOW); + const calls: string[] = []; + const actions = { + async readOutput() { calls.push("readOutput"); return { lines: ["out"], source: "visible" as const }; }, + async readDetection() { calls.push("readDetection"); return ""; }, + async sendOptionKey(_t: string, k: string) { calls.push(`key:${k}`); }, + async sendNavKey(_t: string, k: string) { calls.push(`nav:${k}`); }, + async sendReply(_t: string, text: string) { calls.push(`reply:${text}`); }, + async waitUntilUnblocked() { calls.push("wait"); }, + }; + const app = createApp({ + store, actions, now: () => NOW, health, + hub: new Hub({ now: () => NOW }), + }); + return { app, calls, store }; +} + +const post = (app: ReturnType, path: string, origin: string | null, body: object = {}) => + app.request(path, { + method: "POST", + headers: { + "content-type": "application/json", + ...(origin === null ? {} : { origin }), + }, + body: JSON.stringify(body), + }); + +test("a cross-origin reply is refused, and never reaches the agent", () => { + // THE hole this work closes. `/text` types arbitrary text into a live coding + // agent, and `jsonBody` never looks at the content type — so before this gate + // an `enctype="text/plain"` form on any page the operator visited could post + // valid JSON here with no preflight. The second assertion is the one that + // matters: a 403 that still typed the text would be no fix at all. + return (async () => { + const { app, calls } = harness(); + const res = await post(app, "/api/agents/w1:p1/text", "https://evil.example", { text: "rm -rf /" }); + expect(res.status).toBe(403); + expect(calls).toEqual([]); + })(); +}); + +test("the refusal names cross-origin as the reason", async () => { + const { app } = harness(); + const res = await post(app, "/api/agents/w1:p1/text", "https://evil.example", { text: "hi" }); + const body = await res.json(); + expect(body.ok).toBe(false); + expect(body.detail).toContain("cross-origin"); +}); + +test("a same-origin reply still works", async () => { + const { app, calls } = harness(); + const res = await post(app, "/api/agents/w1:p1/text", "http://localhost", { text: "yes" }); + expect(res.status).toBe(200); + expect(calls).toContain("reply:yes"); +}); + +test("a reply with no Origin still works — the command line is not a CSRF vector", async () => { + const { app, calls } = harness(); + const res = await post(app, "/api/agents/w1:p1/text", null, { text: "yes" }); + expect(res.status).toBe(200); + expect(calls).toContain("reply:yes"); +}); + +test("every write verb is covered, not just the ones with an actions dep", async () => { + // `/ack` touches only paddock's own store and the hub, and is registered even + // in demo mode. It must be gated by the same middleware — the guard belongs to + // the verb, not to a route's dependencies. `done`, because /ack refuses any + // other state with a 409 of its own and this test is about the gate. + const { app } = harness("done"); + expect((await post(app, "/api/agents/w1:p1/ack", "https://evil.example")).status).toBe(403); + expect((await post(app, "/api/agents/w1:p1/ack", "http://localhost")).status).toBe(200); +}); + +test("a foreign Origin on a READ is allowed", async () => { + // Deliberate, and load-bearing twice over. Browsers omit `Origin` on + // same-origin GETs, so a GET guard would have to accept a missing one anyway + // and would gate nothing. And a cross-origin GET cannot READ the response — + // paddock sends no CORS headers — so there is nothing to protect. Guarding + // GETs is how `/sw.js` and the app shell would break instead. + const { app } = harness(); + const res = await app.request("/api/agents", { headers: { origin: "https://evil.example" } }); + expect(res.status).toBe(200); +}); + +/** A `Server` that records whether `upgrade` was reached at all. */ +function fakeServer(): { srv: Server; calls: { upgrades: number } } { + const calls = { upgrades: 0 }; + const srv = { + upgrade() { + calls.upgrades += 1; + return true; + }, + } as unknown as Server; + return { srv, calls }; +} + +const upgradeReq = (origin: string | null) => + new Request("http://127.0.0.1:8787/ws", { + headers: { + upgrade: "websocket", + connection: "Upgrade", + ...(origin === null ? {} : { origin }), + }, + }); + +test("a cross-origin upgrade is refused before `upgrade` is called", () => { + // The socket sends the whole snapshot in `open`, so a refusal that arrived + // after the upgrade would have already disclosed every agent's screen. + const { srv, calls } = fakeServer(); + const res = tryUpgradeWs(upgradeReq("https://evil.example"), srv); + expect(res).toBeInstanceOf(Response); + expect((res as Response).status).toBe(403); + expect(calls.upgrades).toBe(0); +}); + +test("an upgrade with no Origin is refused", () => { + // Browsers always send it on a handshake, so this is `websocat` or a script — + // and every uid on the host can reach a TCP port, unlike herdr's socket file. + const { srv, calls } = fakeServer(); + const res = tryUpgradeWs(upgradeReq(null), srv); + expect((res as Response).status).toBe(403); + expect(calls.upgrades).toBe(0); +}); + +test("a same-origin upgrade proceeds", () => { + const { srv, calls } = fakeServer(); + const res = tryUpgradeWs(upgradeReq("http://127.0.0.1:8787"), srv); + // `undefined` is Bun's own signal that the response IS the upgrade. + expect(res).toBeUndefined(); + expect(calls.upgrades).toBe(1); +}); + +test("a request that is not /ws is still not this route's business", () => { + // The guard must not turn a fall-through into a refusal: `null` is how the + // caller learns the request belongs to the app. + const { srv } = fakeServer(); + expect(tryUpgradeWs(new Request("http://127.0.0.1:8787/api/agents"), srv)).toBeNull(); +}); diff --git a/tests/origin-tunnel.test.ts b/tests/origin-tunnel.test.ts new file mode 100644 index 0000000..122c2b6 --- /dev/null +++ b/tests/origin-tunnel.test.ts @@ -0,0 +1,209 @@ +import { expect, test } from "bun:test"; +import { connect } from "node:net"; +import { createApp } from "@server/routes"; +import { AgentStore } from "@server/state/store"; +import { COOKIE_NAME, Pairing } from "@server/tunnel/pairing"; +import { serveGated } from "@server/tunnel/run"; +import { Hub } from "@server/ws/hub"; +import type { Agent } from "@shared/types"; + +/** + * The three DEPLOYMENT SHAPES, through the real gated listener. + * + * `tests/origin-gate.test.ts` covers the two enforcement points in isolation. + * This file answers the question an operator actually has: does my deployment + * still work? Each test names the shape it stands for — the desk, a quick + * tunnel, a named tunnel — because the same-origin rule is the one change in + * this area that could turn a working phone into a read-only screen, and the + * failure would appear on a device the suite never runs on. + * + * The gated listener rather than the plain app on purpose: a request from a + * tunnel passes the PAIRING gate first and the origin gate second, and only + * this shape exercises both in the order production runs them. + */ + +const NOW = 1_700_000_000_000; +const TUNNEL_HOST = "apple-berry-cat-dog.trycloudflare.com"; +const NAMED_HOST = "paddock.example.com"; + +const health = () => ({ + ok: true, hostId: "dev-box", agents: 1, clients: 0, herdrConnected: true, + lastEventAt: NOW, lastNotifyError: null, version: "0.0.0-dev", latestKnown: null, + herdrProtocol: null, schemaWarning: null, +}); + +function agent(): Agent { + return { + hostId: "dev-box", agentId: "w1:p1", name: "docs-cleanup", + task: "Tidy the README", state: "done", workspaceId: "w1", + workspaceLabel: "docs", cwd: "/srv/project", + stateSince: NOW, updatedAt: NOW, acknowledgedAt: null, + }; +} + +/** + * A live gated listener whose allowlist is whatever the deployment knows. + * + * `publicHosts` is passed as the thunk `index.ts` passes, so what is under test + * is the wiring an operator runs, not a value inlined for the convenience of + * the assertion. + */ +function harness(publicHosts: readonly string[] = []) { + const store = new AgentStore("dev-box"); + store.replaceAll([agent()], NOW); + const pairing = new Pairing({ now: () => NOW }); + // The SAME thunk reaches the app's write middleware and the listener's `/ws` + // upgrade, exactly as `index.ts` hands it to both. Passing it to only one is + // the defect this file caught: writes and upgrades then answer to different + // allowlists. + const hosts = () => publicHosts; + const app = createApp({ + store, pairing, now: () => NOW, health, + hub: new Hub({ now: () => NOW }), + publicHosts: hosts, + }); + const server = serveGated({ + app, pairing, store, + hub: new Hub({ now: () => NOW }), + hostId: "dev-box", + port: 0, // the OS picks, so the suite never collides with a real tunnel + env: {}, + isTty: false, + publicHosts: hosts, + }); + const paired = pairing.attempt(pairing.current().code); + if (paired.kind !== "paired") throw new Error("unreachable: a fresh code must pair"); + return { server, token: paired.token }; +} + +/** + * A write as a browser on `origin` sends it, arriving with `host` as a proxy set + * it — sent over a RAW SOCKET, not `fetch`. + * + * `fetch` silently drops a `Host` header (it is forbidden to script), so every + * request it makes to a loopback listener claims a loopback host. An earlier + * version of this file used it and every tunnel case passed as the DESK case: + * green, and testing nothing it claimed to. Bytes on a socket are also exactly + * what `cloudflared` puts there, so this is the faithful shape rather than a + * clever one. + */ +function rawWrite(port: number, token: string, o: { host: string; origin: string }): Promise { + const body = "{}"; + const req = [ + "POST /api/agents/w1:p1/ack HTTP/1.1", + `Host: ${o.host}`, + `Origin: ${o.origin}`, + `Cookie: ${COOKIE_NAME}=${token}`, + "content-type: application/json", + `content-length: ${body.length}`, + "connection: close", + "", + body, + ].join("\r\n"); + return new Promise((resolve, reject) => { + const sock = connect({ host: "127.0.0.1", port }, () => sock.write(req)); + let out = ""; + sock.on("data", (d) => { out += d.toString("latin1"); }); + sock.on("error", reject); + sock.on("close", () => { + const status = /^HTTP\/1\.1 (\d{3})/.exec(out); + if (status === null) reject(new Error(`no status line: ${out.slice(0, 120)}`)); + else resolve(Number(status[1])); + }); + }); +} + +test("the desk still works: loopback, no public hostname known", async () => { + const { server, token } = harness([]); + try { + const status = await rawWrite(server.port, token, { + host: "127.0.0.1:8788", + origin: "http://127.0.0.1:8788", + }); + expect(status).toBe(200); + } finally { server.stop(); } +}); + +test("a quick tunnel still works: the run's own hostname", async () => { + // `paddock tunnel`. `publicHostsFrom` learns this hostname from the live + // value, since it is never written to settings.json. + const { server, token } = harness([TUNNEL_HOST]); + try { + const status = await rawWrite(server.port, token, { + host: TUNNEL_HOST, + origin: `https://${TUNNEL_HOST}`, + }); + expect(status).toBe(200); + } finally { server.stop(); } +}); + +test("a named tunnel still works: publicUrl's hostname", async () => { + const { server, token } = harness([NAMED_HOST]); + try { + const status = await rawWrite(server.port, token, { + host: NAMED_HOST, + origin: `https://${NAMED_HOST}`, + }); + expect(status).toBe(200); + } finally { server.stop(); } +}); + +test("a named tunnel works with publicUrl unset — the allowlist is inactive", async () => { + // The case that keeps `publicUrl` optional: an operator who does not use + // Telegram has never had a reason to set it, and must not lose the reply path + // for that. Same-origin is still enforced; only rebinding is uncovered. + const { server, token } = harness([]); + try { + const status = await rawWrite(server.port, token, { + host: NAMED_HOST, + origin: `https://${NAMED_HOST}`, + }); + expect(status).toBe(200); + } finally { server.stop(); } +}); + +test("desk browsing survives a populated allowlist", async () => { + // Setting a public URL must not break the loopback the operator uses at their + // own machine, or `make dev`. + const { server, token } = harness([NAMED_HOST]); + try { + const status = await rawWrite(server.port, token, { + host: "127.0.0.1:8788", + origin: "http://127.0.0.1:8788", + }); + expect(status).toBe(200); + } finally { server.stop(); } +}); + +test("a paired but cross-origin write is still refused", async () => { + // Pairing proves the DEVICE reached the tunnel, never which page is asking. + // A paired phone that later visits a hostile page must not become a lever on + // the agents, so the origin gate has to sit behind the pairing gate rather + // than being satisfied by it. + const { server, token } = harness([TUNNEL_HOST]); + try { + const status = await rawWrite(server.port, token, { + host: TUNNEL_HOST, + origin: "https://evil.example", + }); + expect(status).toBe(403); + } finally { server.stop(); } +}); + +test("a publicUrl naming a hostname the deployment is NOT reached on locks out writes", async () => { + // The one way this bites, recorded as a test so it is a known trade-off and + // not a surprise: a stale or mistyped `publicUrl`, or a second legitimate + // hostname for the same paddock, is refused even though Origin and Host + // agree — which is exactly the rebinding protection working as designed, + // pointed at the operator. `docs/gotchas.md` carries the symptom and the fix + // (correct the value, or clear it); the refusal names both headers on stderr + // so it is diagnosable rather than mysterious. + const { server, token } = harness(["stale.example"]); + try { + const status = await rawWrite(server.port, token, { + host: NAMED_HOST, + origin: `https://${NAMED_HOST}`, + }); + expect(status).toBe(403); + } finally { server.stop(); } +}); diff --git a/tests/origin.test.ts b/tests/origin.test.ts new file mode 100644 index 0000000..d58eef7 --- /dev/null +++ b/tests/origin.test.ts @@ -0,0 +1,195 @@ +import { expect, test } from "bun:test"; +import { + allowUpgrade, + allowWrite, + isLoopbackHost, + publicHostsFrom, + refusalReason, +} from "@server/origin"; + +/** + * The gate these predicates implement is decision 12's missing half. + * + * That decision restored the CORS preflight requirement for the three settings + * routes and said plainly what it did not cover: "the pre-existing action + * routes are POST already and carry larger levers, so this is not new in kind. + * It is a floor, not a fix." `POST /api/agents/:id/text` types arbitrary text + * into a coding agent and reads a body with `jsonBody`, which never looks at + * the content type — so it was reachable from an `enctype="text/plain"` form on + * any page the operator visited, and `/ws` handed that same page every agent's + * screen because a WebSocket handshake is exempt from CORS entirely. + * + * These tests pin the rule, not the plumbing. The two predicates differ in + * exactly one place — a missing `Origin` — and that difference is load-bearing + * enough to be asserted from both sides. + */ + +const LOOPBACK = "127.0.0.1:8787"; + +test("a write with no Origin is allowed — a non-browser client is not a CSRF vector", () => { + // curl, a script, `paddock doctor`: none of them carry an attacker's page. + // The threat is a browser acting on a hostile page's behalf, and browsers + // ALWAYS send Origin on a POST. Refusing here would buy nothing and break + // every command-line use of the API. + expect(allowWrite(null, LOOPBACK, [])).toBe(true); +}); + +test("a same-origin write is allowed", () => { + expect(allowWrite("http://127.0.0.1:8787", LOOPBACK, [])).toBe(true); +}); + +test("a cross-origin write is refused", () => { + // The drive-by: any page the operator visits POSTing into a live agent. + expect(allowWrite("https://evil.example", LOOPBACK, [])).toBe(false); +}); + +test("an opaque Origin is refused", () => { + // A sandboxed iframe or a `data:` document sends the literal string "null". + // It parses as a URL nowhere, and it is nobody's first-party request. + expect(allowWrite("null", LOOPBACK, [])).toBe(false); +}); + +test("an unparseable Origin is refused", () => { + expect(allowWrite("not a url", LOOPBACK, [])).toBe(false); +}); + +test("a port mismatch is cross-origin", () => { + // Same host, different port is a different origin to a browser, and must be + // one here: another service on the operator's own machine is exactly the + // attacker the loopback listener is exposed to. + expect(allowWrite("http://127.0.0.1:9999", LOOPBACK, [])).toBe(false); +}); + +test("Host is compared case-insensitively", () => { + // Hostnames are case-insensitive and a proxy may not preserve case; a 403 + // over letter case would be an unexplainable outage. + expect(allowWrite("https://paddock.example.com", "Paddock.Example.COM", [])).toBe(true); +}); + +test("a rebinding request is refused once a public host is known", () => { + // DNS rebinding is the one attack a bare same-origin check cannot see: the + // browser is tricked into resolving evil.example to 127.0.0.1, so Host and + // Origin AGREE and the comparison passes. The allowlist is what catches it. + expect(allowWrite("https://evil.example", "evil.example", ["paddock.example.com"])).toBe(false); +}); + +test("a rebinding request passes when no public host is known — the documented limit", () => { + // Deliberate, and the reason `publicUrl` stays optional. `publicUrl` lives in + // the Notifications section: making it load-bearing for the reply path would + // turn a Telegram setting into the difference between a working dashboard and + // a read-only one, for a reason no operator could guess. So an empty + // allowlist means "we do not know this deployment's hostname", not "allow + // nothing" — CSRF is still closed, rebinding is not. + expect(allowWrite("https://evil.example", "evil.example", [])).toBe(true); +}); + +test("loopback is allowed even when it is not on a populated allowlist", () => { + // Desk browsing and `make dev` must not start failing the moment an operator + // sets a public URL for notification deep links. + expect(allowWrite("http://127.0.0.1:8787", LOOPBACK, ["paddock.example.com"])).toBe(true); +}); + +test("a known public host is allowed", () => { + expect( + allowWrite("https://paddock.example.com", "paddock.example.com", ["paddock.example.com"]), + ).toBe(true); +}); + +test("a host outside a populated allowlist is refused even when Origin agrees", () => { + expect(allowWrite("https://other.example", "other.example", ["paddock.example.com"])).toBe(false); +}); + +test("an upgrade with no Origin is REFUSED, unlike a write", () => { + // The one place the two predicates disagree. Browsers always send Origin on + // a WebSocket handshake — same-origin included, unlike a GET — so requiring + // it costs a browser nothing and shuts out `websocat`. That matters because + // herdr's socket is a FILE, whose permissions keep other local users out, + // while paddock's port is TCP and every uid on the host can reach it. `/ws` + // hands over every agent's screen on connect, so it is the read worth + // closing. + expect(allowUpgrade(null, LOOPBACK, [])).toBe(false); +}); + +test("a same-origin upgrade is allowed", () => { + expect(allowUpgrade("http://127.0.0.1:8787", LOOPBACK, [])).toBe(true); +}); + +test("a cross-origin upgrade is refused", () => { + expect(allowUpgrade("https://evil.example", LOOPBACK, [])).toBe(false); +}); + +test("an upgrade obeys the same allowlist as a write", () => { + expect(allowUpgrade("https://evil.example", "evil.example", ["paddock.example.com"])).toBe(false); +}); + +test("isLoopbackHost accepts the three loopback spellings, with or without a port", () => { + expect(isLoopbackHost("127.0.0.1")).toBe(true); + expect(isLoopbackHost("127.0.0.1:8787")).toBe(true); + expect(isLoopbackHost("localhost")).toBe(true); + expect(isLoopbackHost("localhost:5173")).toBe(true); + expect(isLoopbackHost("[::1]:8787")).toBe(true); +}); + +test("isLoopbackHost is not fooled by a hostname that merely starts with one", () => { + // `127.0.0.1.evil.com` resolves wherever its owner points it. + expect(isLoopbackHost("127.0.0.1.evil.com")).toBe(false); + expect(isLoopbackHost("localhost.evil.com")).toBe(false); + expect(isLoopbackHost("notlocalhost")).toBe(false); +}); + +test("publicHostsFrom returns nothing when no public hostname is known", () => { + // Empty is INACTIVE, not empty-so-deny. See the rebinding test above. + expect(publicHostsFrom(null, null)).toEqual([]); + expect(publicHostsFrom("", null)).toEqual([]); +}); + +test("publicHostsFrom takes the host of a configured publicUrl, port included", () => { + expect(publicHostsFrom("https://paddock.example.com", null)).toEqual(["paddock.example.com"]); + expect(publicHostsFrom("https://paddock.example.com:8443/", null)).toEqual([ + "paddock.example.com:8443", + ]); +}); + +test("publicHostsFrom includes a live tunnel URL", () => { + // A `paddock tunnel` run's hostname is different on every run and is never + // written to settings.json, so it can only come from the live value. + expect(publicHostsFrom(null, "https://apple-berry-cat-dog.trycloudflare.com")).toEqual([ + "apple-berry-cat-dog.trycloudflare.com", + ]); +}); + +test("publicHostsFrom holds both a saved deployment and a live tunnel at once", () => { + // `paddock tunnel` run against a host that also has a named-tunnel publicUrl + // saved: both hostnames are legitimate for the life of that run. + expect( + publicHostsFrom("https://paddock.example.com", "https://apple-berry.trycloudflare.com"), + ).toEqual(["paddock.example.com", "apple-berry.trycloudflare.com"]); +}); + +test("publicHostsFrom ignores a value that is not a URL", () => { + // `publicUrl` is a free-text settings field. A typo must not silently + // populate the allowlist with garbage and lock the operator out — an + // unparseable value is no knowledge at all, which is the inactive case. + expect(publicHostsFrom("paddock.example.com", null)).toEqual([]); + expect(publicHostsFrom("¯\\_(ツ)_/¯", null)).toEqual([]); +}); + +test("publicHostsFrom lowercases what it stores, so the allowlist matches Host", () => { + expect(publicHostsFrom("https://Paddock.Example.COM", null)).toEqual(["paddock.example.com"]); +}); + +/** + * The refusal has TWO causes that need different fixes, and telling an operator + * the wrong one costs them the afternoon. A mismatched `Origin`/`Host` means a + * proxy is rewriting `Host`; a matched pair refused anyway means `publicUrl` + * names a hostname this deployment is not actually reached on. The message that + * blames a proxy for the second is worse than no message. + */ +test("refusalReason distinguishes a rewriting proxy from a wrong publicUrl", () => { + // Origin and Host agree, and the allowlist is what refused: publicUrl's fault. + expect(refusalReason("https://paddock.example.com", "paddock.example.com", ["stale.example"])) + .toBe("host-not-allowed"); + // Origin and Host disagree: a hostile page, or a proxy that rewrote Host. + expect(refusalReason("https://evil.example", "paddock.example.com", [])).toBe("cross-origin"); + expect(refusalReason(null, "paddock.example.com", [])).toBe("no-origin"); +});