From 39d23dbb7d4d1b1ba6644eeb5cdb8de3b4adceaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Tue, 20 Oct 2026 13:53:54 +0200 Subject: [PATCH] fix(api): meter every authenticated write where the caller is resolved 169 of 364 mutating handlers met no rate limit, every PATCH and DELETE on an [id] among them. The shared per-account write bucket (300 a minute, keyed on the acting account) was called by hand in a dozen creates; it is now charged by the identity resolvers for any POST, PUT, PATCH or DELETE, once per request, so a route is metered by resolving its caller. The bulk ingest routes the native client syncs through in bursts (measurements, workouts, intake, mood and cycle bulk, nutrients, the ECG sweep) declare writeBucket: "own" and keep their dedicated ceilings, so a large HealthKit backfill cannot lock the person out of logging a dose. A read-over-POST stays free. write-rate-limit-guard fails on any mutating route that neither resolves its caller through a metering resolver nor calls a limiter, unless it is on a short reasoned exception list; an opt-out must be on its own reviewed list and call its own limiter. --- docs/api/openapi.yaml | 5 +- .../record-write-rate-limit-contract.test.ts | 46 ++-- src/__tests__/write-rate-limit-guard.test.ts | 220 ++++++++++++++++++ src/app/api/allergies/route.ts | 14 +- src/app/api/biomarkers/[id]/merge/route.ts | 10 +- src/app/api/biomarkers/route.ts | 13 +- .../api/custom-metrics/[id]/entries/route.ts | 10 - src/app/api/cycle/day-logs/bulk/route.ts | 5 +- src/app/api/cycle/day-logs/route.ts | 13 +- src/app/api/encounters/route.ts | 13 +- src/app/api/insights/ecg/route.ts | 5 +- .../labs/ocr/commit/__tests__/route.test.ts | 8 +- src/app/api/labs/ocr/commit/route.ts | 13 -- src/app/api/labs/route.ts | 13 +- src/app/api/life-events/route.ts | 9 - src/app/api/measurements/batch/route.ts | 7 +- src/app/api/measurements/route.ts | 11 - src/app/api/medications/[id]/intake/route.ts | 11 - src/app/api/medications/intake/bulk/route.ts | 5 +- src/app/api/medications/intake/route.ts | 11 - src/app/api/mood-entries/bulk/route.ts | 5 +- src/app/api/mood-entries/route.ts | 11 - src/app/api/nutrients/batch/route.ts | 5 +- src/app/api/vaccinations/custom/route.ts | 12 +- src/app/api/vaccinations/route.ts | 13 +- src/app/api/workouts/batch/route.ts | 7 +- src/lib/api-handler.ts | 26 +++ src/lib/auth/__tests__/write-meter.test.ts | 102 ++++++++ src/lib/auth/write-meter.ts | 88 +++++++ src/lib/openapi/routes/shared.ts | 12 +- src/lib/rate-limit.ts | 20 +- tests/integration/write-meter.test.ts | 185 +++++++++++++++ vitest.setup.ts | 18 +- 33 files changed, 730 insertions(+), 216 deletions(-) create mode 100644 src/__tests__/write-rate-limit-guard.test.ts create mode 100644 src/lib/auth/__tests__/write-meter.test.ts create mode 100644 src/lib/auth/write-meter.ts create mode 100644 tests/integration/write-meter.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 380c0a6cb..8ac3d0181 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -396,9 +396,10 @@ paths: "409": *a7 "422": *a4 "429": &a12 - description: "Rate limit exceeded. This route shares one per-account bucket with the other single-record writes — + description: "Rate limit exceeded. This route shares one per-account bucket with every other authenticated write — `record-write:`, 300 requests per 60 seconds — keyed on the ACTING account, so a delegate burns - their own allowance rather than the record owner's. Nothing was written. `meta.errorCode` is + their own allowance rather than the record owner's. Only the bulk ingest endpoints, which meter themselves + with a dedicated bucket, stay outside it. Nothing was written. `meta.errorCode` is `record_write.rate_limited`, which is what tells this refusal apart from a route's own narrower bucket when both can answer 429. The `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining` and `X-RateLimit-Reset` headers describe that bucket; back off on `Retry-After` rather than guessing. Every request counts against diff --git a/src/__tests__/record-write-rate-limit-contract.test.ts b/src/__tests__/record-write-rate-limit-contract.test.ts index 3330c46c4..73a775e93 100644 --- a/src/__tests__/record-write-rate-limit-contract.test.ts +++ b/src/__tests__/record-write-rate-limit-contract.test.ts @@ -4,9 +4,11 @@ * The batch endpoints have been capped at 60 calls a minute since they were * written; the per-record creates a naive or older client loops over were not * capped at all, which is backwards — the batch endpoint is the one a - * well-behaved client uses. Eleven of them share one generous per-account - * bucket now, and the operations publish its name and its cap so a client that - * meets the ceiling can read which one it met. + * well-behaved client uses. Every authenticated write now shares one generous + * per-account bucket, charged where the caller is resolved + * (`src/lib/auth/write-meter.ts`), and the operations listed below publish + * its name and its cap so a client that meets the ceiling can read which one + * it met. * * A published number is only worth reading if it cannot drift from the code, so * this pulls the bucket key and the limit out of `prisma.$queryRaw` — the same @@ -15,9 +17,9 @@ * would prove nothing: the sentence is written by hand, deliberately, and this * is what keeps the hand-written half honest. * - * The route half is structural: every route in the list below calls the shared - * helper. Its limit is that it reads the file rather than the running handler, - * so a call placed after an early return would still count as present. + * The route half is structural: every route in the list below resolves its + * caller through a metering resolver and does not opt out. Its limit is that + * it reads the file rather than the running handler. */ import { readFileSync } from "node:fs"; import { join } from "node:path"; @@ -32,6 +34,7 @@ import { } from "@/lib/rate-limit"; import { buildOpenApiDocument } from "@/lib/openapi/registry"; import { recordWriteRateLimitResponse } from "@/lib/openapi/routes/shared"; +import { RECORD_WRITE_RATE_LIMITED_CODE } from "@/lib/auth/write-meter"; vi.mock("@/lib/db", () => ({ prisma: { $queryRaw: vi.fn() } })); @@ -121,24 +124,27 @@ describe("the shared single-record write bucket", () => { expect(published).toContain("Retry-After"); }); - it("is called by every route that publishes it", () => { - const missing = ON_THE_SHARED_BUCKET.filter(([route]) => { + it("is charged on the path every route that publishes it takes", () => { + // The bucket is charged by the identity resolvers (`meterAccountWrite`), + // not by each route, so a listed route meets it by resolving its caller + // through one of them and not opting out. `write-rate-limit-guard` + // holds every other mutating route to the same rule. + const unmetered = ON_THE_SHARED_BUCKET.filter(([route]) => { const src = readFileSync(join(process.cwd(), route), "utf8"); - return !src.includes("checkRecordWriteRateLimit("); + return ( + !/\brequire(?:Record)?Auth\s*\(/.test(src) || + /\bwriteBucket\s*:\s*["']own["']/.test(src) + ); }).map(([route]) => route); - expect(missing).toEqual([]); + expect(unmetered).toEqual([]); }); - it("names the code it publishes at every site", () => { - // The prose is not the discriminator: two of these routes consult a second - // bucket of their own, and a client that can only read `error` cannot tell - // which one refused it. - const silent = ON_THE_SHARED_BUCKET.filter(([route]) => { - const src = readFileSync(join(process.cwd(), route), "utf8"); - return !src.includes('errorCode: "record_write.rate_limited"'); - }).map(([route]) => route); - expect(silent).toEqual([]); - expect(published).toContain("record_write.rate_limited"); + it("names the code it publishes", () => { + // The prose is not the discriminator: several of these routes consult a + // second bucket of their own, and a client that can only read `error` + // cannot tell which one refused it. + expect(RECORD_WRITE_RATE_LIMITED_CODE).toBe("record_write.rate_limited"); + expect(published).toContain(RECORD_WRITE_RATE_LIMITED_CODE); }); it("is published on every operation that answers it", () => { diff --git a/src/__tests__/write-rate-limit-guard.test.ts b/src/__tests__/write-rate-limit-guard.test.ts new file mode 100644 index 000000000..a579655ee --- /dev/null +++ b/src/__tests__/write-rate-limit-guard.test.ts @@ -0,0 +1,220 @@ +/** + * Structural guard: every mutating route meets a rate limit. + * + * 169 of 364 mutating handlers met no limiter at all: every PATCH and DELETE + * on an `[id]`, and most single-record creates. The shared per-account write + * bucket now sits on the identity resolvers (`meterAccountWrite`, + * `src/lib/auth/write-meter.ts`), so a route that resolves its caller through + * one of them is metered without knowing it. This guard is what keeps that + * true for the routes added next year: + * + * - a route file exporting POST, PUT, PATCH or DELETE must either resolve + * its caller through a metering resolver, call a limiter of its own, or + * stand in {@link UNMETERED} with the reason; + * - a route that opts out of the shared bucket (`writeBucket: "own"`) must + * stand in {@link OWN_BUCKET} with the reason AND call its own limiter, so + * the opt-out can never mean "no limit"; + * - a helper that limits on a route's behalf is accepted only while its own + * source calls a limiter. + * + * The routes are derived from the tree, never listed by hand. The check is + * per file, not per handler: a file whose GET resolves through a metering + * resolver while its POST does not would pass. Comments are stripped first; + * the planted cases at the top prove the matchers fire. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +import { walkSourceFiles } from "./helpers/source-files"; + +const SRC = join(process.cwd(), "src"); + +/** The resolvers in `api-handler.ts` that charge the shared write bucket. */ +const METERING_RESOLVERS = [ + "requireAuth", + "requireRecordAuth", + "requireActorAuth", + "requireGuardianAuth", + "requireAdmin", + "requireCookieAuth", + "requireFreshMfa", + "requireFreshMfaIfEnrolled", + "requireFreshMfaOrElevationIfEnrolled", + "requireRecentProof", + "requireBearerAuth", + "requireMfaManagementAuth", +]; + +/** Helpers that call a limiter on the route's behalf, and where they live. */ +const LIMITING_HELPERS: Record = { + claimGate: ["lib/managed-profiles/claim-gate.ts"], + applyWebhookRateLimit: [ + "lib/withings/webhook-handler.ts", + "lib/whoop/webhook-handler.ts", + ], +}; + +/** Mutating routes with no limit, each with the reason it needs none. */ +const UNMETERED: Record = { + "app/api/auth/logout/route.ts": + "Ends the caller's own session or token and nothing else. It is idempotent, writes no record data, and refusing it would keep alive a credential its holder asked to end.", +}; + +/** Routes metered by a dedicated bucket instead of the shared one. */ +const OWN_BUCKET: Record = { + "app/api/measurements/batch/route.ts": + "The HealthKit uploader sends up to 60 batches of 500 a minute, and an outbox replay of parked pages bypasses the client pacer; the route's own 60-a-minute batch bucket meters it.", + "app/api/workouts/batch/route.ts": + "The workout importer sends page after page during a catch-up pass; the route's own batch bucket meters it.", + "app/api/medications/intake/bulk/route.ts": + "The Apple Health dose import and the standalone adoption send 200 to 500 intakes per request in sequence; the route's own bulk bucket meters it.", + "app/api/mood-entries/bulk/route.ts": + "The standalone adoption sends 200 entries per request in sequence; the route's own bulk bucket meters it.", + "app/api/cycle/day-logs/bulk/route.ts": + "The cycle HealthKit sweep sends 500-day slices in sequence; the route's own bulk bucket meters it.", + "app/api/nutrients/batch/route.ts": + "The nutrient sync sends 500-row batches in sequence; the route's own batch bucket meters it.", + "app/api/insights/ecg/route.ts": + "The ECG sweep sends one recording per request with no client-side delay, newest first, over the whole HealthKit history; the route's own 600-a-minute ingest bucket meters it.", +}; + +function stripComments(text: string): string { + return text.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^\s*\/\/.*$/gm, ""); +} + +const WRITES = + /export\s+(?:const|async\s+function)\s+(POST|PUT|PATCH|DELETE)\b/; +const OWN_LIMITER = /\b(?:check\w*RateLimit|checkRateLimit)\s*\(/; +const OPTS_OUT = /\bwriteBucket\s*:\s*["']own["']/; + +function callsAny(code: string, names: readonly string[]): boolean { + return names.some((name) => new RegExp(`\\b${name}\\s*\\(`).test(code)); +} + +function limited(code: string): boolean { + return ( + OWN_LIMITER.test(code) || callsAny(code, Object.keys(LIMITING_HELPERS)) + ); +} + +describe("every mutating route meets a rate limit", () => { + const files = walkSourceFiles(SRC, { floor: 3000 }).filter( + (file) => !file.startsWith("generated/") && !file.includes("__tests__/"), + ); + const read = (file: string) => + stripComments(readFileSync(join(SRC, file), "utf8")); + const mutating = files + .filter((f) => /^app\/.+\/route\.tsx?$/.test(f)) + .filter((f) => WRITES.test(read(f))); + + it("the matchers fire on planted sources", () => { + expect(WRITES.test("export const PATCH = apiHandler(async () => {")).toBe( + true, + ); + expect(WRITES.test("export async function DELETE(req) {")).toBe(true); + expect(WRITES.test("export const GET = apiHandler(async () => {")).toBe( + false, + ); + expect( + callsAny( + 'const { user } = await requireRecordAuth("write", "labs");', + METERING_RESOLVERS, + ), + ).toBe(true); + // A resolver named in a comment or an import does not count. + expect( + callsAny( + stripComments( + "import { requireAuth } from 'x';\n// requireAuth() here", + ), + METERING_RESOLVERS, + ), + ).toBe(false); + expect(OPTS_OUT.test('requireAuth(SCOPE, { writeBucket: "own" })')).toBe( + true, + ); + expect(limited("const rl = await checkRateLimit(`k:${id}`, 5, 60)")).toBe( + true, + ); + expect(limited("const rl = rateLimitHeaders(x)")).toBe(false); + }); + + it("finds the mutating routes", () => { + expect(mutating.length).toBeGreaterThanOrEqual(300); + }); + + it("meters every mutating route, or names why not", () => { + const unlimited = mutating.filter((f) => { + if (f in UNMETERED) return false; + const code = read(f); + const metered = + callsAny(code, METERING_RESOLVERS) && !OPTS_OUT.test(code); + return !metered && !limited(code); + }); + expect(unlimited).toEqual([]); + }); + + it("holds every opt-out to the reviewed list, and each to its own limiter", () => { + const optedOut = mutating.filter((f) => OPTS_OUT.test(read(f))).sort(); + expect(optedOut).toEqual(Object.keys(OWN_BUCKET).sort()); + const withoutOwn = optedOut.filter((f) => !OWN_LIMITER.test(read(f))); + expect(withoutOwn).toEqual([]); + }); + + it("accepts a limiting helper only while it calls a limiter", () => { + for (const [helper, homes] of Object.entries(LIMITING_HELPERS)) { + for (const home of homes) { + const code = read(home); + expect(code, home).toMatch( + new RegExp(`export\\s+async\\s+function\\s+${helper}\\b`), + ); + expect(OWN_LIMITER.test(code), home).toBe(true); + } + } + }); + + it("keeps the exceptions to routes that exist and still need them", () => { + for (const route of Object.keys(UNMETERED)) { + expect(mutating).toContain(route); + const code = read(route); + expect(callsAny(code, METERING_RESOLVERS) || limited(code), route).toBe( + false, + ); + } + }); + + it("charges the bucket in each metering resolver", () => { + const handler = read("lib/api-handler.ts"); + for (const name of [ + "requireAuth", + "requireActorAuth", + "requireRecordAuth", + "requireGuardianAuth", + "requireAdmin", + "requireCookieAuth", + "requireFreshMfa", + "requireBearerAuth", + ]) { + const start = handler.search( + new RegExp(`export\\s+async\\s+function\\s+${name}\\s*\\(`), + ); + expect(start, name).toBeGreaterThanOrEqual(0); + const next = handler.indexOf("\nexport ", start + 1); + const body = handler.slice(start, next === -1 ? undefined : next); + expect(body, name).toMatch(/\bmeterAccountWrite\s*\(/); + } + // The three that only delegate reach the meter through `requireAuth`. + for (const name of [ + "requireFreshMfaIfEnrolled", + "requireFreshMfaOrElevationIfEnrolled", + "requireRecentProof", + ]) { + const start = handler.search( + new RegExp(`export\\s+async\\s+function\\s+${name}\\s*\\(`), + ); + const next = handler.indexOf("\nexport ", start + 1); + expect(handler.slice(start, next), name).toMatch(/\brequireAuth\s*\(/); + } + }); +}); diff --git a/src/app/api/allergies/route.ts b/src/app/api/allergies/route.ts index 5f5436ef8..57a6b0153 100644 --- a/src/app/api/allergies/route.ts +++ b/src/app/api/allergies/route.ts @@ -15,13 +15,11 @@ import { apiHandler, requireRecordAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; import { auditLog } from "@/lib/auth/audit"; import { - apiError, apiSuccess, getClientIp, returnAllZodIssues, safeJson, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { withIdempotency } from "@/lib/idempotency"; import { encryptToBytes } from "@/lib/ai/coach/bytes-codec"; import { @@ -92,17 +90,7 @@ async function postAllergy(request: NextRequest): Promise { // A READ or WRITE delegate reaches neither: the destination is not on their // Settings list, and this line refuses them with `sharing.access.denied` // regardless. - const { user, actor } = await requireRecordAuth("manage", "profile"); - - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } + const { user } = await requireRecordAuth("manage", "profile"); const { data: rawBody, error: jsonError } = await safeJson(request, { maxBytes: 16 * 1024, diff --git a/src/app/api/biomarkers/[id]/merge/route.ts b/src/app/api/biomarkers/[id]/merge/route.ts index ea266a552..3c0e55465 100644 --- a/src/app/api/biomarkers/[id]/merge/route.ts +++ b/src/app/api/biomarkers/[id]/merge/route.ts @@ -31,7 +31,6 @@ import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; import { withIdempotency } from "@/lib/idempotency"; import { mergeBiomarkers as runMerge } from "@/lib/labs/biomarker-merge"; import { annotate } from "@/lib/logging/context"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { destroyedDetails } from "@/lib/sharing/audit-details"; import { biomarkerMergeSchema } from "@/lib/validations/biomarker-merge"; @@ -45,16 +44,9 @@ async function mergeBiomarkers( request: NextRequest, { params }: RouteParams, ): Promise { - const { user, actor } = await requireRecordAuth("manage", "labs"); + const { user } = await requireRecordAuth("manage", "labs"); const { id } = await params; - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } - const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 4 * 1024, }); diff --git a/src/app/api/biomarkers/route.ts b/src/app/api/biomarkers/route.ts index 2ff94ce4f..a68d39152 100644 --- a/src/app/api/biomarkers/route.ts +++ b/src/app/api/biomarkers/route.ts @@ -9,7 +9,6 @@ import { safeJson, sanitiseZodIssues, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { auditLog } from "@/lib/auth/audit"; import { prisma } from "@/lib/db"; import { withIdempotency } from "@/lib/idempotency"; @@ -76,17 +75,7 @@ async function postBiomarker(request: NextRequest) { // duplicate pre-check and the `@@unique([userId, name])` backstop both scope // to the same resolved id, so a delegate adding a marker the owner already // tracks gets the ordinary 409 rather than a second copy. - const { user, actor } = await requireRecordAuth("write", "labs"); - - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } + const { user } = await requireRecordAuth("write", "labs"); const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 16 * 1024, diff --git a/src/app/api/custom-metrics/[id]/entries/route.ts b/src/app/api/custom-metrics/[id]/entries/route.ts index bb4477ede..2e1832ad5 100644 --- a/src/app/api/custom-metrics/[id]/entries/route.ts +++ b/src/app/api/custom-metrics/[id]/entries/route.ts @@ -9,7 +9,6 @@ import { safeJson, sanitiseZodIssues, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { auditLog } from "@/lib/auth/audit"; import { invalidateUserCorrelationPatterns } from "@/lib/cache/invalidate"; import { serialiseCustomMetricEntry } from "@/lib/custom-metrics/custom-metric-store"; @@ -108,15 +107,6 @@ async function postCustomMetricEntry( // was never wrong, and the GET arm above stays delegable meanwhile. const { user } = await requireAuth(); - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(user.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } const { id } = await params; const metric = await prisma.customMetric.findFirst({ diff --git a/src/app/api/cycle/day-logs/bulk/route.ts b/src/app/api/cycle/day-logs/bulk/route.ts index 66abea65a..b005cfa7f 100644 --- a/src/app/api/cycle/day-logs/bulk/route.ts +++ b/src/app/api/cycle/day-logs/bulk/route.ts @@ -57,7 +57,10 @@ interface EntryResult { export const POST = apiHandler(withIdempotency<[NextRequest]>(postBulk)); async function postBulk(request: NextRequest): Promise { - const { user } = await requireAuth(); + // Metered by this route's own bulk bucket below, not the shared per-account + // write bucket: the native client syncs through here in bursts (see + // `meterAccountWrite`). + const { user } = await requireAuth(undefined, { writeBucket: "own" }); const gate = await requireCycleEnabled(user.id, user.gender); if (!gate.enabled) return gate.response; diff --git a/src/app/api/cycle/day-logs/route.ts b/src/app/api/cycle/day-logs/route.ts index 158d85c84..3f776a0bc 100644 --- a/src/app/api/cycle/day-logs/route.ts +++ b/src/app/api/cycle/day-logs/route.ts @@ -26,7 +26,6 @@ import { returnAllZodIssues, safeJson, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { withIdempotency } from "@/lib/idempotency"; import { requireCycleEnabled } from "@/lib/cycle/gate"; import { @@ -95,21 +94,11 @@ async function postDayLog(request: NextRequest): Promise { // v1.37.0 — MANAGE. The upsert replaces the day, which is what edit means // here; the gate below resolves `user.gender` against the RECORD, so the // module follows the record rather than the caller. - const { user, actor } = await requireRecordAuth("manage", "cycle"); + const { user } = await requireRecordAuth("manage", "cycle"); const gate = await requireCycleEnabled(user.id, user.gender); if (!gate.enabled) return gate.response; - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } - const { data: rawBody, error: jsonError } = await safeJson(request, { maxBytes: 64 * 1024, }); diff --git a/src/app/api/encounters/route.ts b/src/app/api/encounters/route.ts index f44a047f1..5686620b2 100644 --- a/src/app/api/encounters/route.ts +++ b/src/app/api/encounters/route.ts @@ -25,7 +25,6 @@ import { returnAllZodIssues, safeJson, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { apiListSuccess } from "@/lib/api/list-cap"; import { combinedCap, listPage } from "@/lib/http/list-query"; import { withIdempotency } from "@/lib/idempotency"; @@ -167,17 +166,7 @@ export const GET = apiHandler(async (request: NextRequest) => { export const POST = apiHandler(withIdempotency<[NextRequest]>(postEncounter)); async function postEncounter(request: NextRequest): Promise { - const { user, actor, grantId } = await requireRecordAuth("write", "profile"); - - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } + const { user, grantId } = await requireRecordAuth("write", "profile"); const { data: rawBody, error: jsonError } = await safeJson(request, { maxBytes: 32 * 1024, diff --git a/src/app/api/insights/ecg/route.ts b/src/app/api/insights/ecg/route.ts index 83db67783..4a9cfd219 100644 --- a/src/app/api/insights/ecg/route.ts +++ b/src/app/api/insights/ecg/route.ts @@ -203,7 +203,10 @@ export const GET = apiHandler(async () => { * to the same row by construction rather than by a cached response. */ export const POST = apiHandler(async (request: NextRequest) => { - const { user } = await requireAuth(); + // Metered by this route's own bulk bucket below, not the shared per-account + // write bucket: the native client syncs through here in bursts (see + // `meterAccountWrite`). + const { user } = await requireAuth(undefined, { writeBucket: "own" }); // A device ingest carries no model text: switching AI off, or the AI // analysis opt-out, must never refuse a recording. diff --git a/src/app/api/labs/ocr/commit/__tests__/route.test.ts b/src/app/api/labs/ocr/commit/__tests__/route.test.ts index b855c1fd6..435dd77eb 100644 --- a/src/app/api/labs/ocr/commit/__tests__/route.test.ts +++ b/src/app/api/labs/ocr/commit/__tests__/route.test.ts @@ -33,6 +33,12 @@ vi.mock("@/lib/rate-limit", async (importOriginal) => ({ resetAt: new Date(Date.now() + 60_000), })), })); +// The real meter, so the ceiling is proven on the path it now takes: the route +// resolves its caller, the resolver charges the bucket. The unit setup stubs +// the meter for every other route test. +vi.mock("@/lib/auth/write-meter", async (importOriginal) => + importOriginal(), +); const tzMock = vi.hoisted(() => ({ zone: "UTC" })); vi.mock("@/lib/tz/resolver", async (importOriginal) => ({ ...(await importOriginal()), @@ -216,7 +222,7 @@ describe("POST /api/labs/ocr/commit dedup across the user's day", () => { }); describe("POST /api/labs/ocr/commit write ceiling", () => { - it("charges the shared per-account record-write bucket", async () => { + it("charges the shared per-account record-write bucket where the caller is resolved", async () => { const res = await POST(postReq({ rows: [numericRow(95)] })); expect(res.status).toBe(200); expect(checkRecordWriteRateLimit).toHaveBeenCalledWith("user-1"); diff --git a/src/app/api/labs/ocr/commit/route.ts b/src/app/api/labs/ocr/commit/route.ts index 3dd653fc7..b9bd332f0 100644 --- a/src/app/api/labs/ocr/commit/route.ts +++ b/src/app/api/labs/ocr/commit/route.ts @@ -20,7 +20,6 @@ import { fireAndForget } from "@/lib/logging/fire-and-forget"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { - apiError, apiSuccess, getClientIp, returnAllZodIssues, @@ -31,7 +30,6 @@ import { auditLog } from "@/lib/auth/audit"; import { prisma } from "@/lib/db"; import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; import { withIdempotency } from "@/lib/idempotency"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { enqueueReminderSatisfy } from "@/lib/jobs/reminder-satisfy"; import { emitDataArrival } from "@/lib/arrivals/emit-shared"; import { rememberImportAlias } from "@/lib/labs/biomarker-aliases"; @@ -159,17 +157,6 @@ function inBatchKey( async function commitOcrRows(request: NextRequest) { const { user } = await requireAuth(); - // Shared per-account write ceiling, the same bucket every other lab write - // charges (`POST /api/labs`, `POST /api/biomarkers`). A commit can write a - // whole report's rows and mint markers on the way, so it was the one lab - // write a looping client could repeat without any ceiling. - const writeRl = await checkRecordWriteRateLimit(user.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } - const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 256 * 1024, }); diff --git a/src/app/api/labs/route.ts b/src/app/api/labs/route.ts index 60d4dcfbc..31bd837a1 100644 --- a/src/app/api/labs/route.ts +++ b/src/app/api/labs/route.ts @@ -10,7 +10,6 @@ import { safeJson, sanitiseZodIssues, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { auditLog } from "@/lib/auth/audit"; import { prisma } from "@/lib/db"; import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; @@ -156,17 +155,7 @@ async function postLabResult(request: NextRequest) { // it mints it under `user.id`: the marker joins the record's catalogue, not // the caller's, which is the only reading of "add a result to your record" // that leaves the owner with a usable catalogue afterwards. - const { user, actor } = await requireRecordAuth("write", "labs"); - - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } + const { user } = await requireRecordAuth("write", "labs"); const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 16 * 1024, diff --git a/src/app/api/life-events/route.ts b/src/app/api/life-events/route.ts index cdb0d32a0..3d9dd4ee4 100644 --- a/src/app/api/life-events/route.ts +++ b/src/app/api/life-events/route.ts @@ -21,7 +21,6 @@ import { NextRequest } from "next/server"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { - apiError, apiSuccess, getClientIp, returnAllZodIssues, @@ -36,7 +35,6 @@ import { toLifeEventDTO, } from "@/lib/life-events/service"; import { annotate } from "@/lib/logging/context"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; export const GET = apiHandler(async () => { const { user } = await requireAuth(); @@ -53,13 +51,6 @@ export const POST = apiHandler(withIdempotency<[NextRequest]>(postLifeEvent)); async function postLifeEvent(request: NextRequest): Promise { const { user } = await requireAuth(); - const writeRl = await checkRecordWriteRateLimit(user.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } - const { data: rawBody, error: jsonError } = await safeJson(request, { maxBytes: 16 * 1024, }); diff --git a/src/app/api/measurements/batch/route.ts b/src/app/api/measurements/batch/route.ts index f6e04823f..501b03fea 100644 --- a/src/app/api/measurements/batch/route.ts +++ b/src/app/api/measurements/batch/route.ts @@ -293,7 +293,12 @@ async function postBatch(request: NextRequest): Promise { // a native login mints. `requireAuth` refuses any acting-account carrier for // every credential shape, so the scoped token is confined to its owner's own // record here without this route adding an arm of its own. - const auth = await requireAuth(MEASUREMENTS_WRITE_SCOPE); + // Metered by this route's own bulk bucket below, not the shared per-account + // write bucket: the native client syncs through here in bursts (see + // `meterAccountWrite`). + const auth = await requireAuth(MEASUREMENTS_WRITE_SCOPE, { + writeBucket: "own", + }); const { user } = auth; // A scoped credential is a bridge — a scale, a home-automation rule — and not diff --git a/src/app/api/measurements/route.ts b/src/app/api/measurements/route.ts index e707e63d9..655666a85 100644 --- a/src/app/api/measurements/route.ts +++ b/src/app/api/measurements/route.ts @@ -17,7 +17,6 @@ import { safeJson, sanitiseZodIssues, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { createMeasurementSchema, createBatchMeasurementSchema, @@ -403,16 +402,6 @@ async function postMeasurement(request: NextRequest) { const { user } = auth; const scoped = isScopedCredential(auth); - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(auth.actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } - const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 64 * 1024, }); diff --git a/src/app/api/medications/[id]/intake/route.ts b/src/app/api/medications/[id]/intake/route.ts index 0ecd6c9d1..0e05bbecf 100644 --- a/src/app/api/medications/[id]/intake/route.ts +++ b/src/app/api/medications/[id]/intake/route.ts @@ -10,7 +10,6 @@ import { safeJson, sanitiseZodIssues, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { intakeSchema, listIntakeEventsSchema, @@ -62,16 +61,6 @@ async function postIntake(request: NextRequest, { params }: RouteParams) { "medications", ); - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } - // v1.36.1 — the sibling route `POST /api/medications/intake` // refuses a delegate changing a dose the owner already recorded. This route // reaches the same rows through the slot upsert, so without the same rule it diff --git a/src/app/api/medications/intake/bulk/route.ts b/src/app/api/medications/intake/bulk/route.ts index 7b9847940..4b20c810d 100644 --- a/src/app/api/medications/intake/bulk/route.ts +++ b/src/app/api/medications/intake/bulk/route.ts @@ -197,7 +197,10 @@ interface EntryResult { export const POST = apiHandler(withIdempotency<[NextRequest]>(postBulk)); async function postBulk(request: NextRequest): Promise { - const auth = await requireAuth(); + // Metered by this route's own bulk bucket below, not the shared per-account + // write bucket: the native client syncs through here in bursts (see + // `meterAccountWrite`). + const auth = await requireAuth(undefined, { writeBucket: "own" }); const { user } = auth; // A narrow, single-purpose credential (a bridge, an ingest token) is no // Apple Health sync, so it never writes the dose arrival record below. The diff --git a/src/app/api/medications/intake/route.ts b/src/app/api/medications/intake/route.ts index 98d92f114..dbdaf6e1c 100644 --- a/src/app/api/medications/intake/route.ts +++ b/src/app/api/medications/intake/route.ts @@ -21,7 +21,6 @@ import { safeJson, sanitiseZodIssues, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { annotate } from "@/lib/logging/context"; import { prisma } from "@/lib/db"; import { auditLog } from "@/lib/auth/audit"; @@ -212,16 +211,6 @@ export const POST = apiHandler(async (request: NextRequest) => { // notification at the end of this handler. const { user, actor } = await requireRecordAuth("write", "medications"); - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } - const { data: body, error } = await safeJson(request, { maxBytes: 64 * 1024, }); diff --git a/src/app/api/mood-entries/bulk/route.ts b/src/app/api/mood-entries/bulk/route.ts index 8a606cf7d..b0564355e 100644 --- a/src/app/api/mood-entries/bulk/route.ts +++ b/src/app/api/mood-entries/bulk/route.ts @@ -194,7 +194,10 @@ interface EntryResult { export const POST = apiHandler(withIdempotency<[NextRequest]>(postBulk)); async function postBulk(request: NextRequest): Promise { - const { user } = await requireAuth(); + // Metered by this route's own bulk bucket below, not the shared per-account + // write bucket: the native client syncs through here in bursts (see + // `meterAccountWrite`). + const { user } = await requireAuth(undefined, { writeBucket: "own" }); const rl = await checkRateLimit( `mood-entries:bulk:${user.id}`, diff --git a/src/app/api/mood-entries/route.ts b/src/app/api/mood-entries/route.ts index 94a8c4a82..67f38fe4d 100644 --- a/src/app/api/mood-entries/route.ts +++ b/src/app/api/mood-entries/route.ts @@ -8,7 +8,6 @@ import { safeJson, sanitiseZodIssues, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { createMoodEntrySchema, listMoodEntriesSchema, @@ -109,16 +108,6 @@ async function postMoodEntry(request: NextRequest) { // the `externalId` refusal below. const { user, actor } = await requireRecordAuth("manage", "mind"); - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } - const { data: body, error: jsonError } = await safeJson(request, { maxBytes: 64 * 1024, }); diff --git a/src/app/api/nutrients/batch/route.ts b/src/app/api/nutrients/batch/route.ts index 04a3c31a2..ac2d88f2c 100644 --- a/src/app/api/nutrients/batch/route.ts +++ b/src/app/api/nutrients/batch/route.ts @@ -79,7 +79,10 @@ interface EntryResult { export const POST = apiHandler(withIdempotency<[NextRequest]>(postBatch)); async function postBatch(request: NextRequest): Promise { - const { user } = await requireAuth(); + // Metered by this route's own bulk bucket below, not the shared per-account + // write bucket: the native client syncs through here in bursts (see + // `meterAccountWrite`). + const { user } = await requireAuth(undefined, { writeBucket: "own" }); // Module gate first: a disabled module must refuse before any body // work, so iOS can branch on the 403 `module.disabled` errorCode and diff --git a/src/app/api/vaccinations/custom/route.ts b/src/app/api/vaccinations/custom/route.ts index dbb15e488..d114d3a3a 100644 --- a/src/app/api/vaccinations/custom/route.ts +++ b/src/app/api/vaccinations/custom/route.ts @@ -31,7 +31,6 @@ import { returnAllZodIssues, safeJson, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { customVaccineCreateSchema } from "@/lib/validations/vaccinations"; import { toCustomVaccineDTO } from "@/lib/vaccinations/dto"; import { @@ -57,16 +56,7 @@ export const GET = apiHandler(async () => { }); export const POST = apiHandler(async (request: NextRequest) => { - const { user, actor } = await requireRecordAuth("write", "profile"); - - // The shared per-account write ceiling, keyed on the ACTOR so a delegate - // spends their own allowance (`checkRecordWriteRateLimit`). - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } + const { user } = await requireRecordAuth("write", "profile"); const { data: rawBody, error: jsonError } = await safeJson(request, { maxBytes: 8 * 1024, diff --git a/src/app/api/vaccinations/route.ts b/src/app/api/vaccinations/route.ts index 3a15bc2c8..afb938b15 100644 --- a/src/app/api/vaccinations/route.ts +++ b/src/app/api/vaccinations/route.ts @@ -29,7 +29,6 @@ import { returnAllZodIssues, safeJson, } from "@/lib/api-response"; -import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; import { withIdempotency } from "@/lib/idempotency"; import { apiListSuccess } from "@/lib/api/list-cap"; import { listPage } from "@/lib/http/list-query"; @@ -150,17 +149,7 @@ export const GET = apiHandler(async (request: NextRequest) => { export const POST = apiHandler(withIdempotency<[NextRequest]>(postVaccination)); async function postVaccination(request: NextRequest): Promise { - const { user, actor } = await requireRecordAuth("write", "profile"); - - // Shared per-account write ceiling — see `checkRecordWriteRateLimit`. The - // batch siblings have always been capped; the per-record creates a looping - // client hits were not. - const writeRl = await checkRecordWriteRateLimit(actor.id); - if (!writeRl.allowed) { - return apiError("Too many writes, try again later", 429, { - errorCode: "record_write.rate_limited", - }); - } + const { user } = await requireRecordAuth("write", "profile"); const { data: rawBody, error: jsonError } = await safeJson(request, { maxBytes: 32 * 1024, diff --git a/src/app/api/workouts/batch/route.ts b/src/app/api/workouts/batch/route.ts index c25beccca..37ca6dfaf 100644 --- a/src/app/api/workouts/batch/route.ts +++ b/src/app/api/workouts/batch/route.ts @@ -239,7 +239,12 @@ async function postBatch(request: NextRequest): Promise { // native login mints. The only route that names it. `requireAuth` refuses // any acting-account carrier for every credential shape, so the scoped token // is confined to its owner's own record without an arm of its own here. - const auth = await requireAuth(WORKOUTS_WRITE_SCOPE); + // Metered by this route's own bulk bucket below, not the shared per-account + // write bucket: the native client syncs through here in bursts (see + // `meterAccountWrite`). + const auth = await requireAuth(WORKOUTS_WRITE_SCOPE, { + writeBucket: "own", + }); const { user } = auth; // A scoped credential is a bridge, not the native client: the source of diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 564bba265..8166b60d5 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -55,6 +55,7 @@ import { attachRecordContextEcho, } from "./sharing/record-session-fence"; import { refuseOnKeyMismatch } from "./boot/key-mismatch-refusal"; +import { meterAccountWrite, type WriteMeterOptions } from "./auth/write-meter"; import { acceptIncomingRequestId } from "./http/request-id"; import { withErrorEnvelopeDefaults } from "./api-response"; import { @@ -545,12 +546,16 @@ export function isScopedCredential(auth: AuthContext): boolean { */ export async function requireAuth( requiredPermission?: string, + options?: Pick, ): Promise { const auth = await authenticateCaller(requiredPermission); const carrier = await readActingCarrier(auth); if (carrier.kind !== "none") { throw refuseUndeclaredMode(auth, carrier); } + // The shared write ceiling, for every write this resolver admits — see + // `meterAccountWrite`. + await meterAccountWrite(auth.user.id, options); return auth; } @@ -866,6 +871,7 @@ export async function requireActorAuth(): Promise { auditRefusal(auth, { kind: "header", accountId: header }, "actor_surface"); throw new SharingNotPermittedError(); } + await meterAccountWrite(auth.user.id); return auth; } @@ -913,6 +919,13 @@ export interface RecordAuthOptions { * above `"read"`. */ readonly readOverPost?: true; + /** + * This route meters its writes with a dedicated bucket instead of the shared + * per-account one (`meterAccountWrite`). Frozen per route, with the reason, + * by `write-rate-limit-guard.test.ts`, which also requires the route to call + * its own limiter. + */ + readonly writeBucket?: "own"; } /** @@ -966,6 +979,13 @@ export async function requireRecordAuth( // fence reads, so the invariant is "before the carrier", not "before the // first database await". await assertRecordSessionFence(auth); + // The shared write ceiling, charged to the ACTING account before any grant + // is read, so a refused write counts like an admitted one — see + // `meterAccountWrite`. + await meterAccountWrite(auth.user.id, { + readOverPost: options?.readOverPost, + writeBucket: options?.writeBucket, + }); const carrier = await readActingCarrier(auth); // A narrow, single-purpose Bearer never acts on another record. @@ -1063,6 +1083,7 @@ export async function requireGuardianAuth(): Promise { // a context that has since moved must be refused before the marker is read, // not after. await assertRecordSessionFence(auth); + await meterAccountWrite(auth.user.id); const carrier = await readActingCarrier(auth); if (carrier.kind === "none") { @@ -1207,6 +1228,7 @@ export async function requireAdmin(): Promise { if (sessionData.user.role !== "ADMIN") { throw new HttpError(403, "Admin access required", AUTH_ERROR_CODES.admin); } + await meterAccountWrite(sessionData.user.id); return { ...sessionData, authMethod: "cookie" }; } @@ -1233,6 +1255,7 @@ export async function requireCookieAuth(): Promise { auth_method: "session", }); } + await meterAccountWrite(sessionData.user.id); return { ...sessionData, authMethod: "cookie" }; } @@ -1343,6 +1366,8 @@ export async function requireFreshMfa( }); } + await meterAccountWrite(sessionData.user.id); + // The account must hold a credential that can stamp `mfaVerifiedAt` in the // first place. Without one a step-up-gated action is unreachable by design // rather than merely refused, so the refusal is named differently @@ -1661,6 +1686,7 @@ export async function requireBearerAuth(): Promise { } const raw = authHeader.slice(7); const auth = await authenticateBearer(raw, undefined); + await meterAccountWrite(auth.user.id); return { ...auth, apiTokenId: auth.session.id, diff --git a/src/lib/auth/__tests__/write-meter.test.ts b/src/lib/auth/__tests__/write-meter.test.ts new file mode 100644 index 000000000..ac6069b83 --- /dev/null +++ b/src/lib/auth/__tests__/write-meter.test.ts @@ -0,0 +1,102 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// The unit setup stubs the meter for every route test; this file is where the +// real one is pinned. +vi.mock("@/lib/auth/write-meter", async (importOriginal) => + importOriginal(), +); + +const state = vi.hoisted(() => ({ + event: null as null | { getHttpMethod: () => string | undefined }, + allowed: true, +})); + +vi.mock("@/lib/logging/context", () => ({ + getEvent: () => state.event, + annotate: vi.fn(), +})); + +vi.mock("@/lib/rate-limit", () => ({ + checkRecordWriteRateLimit: vi.fn(async () => ({ + allowed: state.allowed, + limit: 300, + remaining: state.allowed ? 299 : 0, + resetAt: Date.now() + 60_000, + })), +})); + +import { + meterAccountWrite, + RECORD_WRITE_RATE_LIMITED_CODE, +} from "../write-meter"; +import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; +import { HttpError } from "@/lib/api-errors"; + +function request(method: string | undefined) { + state.event = { getHttpMethod: () => method }; +} + +beforeEach(() => { + vi.mocked(checkRecordWriteRateLimit).mockClear(); + state.allowed = true; + state.event = null; +}); + +describe("meterAccountWrite", () => { + it.each(["POST", "PUT", "PATCH", "DELETE", "patch"])( + "charges the acting account for a %s", + async (method) => { + request(method); + await meterAccountWrite("account-1"); + expect(checkRecordWriteRateLimit).toHaveBeenCalledWith("account-1"); + }, + ); + + it.each(["GET", "HEAD", "OPTIONS"])("leaves a %s alone", async (method) => { + request(method); + await meterAccountWrite("account-1"); + expect(checkRecordWriteRateLimit).not.toHaveBeenCalled(); + }); + + it("charges nothing outside a request", async () => { + await meterAccountWrite("account-1"); + expect(checkRecordWriteRateLimit).not.toHaveBeenCalled(); + }); + + it("charges one request once, however many resolvers it passes", async () => { + request("POST"); + await meterAccountWrite("account-1"); + await meterAccountWrite("account-1"); + expect(checkRecordWriteRateLimit).toHaveBeenCalledTimes(1); + + request("POST"); + await meterAccountWrite("account-1"); + expect(checkRecordWriteRateLimit).toHaveBeenCalledTimes(2); + }); + + it("leaves a read-over-POST alone, and only the POST", async () => { + request("POST"); + await meterAccountWrite("account-1", { readOverPost: true }); + expect(checkRecordWriteRateLimit).not.toHaveBeenCalled(); + request("DELETE"); + await meterAccountWrite("account-1", { readOverPost: true }); + expect(checkRecordWriteRateLimit).toHaveBeenCalledTimes(1); + }); + + it("leaves a route with its own bucket alone", async () => { + request("POST"); + await meterAccountWrite("account-1", { writeBucket: "own" }); + expect(checkRecordWriteRateLimit).not.toHaveBeenCalled(); + }); + + it("refuses a spent bucket with 429 and the published code", async () => { + state.allowed = false; + request("PATCH"); + const refusal = await meterAccountWrite("account-1").catch((e) => e); + expect(refusal).toBeInstanceOf(HttpError); + expect((refusal as HttpError).statusCode).toBe(429); + expect((refusal as HttpError).errorCode).toBe( + RECORD_WRITE_RATE_LIMITED_CODE, + ); + }); +}); diff --git a/src/lib/auth/write-meter.ts b/src/lib/auth/write-meter.ts new file mode 100644 index 000000000..25c4a1895 --- /dev/null +++ b/src/lib/auth/write-meter.ts @@ -0,0 +1,88 @@ +/** + * The shared per-account write ceiling, applied where the caller is resolved. + * + * `checkRecordWriteRateLimit` (300 writes a minute per acting account) used to + * be called by hand in a dozen create routes, and the other mutating routes + * had no ceiling at all: of 364 mutating handlers, 169 met no limiter, every + * PATCH and DELETE on an `[id]` among them. A positive list of routes that + * remember to call it can only fall behind the routes that exist, so the + * ceiling now sits on the path every authenticated write already takes: the + * identity resolvers in `src/lib/api-handler.ts` call {@link meterAccountWrite} + * once the caller is known, and it charges the bucket for any request whose + * method writes. + * + * What it does not do: + * + * - charge a read. GET, HEAD and OPTIONS pass untouched, and so does a POST a + * route declared `readOverPost` (a search whose query must not stand in a + * URL). + * - charge one request twice. A route that resolves the caller through two + * helpers (step-up on top of a session) is charged once. + * - meter a route that declared its own bucket. The bulk ingest endpoints + * the native client syncs through have dedicated ceilings sized for that + * volume; charging the shared bucket as well would let a large HealthKit + * backfill lock the person out of logging a single dose by hand. Declaring + * `writeBucket: "own"` is frozen per route by + * `src/__tests__/write-rate-limit-guard.test.ts`, which also requires the + * route to call its own limiter. + * + * The bucket is keyed on the ACTING account, the precedent from the delegable + * medication routes: a delegate spends their own allowance rather than the + * record owner's, and cannot collect a fresh one by switching records. A + * refused request is counted too, since the charge happens before the body is + * read. + */ +import { HttpError } from "@/lib/api-errors"; +import { annotate, getEvent } from "@/lib/logging/context"; +import { checkRecordWriteRateLimit } from "@/lib/rate-limit"; + +/** The code a refusal by the shared write bucket carries. */ +export const RECORD_WRITE_RATE_LIMITED_CODE = "record_write.rate_limited"; + +const WRITE_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]); + +/** Requests already charged, so a second resolver in one request is free. */ +const charged = new WeakSet(); + +export interface WriteMeterOptions { + /** The route answers this POST without writing (see `readOverPost`). */ + readOverPost?: true; + /** + * The route meters its writes with a dedicated bucket instead of the shared + * one. Reviewed per route by the write rate-limit guard. + */ + writeBucket?: "own"; +} + +/** + * Charge the acting account's shared write bucket for this request, or refuse + * it with 429 when the bucket is spent. + * + * Outside a request (no wide event, so no method) there is nothing to charge. + */ +export async function meterAccountWrite( + accountId: string, + options: WriteMeterOptions = {}, +): Promise { + const evt = getEvent(); + if (!evt) return; + const method = (evt.getHttpMethod() ?? "").toUpperCase(); + if (!WRITE_METHODS.has(method)) return; + if (options.readOverPost === true && method === "POST") return; + if (options.writeBucket === "own") { + annotate({ meta: { write_bucket: "own" } }); + return; + } + if (charged.has(evt)) return; + charged.add(evt); + + const rl = await checkRecordWriteRateLimit(accountId); + if (!rl.allowed) { + annotate({ meta: { record_write_rate_limited: true } }); + throw new HttpError( + 429, + "Too many writes, try again later", + RECORD_WRITE_RATE_LIMITED_CODE, + ); + } +} diff --git a/src/lib/openapi/routes/shared.ts b/src/lib/openapi/routes/shared.ts index 4c20a843b..afa7f2533 100644 --- a/src/lib/openapi/routes/shared.ts +++ b/src/lib/openapi/routes/shared.ts @@ -356,10 +356,12 @@ export const stdResponses = { * * The batch endpoints have been capped at 60 calls a minute since they were * written and the per-record siblings were not capped at all, which is exactly - * backwards: the batch endpoint is the one a well-behaved client uses. The - * eleven single-record creates now share one generous per-account bucket, and - * this is the response that names it — a client that meets a ceiling should be - * able to read which one it met without a bug report. + * backwards: the batch endpoint is the one a well-behaved client uses. Every + * authenticated write now shares one generous per-account bucket, charged where + * the caller is resolved (`src/lib/auth/write-meter.ts`), and this is the + * response that names it on the single-record creates that publish it — a + * client that meets a ceiling should be able to read which one it met without + * a bug report. * * Spread AFTER `...stdResponses` so it replaces the generic 429 on those * operations. The headers block is the one the standard 429 already declares. @@ -371,7 +373,7 @@ export const stdResponses = { export const recordWriteRateLimitResponse = { "429": { description: - "Rate limit exceeded. This route shares one per-account bucket with the other single-record writes — `record-write:`, 300 requests per 60 seconds — keyed on the ACTING account, so a delegate burns their own allowance rather than the record owner's. Nothing was written. `meta.errorCode` is `record_write.rate_limited`, which is what tells this refusal apart from a route's own narrower bucket when both can answer 429. The `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining` and `X-RateLimit-Reset` headers describe that bucket; back off on `Retry-After` rather than guessing. Every request counts against the bucket, refused ones included: the check runs before the body is read, so a client looping on a 422 spends the same allowance as one that writes rows. A client with more than a handful of rows to send should use the batch endpoint for its domain instead of looping this one.", + "Rate limit exceeded. This route shares one per-account bucket with every other authenticated write — `record-write:`, 300 requests per 60 seconds — keyed on the ACTING account, so a delegate burns their own allowance rather than the record owner's. Only the bulk ingest endpoints, which meter themselves with a dedicated bucket, stay outside it. Nothing was written. `meta.errorCode` is `record_write.rate_limited`, which is what tells this refusal apart from a route's own narrower bucket when both can answer 429. The `Retry-After`, `X-RateLimit-Limit`, `X-RateLimit-Remaining` and `X-RateLimit-Reset` headers describe that bucket; back off on `Retry-After` rather than guessing. Every request counts against the bucket, refused ones included: the check runs before the body is read, so a client looping on a 422 spends the same allowance as one that writes rows. A client with more than a handful of rows to send should use the batch endpoint for its domain instead of looping this one.", content: { "application/json": { schema: errorEnvelope } }, headers: stdResponses["429"].headers, }, diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index 9c9daa3cd..9951a2bd9 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -136,25 +136,23 @@ export async function checkEnvironmentBackfillRateLimit( } /** - * Shared per-account ceiling for the single-record writes. + * Shared per-account ceiling for every authenticated write. * * The batch endpoints have been capped at 60 calls a minute since they were * written; the per-record siblings a naive or older client loops over were not * capped at all, which is backwards — the batch endpoint is the one a - * well-behaved client uses. This is the floor under the other direction: - * `POST /api/measurements`, `/api/mood-entries`, both medication-intake writes, - * `/api/labs`, a custom-metric entry, a cycle day-log, a vaccination, an - * allergy, a biomarker and an encounter. + * well-behaved client uses. This was first the floor under a dozen creates, + * called by hand; it is now charged where the caller is resolved + * (`meterAccountWrite`, `src/lib/auth/write-meter.ts`), so every POST, PUT, + * PATCH and DELETE an authenticated caller sends counts against it, except on + * the bulk ingest routes that meter themselves. * * One shared bucket, not one per route, for the reason - * `checkAnalyticsReadRateLimit` gives: a loop that rotates across ten creates + * `checkAnalyticsReadRateLimit` gives: a loop that rotates across ten routes * should still meet a cap. Three hundred a minute is five writes a second * sustained for a full minute — no hand-driven session and no sync burst - * reaches it. Ten of the eleven are wrapped in `withIdempotency`, so an honest - * retry storm is deduplicated before it is counted; `POST - * /api/medications/intake` is not, because it updates a named `intakeId` - * rather than creating a row, so a replay writes the status that is already - * there. It exists to cut off the pathological loop, not to shape traffic. + * reaches it. It exists to cut off the pathological loop, not to shape + * traffic. * * A refused request is counted too: the check runs before the body is read, * so a client looping on a 422 spends its allowance without writing a row. diff --git a/tests/integration/write-meter.test.ts b/tests/integration/write-meter.test.ts new file mode 100644 index 000000000..5b448a8f8 --- /dev/null +++ b/tests/integration/write-meter.test.ts @@ -0,0 +1,185 @@ +/** + * The shared write ceiling, against Postgres, on routes that never call it. + * + * The bucket is charged where the caller is resolved, so the routes below + * carry no limiter of their own; a PATCH and a DELETE on an `[id]` met none + * before. What has to hold: a write is counted, a read is not, a spent bucket + * refuses with the published code and the headers that describe it and + * writes nothing, and a bulk route on its own bucket keeps working while the + * shared one is spent. + */ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; +import { + RECORD_WRITE_BUCKET_PREFIX, + RECORD_WRITE_LIMIT, +} from "@/lib/rate-limit"; + +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), +})); + +const USER_ID = "user-write-meter"; +const BUCKET = `${RECORD_WRITE_BUCKET_PREFIX}:${USER_ID}`; + +async function bucketCount(): Promise { + const row = await getPrismaClient().rateLimit.findUnique({ + where: { key: BUCKET }, + }); + return row?.count ?? 0; +} + +async function spendBucket(): Promise { + await getPrismaClient().rateLimit.upsert({ + where: { key: BUCKET }, + create: { + key: BUCKET, + count: RECORD_WRITE_LIMIT, + resetAt: new Date(Date.now() + 60_000), + }, + update: { + count: RECORD_WRITE_LIMIT, + resetAt: new Date(Date.now() + 60_000), + }, + }); +} + +let allergyId: string; + +beforeEach(async () => { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + cookieJar.clear(); + headerJar.clear(); + await prisma.user.create({ + data: { + id: USER_ID, + username: "write-meter", + email: "write-meter@example.test", + timezone: "UTC", + }, + }); + const session = await prisma.session.create({ + data: { userId: USER_ID, expiresAt: new Date(Date.now() + 3_600_000) }, + }); + cookieJar.set("healthlog_session", session.id); + const allergy = await prisma.allergy.create({ + data: { + userId: USER_ID, + substance: "Pollen", + category: "ENVIRONMENT", + type: "ALLERGY", + status: "ACTIVE", + }, + }); + allergyId = allergy.id; +}); + +function patchAllergy(body: unknown) { + return import("@/app/api/allergies/[id]/route").then(({ PATCH }) => + PATCH( + new NextRequest(`http://localhost/api/allergies/${allergyId}`, { + method: "PATCH", + body: JSON.stringify(body), + headers: { "content-type": "application/json" }, + }), + { params: Promise.resolve({ id: allergyId }) }, + ), + ); +} + +describe("the shared write bucket on routes without a limiter of their own", () => { + it("counts a PATCH and a DELETE, and not a GET", async () => { + const { GET, DELETE } = await import("@/app/api/allergies/[id]/route"); + + const read = await GET( + new NextRequest(`http://localhost/api/allergies/${allergyId}`), + { params: Promise.resolve({ id: allergyId }) }, + ); + expect(read.status).toBe(200); + expect(await bucketCount()).toBe(0); + + expect((await patchAllergy({ substance: "Birch pollen" })).status).toBe( + 200, + ); + expect(await bucketCount()).toBe(1); + + const gone = await DELETE( + new NextRequest(`http://localhost/api/allergies/${allergyId}`, { + method: "DELETE", + }), + { params: Promise.resolve({ id: allergyId }) }, + ); + expect(gone.status).toBe(200); + expect(await bucketCount()).toBe(2); + }); + + it("refuses a write once the bucket is spent, says why, and writes nothing", async () => { + await spendBucket(); + + const res = await patchAllergy({ substance: "Birch pollen" }); + expect(res.status).toBe(429); + const body = (await res.json()) as { meta?: { errorCode?: string } }; + expect(body.meta?.errorCode).toBe("record_write.rate_limited"); + expect(Number(res.headers.get("Retry-After"))).toBeGreaterThan(0); + expect(res.headers.get("X-RateLimit-Limit")).toBe( + String(RECORD_WRITE_LIMIT), + ); + + const row = await getPrismaClient().allergy.findUniqueOrThrow({ + where: { id: allergyId }, + }); + expect(row.substance).toBe("Pollen"); + }); + + it("keeps a bulk route on its own bucket working while the shared one is spent", async () => { + await spendBucket(); + const { POST } = await import("@/app/api/measurements/batch/route"); + const res = await POST( + new NextRequest("http://localhost/api/measurements/batch", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + measurements: [ + { + type: "WEIGHT", + value: 72.4, + unit: "kg", + measuredAt: new Date(Date.now() - 3_600_000).toISOString(), + source: "APPLE_HEALTH", + externalId: "write-meter-weight-1", + }, + ], + }), + }), + ); + expect(res.status).toBeLessThan(300); + expect( + await getPrismaClient().measurement.count({ where: { userId: USER_ID } }), + ).toBe(1); + }); +}); diff --git a/vitest.setup.ts b/vitest.setup.ts index 2b48ae93d..8e1a180de 100644 --- a/vitest.setup.ts +++ b/vitest.setup.ts @@ -18,7 +18,7 @@ * Server-side translation (`getServerTranslator`, `shared-resolve`) holds * every catalog on its own and needs no priming. */ -import { afterEach } from "vitest"; +import { afterEach, vi } from "vitest"; import { settleBackgroundTasks } from "@/lib/logging/background-tasks"; import { primeMessages } from "@/lib/i18n/load-locale"; import enMessages from "./messages/en.json"; @@ -43,3 +43,19 @@ primeMessages("en", enMessages); afterEach(async () => { await settleBackgroundTasks(); }); + +/** + * The shared write ceiling is charged by every identity resolver on a write + * (`src/lib/auth/write-meter.ts`), which is a database round-trip most route + * tests have no fake for: their Prisma mock carries the models the handler + * touches and nothing else. The meter is therefore a no-op in unit tests by + * default. Its own behaviour is pinned by + * `src/lib/auth/__tests__/write-meter.test.ts`, which loads the real module, + * and against Postgres by `tests/integration/write-meter.test.ts`; a route test + * that wants the refusal mocks `meterAccountWrite` to throw. + */ +vi.mock("@/lib/auth/write-meter", async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, meterAccountWrite: vi.fn(async () => undefined) }; +});