From 3e9594b3b8e7d1353290e0c4ac7cdaa8af1fd53e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 13:53:54 +0200 Subject: [PATCH] test(guards): read whole calls in the Bearer scope guard The guard's matchers stopped at the first closing parenthesis and knew one spelling of a Bearer lookup. requireRecordAuth(need(), "x", { scope: X }) read as naming no scope, a findFirst or findUniqueOrThrow by tokenHash was invisible, and the /mcp route sat on the allowlist only because a comment named the resolver. Calls are now parsed whole, comments stripped, with the first requireAuth argument read as the scope and any later options ignored. A Bearer resolution is the shared resolver, the MCP edge's wrapper, any apiToken find whose arguments name tokenHash, or a raw SELECT by token_hash. The stricter matcher found one resolver the guard never saw: the local stdio MCP entrypoint, now on the allowlist with its reason. Planted cases prove each matcher fires. --- .../bearer-scope-enforcement-guard.test.ts | 242 +++++++++++++++--- 1 file changed, 205 insertions(+), 37 deletions(-) diff --git a/src/__tests__/bearer-scope-enforcement-guard.test.ts b/src/__tests__/bearer-scope-enforcement-guard.test.ts index 5b2562c98..6c0298f4b 100644 --- a/src/__tests__/bearer-scope-enforcement-guard.test.ts +++ b/src/__tests__/bearer-scope-enforcement-guard.test.ts @@ -40,6 +40,177 @@ function filesMatching(re: RegExp): string[] { return sourceFiles().filter((rel) => re.test(read(rel))); } +/** + * The argument text of every call to `callee` in `src`, parens balanced. + * + * The matchers this file used to rely on stopped at the first `)` — + * `requireRecordAuth\([^)]*scope:` read `requireRecordAuth(need(), "x", { + * scope: X })` as having no scope at all — and demanded one spelling of a + * Bearer lookup. A guard whose matcher misses a real call site is green + * because it matched nothing. So calls are read whole: from the `(` after the + * name to its matching `)`, skipping string and template contents, and a + * function DECLARATION of the same name is not a call. + */ +function callArgs(src: string, callee: string): string[] { + const code = stripComments(src); + const out: string[] = []; + const re = new RegExp(`(? { + return callArgs(src, "requireAuth").map((args) => { + const first = topLevelArgs(args)[0]; + return first === undefined || first === "undefined" ? null : first; + }); +} + +/** The scopes the `requireRecordAuth` calls in `src` name in their options. */ +function recordAuthScopes(src: string): string[] { + const out: string[] = []; + for (const args of callArgs(src, "requireRecordAuth")) { + for (const m of args.matchAll(/\bscope\s*:\s*([A-Za-z_$][\w$]*)/g)) { + out.push(m[1]); + } + } + return out; +} + +/** + * Whether `src` turns a Bearer value into an identity: the shared resolver, or + * any `apiToken` lookup (`findUnique`, `findFirst`, `findMany`, their + * `OrThrow` forms, on `prisma` or a transaction) whose arguments name + * `tokenHash`, or raw SQL reading `api_tokens` by `token_hash`. + */ +function resolvesBearer(src: string): boolean { + const code = stripComments(src); + // The shared resolver, and the MCP edge's wrapper around it. The `/mcp` + // route was on the allowlist only because a COMMENT named the resolver; + // with comments stripped it is found by the call it actually makes. + if (/\bresolveBearerToken\b|\bresolveMcpAuthContext\s*\(/.test(code)) { + return true; + } + const lookups = + /\bapiToken\s*\.\s*find(?:Unique|First|Many)(?:OrThrow)?\s*\(/g; + for (const m of code.matchAll(lookups)) { + const args = argsFrom(code, m.index + m[0].length - 1); + if (/\btokenHash\b/.test(args)) return true; + } + // A raw read of the table by hash. An UPDATE that revokes by hash turns + // nothing into an identity and is not one. + return /\bSELECT\b[\s\S]{0,300}?\bFROM\s+"?api_tokens"?[\s\S]{0,300}?\btoken_hash\b/i.test( + code, + ); +} + +describe("the call matchers read whole calls", () => { + it("finds a scope behind nested parentheses and across lines", () => { + const planted = `const ctx = await requireRecordAuth( + needFor(method()), + "measurements", + { scope: MEASUREMENTS_WRITE_SCOPE }, + );`; + expect(recordAuthScopes(planted)).toEqual(["MEASUREMENTS_WRITE_SCOPE"]); + // The old matcher stopped at the first ")" and saw nothing. + expect(/requireRecordAuth\([^)]*\bscope:/.test(planted)).toBe(false); + }); + + it("reads the first requireAuth argument and ignores the options", () => { + expect( + requireAuthScopes( + 'await requireAuth(WORKOUTS_WRITE_SCOPE, { writeBucket: "own" });\n' + + 'await requireAuth(undefined, { writeBucket: "own" });\n' + + "await requireAuth();\n" + + "export async function requireAuth(requiredPermission?: string) {}\n" + + 'await requireAuth("health:write");', + ), + ).toEqual(["WORKOUTS_WRITE_SCOPE", null, null, '"health:write"']); + }); + + it("recognises every spelling of a Bearer lookup", () => { + for (const planted of [ + "prisma.apiToken.findUnique({ where: { tokenHash: h } })", + "prisma.apiToken\n .findUnique({\n where: { userId, tokenHash: hash },\n })", + "tx.apiToken.findFirst({ where: { revokedAt: null, tokenHash: digest(x) } })", + "await prisma.apiToken.findUniqueOrThrow({ where: { tokenHash } })", + "prisma.$queryRaw`SELECT user_id FROM api_tokens WHERE token_hash = ${h}`", + ]) { + expect(resolvesBearer(planted), planted).toBe(true); + } + // A lookup by something else is not a Bearer resolution, even with the + // word elsewhere in the file, and neither is a revocation by hash. + expect( + resolvesBearer( + "prisma.apiToken.findMany({ where: { userId } });\nconst tokenHash = 1;", + ), + ).toBe(false); + expect( + resolvesBearer( + "prisma.$executeRaw`UPDATE api_tokens SET revoked = true WHERE token_hash = ${h}`", + ), + ).toBe(false); + }); +}); + describe("T1 — the Bearer resolution set is frozen", () => { /** * Files allowed to turn a raw `Authorization: Bearer` value into a user. @@ -58,6 +229,13 @@ describe("T1 — the Bearer resolution set is frozen", () => { "lib/mcp/auth.ts", // The `/mcp` transport, via `resolveMcpAuthContext`. "app/mcp/route.ts", + // The local stdio MCP entrypoint, via the same `resolveMcpAuthContext`: a + // pasted token for a power-user's own MCP client, run in a plain Node + // process outside the web server. Same edge, same posture as `/mcp`. It + // was a Bearer resolution this guard never saw, because the matcher only + // knew the resolver's own name and the `/mcp` entry above was matched + // through a comment. + "cli/mcp-stdio.ts", // Hand-rolled Bearer authentication: the external medication-ingest // surface, which gates on BOTH `medication:ingest` and the per-medication // `medication::ingest` grant and never touches `requireAuth`. @@ -90,9 +268,7 @@ describe("T1 — the Bearer resolution set is frozen", () => { // `tokenHash` may sit anywhere inside the `where` object — demanding it // first would let a formatting change (another key ahead of it) walk a // resolver out of the guard's sight. - const resolvers = filesMatching( - /resolveBearerToken|apiToken\s*\.\s*findUnique\(\s*\{[\s\S]*?where:\s*\{[^}]*?tokenHash/, - ); + const resolvers = sourceFiles().filter((rel) => resolvesBearer(read(rel))); // Non-zero proof: an empty match set must fail rather than agree with an // emptied allowlist. @@ -207,18 +383,12 @@ describe("T3 — the mint sites are frozen", () => { }); describe("T4 — declared scopes are exported constants, never string literals", () => { - it("every requireAuth argument is an identifier", () => { + it("every requireAuth scope argument is an identifier", () => { const offenders: string[] = []; for (const rel of sourceFiles()) { - const src = read(rel); - // Call sites only — skip the declaration in `api-handler.ts`, whose - // parameter list is not an argument. - for (const m of src.matchAll( - /(?(); for (const rel of sourceFiles()) { const src = read(rel); - for (const m of src.matchAll( - /(? { // an importer can skip a download. Nothing else: not the list or detail, // not the original or thumbnail, not bulk, not the AI legs. A third route // here is a widening and belongs in review. - const declaring = filesMatching( - /requireAuth\(\s*DOCUMENTS_WRITE_SCOPE\s*\)|requireRecordAuth\([^)]*\bscope:\s*DOCUMENTS_WRITE_SCOPE\b/, - ); + const declaring = sourceFiles().filter((rel) => { + const src = read(rel); + return ( + requireAuthScopes(src).includes("DOCUMENTS_WRITE_SCOPE") || + recordAuthScopes(src).includes("DOCUMENTS_WRITE_SCOPE") + ); + }); expect(declaring).toEqual([ "app/api/documents/inbound/route.ts", "app/api/documents/inbound/source/route.ts", @@ -277,7 +445,7 @@ describe("T4b — `documents:write` opens exactly two doors", () => { // name it. const upload = read("app/api/documents/inbound/route.ts"); expect( - upload.match(/requireAuth\(\s*DOCUMENTS_WRITE_SCOPE\s*\)/g), + requireAuthScopes(upload).filter((x) => x === "DOCUMENTS_WRITE_SCOPE"), ).toHaveLength(1); // And the lookup is a read of one key and nothing else: a single GET. const lookup = read("app/api/documents/inbound/source/route.ts"); @@ -294,13 +462,17 @@ describe("T4c — `workouts:write` opens exactly one door", () => { // The set of files naming the scope IS its reach. The batch ingest and // nothing else: not the workout list or detail, which would hand a // bridge the history it has no business reading back. - const declaring = filesMatching( - /requireAuth\(\s*WORKOUTS_WRITE_SCOPE\s*\)|requireRecordAuth\([^)]*\bscope:\s*WORKOUTS_WRITE_SCOPE\b/, - ); + const declaring = sourceFiles().filter((rel) => { + const src = read(rel); + return ( + requireAuthScopes(src).includes("WORKOUTS_WRITE_SCOPE") || + recordAuthScopes(src).includes("WORKOUTS_WRITE_SCOPE") + ); + }); expect(declaring).toEqual(["app/api/workouts/batch/route.ts"]); const batch = read("app/api/workouts/batch/route.ts"); expect( - batch.match(/requireAuth\(\s*WORKOUTS_WRITE_SCOPE\s*\)/g), + requireAuthScopes(batch).filter((x) => x === "WORKOUTS_WRITE_SCOPE"), ).toHaveLength(1); expect( [...batch.matchAll(/export const (GET|POST|PUT|PATCH|DELETE)\b/g)].map( @@ -330,8 +502,8 @@ describe("T5 — a delegable route naming a Bearer scope is frozen", () => { }; it("only the known routes pass a scope to the record resolver", () => { - const naming = filesMatching( - /requireRecordAuth\([^)]*\bscope:\s*[A-Za-z_$][\w$]*/, + const naming = sourceFiles().filter( + (rel) => recordAuthScopes(read(rel)).length > 0, ); // Non-zero proof: an empty match set must fail rather than agree with an // emptied table. This is the leg most likely to rot — the matcher spans a @@ -343,11 +515,7 @@ describe("T5 — a delegable route naming a Bearer scope is frozen", () => { it("each names the scope this table says it does", () => { for (const [rel, scope] of Object.entries(SCOPED_RECORD_ROUTES)) { - const named = [ - ...read(rel).matchAll( - /requireRecordAuth\([^)]*\bscope:\s*([A-Za-z_$][\w$]*)/g, - ), - ].map((m) => m[1]); + const named = recordAuthScopes(read(rel)); expect(named).toEqual([scope]); } });