diff --git a/CHANGELOG.md b/CHANGELOG.md index e9e44e01f..0204f0d31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ ## [Unreleased] +### Changed + +- **You can hold up to ten measurement tokens at a time.** Past that, minting + another is refused until you revoke one, so a script stuck in a retry loop + meets a wall instead of leaving you hundreds of live credentials to clear + out by hand. Only tokens that are actually alive count: revoking one frees + a slot immediately, an expired one stops taking up space on its own, and + the ordinary tokens your browser and phone use to sign in are counted + separately and never eat into it. A household with a scale, a watch bridge + and a couple of scripts sits at three or four, so the limit is there to + catch a runaway rather than to ration. + ### Fixed - **The heart-rate-variability tile never appeared on the dashboard for diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index bfdb509ff..41cbd636e 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -2782,8 +2782,8 @@ paths: not because a wildcard lacks the reach, but because of the lifetimes involved: a native access token lives a day and what it could mint here lives a year, so admitting one would let a short-lived compromise leave behind a credential that outlives revoking it. Gated by the operator's instance-wide API switch. Body capped at 16 KiB; - 10 mints per user per minute. Tokens appear in `GET /api/tokens` and are revoked at `DELETE /api/tokens/{id}` - like any other. + 10 mints per user per minute, and at most 10 live tokens held at once. Tokens appear in `GET /api/tokens` and + are revoked at `DELETE /api/tokens/{id}` like any other — revoking frees a slot against the ceiling. requestBody: required: true content: @@ -2811,6 +2811,17 @@ paths: application/json: schema: $ref: "#/components/schemas/ErrorEnvelope" + "409": + description: "The account already holds 10 live measurement tokens (`meta.errorCode` = + `tokens.measurements.ceiling_reached`). Nothing was minted. Distinct from the 429 above: that one is a rate + and clears by waiting, this one is a conflict with the account's current state and clears by revoking a + token at `DELETE /api/tokens/{id}`. Only LIVE tokens count — revoked and expired rows do not occupy a slot — + and only those carrying `measurements:write`, so the wildcard tokens a sign-in mints never consume the + budget." + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" "413": description: Body exceeds 16 KiB. content: diff --git a/docs/integrations/home-assistant.md b/docs/integrations/home-assistant.md index 89f787060..b1b92e337 100644 --- a/docs/integrations/home-assistant.md +++ b/docs/integrations/home-assistant.md @@ -22,6 +22,14 @@ Tokens expire after a year by default. They appear in the API Tokens list on the same page alongside every other credential on the account, and are revoked there. +An account can hold at most ten live measurement tokens at once — across every +automation platform combined, not ten per platform. Past that, minting another +is refused with a 409 until you revoke one; revoking frees the slot +immediately, and an expired token stops counting on its own. It is not a +security boundary, just a backstop against a script stuck minting in a retry +loop, so a household running a scale, a watch bridge and a couple of scripts +will never come close. + ## What the token can and cannot do It can add readings, on your own record, through: diff --git a/src/app/api/tokens/measurements/__tests__/route.test.ts b/src/app/api/tokens/measurements/__tests__/route.test.ts index a5bad79c9..21ebe8bfd 100644 --- a/src/app/api/tokens/measurements/__tests__/route.test.ts +++ b/src/app/api/tokens/measurements/__tests__/route.test.ts @@ -37,12 +37,17 @@ vi.mock("@/lib/logging/context", () => ({ getEvent: vi.fn(() => null), })); +vi.mock("@/lib/db", () => ({ + prisma: { apiToken: { count: vi.fn() } }, +})); + import { POST } from "../route"; import { requireCookieAuth } from "@/lib/api-handler"; import { issueApiToken } from "@/lib/auth/issue-token"; import { isApiGloballyEnabled } from "@/lib/app-settings"; import { checkRateLimit } from "@/lib/rate-limit"; import { auditLog } from "@/lib/auth/audit"; +import { prisma } from "@/lib/db"; import { MEASUREMENTS_WRITE_SCOPE } from "@/lib/measurements/scopes"; const USER = { id: "user-1", role: "USER" as const }; @@ -62,6 +67,8 @@ beforeEach(() => { vi.mocked(isApiGloballyEnabled).mockResolvedValue(true); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true } as never); vi.mocked(auditLog).mockResolvedValue(undefined as never); + // Below the ceiling by default; the cases that care set their own count. + vi.mocked(prisma.apiToken.count).mockResolvedValue(0 as never); vi.mocked(issueApiToken).mockResolvedValue({ token: "hlk_" + "a".repeat(64), expiresAt: EXPIRES, @@ -114,6 +121,25 @@ describe("what it mints", () => { }), ); }); + + it("counts only live tokens carrying this scope", async () => { + // The whole shape of the query, asserted rather than assumed. Counting + // every row would let the `["*"]` tokens a login mints consume the budget + // and refuse a mint for a reason the person cannot see; counting revoked + // or expired rows would mean revoking never frees a slot. + await POST(req({ name: "Scale" }) as never); + + const where = vi.mocked(prisma.apiToken.count).mock.calls[0][0]?.where; + expect(where).toMatchObject({ + userId: USER.id, + revoked: false, + permissions: { has: MEASUREMENTS_WRITE_SCOPE }, + }); + expect(where?.OR).toEqual([ + { expiresAt: null }, + { expiresAt: { gt: expect.any(Date) } }, + ]); + }); }); describe("when it refuses", () => { @@ -145,6 +171,36 @@ describe("when it refuses", () => { expect(issueApiToken).not.toHaveBeenCalled(); }); + it("409s at the live-token ceiling, without minting", async () => { + vi.mocked(prisma.apiToken.count).mockResolvedValue(10 as never); + const res = await POST(req({ name: "Scale" }) as never); + + // 409, not 429: a conflict with the account's current state, whose remedy + // is revoking rather than waiting. A 429 would say the opposite. + expect(res.status).toBe(409); + expect((await res.json()).meta?.errorCode).toBe( + "tokens.measurements.ceiling_reached", + ); + expect(issueApiToken).not.toHaveBeenCalled(); + }); + + it("still mints on the last free slot", async () => { + // The off-by-one that would make the ceiling nine. Worth pinning, because + // `>=` versus `>` is invisible in review and only one of them is right. + vi.mocked(prisma.apiToken.count).mockResolvedValue(9 as never); + const res = await POST(req({ name: "Scale" }) as never); + expect(res.status).toBe(201); + }); + + it("checks the ceiling only after the body validates", async () => { + // A malformed request should hear about the malformation, not the + // ceiling — and should not spend a query establishing it. + vi.mocked(prisma.apiToken.count).mockResolvedValue(10 as never); + const res = await POST(req({}) as never); + expect(res.status).toBe(422); + expect(prisma.apiToken.count).not.toHaveBeenCalled(); + }); + it("ignores a scope the body tries to smuggle in", async () => { // There is no `scope` field to validate, so an unknown key is simply not // read — the permission array is a literal either way. Asserted because diff --git a/src/app/api/tokens/measurements/route.ts b/src/app/api/tokens/measurements/route.ts index ccbe80e87..d1c30410b 100644 --- a/src/app/api/tokens/measurements/route.ts +++ b/src/app/api/tokens/measurements/route.ts @@ -40,6 +40,7 @@ import { } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; +import { prisma } from "@/lib/db"; import { issueApiToken } from "@/lib/auth/issue-token"; import { isApiGloballyEnabled } from "@/lib/app-settings"; import { checkRateLimit } from "@/lib/rate-limit"; @@ -62,6 +63,23 @@ const DEFAULT_EXPIRY_DAYS = 365; const MINT_RATE_LIMIT_MAX = 10; const MINT_RATE_LIMIT_WINDOW_MS = 60 * 1000; +/** + * How many ingest tokens an account may hold at once. + * + * The rate limit above and this are different guards and neither substitutes + * for the other. The bucket bounds a burst; it does nothing about + * accumulation, and a stuck automation retrying politely inside its allowance + * still reaches several hundred live credentials in an afternoon. This bounds + * the standing set, so a runaway script meets a refusal rather than leaving a + * thousand rows for somebody to revoke by hand. + * + * Ten is meant to be a number nobody legitimately reaches: a household running + * Home Assistant, a scale bridge and a couple of scripts sits at three or four. + * It is not a security boundary — the person can revoke and mint freely — so + * it is set to catch a loop, not to ration. + */ +const MAX_LIVE_TOKENS = 10; + export const POST = apiHandler(async (request: NextRequest) => { // Cookie-only, and the reason is lifetime rather than reach. // @@ -102,6 +120,56 @@ export const POST = apiHandler(async (request: NextRequest) => { return returnAllZodIssues(parsed.error, 422); } + // Counted LIVE, not minted-ever, so revoking frees a slot and an expired + // token stops occupying one — the ceiling bounds what currently exists + // rather than what the account has ever done. Scoped to this grant for the + // same reason: a login mints a `["*"]` access token per session and those + // accumulate on their own, so counting every row would let ordinary browser + // use exhaust the budget and then refuse a mint for a reason invisible to + // the person hitting it. + // + // Placed after validation and before the create: a request that is going to + // be refused should not cost a credential, and one that is malformed should + // hear about the malformation rather than the ceiling. + // + // Count-then-create is NOT atomic, and deliberately so. Two mints racing at + // nine both read nine, both pass, and the account lands eleven. Nothing here + // prevents that; what bounds it is the rate limit above, which is a single + // `INSERT … ON CONFLICT DO UPDATE … RETURNING` (`lib/rate-limit.ts`) and so + // admits at most `MINT_RATE_LIMIT_MAX` mints per user per window. The race + // can overshoot the ceiling; it cannot run away from it. + // + // Left unserialised because of what this limit is for: catching a runaway + // automation, not metering a quota anybody is billed against. The holder can + // revoke and mint freely, so eleven tokens instead of ten costs nothing, + // while a lock on a credential mint is a contention point on a write that + // must not hang. If it ever does need to be exact, the house pattern is + // `pg_advisory_xact_lock(hashtextextended(, 0))` inside the + // transaction — see `lib/managed-profiles/lifecycle.ts:47` or + // `lib/integrations/oauth-refresh.ts:89`. + const live = await prisma.apiToken.count({ + where: { + userId: user.id, + revoked: false, + permissions: { has: MEASUREMENTS_WRITE_SCOPE }, + OR: [{ expiresAt: null }, { expiresAt: { gt: new Date() } }], + }, + }); + if (live >= MAX_LIVE_TOKENS) { + annotate({ + action: { name: "tokens.measurements.create" }, + meta: { outcome: "ceiling_reached", live_token_count: live }, + }); + // 409 rather than 429: this is a conflict with the account's current + // state, not a rate, and the remedy is to revoke one rather than to wait. + // A 429 would tell the caller the opposite of what is true. + return apiError( + `You already have ${MAX_LIVE_TOKENS} measurement tokens. Revoke one before creating another.`, + 409, + { errorCode: "tokens.measurements.ceiling_reached" }, + ); + } + const issued = await issueApiToken({ userId: user.id, name: parsed.data.name, diff --git a/src/lib/openapi/routes/auth.ts b/src/lib/openapi/routes/auth.ts index 96d5efdd9..be835e889 100644 --- a/src/lib/openapi/routes/auth.ts +++ b/src/lib/openapi/routes/auth.ts @@ -1620,7 +1620,7 @@ export const authPaths: NonNullable = { "Mints a Bearer scoped to exactly `measurements:write` and audits the mint. THE RESPONSE CARRIES THE RAW TOKEN — the only place it ever exists; it is stored as an HMAC and no path re-reveals it.\n\n" + "**What it can do.** `POST /api/measurements` and `POST /api/measurements/batch`, on its owner's own record, attributing `source: MANUAL`. An entry naming `APPLE_HEALTH` is refused 422 rather than relabelled: that source is half a dedup key the phone also writes into, it participates in the cross-source merge, and it is what decides the Apple Health card may claim a sync happened. For the same reason a write through this credential does not move the native client's sync checkpoint.\n\n" + "**What it cannot do.** Everything else, including the measurement reads on the same paths, the edit and delete legs, and the export — a scope grants what it names and nothing adjacent. It cannot be pointed at a shared record: a request carrying the account selector is refused 403 before any grant is read, whatever grants its holder actually has. And it cannot mint another token, this endpoint included.\n\n" + - "Minting requires a COOKIE SESSION. No Bearer credential reaches this endpoint at any scope, wildcard included — not because a wildcard lacks the reach, but because of the lifetimes involved: a native access token lives a day and what it could mint here lives a year, so admitting one would let a short-lived compromise leave behind a credential that outlives revoking it. Gated by the operator's instance-wide API switch. Body capped at 16 KiB; 10 mints per user per minute. Tokens appear in `GET /api/tokens` and are revoked at `DELETE /api/tokens/{id}` like any other.", + "Minting requires a COOKIE SESSION. No Bearer credential reaches this endpoint at any scope, wildcard included — not because a wildcard lacks the reach, but because of the lifetimes involved: a native access token lives a day and what it could mint here lives a year, so admitting one would let a short-lived compromise leave behind a credential that outlives revoking it. Gated by the operator's instance-wide API switch. Body capped at 16 KiB; 10 mints per user per minute, and at most 10 live tokens held at once. Tokens appear in `GET /api/tokens` and are revoked at `DELETE /api/tokens/{id}` like any other — revoking frees a slot against the ceiling.", requestBody: { required: true, content: { @@ -1671,6 +1671,11 @@ export const authPaths: NonNullable = { "More than 10 mints from this account in a minute. Nothing was minted.", content: { "application/json": { schema: errorEnvelope } }, }, + "409": { + description: + "The account already holds 10 live measurement tokens (`meta.errorCode` = `tokens.measurements.ceiling_reached`). Nothing was minted. Distinct from the 429 above: that one is a rate and clears by waiting, this one is a conflict with the account's current state and clears by revoking a token at `DELETE /api/tokens/{id}`. Only LIVE tokens count — revoked and expired rows do not occupy a slot — and only those carrying `measurements:write`, so the wildcard tokens a sign-in mints never consume the budget.", + content: { "application/json": { schema: errorEnvelope } }, + }, }, }, }, diff --git a/tests/integration/bearer-scope-enforcement.test.ts b/tests/integration/bearer-scope-enforcement.test.ts index 08c759e24..c4e2d32dc 100644 --- a/tests/integration/bearer-scope-enforcement.test.ts +++ b/tests/integration/bearer-scope-enforcement.test.ts @@ -692,6 +692,82 @@ describe("B8 — the measurement-ingest scope reaches its two routes and no othe ).toBe(0); }); + /** + * The live-token ceiling, on a cookie session because that is the only + * credential the mint admits. + * + * Here rather than in a file of its own for one reason: the assertion that + * needs a real database is `permissions: { has: … }` against a Postgres + * `text[]`, and the harness for that already exists in this file. A mock + * can confirm the query was built; only Postgres can confirm it selects the + * rows it means to. + */ + it("refuses a mint once the account holds the ceiling, and revoking frees a slot", async () => { + const CEILING = 10; + const now = Date.now(); + await getPrismaClient().apiToken.createMany({ + data: Array.from({ length: CEILING }, (_, i) => ({ + userId: USER_ID, + name: `bridge ${i}`, + tokenHash: hashToken(`hlk_ceiling_${i}_${"0".repeat(40)}`), + permissions: ["measurements:write"], + expiresAt: new Date(now + 86_400_000), + })), + }); + // Neither of these may occupy a slot: one is a different scope, the other + // has already expired. Seeded so a query that counted either would push + // the live figure past the ceiling and pass this test for the wrong + // reason on the revoke leg below. + await getPrismaClient().apiToken.createMany({ + data: [ + { + userId: USER_ID, + name: "a browser session", + tokenHash: hashToken(`hlk_wild_${"0".repeat(44)}`), + permissions: ["*"], + expiresAt: new Date(now + 86_400_000), + }, + { + userId: USER_ID, + name: "long expired", + tokenHash: hashToken(`hlk_expired_${"0".repeat(40)}`), + permissions: ["measurements:write"], + expiresAt: new Date(now - 86_400_000), + }, + ], + }); + + await useCookieSession(); + const { POST } = await import("@/app/api/tokens/measurements/route"); + const mint = () => + POST( + new NextRequest("https://health.example/api/tokens/measurements", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ name: "one too many" }), + } as never), + ); + + const refused = await mint(); + expect(refused.status).toBe(409); + expect((await refused.json()).meta?.errorCode).toBe( + "tokens.measurements.ceiling_reached", + ); + + // Revoking one live row — not the expired or wildcard decoys — must open + // exactly one slot. This is the leg that fails if the count is of mints + // ever made rather than of tokens currently alive. + const victim = await getPrismaClient().apiToken.findFirstOrThrow({ + where: { name: "bridge 0" }, + }); + await getPrismaClient().apiToken.update({ + where: { id: victim.id }, + data: { revoked: true }, + }); + + expect((await mint()).status).toBe(201); + }); + it("cannot reach the medication ingest surface", async () => { await armToken(["measurements:write"], "mwrite8"); const { POST } = await import("@/app/api/ingest/medication/route");