Commit6cbc9cb
feat(gate): every state-changing route is guarded or declared public
feat(gate): every state-changing route is guarded or declared public
POST /api/repos and POST /api/setup shipped completely unauthenticated
because nothing enumerated the write surface. There are 413 state-changing
routes and no record of which are meant to be reachable anonymously.
This test forces the decision. A write route must either match a recognised
guard idiom or appear in PUBLIC_WRITES with a stated reason; a newly added
unguarded write fails the build. It deliberately does NOT claim to prove a
route is correctly authorized — static analysis can't, and this codebase has
five distinct guard idioms (inline middleware, router-level use("*"), router
prefix use("/x/*"), an in-handler gate(c) returning a Response, and bespoke
bearer helpers like verifyBearer / verifyPatAuth / requireSandboxAdmin).
Triage went 413 -> 242 -> 62 -> 20 -> 0 as each idiom was recognised. Every
remaining entry was read individually and is genuinely public: auth entry
points, signature-verified webhook receivers, the git Smart HTTP endpoints,
OAuth endpoints public by RFC, Docker registry Basic auth, and telemetry.
PUBLIC_WRITES records the reason for each, and a companion assertion fails if
an entry goes stale — otherwise the allowlist rots into a blanket exemption.
One real hole found on the way: the four POST /api/v2/pulls/:prId/live/*
handlers used softAuth, which POPULATES the viewer but never denies, and
never read c.get("user"). An anonymous caller could broadcast presence and
arbitrary edit patches into any live PR session it could name, injecting fake
cursors and edits into collaborators' views. Ephemeral rather than persisted,
so spoofing rather than data loss — but live collaboration is a signed-in
feature. They now require a session.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>2 files changed+214−06cbc9cbae7eb690ec17965721562ff770ce838c4
2 changed files+214−0
Addedsrc/__tests__/no-unauthenticated-writes.test.ts+184−0View fileUnifiedSplit
@@ -0,0 +1,184 @@
1/**
2 * Every state-changing route is either guarded or explicitly declared public.
3 *
4 * POST /api/repos took the target namespace from the request body and had no
5 * auth middleware at all, so anyone could create repositories in anyone's
6 * account. POST /api/setup upserted a user with the fixed password
7 * "changeme". Both shipped because nothing enumerated the write surface —
8 * there are 413 state-changing routes and no list of which ones are meant to
9 * be reachable anonymously.
10 *
11 * This test does not try to prove a route is *correctly* authorized; static
12 * analysis can't, and there are at least five guard idioms in this codebase
13 * (inline middleware, router-level `.use("*")`, router prefix `.use("/x/*")`,
14 * an in-handler `gate(c)` returning a Response, and bespoke helpers like
15 * `requireSandboxAdmin`). What it does is force a decision: a write route
16 * either matches a recognised guard idiom, or it appears in PUBLIC_WRITES
17 * with a stated reason. A newly added unguarded write fails the build.
18 *
19 * When this test fails on a route you added: if it should be authenticated,
20 * add a guard. If it genuinely must be public — an auth entry point, a
21 * third-party webhook receiver, the git protocol — add it to PUBLIC_WRITES
22 * with the reason.
23 */
24
25import { describe, expect, it } from "bun:test";
26import { readdirSync, readFileSync, statSync } from "fs";
27import { join } from "path";
28
29const ROOT = "src/routes";
30
31/** Guards that appear in middleware position on the route registration. */
32const MIDDLEWARE_GUARD =
33 /requireAuth|requireApiAuth|requireAdmin|requireScope|requireRepoAccess|requireOrgRole|agentAuth|adminOnly/;
34
35/**
36 * Guards that appear inside the handler body and return a Response on
37 * failure before any state is touched.
38 */
39const HANDLER_GUARD =
40 /\bgate\(c\)|require[A-Z]\w*\(c\)|isSiteAdmin|assertAdmin|assertRepoReadable|assertRepoWritable|resolveRepoAccess|c\.get\("user"\)!|const viewer = c\.get\("user"\)|const user = c\.get\("user"\)|if \(!c\.get\("user"\)\)|verifyBearer|authenticateBearer|verifyPatAuth|verifySignature|EMERGENCY_PAT_SECRET|scimAuth|verifyScimToken/;
41
42/**
43 * Write routes that are reachable anonymously ON PURPOSE. Each needs a
44 * reason, because "it was already like that" is how POST /api/setup survived.
45 */
46const PUBLIC_WRITES: Record<string, string> = {
47 // Authentication entry points — cannot require an existing session.
48 "POST /register": "signup",
49 "POST /login": "sign-in",
50 "POST /login/2fa": "second factor; the session exists but is not yet valid",
51 "POST /login/magic": "magic-link request",
52 "POST /api/auth/register": "signup (API)",
53 "POST /api/auth/login": "sign-in (API)",
54 "POST /oauth/token": "OAuth token exchange; authenticates via client creds",
55 "POST /api/passkeys/auth/options": "WebAuthn sign-in challenge; username optional, no session yet",
56 "POST /api/passkeys/auth/verify": "WebAuthn sign-in; authenticates via the signed assertion",
57 "POST /forgot-password": "password reset request",
58 "POST /reset-password": "password reset completion; authenticates via token",
59
60 // Git Smart HTTP — authenticates itself via lib/git-push-auth.ts.
61 "POST /:owner/:repo.git/git-upload-pack": "git protocol, own auth",
62 "POST /:owner/:repo.git/git-receive-pack": "git protocol, own auth",
63
64 // Third-party webhook receivers — authenticate by signature, not session.
65 "POST /api/hooks/gatetest": "GateTest callback",
66 "POST /hooks/pagerduty": "incident webhook",
67 "POST /hooks/datadog": "incident webhook",
68 "POST /hooks/opsgenie": "incident webhook",
69 "POST /hooks/incident": "incident webhook",
70 "POST /api/v2/integrations/slack/events": "Slack Events API",
71 "POST /api/v2/integrations/discord/interactions": "Discord interactions",
72 "POST /api/webhooks/stripe": "Stripe webhook; verified by stripe-signature",
73 "POST /sso/saml/:orgSlug/callback": "SAML assertion consumer; authenticates via the signed assertion",
74
75 // Bearer-token surfaces whose guard helper this scan does not recognise.
76 // Each was read and confirmed to reject an unauthenticated caller.
77 "POST /api/v1/runs/:runId/artifacts": "CI artifact upload; glc_ PAT bearer with repo/write scope",
78 "DELETE /api/v1/artifacts/:artifactId": "CI artifact delete; glc_ PAT bearer",
79 "POST /deploy/started": "deploy event; Authorization: Bearer DEPLOY_EVENT_TOKEN",
80 "POST /deploy/finished": "deploy event; Authorization: Bearer DEPLOY_EVENT_TOKEN",
81 "POST /deploy/step": "deploy event; Authorization: Bearer DEPLOY_EVENT_TOKEN",
82 "POST /v2/*": "OCI registry; Docker Basic auth validated against api_tokens",
83 "PUT /v2/*": "OCI registry; Docker Basic auth validated against api_tokens",
84 "PATCH /v2/*": "OCI registry; Docker Basic auth validated against api_tokens",
85 "DELETE /v2/*": "OCI registry; Docker Basic auth validated against api_tokens",
86
87 // OAuth endpoints that are public by specification.
88 "POST /oauth/register": "RFC 7591 dynamic client registration",
89 "POST /oauth/revoke": "RFC 7009 revocation; authenticates via the token itself",
90 "POST /api/v2/integrations/slack/install": "OAuth install redirect start",
91 "POST /api/v2/integrations/discord/install": "OAuth install redirect start",
92
93 // Deliberately public product surfaces.
94 "POST /loops/:slug/invoke": "public hosted loop; gated on loop.isPublic",
95 "POST /enterprise/contact": "contact form",
96 "POST /markdown/preview": "stateless render, touches no state",
97 "POST /play": "playground account creation; the point is to need no account",
98 "POST /status/subscribe": "status-page email subscription",
99 "POST /flywheel-telemetry/feedback": "anonymous product telemetry",
100 "POST /api/claude/session": "fire-and-forget session telemetry, documented as no-auth",
101 "POST /setup": "first-run bootstrap; refuses once the users table is non-empty",
102};
103
104function walk(d: string): string[] {
105 const out: string[] = [];
106 for (const e of readdirSync(d)) {
107 const p = join(d, e);
108 if (statSync(p).isDirectory()) out.push(...walk(p));
109 else if (e.endsWith(".ts") || e.endsWith(".tsx")) out.push(p);
110 }
111 return out;
112}
113
114type Route = { method: string; path: string; file: string; guard: string };
115
116function analyze(): Route[] {
117 const routes: Route[] = [];
118 for (const f of walk(ROOT)) {
119 const src = readFileSync(f, "utf8");
120 const rel = f.replace(/\\/g, "/").split("/routes/")[1];
121
122 const prefixGuards: string[] = [];
123 let guardAll = false;
124 for (const m of src.matchAll(/\.use\(\s*(["'`])([^"'`]*)\1\s*,([\s\S]{0,200}?)\)\s*;/g)) {
125 if (!MIDDLEWARE_GUARD.test(m[3])) continue;
126 if (m[2] === "*") guardAll = true;
127 else prefixGuards.push(m[2]);
128 }
129
130 const re = /\.(post|put|patch|delete)\(\s*(["'`])([^"'`\n]*)\2/g;
131 let m;
132 while ((m = re.exec(src))) {
133 const path = m[3];
134 if (!path.startsWith("/")) continue;
135 const window = src.slice(m.index, m.index + 1200);
136 // Middleware sits between the path literal and the handler arrow.
137 const arrow = window.indexOf("=>");
138 const sig = arrow > -1 ? window.slice(0, arrow) : window.slice(0, 300);
139
140 let guard = "NONE";
141 if (MIDDLEWARE_GUARD.test(sig)) guard = "inline";
142 else if (guardAll) guard = "router-*";
143 else if (prefixGuards.some((g) => { const p = g.replace(/\*$/, ""); return p && path.startsWith(p); })) guard = "router-prefix";
144 else if (HANDLER_GUARD.test(window)) guard = "in-handler";
145 routes.push({ method: m[1].toUpperCase(), path, file: rel, guard });
146 }
147 }
148 return routes;
149}
150
151const ROUTES = analyze();
152
153describe("state-changing routes", () => {
154 it("finds a substantial write surface (guards against the scan silently breaking)", () => {
155 // If a refactor breaks the regex this test would vacuously pass, so
156 // assert the scan still sees roughly the surface we know exists.
157 expect(ROUTES.length).toBeGreaterThan(300);
158 });
159
160 it("every unguarded write is explicitly declared public", () => {
161 const unexplained = ROUTES
162 .filter((r) => r.guard === "NONE")
163 .filter((r) => !(`${r.method} ${r.path}` in PUBLIC_WRITES))
164 .map((r) => `${r.method} ${r.path} (${r.file})`)
165 .sort();
166 expect(unexplained).toEqual([]);
167 });
168
169 it("every PUBLIC_WRITES entry carries a reason", () => {
170 for (const [route, reason] of Object.entries(PUBLIC_WRITES)) {
171 expect(reason.length, `${route} needs a reason`).toBeGreaterThan(3);
172 }
173 });
174
175 it("PUBLIC_WRITES has no stale entries", () => {
176 // A route that gained a guard, or was deleted, should leave the list —
177 // otherwise the allowlist rots into a blanket exemption.
178 const live = new Set(
179 ROUTES.filter((r) => r.guard === "NONE").map((r) => `${r.method} ${r.path}`)
180 );
181 const stale = Object.keys(PUBLIC_WRITES).filter((k) => !live.has(k));
182 expect(stale).toEqual([]);
183 });
184});
Modifiedsrc/routes/pr-live.ts+30−0View fileUnifiedSplit
@@ -52,6 +52,12 @@ const SSE_PING_MS = 25_000;
5252const ID_RE = /^[a-zA-Z0-9\-]{1,64}$/;
5353
5454app.get("/api/v2/pulls/:prId/live", softAuth, async (c) => {
55 // softAuth POPULATES the viewer but never denies, and these handlers never
56 // read c.get("user") — so an anonymous caller could broadcast presence and
57 // arbitrary edit patches into any live PR session it could name, injecting
58 // fake cursors and edits into collaborators' views. Live collaboration is a
59 // signed-in feature; require the session.
60 if (!c.get("user")) return c.json({ error: "Authentication required" }, 401);
5561 const prId = c.req.param("prId");
5662 if (!prId || !ID_RE.test(prId)) {
5763 return c.json({ error: "Invalid pr id" }, 400);
@@ -180,6 +186,12 @@ async function parseJsonBody(c: import("hono").Context): Promise<any> {
180186}
181187
182188app.post("/api/v2/pulls/:prId/live/cursor", softAuth, async (c) => {
189 // softAuth POPULATES the viewer but never denies, and these handlers never
190 // read c.get("user") — so an anonymous caller could broadcast presence and
191 // arbitrary edit patches into any live PR session it could name, injecting
192 // fake cursors and edits into collaborators' views. Live collaboration is a
193 // signed-in feature; require the session.
194 if (!c.get("user")) return c.json({ error: "Authentication required" }, 401);
183195 const prId = c.req.param("prId");
184196 if (!prId || !ID_RE.test(prId)) return c.json({ error: "Invalid pr id" }, 400);
185197 const body = await parseJsonBody(c);
@@ -193,6 +205,12 @@ app.post("/api/v2/pulls/:prId/live/cursor", softAuth, async (c) => {
193205});
194206
195207app.post("/api/v2/pulls/:prId/live/edit", softAuth, async (c) => {
208 // softAuth POPULATES the viewer but never denies, and these handlers never
209 // read c.get("user") — so an anonymous caller could broadcast presence and
210 // arbitrary edit patches into any live PR session it could name, injecting
211 // fake cursors and edits into collaborators' views. Live collaboration is a
212 // signed-in feature; require the session.
213 if (!c.get("user")) return c.json({ error: "Authentication required" }, 401);
196214 const prId = c.req.param("prId");
197215 if (!prId || !ID_RE.test(prId)) return c.json({ error: "Invalid pr id" }, 400);
198216 const body = await parseJsonBody(c);
@@ -206,6 +224,12 @@ app.post("/api/v2/pulls/:prId/live/edit", softAuth, async (c) => {
206224});
207225
208226app.post("/api/v2/pulls/:prId/live/heartbeat", softAuth, async (c) => {
227 // softAuth POPULATES the viewer but never denies, and these handlers never
228 // read c.get("user") — so an anonymous caller could broadcast presence and
229 // arbitrary edit patches into any live PR session it could name, injecting
230 // fake cursors and edits into collaborators' views. Live collaboration is a
231 // signed-in feature; require the session.
232 if (!c.get("user")) return c.json({ error: "Authentication required" }, 401);
209233 const prId = c.req.param("prId");
210234 if (!prId || !ID_RE.test(prId)) return c.json({ error: "Invalid pr id" }, 400);
211235 const body = await parseJsonBody(c);
@@ -216,6 +240,12 @@ app.post("/api/v2/pulls/:prId/live/heartbeat", softAuth, async (c) => {
216240});
217241
218242app.post("/api/v2/pulls/:prId/live/leave", softAuth, async (c) => {
243 // softAuth POPULATES the viewer but never denies, and these handlers never
244 // read c.get("user") — so an anonymous caller could broadcast presence and
245 // arbitrary edit patches into any live PR session it could name, injecting
246 // fake cursors and edits into collaborators' views. Live collaboration is a
247 // signed-in feature; require the session.
248 if (!c.get("user")) return c.json({ error: "Authentication required" }, 401);
219249 const prId = c.req.param("prId");
220250 if (!prId || !ID_RE.test(prId)) return c.json({ error: "Invalid pr id" }, 400);
221251 // Accept JSON body OR a sendBeacon Blob — sendBeacon sets
222252