Public: anyone can read this; the crew can change it.

Every kind of response carries the security floor and every mutation checks its origin, proven by two route-walking tests #10931

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

mason/forge:mason/10918-every-kind-of-response-carries-the-secur → main 1 commit fork

  • Merged. mason/10918-every-kind-of-response-carries-the-secur was merged into main as a9a1941.

Review

Sign in to review this change.
2 files changed +212 −16
test/cross-origin-mutations.test.ts → test/origin-every-mutation.test.ts renamed +37 −16
@@ -1,11 +1,16 @@
11 /**
2 * Same-origin on every mutation, tested as a whole (#5848). Every POST, PUT,
3 * PATCH and DELETE either router registers is sent signed in with a session
4 * cookie and `Origin: https://evil.example`; each must answer 403 and leave
5 * the event log as it was. Routes a bearer token alone opens are exempt by
6 * one rule (BEARER_ONLY) and proven to refuse a cookie-only request. Routes
7 * the test cannot reach are named in UNCALLABLE with why, and that list is
8 * checked against the routers both ways.
2 * Same-origin on every mutation, tested as a whole (#5848, #10918). Every POST,
3 * PUT, PATCH and DELETE either router registers is sent signed in with a
4 * session cookie and `Origin: https://evil.example`; each must answer 403,
5 * leave the event log as it was, and have been refused by an origin check
6 * (`crossOrigin` in src/http/context.ts, which the gate in
7 * src/http/origin_gate.ts and the handlers' second locks share) — a 403 for
8 * any other reason would hide a route with no check at all.
9 *
10 * A route with no origin check is named in EXEMPT, one line each with its
11 * reason, and proven to refuse a cookie-only request. Routes the test cannot
12 * reach are named in UNCALLABLE with why. Both lists are checked against the
13 * routers, so neither can name a route that is gone or grow without a reason.
914 */
1015 import { afterAll, beforeAll, expect, test } from "bun:test";
1116 import { mkdtemp, rm } from "node:fs/promises";
@@ -23,11 +28,16 @@ import { quietLogs } from "./helpers/globals.ts";
2328 quietLogs();
2429
2530 /**
26 * The rule: these routes read only `Authorization: Bearer`, never a cookie, so
27 * a browser cannot be made to call them as someone. A cookie alone must get a
28 * 401 from them, which the second test proves.
31 * Mutations with no origin check, each with why. Every one reads only
32 * `Authorization: Bearer`, never a cookie, so a browser cannot be made to call
33 * it as someone: a cookie alone must get a 401 (or 405), which a test proves.
34 * Add a line here only with a reason a reviewer can check.
2935 */
30 const BEARER_ONLY = new Set(["POST /mcp", "DELETE /mcp", "POST /oauth/userinfo"]);
36 const EXEMPT: Record<string, string> = {
37 "POST /mcp": "the MCP endpoint reads a bearer token only; a cookie is never an identity there",
38 "DELETE /mcp": "ends an MCP session named by a bearer's session header; a cookie is never an identity there",
39 "POST /oauth/userinfo": "OAuth userinfo answers the bearer access token it is sent, never a cookie",
40 };
3141
3242 /** Routes the test cannot call, with why. Each must be a registered mutation. */
3343 const UNCALLABLE: Record<string, string> = {};
@@ -45,6 +55,8 @@ const mutations = (): { method: string; pattern: string }[] =>
4555 [...buildRouter().routes(), ...buildApiRouter().routes()].filter((r) => MUTATING.has(r.method));
4656
4757 let dataDir = "", app: App, cookie = "";
58 /** How many times an origin check has refused a request, read from the log line `crossOrigin` writes. */
59 let originRefusals = 0;
4860 const cells = process.env["FORGE_CELLS"];
4961
5062 beforeAll(async () => {
@@ -66,6 +78,11 @@ beforeAll(async () => {
6678 const pull = await openPull(app, admin.value, repo.value, { title: "t", body: "", baseBranch: "main", headBranch: "topic" });
6779 if (!pull.ok) throw new Error(JSON.stringify(pull.error));
6880 cookie = `forge_session=${createSession(app.db, admin.value.id).token}`;
81 const warn = app.log.warn.bind(app.log);
82 app.log.warn = (message, fields) => {
83 if (message === "refused cross-origin mutation") originRefusals++;
84 warn(message, fields);
85 };
6986 });
7087
7188 afterAll(async () => {
@@ -92,18 +109,22 @@ test("every mutating route refuses a signed-in request from a foreign Origin, wr
92109 expect(routes.length).toBeGreaterThan(100);
93110 const wrong: string[] = [];
94111 for (const r of routes) {
95 if (BEARER_ONLY.has(key(r)) || key(r) in UNCALLABLE) continue;
112 if (key(r) in EXEMPT || key(r) in UNCALLABLE) continue;
96113 const before = events();
114 const refusedBefore = originRefusals;
97115 const status = await send(r, "https://evil.example");
98 if (status !== 403 || events() !== before) wrong.push(`${key(r)} → ${status}, ${events() - before} events`);
116 const checked = originRefusals > refusedBefore;
117 if (status !== 403 || events() !== before || !checked)
118 wrong.push(`${key(r)} → ${status}, ${events() - before} events${checked ? "" : ", no origin check"}`);
99119 }
100120 expect(wrong).toEqual([]);
101121 });
102122
103 test("bearer-only routes carry no cookie: a cookie alone is refused", async () => {
123 test("every exempt route is a registered mutation with a reason, and a cookie alone is refused there", async () => {
104124 const registered = new Set(mutations().map(key));
105 for (const route of BEARER_ONLY) {
125 for (const [route, why] of Object.entries(EXEMPT)) {
106126 expect(registered.has(route)).toBe(true);
127 expect(why.length).toBeGreaterThan(20);
107128 const [method, pattern] = route.split(" ") as [string, string];
108129 const before = events();
109130 // 401 for no credential; 405 where the route answers nothing but a bearer's POST.
@@ -116,7 +137,7 @@ test("the uncallable list names only registered routes, each with a reason", ()
116137 const registered = new Set(mutations().map(key));
117138 for (const [route, why] of Object.entries(UNCALLABLE)) {
118139 expect(registered.has(route)).toBe(true);
119 expect(why.length).toBeGreaterThan(0);
140 expect(why.length).toBeGreaterThan(20);
120141 }
121142 });
122143
test/security-floor-every-route.test.ts added +175
@@ -0,0 +1,175 @@
1 /**
2 * The security floor (`securityFloor` in src/http/respond.ts) on every kind of
3 * response, not just every GET route (#10918): a page, a JSON answer, each
4 * shape of refusal and error, the 404s, the 304s, the assets, HEAD and
5 * OPTIONS, a redirect, the probes and a 500. `security-floor-routes.test.ts`
6 * walks the GET route table; this one walks the kinds of answer, which is
7 * where a responder that skips the floor would hide — a 304 with no body, a
8 * refusal written before any route, a page that writes its own policy.
9 *
10 * Each sample names its kind and the status it must answer with, so a sample
11 * that drifted to another kind of answer fails loudly instead of proving
12 * nothing. The three headers asserted are the issue's; HTML writes its own
13 * nonce'd policy and the floor fills the rest.
14 */
15 import { afterAll, beforeAll, expect, test } from "bun:test";
16 import { mkdtemp, rm } from "node:fs/promises";
17 import { tmpdir } from "node:os";
18 import { join } from "node:path";
19 import { createApp, type App } from "../src/app.ts";
20 import { commitToBranch } from "../src/actions/repos.ts";
21 import { createRepo } from "../src/core/repos.ts";
22 import { createSession } from "../src/core/sessions.ts";
23 import { serve, type Servers } from "../src/http/server.ts";
24 import { spriteAsset, stylesheetAsset } from "../src/web/asset.ts";
25 import { seedAdmin } from "./helpers/admin.ts";
26 import { quietLogs } from "./helpers/globals.ts";
27
28 quietLogs();
29
30 const FLOOR = ["content-security-policy", "x-content-type-options", "referrer-policy"] as const;
31
32 type Sample = {
33 kind: string;
34 path: string | (() => string);
35 status: number;
36 init?: RequestInit;
37 signedIn?: boolean;
38 /** Headers the sample needs that only exist once the first answer is in (a 304's tag). */
39 conditional?: boolean;
40 };
41
42 let dataDir = "";
43 let app: App, servers: Servers;
44 let cookie = "";
45
46 beforeAll(async () => {
47 dataDir = await mkdtemp(join(tmpdir(), "forge-floor-kinds-"));
48 app = createApp({ dataDir, port: 0 });
49 servers = await serve(app);
50 const admin = await seedAdmin(app, { username: "ada", email: "[email protected]", password: "correct-horse-battery" });
51 if (!admin.ok) throw new Error(admin.error);
52 const made = await createRepo(app.db, app.reposRoot, { ownerId: admin.value.id, name: "notes", visibility: "public" });
53 if (!made.ok) throw new Error(String(made.error));
54 const seed = await commitToBranch(app, admin.value, made.value, { message: "seed", files: [{ path: "a.txt", content: "a\n" }] });
55 if (!seed.ok) throw new Error(String(seed.error));
56 cookie = `forge_session=${createSession(app.db, admin.value.id).token}`;
57 }, 30_000);
58
59 afterAll(async () => {
60 servers?.stop();
61 app?.close();
62 await rm(dataDir, { recursive: true, force: true });
63 });
64
65 const FOREIGN = { origin: "https://evil.example", "content-type": "application/json" };
66
67 const SAMPLES: Sample[] = [
68 { kind: "page", path: "/", status: 200 },
69 { kind: "page, signed in", path: "/", status: 200, signedIn: true },
70 { kind: "repository page", path: "/ada/notes", status: 200 },
71 { kind: "HEAD of a page", path: "/", status: 200, init: { method: "HEAD" } },
72 { kind: "JSON answer", path: "/api/v1/repos/ada/notes", status: 200 },
73 { kind: "JSON answer, signed in", path: "/api/v1/user", status: 200, signedIn: true },
74 { kind: "plain text", path: "/robots.txt", status: 200 },
75 { kind: "stylesheet asset", path: () => `/assets/${stylesheetAsset().name}.css`, status: 200 },
76 { kind: "sprite asset", path: () => `/assets/${spriteAsset().name}.svg`, status: 200 },
77 { kind: "304 of an asset", path: () => `/assets/${stylesheetAsset().name}.css`, status: 304, conditional: true },
78 { kind: "304 of a JSON answer", path: "/api/v1/repos/ada/notes/issues", status: 304, conditional: true },
79 { kind: "404 page", path: "/no/such/path/at/all", status: 404 },
80 { kind: "404 of a repository", path: "/ada/nothing-here", status: 404 },
81 { kind: "404 JSON, no route", path: "/api/v1/no-such-route", status: 404 },
82 { kind: "404 JSON, no resource", path: "/api/v1/repos/ada/nothing-here", status: 404 },
83 { kind: "404 of an asset", path: "/assets/not-a-real-hash.css", status: 404 },
84 { kind: "404 of a stale asset hash", path: "/assets/stale.js", status: 404 },
85 { kind: "405 JSON", path: "/api/v1/repos/ada/notes", status: 405, init: { method: "PUT" } },
86 { kind: "401 JSON", path: "/api/v1/user", status: 401 },
87 {
88 kind: "422 JSON, a body that is not JSON",
89 path: "/api/v1/repos/ada/notes/issues",
90 status: 422,
91 signedIn: true,
92 init: { method: "POST", headers: { "content-type": "application/json" }, body: "{not json" },
93 },
94 {
95 kind: "403 JSON, the origin gate",
96 path: "/api/v1/repos/ada/notes/issues",
97 status: 403,
98 signedIn: true,
99 init: { method: "POST", headers: FOREIGN, body: "{}" },
100 },
101 {
102 kind: "403 page, the origin gate to a form",
103 path: "/new",
104 status: 403,
105 signedIn: true,
106 init: {
107 method: "POST",
108 headers: { origin: "https://evil.example", accept: "text/html", "content-type": "application/x-www-form-urlencoded" },
109 body: "name=x",
110 },
111 },
112 { kind: "redirect", path: "/openapi.json", status: 308 },
113 { kind: "OPTIONS, answered as no route", path: "/api/v1", status: 404, init: { method: "OPTIONS" } },
114 { kind: "liveness probe", path: "/healthz", status: 200 },
115 { kind: "readiness probe", path: "/readyz", status: 200 },
116 ];
117
118 async function ask(sample: Sample, extra: Record<string, string> = {}): Promise<Response> {
119 const path = typeof sample.path === "function" ? sample.path() : sample.path;
120 const headers = new Headers(sample.init?.headers);
121 headers.set("accept-encoding", "identity");
122 if (sample.signedIn === true) headers.set("cookie", cookie);
123 for (const [k, v] of Object.entries(extra)) headers.set(k, v);
124 return fetch(`${servers.url}${path}`, {
125 ...sample.init,
126 headers,
127 redirect: "manual",
128 signal: AbortSignal.timeout(5_000),
129 });
130 }
131
132 /** What is wrong with one sample's answer: a missing floor header, or the wrong kind of answer. */
133 async function wrongWith(sample: Sample): Promise<string[]> {
134 let res = await ask(sample);
135 if (sample.conditional === true) {
136 const etag = res.headers.get("etag");
137 await res.body?.cancel();
138 if (etag === null) return [`${sample.kind}: no etag to revalidate with`];
139 res = await ask(sample, { "if-none-match": etag });
140 }
141 await res.body?.cancel();
142 const wrong = FLOOR.filter((h) => !res.headers.get(h)).map((h) => `${sample.kind} (${res.status}) lacks ${h}`);
143 if (res.status !== sample.status) wrong.push(`${sample.kind} answered ${res.status}, not ${sample.status}`);
144 return wrong;
145 }
146
147 test("every kind of response carries the floor", async () => {
148 const wrong = (await Promise.all(SAMPLES.map(wrongWith))).flat();
149 expect(wrong).toEqual([]);
150 }, 15_000);
151
152 test("a handler's throw, answered as a 500, carries the floor", async () => {
153 // The session read is the first statement a signed-in request makes; a
154 // database that throws there is the fault the 500 path exists for.
155 const query = app.db.query.bind(app.db);
156 app.db.query = (() => {
157 throw new Error("the disk went away");
158 }) as typeof app.db.query;
159 let res: Response;
160 try {
161 res = await ask({ kind: "500", path: "/api/v1/user", status: 500, signedIn: true });
162 } finally {
163 app.db.query = query;
164 }
165 await res.body?.cancel();
166 expect(res.status).toBe(500);
167 for (const h of FLOOR) expect(res.headers.get(h)).toBeTruthy();
168 });
169
170 test("the floor's own policy is the inert one on anything that is not a page", async () => {
171 const res = await ask({ kind: "JSON", path: "/api/v1/repos/ada/notes", status: 200 });
172 await res.body?.cancel();
173 expect(res.headers.get("content-security-policy")).toContain("default-src 'none'");
174 expect(res.headers.get("x-content-type-options")).toBe("nosniff");
175 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs