diff --git a/CHANGELOG.md b/CHANGELOG.md index 76bea5ed6..709010fdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## [Unreleased] +API tokens now enforce their scope. + +- **A token minted for medication intake is refused outside the ingest endpoint.** Previously any valid token reached every authenticated route regardless of the scope it carried, so a token handed to a third-party automation could read the full-account backup export, lab results, measurements and the assistant. A route that accepts a narrow-scope token now has to name that scope; a route that names none accepts browser sessions and the app's own full-access tokens only. Tokens issued to the web session, the native client and the MCP connector are unaffected. +- **Revoke and re-issue any token you handed to a third party.** A token that could reach the backup export should be treated as having had that reach for its whole lifetime, not only from the moment it was noticed. +- **The generic "create API token" button is gone from settings.** The token it minted never worked for the medication intake it advertised — the ingest endpoint also requires a per-medication grant that this button never issued — while reaching everything else. Tokens are issued per medication from the medication's own API-endpoint toggle, which grants exactly that one medication. Existing tokens stay listed and revocable. +- **MCP connector tokens are now bound to the MCP endpoint alone.** They previously also passed on read-only REST requests. No connector flow used that. + ## [1.30.16] — 2026-07-19 Stops an ongoing loss of resting heart-rate history. diff --git a/CLAUDE.md b/CLAUDE.md index 0520b6acf..ac5b3c414 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -88,7 +88,8 @@ The architecture map in `.planning/codebase/arch.md` walks each layer with file: - **RSC by default.** Server components are the default; add `"use client"` only when a hook, state, or browser API actually needs it. - **Every API route wraps in `apiHandler`.** Verified — zero exceptions in the current tree. Every body-accepting route runs Zod `safeParse` and returns 422 via `returnAllZodIssues` (multi-issue envelope sanitised against echoed input through `sanitiseZodIssues`). - **`userId` is always narrowed from session or Bearer.** No route accepts `userId` as a body field — it comes from `requireAuth()` and feeds the Prisma `where`. The batch endpoints (`/api/measurements/batch`, `/api/mood-entries/bulk`, `/api/medications/intake/bulk`, `/api/workouts/batch`) don't even declare a `userId` field in their Zod schemas. -- **No mass assignment.** Every `prisma.X.{create,update}({ data: ... })` builds its `data` object field-by-field from `parsed.data`, never by spreading the parsed object whole. `tokens.create` hardcodes the narrow scope so the user-facing endpoint can never mint a wildcard token. +- **No mass assignment.** Every `prisma.X.{create,update}({ data: ... })` builds its `data` object field-by-field from `parsed.data`, never by spreading the parsed object whole. `tokens.create` builds `permissions` from a hardcoded literal, never from request input. +- **Bearer scope is fail-closed.** `requireAuth()` with no argument admits a cookie session or a wildcard (`["*"]`) token only — a narrow-scope token is refused with 403 and an `undeclared_scope` audit row. A route that means to accept a narrow token names the scope: `requireAuth(FHIR_READ_SCOPE)`. Adding a route therefore cannot silently widen an existing token's reach, because the default arm denies. Only the login / passkey / refresh paths mint `["*"]`; every user-facing mint is narrow, and narrow now means narrow. Two surfaces resolve a Bearer token outside `requireAuth` — `/api/ingest/medication` and `/mcp` — and that set is frozen by a structural test (`src/__tests__/bearer-scope-enforcement-guard.test.ts`). - **`requireAdmin()` is cookie-only.** Bearer tokens — even with `["*"]` scope — cannot reach admin endpoints. This is a structural boundary in `requireAdmin()` (`src/lib/api-handler.ts`), not a runtime check that future code can soften. - **Raw SQL is parameter-bound or whitelist-spliced.** Every `$queryRaw` uses tagged-template parameters. Every `$queryRawUnsafe` / `$executeRawUnsafe` either passes positional `$N` placeholders with an argument array OR splices a value already asserted against a closed enum + regex (see the whitelist-splice block in `src/lib/rollups/measurement-rollups.ts`, around the `$queryRawUnsafe` calls). Inline comments document the whitelist at the splice point. - **TanStack Query keys live in the centralised factory.** `src/lib/query-keys/` (16 modules, `index.ts` barrel) is the only legal source of `queryKey` / `mutationKey` arrays. The in-repo ESLint rule `healthlog/queryKey-factory` (set to `error`) flags any bare array. Same-key + different `queryFn` shape silently poisons the cache; the factory prevents it. Every read unwraps `(await res.json()).data` from the envelope. diff --git a/docs/self-hosting/mcp.md b/docs/self-hosting/mcp.md index ae94a55ae..db632db0b 100644 --- a/docs/self-hosting/mcp.md +++ b/docs/self-hosting/mcp.md @@ -65,10 +65,10 @@ Still in **Settings → MCP → Tokens**: 3. **Copy the `hlk_…` value now** — it is shown exactly once and cannot be retrieved again. Tokens default to a 90-day lifetime. -The token never carries the `*` wildcard and never the -`medication:ingest` scope the generic `/api/tokens` mints — it is the -least-privilege MCP scope and nothing more. The same list shows -last-used time and lets you revoke any token. +The token never carries the `*` wildcard and never a medication-ingest +scope — it is the least-privilege MCP scope and nothing more, and it is +bound to the MCP surface: it is refused on every REST route. The same +list shows last-used time and lets you revoke any token. Cloud connectors that use OAuth (next section) mint the same `health:read` scope automatically, so for Claude.ai / ChatGPT you do not diff --git a/tests/integration/bearer-scope-enforcement.test.ts b/tests/integration/bearer-scope-enforcement.test.ts new file mode 100644 index 000000000..2b158e1e2 --- /dev/null +++ b/tests/integration/bearer-scope-enforcement.test.ts @@ -0,0 +1,322 @@ +/** + * Behavioural suite for the fail-closed Bearer-scope default, against a real + * Postgres and the real resolver — no mocked Prisma, no mocked `requireAuth`. + * + * The gap this pins: `requireAuth()` enforced a token's scope only when the + * route passed one, and 324 of 330 route files pass none. A token minted for + * medication intake therefore reached the full-backup export, the labs surface, + * the coach, and the bulk deletes. B1 is the direct regression test — it fails + * on the pre-fix tree. + * + * Seven cases against 300-odd routes prove nothing about the other routes by + * enumeration. The guarantee for those is structural (one resolution path, one + * authorisation arm) and is held by + * `src/__tests__/bearer-scope-enforcement-guard.test.ts`. What these cases do + * is prove the arm itself behaves, end to end, for every credential shape the + * app actually mints. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +process.env.API_TOKEN_HMAC_KEY ??= + "test-hmac-key-bearer-scope-enforcement-32-bytes-min-0987654321"; + +const { hashToken } = await import("@/lib/auth/hmac"); + +const USER_ID = "user-bearer-scope-test"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + headerJar.clear(); + await getPrismaClient().user.create({ + data: { + id: USER_ID, + username: "bearer-scope", + email: "bearer-scope@example.test", + timezone: "UTC", + }, + }); +}); + +/** Mint a real `ApiToken` row with the given scopes and arm the Bearer header. */ +async function useToken(permissions: string[], label = "t"): Promise { + const raw = `hlk_${label}_${"0".repeat(48)}`; + await getPrismaClient().apiToken.create({ + data: { + userId: USER_ID, + name: label, + tokenHash: hashToken(raw), + permissions, + }, + }); + headerJar.set("authorization", `Bearer ${raw}`); + return raw; +} + +/** Arm a real cookie session instead of a Bearer token. */ +async function useCookieSession(): Promise { + const session = await getPrismaClient().session.create({ + data: { userId: USER_ID, expiresAt: new Date(Date.now() + 3_600_000) }, + }); + cookieJar.set("healthlog_session", session.id); +} + +function req(path: string, method = "GET"): NextRequest { + const init: RequestInit = { method }; + const raw = headerJar.get("authorization"); + if (raw) init.headers = { authorization: raw }; + return new NextRequest(`https://health.example${path}`, init as never); +} + +/** The most recent `auth.bearer.failure` audit row's machine reason. */ +async function lastBearerFailureReason(): Promise { + const row = await getPrismaClient().auditLog.findFirst({ + where: { action: "auth.bearer.failure" }, + orderBy: { createdAt: "desc" }, + }); + if (!row?.details) return undefined; + return (JSON.parse(row.details) as { reason?: string }).reason; +} + +describe("B1 — a narrow token cannot read the full-account export", () => { + it("refuses ['medication:ingest'] on GET /api/export/full-backup with 403", async () => { + // The vulnerability in one request: this is the token the settings card + // told users to hand to third-party automations, reading every row the + // account holds. + await useToken(["medication:ingest"], "narrow"); + const { GET } = await import("@/app/api/export/full-backup/route"); + + const res = await GET(req("/api/export/full-backup")); + + expect(res.status).toBe(403); + const json = (await res.json()) as { data: null; error: string }; + expect(json.data).toBeNull(); + expect(json.error).toBe("Insufficient permissions"); + + // The break is visible AND attributable: an operator can name the token. + // Audit writes are fire-and-forget; give the microtask a beat to land. + await new Promise((r) => setTimeout(r, 100)); + expect(await lastBearerFailureReason()).toBe("undeclared_scope"); + }); + + it("refuses the same token on GET /api/labs with 403", async () => { + await useToken(["medication:ingest"], "narrow2"); + const { GET } = await import("@/app/api/labs/route"); + const res = await GET(req("/api/labs")); + expect(res.status).toBe(403); + }); +}); + +describe("B2/B3 — the medication-ingest surface is unchanged", () => { + async function seedMedication(): Promise { + const med = await getPrismaClient().medication.create({ + data: { userId: USER_ID, name: "Test Med", dose: "1 mg" }, + }); + return med.id; + } + + it("B2 — the per-medication token still ingests", async () => { + // The credential that actually performs medication intake. It never + // touches `requireAuth` — `/api/ingest/medication` hand-rolls the whole + // resolution — so the fail-closed default cannot reach it. This is the + // one flow a fail-closed default could plausibly have broken. + const medId = await seedMedication(); + await useToken(["medication:ingest", `medication:${medId}:ingest`], "pair"); + const { POST } = await import("@/app/api/ingest/medication/route"); + + const request = new NextRequest( + "https://health.example/api/ingest/medication", + { + method: "POST", + headers: { + authorization: headerJar.get("authorization")!, + "content-type": "application/json", + }, + body: JSON.stringify({ + medicationName: "Test Med", + idempotencyKey: "bearer-scope-b2", + }), + } as never, + ); + const res = await POST(request); + + expect([200, 201]).toContain(res.status); + const events = await getPrismaClient().medicationIntakeEvent.count({ + where: { userId: USER_ID }, + }); + expect(events).toBe(1); + }); + + it("B3 — a token without the per-medication grant is still refused", async () => { + // The pre-existing second gate. This is why the retired `/api/tokens` + // mint never worked for its advertised purpose. + await seedMedication(); + await useToken(["medication:ingest"], "familyonly"); + const { POST } = await import("@/app/api/ingest/medication/route"); + + const request = new NextRequest( + "https://health.example/api/ingest/medication", + { + method: "POST", + headers: { + authorization: headerJar.get("authorization")!, + "content-type": "application/json", + }, + body: JSON.stringify({ + medicationName: "Test Med", + idempotencyKey: "bearer-scope-b3", + }), + } as never, + ); + const res = await POST(request); + + expect(res.status).toBe(403); + }); +}); + +describe("B4 — the native client is not broken", () => { + it("admits a ['*'] token on GET /api/export/full-backup", async () => { + // Login, passkey login-verify and refresh rotation all mint `["*"]`, so + // this is exactly the credential the iOS app holds. If this case ever + // goes red, the native client is down. + await useToken(["*"], "wildcard"); + const { GET } = await import("@/app/api/export/full-backup/route"); + const res = await GET(req("/api/export/full-backup")); + expect(res.status).toBe(200); + }); + + it("admits a ['*'] token on a batch ingest route", async () => { + await useToken(["*"], "wildcardbatch"); + const { POST } = await import("@/app/api/measurements/batch/route"); + const request = new NextRequest( + "https://health.example/api/measurements/batch", + { + method: "POST", + headers: { + authorization: headerJar.get("authorization")!, + "content-type": "application/json", + }, + body: JSON.stringify({ + entries: [ + { + hkIdentifier: "HKQuantityTypeIdentifierBodyMass", + value: 70, + unit: "kg", + startDate: new Date().toISOString(), + endDate: new Date().toISOString(), + externalId: "uuid-bearer-scope-1", + }, + ], + }), + } as never, + ); + const res = await POST(request); + expect(res.status).toBe(200); + }); +}); + +describe("B5 — an MCP token is audience-bound to /mcp", () => { + it("refuses ['health:read'] on a REST read", async () => { + // Narrowing, not breakage: the token's REST read leg was never a feature + // an MCP client used. Its audience is now /mcp alone. + await useToken(["health:read"], "mcpread"); + const { GET } = await import("@/app/api/export/full-backup/route"); + const res = await GET(req("/api/export/full-backup")); + expect(res.status).toBe(403); + }); + + it("still resolves the same token on the /mcp wire", async () => { + const raw = await useToken(["health:read"], "mcpread2"); + const { resolveMcpAuthContext } = await import("@/lib/mcp/auth"); + + const ctx = await resolveMcpAuthContext(raw); + + expect(ctx.userId).toBe(USER_ID); + expect(ctx.canRead).toBe(true); + // Read-only token: the write tools stay shut. + expect(ctx.canWrite).toBe(false); + }); + + it("grants write on /mcp only for a consented health:write token", async () => { + const raw = await useToken(["health:read", "health:write"], "mcprw"); + const { resolveMcpAuthContext } = await import("@/lib/mcp/auth"); + const ctx = await resolveMcpAuthContext(raw); + expect(ctx.canWrite).toBe(true); + }); +}); + +describe("B6 — a declared scope grants only what it names", () => { + it("admits ['fhir:read'] on the FHIR face", async () => { + await useToken(["fhir:read"], "fhir"); + const { GET } = await import("@/app/api/fhir/Observation/route"); + const res = await GET(req("/api/fhir/Observation")); + expect(res.status).toBe(200); + }); + + it("refuses the same token on /api/labs, which declares no scope", async () => { + // The whole point of the inversion: holding a scope buys the routes that + // name it, and nothing else. + await useToken(["fhir:read"], "fhir2"); + const { GET } = await import("@/app/api/labs/route"); + const res = await GET(req("/api/labs")); + expect(res.status).toBe(403); + }); +}); + +describe("B7 — cookie sessions are untouched", () => { + it("reaches every route the narrow token was refused on", async () => { + await useCookieSession(); + + const backup = await import("@/app/api/export/full-backup/route"); + expect((await backup.GET(req("/api/export/full-backup"))).status).toBe(200); + + const labs = await import("@/app/api/labs/route"); + expect((await labs.GET(req("/api/labs"))).status).toBe(200); + + const fhir = await import("@/app/api/fhir/Observation/route"); + expect((await fhir.GET(req("/api/fhir/Observation"))).status).toBe(200); + }); + + it("requireAdmin still refuses a ['*'] Bearer token", async () => { + // The cookie-only property of `requireAdmin()` is orthogonal to this + // change and must stay orthogonal. A wildcard token is the strongest + // Bearer credential the app mints and it still cannot elevate. + await getPrismaClient().user.update({ + where: { id: USER_ID }, + data: { role: "ADMIN" }, + }); + await useToken(["*"], "adminwild"); + + const { requireAdmin, HttpError } = await import("@/lib/api-handler"); + await expect(requireAdmin()).rejects.toBeInstanceOf(HttpError); + }); +}); diff --git a/tests/integration/workout-batch-create.test.ts b/tests/integration/workout-batch-create.test.ts index e4f03aca1..556e5d634 100644 --- a/tests/integration/workout-batch-create.test.ts +++ b/tests/integration/workout-batch-create.test.ts @@ -10,7 +10,8 @@ * - Over-cap routes return 400 with the documented error code * - Idempotency-Key replay returns the cached envelope * - Rate-limit kicks in at the documented ceiling - * - Narrow-scope Bearer tokens are admitted (W10 fix-C parity) + * - Narrow-scope Bearer tokens are refused; the wildcard token iOS + * actually holds is admitted * * A separate file at this directory carries the concurrent-write race * test so the contention assertion stays focused on the invariant that @@ -434,34 +435,66 @@ describe("POST /api/workouts/batch (real Postgres)", () => { expect(stored).toBe(0); }); - it("admits a narrow-scope Bearer token (W10 fix-C parity)", async () => { + it("refuses a narrow-scope Bearer token, admits the wildcard one", async () => { + // Supersedes the W10 fix-C case, whose comment claimed "the iOS + // narrow-scope sync depends on" a narrow token being admitted here. It + // does not: `workouts:ingest` has no mint site anywhere in the tree, and + // iOS holds a `["*"]` token from login / passkey / refresh rotation. What + // that case actually pinned was the fail-open default — a token scoped to + // anything at all reaching a route that named no scope. + // + // Both halves are asserted together so the real iOS contract is the thing + // under test, not an inference from the deny case. const { POST } = await import("@/app/api/workouts/batch/route"); - // Drop the cookie session so the Bearer path is the ONLY auth in - // play. A narrow-scope token (no wildcard) MUST still admit on a - // route that declares no required permission — that's the W10 - // fix-C correctness fix the iOS narrow-scope sync depends on. + // Drop the cookie session so the Bearer path is the ONLY auth in play. cookieJar.delete("healthlog_session"); - const rawToken = "hlk_narrow_scope_workout_ingest_token_value_a1b2c3d4e5"; + const narrowToken = + "hlk_narrow_scope_workout_ingest_token_value_a1b2c3d4e5"; await getPrismaClient().apiToken.create({ data: { userId: TEST_USER_ID, name: "narrow-scope-workouts", - tokenHash: hashToken(rawToken), + tokenHash: hashToken(narrowToken), permissions: ["workouts:ingest"], // NO wildcard }, }); - headerJar.set("authorization", `Bearer ${rawToken}`); + headerJar.set("authorization", `Bearer ${narrowToken}`); - const res = await POST( + const denied = await POST( makeRequest( { workouts: [baseWorkout("hk-uuid-narrow-1")] }, - { bearer: rawToken }, + { bearer: narrowToken }, ), ); - expect(res.status).toBe(200); - const json = (await res.json()) as { data: { inserted: number } }; + expect(denied.status).toBe(403); + expect( + await getPrismaClient().workout.count({ + where: { userId: TEST_USER_ID }, + }), + ).toBe(0); + + // The credential iOS actually holds. + const wildcardToken = "hlk_wildcard_workout_ingest_token_value_f6g7h8i9j0"; + await getPrismaClient().apiToken.create({ + data: { + userId: TEST_USER_ID, + name: "wildcard-workouts", + tokenHash: hashToken(wildcardToken), + permissions: ["*"], + }, + }); + headerJar.set("authorization", `Bearer ${wildcardToken}`); + + const admitted = await POST( + makeRequest( + { workouts: [baseWorkout("hk-uuid-wildcard-1")] }, + { bearer: wildcardToken }, + ), + ); + expect(admitted.status).toBe(200); + const json = (await admitted.json()) as { data: { inserted: number } }; expect(json.data.inserted).toBe(1); }); });