From a08631e0a1fd388df58786efb9134b676231036d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 1 Aug 2026 22:39:50 +0200 Subject: [PATCH 1/2] Freeze the set of surfaces that resolve a caller by any mechanism The Bearer guard freezes one mechanism: the files that turn an Authorization header into a user. It reads like coverage of "who can resolve a caller" and it is not. Two other families do the same job by other means and no guard watches either. Fifteen files under src/app read the session cookie directly instead of going through requireAuth: eight API routes and seven pages. Nine more authenticate on a token carried in the URL, including the clinician share tree, which serves one person's whole health record to an anonymous caller holding a path segment. A sweep that greps for getSession sees none of the second family. Freeze both as literal lists with a reason per entry, matched by five independent legs: the session-deriving helpers, a direct read of the session cookie name, a dynamic route segment carrying a credential, share-token resolution, and a query-string credential. Legs overlap, so each asserts its own membership and its own non-zero count. A leg whose matcher stops matching then fails instead of letting the union carry it. --- src/__tests__/session-surface-guard.test.ts | 314 ++++++++++++++++++++ 1 file changed, 314 insertions(+) create mode 100644 src/__tests__/session-surface-guard.test.ts diff --git a/src/__tests__/session-surface-guard.test.ts b/src/__tests__/session-surface-guard.test.ts new file mode 100644 index 000000000..067bd97d3 --- /dev/null +++ b/src/__tests__/session-surface-guard.test.ts @@ -0,0 +1,314 @@ +/** + * Structural guards on the surfaces that resolve a caller's identity. + * + * `bearer-scope-enforcement-guard.test.ts` freezes ONE mechanism: the files + * that turn an `Authorization: Bearer` value into a user. That guard is + * correct and it holds. What it is not is coverage of "who can resolve a + * caller". Two other families do the same job by other means, and a sweep + * written by copying the Bearer guard sees neither of them: + * + * A. A route or page that reads the session cookie itself, instead of + * going through `requireAuth()` inside `apiHandler`. `requireAuth` is + * where the fail-closed scope arm, the audit row, and the admin + * boundary live; a direct `getSession()` has none of them and answers + * to nothing but the file it sits in. + * B. A surface whose whole credential is a token in the URL — the + * clinician share tree serves one person's complete health record to + * an anonymous caller holding an `hls_` path segment, and never + * touches a session at all. No amount of grepping for `getSession` + * will ever show it to you. + * + * Neither family is wrong. Both are deliberate, and both are documented at + * their own call sites. The defect this file closes is that nothing said so + * when a new member joined. + * + * These are tripwires, not proofs. They cannot show an allowlist is + * correct — only that it has not changed without someone editing this file. + * + * A note on why every matcher carries its own count assertion: several legs + * below overlap (four of the five share-token files also sit under a + * `[token]` segment). If one leg's regex silently stopped matching, the + * union would be unchanged and the guard would stay green for the wrong + * reason. Each leg therefore asserts its own membership, so a matcher that + * matches nothing fails instead of passing. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join, sep } from "node:path"; +import { globSync } from "node:fs"; + +const SRC = join(process.cwd(), "src"); + +/** + * Every non-test `.ts` / `.tsx` under `src/app/`, which is the whole of the + * request-handling tree: API routes, RSC pages, layouts, route handlers. + * The generated Prisma client is excluded by construction (it is not under + * `app/`) and never read — see CLAUDE.md. + */ +function appFiles(): string[] { + return globSync("app/**/*.{ts,tsx}", { cwd: SRC }) + .map((p) => p.split(sep).join("/")) + .filter((p) => !p.includes("__tests__")) + .filter((p) => !p.endsWith(".test.ts") && !p.endsWith(".test.tsx")) + .sort(); +} + +function read(rel: string): string { + return readFileSync(join(SRC, rel), "utf8"); +} + +function appFilesMatching(re: RegExp): string[] { + return appFiles().filter((rel) => re.test(read(rel))); +} + +describe("S1 — the direct session-resolution set is frozen", () => { + /** + * Files under `src/app` that resolve the caller from the session cookie + * WITHOUT going through `requireAuth()`. Every other authenticated route + * in the tree — 300-odd of them — reaches its user through `apiHandler` + * plus `requireAuth`, which is what makes this list short enough to read. + * + * An entry here is not a finding. It is a place where the protections + * `requireAuth` applies centrally have to be argued locally, so the + * reason is the point of the entry. + */ + const SESSION_RESOLUTION_ALLOWLIST: Record = { + // Completes the native ASWebAuthenticationSession handoff. Validates the + // browser session itself because it also needs `session.createdAt` for + // the freshness comparison, which `requireAuth` does not project. + "app/api/auth/native/complete/route.ts": + "reads createdAt for the handoff freshness check", + // Anonymous IdP callback. Probes for an already-valid session so a + // replayed callback redirects instead of minting a second identity. + "app/api/auth/oidc/callback/route.ts": + "duplicate-callback probe on an otherwise anonymous route", + // Public liveness probe. The session read only decides whether the body + // carries operational detail; the status code is computed without it. + "app/api/health/route.ts": "admin-only detail on a public probe", + // OAuth authorize runs under `withBackgroundEvent`, not `apiHandler`, + // because it owes RFC-format error bodies — so it resolves the + // consenting human itself. + "app/api/mcp/oauth/authorize/route.ts": + "consent decision on a route outside apiHandler", + // Provider OAuth callbacks. The browser arrives as a top-level + // navigation, so the code is bound to the session user and redirected. + "app/api/oura/callback/route.ts": + "binds the OAuth code to the session user", + "app/api/polar/callback/route.ts": + "binds the OAuth code to the session user", + "app/api/strava/callback/route.ts": + "binds the OAuth code to the session user", + "app/api/whoop/callback/route.ts": + "binds the OAuth code to the session user", + // RSC prefetch: the server component hydrates the client query cache + // with the session user's own data. Absence of a session skips the + // prefetch rather than failing the render. + "app/coach/page.tsx": "RSC prefetch of the session user's coach state", + "app/insights/page.tsx": "RSC prefetch of the session user's insights", + "app/insights/workouts/page.tsx": + "RSC prefetch of the session user's workouts", + "app/medications/page.tsx": + "RSC prefetch of the session user's medications", + "app/page.tsx": "RSC prefetch of the session user's dashboard snapshot", + // Onboarding routing. Reads the actor's own onboarding step to redirect; + // renders no record data. + "app/onboarding/[step]/page.tsx": + "redirects on the actor's onboarding step", + "app/onboarding/page.tsx": "redirects on the actor's onboarding step", + }; + + /** + * The identity-deriving helpers exported by `lib/auth/session.ts`. The + * lifecycle exports (`createSession`, `destroySession`, `destroyAllSessions`, + * `destroySessionById`, `destroyOtherSessions`) are deliberately NOT here: + * they act on a session, they do not hand the handler an identity to scope + * a query by. Adding a sixth deriving helper to `session.ts` means adding + * it to this regex. + */ + const SESSION_DERIVING_HELPERS = + /\b(getSession|getSessionUserLocale|validateSessionFromCookieValue|validateSessionWithCreatedAt)\s*\(/; + + /** + * Reaching for the cookie by name is the way around the helpers. Both the + * exported constant and the raw literal count. + */ + const RAW_SESSION_COOKIE = /\bSESSION_COOKIE_NAME\b|"healthlog_session"/; + + it("no file under src/app derives a session outside requireAuth unlisted", () => { + const resolvers = appFilesMatching(SESSION_DERIVING_HELPERS); + + // Non-zero proof. If the helper names are ever renamed out from under + // this regex, the match set collapses to empty — which must fail here + // rather than quietly agree with an emptied allowlist. + expect(resolvers.length).toBeGreaterThan(0); + expect(resolvers).toEqual(Object.keys(SESSION_RESOLUTION_ALLOWLIST).sort()); + }); + + it("no file under src/app reads the session cookie by name unlisted", () => { + const cookieReaders = appFilesMatching(RAW_SESSION_COOKIE); + + // Same non-zero proof, for this leg's own matcher. These two files also + // appear in the leg above, which is exactly why this count is asserted + // separately: an overlapping leg cannot borrow another leg's evidence. + expect(cookieReaders.length).toBeGreaterThan(0); + expect(cookieReaders).toEqual([ + "app/api/auth/native/complete/route.ts", + "app/api/auth/oidc/callback/route.ts", + ]); + + // And whatever it finds must already be an argued session surface. + for (const rel of cookieReaders) { + expect(Object.keys(SESSION_RESOLUTION_ALLOWLIST)).toContain(rel); + } + }); + + it("every allowlist entry carries a reason", () => { + for (const [rel, reason] of Object.entries(SESSION_RESOLUTION_ALLOWLIST)) { + expect(reason.trim().length, `${rel} has no reason`).toBeGreaterThan(0); + } + }); +}); + +describe("S2 — the URL-token authentication set is frozen", () => { + /** + * Files whose credential is a token in the URL — a path segment or a + * query parameter — rather than a session cookie or a Bearer header. + * + * This is the family the Bearer guard cannot see and a `getSession` sweep + * cannot see. `src/app/c/[token]/**` resolves one user's entire health + * record from an `hls_` path segment held by an anonymous caller. That is + * the product's design, it predates every guard in this file, and it is + * not a defect. What was a defect is that a sixth door could have been + * cut next to it without anything saying so. + */ + const URL_TOKEN_ALLOWLIST: Record = { + // Clinician share link: the `hls_` path segment is the whole credential. + "app/c/[token]/page.tsx": "serves the shared record view", + "app/c/[token]/d/[id]/route.ts": "serves a shared document's bytes", + "app/c/[token]/fhir/route.ts": "serves the shared record as a FHIR bundle", + "app/c/[token]/report.pdf/route.ts": "serves the shared record as a PDF", + // The passphrase gate for the same token. Anonymous by design; mints the + // token-scoped unlock cookie the view above checks. + "app/api/c/[token]/unlock/route.ts": "verifies the share-link passphrase", + // Webhook entrypoints. The provider supports no headers on a delivery, + // so the shared secret rides the path segment and is scrubbed from logs + // by `PATH_SECRET_PATHS`. These resolve a record from the payload after + // the secret compares equal. + "app/api/whoop/webhook/[token]/route.ts": + "path-segment shared secret, plus an HMAC body signature", + "app/api/withings/webhook/[token]/route.ts": + "path-segment shared secret; no body signature is available", + // The legacy form of the same Withings secret, still alive for + // subscriptions that have not rotated to the path-segment URL. + "app/api/withings/webhook/route.ts": + "legacy query-parameter form of the Withings webhook secret", + // Shape check only. Validates the `hlv_` prefix and redirects; touches + // no database, resolves nothing, and is not an enumeration oracle. + // Listed because it is a URL-token surface, with the reason being that + // it authenticates nothing. + "app/invite/[token]/page.tsx": + "validates the invite token's shape and redirects; no lookup", + }; + + /** + * Leg 1 — route shape. A dynamic segment whose name carries `token` or + * `secret` puts a credential in the URL by construction. This matcher + * reads the filesystem, not the file, so no variable name can slip past + * it; a new such route is caught before anyone reads its body. + */ + function urlCredentialSegmentFiles(): string[] { + return appFiles().filter((rel) => + rel.split("/").some((seg) => /^\[.*(token|secret).*\]$/i.test(seg)), + ); + } + + /** + * Leg 2 — share-token resolution by content, so a share surface that ever + * moves out from under a `[token]` segment is still caught. + */ + const SHARE_TOKEN_RESOLVERS = + /\b(resolveShareToken|resolveShareGateState|resolveShareReportDownload|loadShareViewData)\s*\(/; + + /** + * Leg 3 — a credential read out of the query string. + */ + const QUERY_CREDENTIAL = + /searchParams\.get\(\s*["'](secret|token|key|passphrase)["']\s*\)/; + + it("every URL-credential route segment is named", () => { + const bySegment = urlCredentialSegmentFiles(); + + expect(bySegment.length).toBeGreaterThan(0); + expect(bySegment).toEqual([ + "app/api/c/[token]/unlock/route.ts", + "app/api/whoop/webhook/[token]/route.ts", + "app/api/withings/webhook/[token]/route.ts", + "app/c/[token]/d/[id]/route.ts", + "app/c/[token]/fhir/route.ts", + "app/c/[token]/page.tsx", + "app/c/[token]/report.pdf/route.ts", + "app/invite/[token]/page.tsx", + ]); + }); + + it("every share-token resolver is named", () => { + const byResolver = appFilesMatching(SHARE_TOKEN_RESOLVERS); + + expect(byResolver.length).toBeGreaterThan(0); + expect(byResolver).toEqual([ + "app/api/c/[token]/unlock/route.ts", + "app/c/[token]/d/[id]/route.ts", + "app/c/[token]/fhir/route.ts", + "app/c/[token]/page.tsx", + "app/c/[token]/report.pdf/route.ts", + ]); + }); + + it("every query-string credential read is named", () => { + const byQuery = appFilesMatching(QUERY_CREDENTIAL); + + expect(byQuery.length).toBeGreaterThan(0); + expect(byQuery).toEqual(["app/api/withings/webhook/route.ts"]); + }); + + it("the three legs together equal the frozen allowlist", () => { + const union = [ + ...new Set([ + ...urlCredentialSegmentFiles(), + ...appFilesMatching(SHARE_TOKEN_RESOLVERS), + ...appFilesMatching(QUERY_CREDENTIAL), + ]), + ].sort(); + + expect(union.length).toBeGreaterThan(0); + expect(union).toEqual(Object.keys(URL_TOKEN_ALLOWLIST).sort()); + }); + + it("every allowlist entry carries a reason", () => { + for (const [rel, reason] of Object.entries(URL_TOKEN_ALLOWLIST)) { + expect(reason.trim().length, `${rel} has no reason`).toBeGreaterThan(0); + } + }); +}); + +describe("S3 — the two families stay distinct", () => { + /** + * A file that both resolves a session AND authenticates by a URL token is + * not forbidden, but it is a shape nobody has reviewed. If one appears, + * it should appear in a diff that says so rather than in production. + */ + it("no file authenticates by both a session and a URL token", () => { + const sessionFiles = appFilesMatching( + /\b(getSession|getSessionUserLocale|validateSessionFromCookieValue|validateSessionWithCreatedAt)\s*\(/, + ); + const tokenFiles = appFiles().filter((rel) => + rel.split("/").some((seg) => /^\[.*(token|secret).*\]$/i.test(seg)), + ); + + expect(sessionFiles.length).toBeGreaterThan(0); + expect(tokenFiles.length).toBeGreaterThan(0); + + const both = sessionFiles.filter((rel) => tokenFiles.includes(rel)); + expect(both).toEqual([]); + }); +}); From da85e64cfad012c046075cacd129d132d206a80b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 1 Aug 2026 22:45:04 +0200 Subject: [PATCH 2/2] Catch the Bearer resolver the guard's matcher walked past The Bearer guard demanded the literal `apiToken.findUnique(`. `lib/idempotency.ts` writes the same call fluent-style, with a newline between `prisma.apiToken` and `.findUnique({`, so the matcher never saw it. It has been resolving a raw Bearer value to a user id since the idempotency work landed, outside the frozen set, while the guard stayed green on five files. Tolerate whitespace around the member access and name the file with what it does: it decides which user's idempotency cell a replay lands in, and authorises nothing, because the handler still runs its own requireAuth. Add the non-zero count assertion the other guards now carry. Narrowing the regex back to its old form turns this test red, which is the proof the broadened matcher is what finds the file. The mint-site matcher in T3 was checked for the same defect and has no hidden members. --- .../bearer-scope-enforcement-guard.test.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/__tests__/bearer-scope-enforcement-guard.test.ts b/src/__tests__/bearer-scope-enforcement-guard.test.ts index 21c938fdc..b1c721cbb 100644 --- a/src/__tests__/bearer-scope-enforcement-guard.test.ts +++ b/src/__tests__/bearer-scope-enforcement-guard.test.ts @@ -64,16 +64,34 @@ describe("T1 — the Bearer resolution set is frozen", () => { // surface, which gates on BOTH `medication:ingest` and the per-medication // `medication::ingest` grant and never touches `requireAuth`. "app/api/ingest/medication/route.ts", + // `defaultUserIdResolver` — resolves the Bearer token to a user id so an + // idempotency key files under its owner. It authorises nothing: the + // handler still runs its own `requireAuth()`, and this resolver only + // decides which user's cache cell a replay lands in. Narrow in reach and + // deliberate, but it is a raw Bearer value becoming a user id, so it + // belongs on this list rather than beside it. + "lib/idempotency.ts", ].sort(); it("no file outside the allowlist resolves a Bearer token to a user", () => { // `resolveBearerToken` is the shared primitive; a `tokenHash` lookup is the // hand-rolled equivalent. Either one turns a bearer credential into an // identity, so both have to stay inside the allowlist. + // + // The whitespace classes around `.findUnique` are load-bearing. The + // original matcher demanded the literal `apiToken.findUnique(`, and + // `lib/idempotency.ts` writes the call fluent-style — `prisma.apiToken`, + // newline, `.findUnique({`. It resolved a Bearer token to a user id for + // three releases without this guard ever seeing it. A matcher that misses + // a real resolver is green because it matched nothing, not because + // nothing was there. const resolvers = filesMatching( - /resolveBearerToken|apiToken\.findUnique\([\s\S]*?where:\s*\{\s*tokenHash/, + /resolveBearerToken|apiToken\s*\.\s*findUnique\(\s*\{[\s\S]*?where:\s*\{\s*tokenHash/, ); + // Non-zero proof: an empty match set must fail rather than agree with an + // emptied allowlist. + expect(resolvers.length).toBeGreaterThan(0); expect(resolvers).toEqual(RESOLUTION_ALLOWLIST); });