mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 21:10:59 +00:00
Merge work/session-surface-guard
This commit is contained in:
commit
7a6335a2c7
2 files changed
+333
-1
No files matched your search
@@ -64,16 +64,34 @@ describe("T1 — the Bearer resolution set is frozen", () => {
|
||||
// surface, which gates on BOTH `medication:ingest` and the per-medication
|
||||
// `medication:<id>: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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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<string, string> = {
|
||||
// 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<string, string> = {
|
||||
// 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([]);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user