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); }); 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([]); + }); +});