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.
This commit is contained in:
Marc-André Bombeck committed 2026-10-20 13:53:54 +02:00
1 parent 3e9594b3b8
commit 39d23dbb7d
33 files changed
+730 -216

No files matched your search

@@ -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", () => {
@@ -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<string, string[]> = {
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<string, string> = {
"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<string, string> = {
"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*\(/);
}
});
});