From 08100b6207500ea1f97b9b26a3686e524e5f5d04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 10 Sep 2026 03:55:46 +0200 Subject: [PATCH] fix(api): put Retry-After and the rate-limit triple on every 429 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A refused client had nothing to back off against. `rateLimitHeaders()` emitted `X-RateLimit-Remaining` and an ISO `X-RateLimit-Reset`, no `Retry-After` and no cap, and only about 70 of the 199 route files that consult a limiter attached even those two. The rest answered with a bare prose envelope, including every endpoint an offline replay queue hits: the four batch ingests, `/api/sync/changes` and `/api/auth/refresh`. A first-run Apple Health import trips the 60-per-minute batch ceiling reliably, and the only strategy left to the client was a blind backoff. Threading headers through 129 handlers is 129 edits the next new route forgets, so the verdict is carried instead: `checkRateLimit` records it in a request-scoped cell and `apiHandler` dresses any 429 that leaves without headers. A route that already built its own is left alone — `Retry-After` is the presence test, and it comes from the same builder. `Retry-After` is whole seconds rounding UP (RFC 9110 §10.2.3), floored at zero. `X-RateLimit-Limit` is new and needs the cap, so the limiter result carries it. `X-RateLimit-Reset` keeps its ISO form: it is the wrong convention for the name, but it is on the wire today and changing it is a decision of its own. The cell lives in its own leaf module because 177 test files replace `@/lib/rate-limit` wholesale with a mock factory, and `apiHandler` must not depend on anything those factories stand in for. --- docs/api/openapi.yaml | 5 +- .../api/auth/check-user/route.test.ts | 5 + .../auth/me/diabetes/__tests__/route.test.ts | 2 + .../me/disable-coach/__tests__/route.test.ts | 2 + .../me/glucose-unit/__tests__/route.test.ts | 2 + .../__tests__/route.test.ts | 2 + .../auth/me/modules/__tests__/route.test.ts | 2 + .../__tests__/route.test.ts | 2 + .../c/[token]/unlock/__tests__/route.test.ts | 2 + .../api/consent/ai/__tests__/route.test.ts | 8 +- .../consent/ai/latest/__tests__/route.test.ts | 9 +- .../consent/ai/web/__tests__/route.test.ts | 8 +- .../digest/dismiss/__tests__/route.test.ts | 2 + .../[id]/extract/__tests__/route.test.ts | 2 + .../[id]/index/__tests__/route.test.ts | 3 + .../__tests__/measurement-streaming.test.ts | 1 + .../export/__tests__/per-type-routes.test.ts | 11 + .../__tests__/soft-delete-filter.test.ts | 1 + .../comprehensive/__tests__/route.test.ts | 1 + .../correlations/__tests__/route.test.ts | 2 + .../insights/generate/__tests__/route.test.ts | 5 + .../web-vitals/__tests__/route.test.ts | 2 + src/app/api/mcp/oauth/__tests__/token.test.ts | 4 + src/app/api/mcp/oauth/authorize/route.ts | 12 +- src/app/api/mcp/oauth/register/route.ts | 6 +- src/app/api/mcp/oauth/token/route.ts | 6 +- .../__tests__/aggregated-hr-wire.test.ts | 1 + .../__tests__/hr-bucket-observability.test.ts | 1 + .../insert-returning-arrivals.test.ts | 1 + .../batch/__tests__/pr-detection-hook.test.ts | 1 + .../batch/__tests__/sync-trigger.test.ts | 2 + .../__tests__/timestamp-plausibility.test.ts | 1 + .../__tests__/unstable-external-id.test.ts | 1 + .../[id]/compliance/__tests__/route.test.ts | 3 + .../[id]/dose-history/__tests__/route.test.ts | 3 + .../[id]/glp1/__tests__/route.test.ts | 3 + .../bulk-delete/__tests__/route.test.ts | 3 + .../intake/import/__tests__/route.test.ts | 2 + .../[id]/inventory/__tests__/route.test.ts | 3 + .../[id]/side-effects/__tests__/route.test.ts | 3 + .../compliance/__tests__/route.test.ts | 3 + .../extract/__tests__/route.test.ts | 2 + .../assessments/__tests__/route.test.ts | 1 + .../nutrients/water/__tests__/route.test.ts | 2 + src/app/api/onboarding/__tests__/step.test.ts | 3 + .../api/user/avatar/__tests__/route.test.ts | 3 + .../workouts/__tests__/batch-create.test.ts | 2 + src/app/mcp/__tests__/route.test.ts | 2 + src/app/mcp/route.ts | 9 +- src/lib/__tests__/rate-limit-headers.test.ts | 141 +++++++++ src/lib/api-handler.ts | 277 ++++++++++-------- .../__tests__/ai-route-support-limit.test.ts | 1 + src/lib/openapi/routes/shared.ts | 3 +- src/lib/rate-limit-context.ts | 97 ++++++ src/lib/rate-limit.ts | 26 +- .../rate-limit-429-headers.test.ts | 135 +++++++++ 56 files changed, 681 insertions(+), 161 deletions(-) create mode 100644 src/lib/__tests__/rate-limit-headers.test.ts create mode 100644 src/lib/rate-limit-context.ts create mode 100644 tests/integration/rate-limit-429-headers.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 72915fbc3..580ddaf59 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -101,7 +101,10 @@ paths: schema: $ref: "#/components/schemas/ErrorEnvelope" "429": &a5 - description: Rate limit exceeded. + description: Rate limit exceeded. The response carries `Retry-After` (whole seconds, rounded up — wait at least that + long before retrying), `X-RateLimit-Limit` (the bucket's cap), `X-RateLimit-Remaining` and + `X-RateLimit-Reset` (the reset instant, ISO-8601, not epoch seconds). Back off on `Retry-After` rather than + guessing. content: application/json: schema: diff --git a/src/__tests__/api/auth/check-user/route.test.ts b/src/__tests__/api/auth/check-user/route.test.ts index 7cd4e5eae..8ae3cbd40 100644 --- a/src/__tests__/api/auth/check-user/route.test.ts +++ b/src/__tests__/api/auth/check-user/route.test.ts @@ -27,6 +27,7 @@ vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 15 * 60 * 1000, }), @@ -34,6 +35,7 @@ vi.mock("@/lib/rate-limit", () => ({ // wrapper. Default to a clean per-IP result. checkAuthSurfaceRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 15 * 60 * 1000, ip: "203.0.113.1", @@ -84,11 +86,13 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 15 * 60 * 1000, }); vi.mocked(checkAuthSurfaceRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 15 * 60 * 1000, ip: "203.0.113.1", @@ -175,6 +179,7 @@ describe("POST /api/auth/check-user", () => { it("returns 429 when the per-IP rate-limit is exhausted", async () => { vi.mocked(checkAuthSurfaceRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 15 * 60 * 1000, ip: "203.0.113.1", diff --git a/src/app/api/auth/me/diabetes/__tests__/route.test.ts b/src/app/api/auth/me/diabetes/__tests__/route.test.ts index 5919ff386..0cee31013 100644 --- a/src/app/api/auth/me/diabetes/__tests__/route.test.ts +++ b/src/app/api/auth/me/diabetes/__tests__/route.test.ts @@ -32,6 +32,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -192,6 +193,7 @@ describe("PATCH /api/auth/me/diabetes", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/disable-coach/__tests__/route.test.ts b/src/app/api/auth/me/disable-coach/__tests__/route.test.ts index 8042a65c6..5ed9ad5b2 100644 --- a/src/app/api/auth/me/disable-coach/__tests__/route.test.ts +++ b/src/app/api/auth/me/disable-coach/__tests__/route.test.ts @@ -24,6 +24,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -247,6 +248,7 @@ describe("PATCH /api/auth/me/disable-coach", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/glucose-unit/__tests__/route.test.ts b/src/app/api/auth/me/glucose-unit/__tests__/route.test.ts index 943f37294..a699059cb 100644 --- a/src/app/api/auth/me/glucose-unit/__tests__/route.test.ts +++ b/src/app/api/auth/me/glucose-unit/__tests__/route.test.ts @@ -36,6 +36,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -197,6 +198,7 @@ describe("PATCH /api/auth/me/glucose-unit", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/health-score-config/__tests__/route.test.ts b/src/app/api/auth/me/health-score-config/__tests__/route.test.ts index 75d4ff20f..da967879f 100644 --- a/src/app/api/auth/me/health-score-config/__tests__/route.test.ts +++ b/src/app/api/auth/me/health-score-config/__tests__/route.test.ts @@ -35,6 +35,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -397,6 +398,7 @@ describe("PATCH — envelope and transport", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/modules/__tests__/route.test.ts b/src/app/api/auth/me/modules/__tests__/route.test.ts index 1e23957ac..7a14aef25 100644 --- a/src/app/api/auth/me/modules/__tests__/route.test.ts +++ b/src/app/api/auth/me/modules/__tests__/route.test.ts @@ -35,6 +35,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -300,6 +301,7 @@ describe("PATCH /api/auth/me/modules", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/auth/me/notification-prefs/__tests__/route.test.ts b/src/app/api/auth/me/notification-prefs/__tests__/route.test.ts index 088cecd53..c8d503649 100644 --- a/src/app/api/auth/me/notification-prefs/__tests__/route.test.ts +++ b/src/app/api/auth/me/notification-prefs/__tests__/route.test.ts @@ -26,6 +26,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -587,6 +588,7 @@ describe("PATCH /api/auth/me/notification-prefs", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/c/[token]/unlock/__tests__/route.test.ts b/src/app/api/c/[token]/unlock/__tests__/route.test.ts index 7c0828da3..e7cf2ef56 100644 --- a/src/app/api/c/[token]/unlock/__tests__/route.test.ts +++ b/src/app/api/c/[token]/unlock/__tests__/route.test.ts @@ -68,6 +68,7 @@ beforeEach(() => { vi.stubEnv("API_TOKEN_HMAC_KEY", TEST_HMAC_KEY); vi.mocked(checkAuthSurfaceRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 60_000, ip: "1.2.3.4", @@ -137,6 +138,7 @@ describe("POST /api/c/[token]/unlock", () => { }); vi.mocked(checkAuthSurfaceRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, ip: "1.2.3.4", diff --git a/src/app/api/consent/ai/__tests__/route.test.ts b/src/app/api/consent/ai/__tests__/route.test.ts index cfb9c52c3..8f4a55859 100644 --- a/src/app/api/consent/ai/__tests__/route.test.ts +++ b/src/app/api/consent/ai/__tests__/route.test.ts @@ -55,7 +55,12 @@ import { getSession } from "@/lib/auth/session"; import { auditLog } from "@/lib/auth/audit"; import { checkConsentRateLimit } from "@/lib/rate-limit"; -const RL_OK = { allowed: true, remaining: 19, resetAt: Date.now() + 60_000 }; +const RL_OK = { + allowed: true, + limit: 20, + remaining: 19, + resetAt: Date.now() + 60_000, +}; const $transaction = vi.mocked(prisma.$transaction) as unknown as { mockImplementation: (impl: (fn: TxFn) => unknown) => void; @@ -109,6 +114,7 @@ describe("POST /api/consent/ai", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkConsentRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/consent/ai/latest/__tests__/route.test.ts b/src/app/api/consent/ai/latest/__tests__/route.test.ts index b21a15eeb..db3ed6ddb 100644 --- a/src/app/api/consent/ai/latest/__tests__/route.test.ts +++ b/src/app/api/consent/ai/latest/__tests__/route.test.ts @@ -54,7 +54,12 @@ import { getSession } from "@/lib/auth/session"; import { auditLog } from "@/lib/auth/audit"; import { checkConsentRateLimit } from "@/lib/rate-limit"; -const RL_OK = { allowed: true, remaining: 19, resetAt: Date.now() + 60_000 }; +const RL_OK = { + allowed: true, + limit: 20, + remaining: 19, + resetAt: Date.now() + 60_000, +}; const $transaction = vi.mocked(prisma.$transaction) as unknown as { mockImplementation: (impl: (fn: TxFn) => unknown) => void; @@ -99,6 +104,7 @@ describe("GET /api/consent/ai/latest", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkConsentRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); @@ -197,6 +203,7 @@ describe("DELETE /api/consent/ai/latest", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkConsentRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/consent/ai/web/__tests__/route.test.ts b/src/app/api/consent/ai/web/__tests__/route.test.ts index d646dcb1f..6d5651727 100644 --- a/src/app/api/consent/ai/web/__tests__/route.test.ts +++ b/src/app/api/consent/ai/web/__tests__/route.test.ts @@ -43,7 +43,12 @@ const SESSION_OK = { user: { id: "user-1", username: "testuser", role: "USER" as const }, }; -const RL_OK = { allowed: true, remaining: 19, resetAt: Date.now() + 60_000 }; +const RL_OK = { + allowed: true, + limit: 20, + remaining: 19, + resetAt: Date.now() + 60_000, +}; /** The mount heal posts no body and no content-type. */ function mkHeal(): Request { @@ -79,6 +84,7 @@ describe("POST /api/consent/ai/web", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkConsentRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/daily/digest/dismiss/__tests__/route.test.ts b/src/app/api/daily/digest/dismiss/__tests__/route.test.ts index 8c2ab3e05..795f7ef8c 100644 --- a/src/app/api/daily/digest/dismiss/__tests__/route.test.ts +++ b/src/app/api/daily/digest/dismiss/__tests__/route.test.ts @@ -24,6 +24,7 @@ vi.mock("@/lib/modules/gate", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn(async () => ({ allowed: true, + limit: 60, remaining: 39, resetAt: Date.now() + 60_000, })), @@ -159,6 +160,7 @@ describe("POST /api/daily/digest/dismiss", () => { const { checkRateLimit } = await import("@/lib/rate-limit"); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/documents/inbound/[id]/extract/__tests__/route.test.ts b/src/app/api/documents/inbound/[id]/extract/__tests__/route.test.ts index c5b3ba303..6acb80659 100644 --- a/src/app/api/documents/inbound/[id]/extract/__tests__/route.test.ts +++ b/src/app/api/documents/inbound/[id]/extract/__tests__/route.test.ts @@ -168,6 +168,7 @@ beforeEach(() => { vi.mocked(prisma.extractedFact.count).mockResolvedValue(0 as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 5, resetAt: Date.now() + 60 * 60 * 1000, }); @@ -304,6 +305,7 @@ describe("POST /api/documents/inbound/[id]/extract — stored mode", () => { it("consumes the shared document-AI bucket honestly", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30 * 60_000, }); diff --git a/src/app/api/documents/inbound/[id]/index/__tests__/route.test.ts b/src/app/api/documents/inbound/[id]/index/__tests__/route.test.ts index 5b6540011..aa5b6c7d5 100644 --- a/src/app/api/documents/inbound/[id]/index/__tests__/route.test.ts +++ b/src/app/api/documents/inbound/[id]/index/__tests__/route.test.ts @@ -128,6 +128,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 5, resetAt: Date.now() + 60 * 60 * 1000, }); @@ -222,6 +223,7 @@ describe("POST /api/documents/inbound/[id]/index — bucket honesty", () => { const resetAt = Date.now() + 47 * 60_000; vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt, }); @@ -309,6 +311,7 @@ describe("POST /api/documents/inbound/[id]/index — staging continuation", () = it("does not reach staging when the index itself was refused", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/export/__tests__/measurement-streaming.test.ts b/src/app/api/export/__tests__/measurement-streaming.test.ts index 3fedbe375..aa674b460 100644 --- a/src/app/api/export/__tests__/measurement-streaming.test.ts +++ b/src/app/api/export/__tests__/measurement-streaming.test.ts @@ -71,6 +71,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3_600_000, }); diff --git a/src/app/api/export/__tests__/per-type-routes.test.ts b/src/app/api/export/__tests__/per-type-routes.test.ts index f29671ba1..f3ca60494 100644 --- a/src/app/api/export/__tests__/per-type-routes.test.ts +++ b/src/app/api/export/__tests__/per-type-routes.test.ts @@ -190,6 +190,7 @@ describe("GET /api/export/measurements", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -229,6 +230,7 @@ describe("GET /api/export/measurements", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -258,6 +260,7 @@ describe("GET /api/export/measurements", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -289,6 +292,7 @@ describe("GET /api/export/measurements", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -327,6 +331,7 @@ describe("GET /api/export/medications", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -370,6 +375,7 @@ describe("GET /api/export/medications", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -390,6 +396,7 @@ describe("GET /api/export/medications", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -425,6 +432,7 @@ describe("GET /api/export/medications", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -452,6 +460,7 @@ describe("GET /api/export/mood", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -492,6 +501,7 @@ describe("GET /api/export/full-backup", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -562,6 +572,7 @@ describe("GET /api/export/full-backup", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); diff --git a/src/app/api/export/__tests__/soft-delete-filter.test.ts b/src/app/api/export/__tests__/soft-delete-filter.test.ts index 329405629..bd4a5669c 100644 --- a/src/app/api/export/__tests__/soft-delete-filter.test.ts +++ b/src/app/api/export/__tests__/soft-delete-filter.test.ts @@ -130,6 +130,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3_600_000, }); diff --git a/src/app/api/insights/comprehensive/__tests__/route.test.ts b/src/app/api/insights/comprehensive/__tests__/route.test.ts index 372a313ea..6f776d239 100644 --- a/src/app/api/insights/comprehensive/__tests__/route.test.ts +++ b/src/app/api/insights/comprehensive/__tests__/route.test.ts @@ -117,6 +117,7 @@ beforeEach(() => { // v1.15.20 — default to an allowing analytics-read budget. vi.mocked(checkAnalyticsReadRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 119, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/insights/correlations/__tests__/route.test.ts b/src/app/api/insights/correlations/__tests__/route.test.ts index 0a110f9ed..bc481a4f4 100644 --- a/src/app/api/insights/correlations/__tests__/route.test.ts +++ b/src/app/api/insights/correlations/__tests__/route.test.ts @@ -103,6 +103,7 @@ beforeEach(() => { // v1.15.20 — default to an allowing analytics-read budget. vi.mocked(checkAnalyticsReadRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 119, resetAt: Date.now() + 60_000, }); @@ -229,6 +230,7 @@ describe("GET /api/insights/correlations", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkAnalyticsReadRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/insights/generate/__tests__/route.test.ts b/src/app/api/insights/generate/__tests__/route.test.ts index 55516a66d..5289fa0d1 100644 --- a/src/app/api/insights/generate/__tests__/route.test.ts +++ b/src/app/api/insights/generate/__tests__/route.test.ts @@ -188,6 +188,7 @@ beforeEach(() => { selfContextText.mockResolvedValue(null); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3600_000, }); @@ -366,6 +367,7 @@ describe("POST /api/insights/generate — rate limit (v1.4.16 A7.1)", () => { expect(limit).toBe(10); return { allowed: callCount <= 10, + limit: 10, remaining: Math.max(0, 10 - callCount), resetAt: Date.now() + 3600_000, }; @@ -392,6 +394,7 @@ describe("POST /api/insights/generate — rate limit (v1.4.16 A7.1)", () => { expect(limit).toBe(3); return { allowed: callCount <= 3, + limit: 3, remaining: Math.max(0, 3 - callCount), resetAt: Date.now() + 3600_000, }; @@ -673,6 +676,7 @@ describe("POST /api/insights/generate — briefingless fresh cache (v1.28.30)", mockBriefinglessFreshCache(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 3600_000, }); @@ -691,6 +695,7 @@ describe("POST /api/insights/generate — briefingless fresh cache (v1.28.30)", mockBriefinglessFreshCache(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 3600_000, }); diff --git a/src/app/api/internal/web-vitals/__tests__/route.test.ts b/src/app/api/internal/web-vitals/__tests__/route.test.ts index 81a792abf..e3e1ac303 100644 --- a/src/app/api/internal/web-vitals/__tests__/route.test.ts +++ b/src/app/api/internal/web-vitals/__tests__/route.test.ts @@ -50,6 +50,7 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); @@ -114,6 +115,7 @@ describe("POST /api/internal/web-vitals", () => { it("returns 429 when the rate-limit fires", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/mcp/oauth/__tests__/token.test.ts b/src/app/api/mcp/oauth/__tests__/token.test.ts index 54bf4e451..df17975e9 100644 --- a/src/app/api/mcp/oauth/__tests__/token.test.ts +++ b/src/app/api/mcp/oauth/__tests__/token.test.ts @@ -37,12 +37,14 @@ vi.mock("@/lib/auth/issue-token", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkAuthSurfaceRateLimit: vi.fn(async () => ({ allowed: true, + limit: 60, remaining: 100, resetAt: Date.now() + 60_000, ip: "1.2.3.4", })), checkRateLimit: vi.fn(async () => ({ allowed: true, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, })), @@ -103,6 +105,7 @@ beforeEach(() => { vi.clearAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); @@ -215,6 +218,7 @@ describe("authorization_code grant", () => { it("rejects a replayed code (single-use jti already claimed)", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/mcp/oauth/authorize/route.ts b/src/app/api/mcp/oauth/authorize/route.ts index 1abc2c847..04de08df8 100644 --- a/src/app/api/mcp/oauth/authorize/route.ts +++ b/src/app/api/mcp/oauth/authorize/route.ts @@ -302,11 +302,7 @@ export async function GET(request: NextRequest): Promise { { error: "temporarily_unavailable" }, { status: 429, - headers: rateLimitHeaders({ - allowed: false, - remaining: rl.remaining, - resetAt: rl.resetAt, - }), + headers: rateLimitHeaders(rl), }, ); } @@ -407,11 +403,7 @@ export async function POST(request: NextRequest): Promise { { error: "temporarily_unavailable" }, { status: 429, - headers: rateLimitHeaders({ - allowed: false, - remaining: rl.remaining, - resetAt: rl.resetAt, - }), + headers: rateLimitHeaders(rl), }, ); } diff --git a/src/app/api/mcp/oauth/register/route.ts b/src/app/api/mcp/oauth/register/route.ts index 046453158..aa5d76a83 100644 --- a/src/app/api/mcp/oauth/register/route.ts +++ b/src/app/api/mcp/oauth/register/route.ts @@ -80,11 +80,7 @@ export async function POST(request: NextRequest): Promise { { error: "temporarily_unavailable" }, { status: 429, - headers: rateLimitHeaders({ - allowed: false, - remaining: rl.remaining, - resetAt: rl.resetAt, - }), + headers: rateLimitHeaders(rl), }, ); } diff --git a/src/app/api/mcp/oauth/token/route.ts b/src/app/api/mcp/oauth/token/route.ts index e9493db20..a82b732a5 100644 --- a/src/app/api/mcp/oauth/token/route.ts +++ b/src/app/api/mcp/oauth/token/route.ts @@ -222,11 +222,7 @@ export async function POST(request: NextRequest): Promise { { error: "temporarily_unavailable" }, { status: 429, - headers: rateLimitHeaders({ - allowed: false, - remaining: rl.remaining, - resetAt: rl.resetAt, - }), + headers: rateLimitHeaders(rl), }, ); } diff --git a/src/app/api/measurements/batch/__tests__/aggregated-hr-wire.test.ts b/src/app/api/measurements/batch/__tests__/aggregated-hr-wire.test.ts index a46987f29..3f766d9fb 100644 --- a/src/app/api/measurements/batch/__tests__/aggregated-hr-wire.test.ts +++ b/src/app/api/measurements/batch/__tests__/aggregated-hr-wire.test.ts @@ -189,6 +189,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/hr-bucket-observability.test.ts b/src/app/api/measurements/batch/__tests__/hr-bucket-observability.test.ts index b5e36298c..7597199f4 100644 --- a/src/app/api/measurements/batch/__tests__/hr-bucket-observability.test.ts +++ b/src/app/api/measurements/batch/__tests__/hr-bucket-observability.test.ts @@ -186,6 +186,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/insert-returning-arrivals.test.ts b/src/app/api/measurements/batch/__tests__/insert-returning-arrivals.test.ts index 9b551c5ac..e7eb79f74 100644 --- a/src/app/api/measurements/batch/__tests__/insert-returning-arrivals.test.ts +++ b/src/app/api/measurements/batch/__tests__/insert-returning-arrivals.test.ts @@ -159,6 +159,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(session as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/pr-detection-hook.test.ts b/src/app/api/measurements/batch/__tests__/pr-detection-hook.test.ts index 01f5cb72d..a36974a0c 100644 --- a/src/app/api/measurements/batch/__tests__/pr-detection-hook.test.ts +++ b/src/app/api/measurements/batch/__tests__/pr-detection-hook.test.ts @@ -114,6 +114,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/sync-trigger.test.ts b/src/app/api/measurements/batch/__tests__/sync-trigger.test.ts index fa834286f..83a7416d5 100644 --- a/src/app/api/measurements/batch/__tests__/sync-trigger.test.ts +++ b/src/app/api/measurements/batch/__tests__/sync-trigger.test.ts @@ -194,6 +194,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); @@ -271,6 +272,7 @@ describe("POST /api/measurements/batch — syncTrigger diagnostic (iOS #66)", () vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/timestamp-plausibility.test.ts b/src/app/api/measurements/batch/__tests__/timestamp-plausibility.test.ts index 5c79d949e..8fdeb46da 100644 --- a/src/app/api/measurements/batch/__tests__/timestamp-plausibility.test.ts +++ b/src/app/api/measurements/batch/__tests__/timestamp-plausibility.test.ts @@ -148,6 +148,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts b/src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts index e7369d833..a68fdd012 100644 --- a/src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts +++ b/src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts @@ -151,6 +151,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/compliance/__tests__/route.test.ts b/src/app/api/medications/[id]/compliance/__tests__/route.test.ts index 4b6de773f..128bcfe87 100644 --- a/src/app/api/medications/[id]/compliance/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/compliance/__tests__/route.test.ts @@ -46,6 +46,7 @@ vi.mock("@/lib/cache/server-cache", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -139,6 +140,7 @@ beforeEach(() => { ); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); @@ -416,6 +418,7 @@ describe("GET /api/medications/[id]/compliance — caching and limits", () => { it("returns 429 when the per-user rate limit is exhausted", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/dose-history/__tests__/route.test.ts b/src/app/api/medications/[id]/dose-history/__tests__/route.test.ts index ccc4f707d..4a5c8fed8 100644 --- a/src/app/api/medications/[id]/dose-history/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/dose-history/__tests__/route.test.ts @@ -30,6 +30,7 @@ vi.mock("@/lib/medications/route-guards", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }), @@ -92,6 +93,7 @@ beforeEach(() => { vi.mocked(assertMedicationOwnership).mockResolvedValue(null); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); @@ -134,6 +136,7 @@ describe("GET /api/medications/[id]/dose-history", () => { it("returns 429 when the per-user cap is exhausted", async () => { vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/glp1/__tests__/route.test.ts b/src/app/api/medications/[id]/glp1/__tests__/route.test.ts index 5e6bbad29..3d52191ee 100644 --- a/src/app/api/medications/[id]/glp1/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/glp1/__tests__/route.test.ts @@ -38,6 +38,7 @@ vi.mock("@/lib/auth/audit", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -85,6 +86,7 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -251,6 +253,7 @@ describe("POST /api/medications/[id]/glp1", () => { vi.mocked(prisma.medication.findUnique).mockResolvedValue(MED_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/intake/bulk-delete/__tests__/route.test.ts b/src/app/api/medications/[id]/intake/bulk-delete/__tests__/route.test.ts index 381421305..c087df6f9 100644 --- a/src/app/api/medications/[id]/intake/bulk-delete/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/intake/bulk-delete/__tests__/route.test.ts @@ -39,6 +39,7 @@ vi.mock("@/lib/medications/route-guards", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -124,6 +125,7 @@ beforeEach(() => { vi.mocked(assertMedicationOwnership).mockResolvedValue(null); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -239,6 +241,7 @@ describe("POST /api/medications/[id]/intake/bulk-delete — F-1 H-6 rate limit", it("returns 429 when the per-user cap is exhausted", async () => { vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/intake/import/__tests__/route.test.ts b/src/app/api/medications/[id]/intake/import/__tests__/route.test.ts index f4c5a0664..a66164c05 100644 --- a/src/app/api/medications/[id]/intake/import/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/intake/import/__tests__/route.test.ts @@ -97,6 +97,7 @@ beforeEach(() => { vi.mocked(auditLog).mockResolvedValue(undefined as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); @@ -407,6 +408,7 @@ describe("POST /api/medications/[id]/intake/import — future bound and limiter" it("refuses with 429 once the import limiter trips", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/inventory/__tests__/route.test.ts b/src/app/api/medications/[id]/inventory/__tests__/route.test.ts index cd9d4251a..5231ae2af 100644 --- a/src/app/api/medications/[id]/inventory/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/inventory/__tests__/route.test.ts @@ -30,6 +30,7 @@ vi.mock("@/lib/auth/audit", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -87,6 +88,7 @@ beforeEach(() => { ); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -253,6 +255,7 @@ describe("POST /api/medications/[id]/inventory", () => { vi.mocked(prisma.medication.findUnique).mockResolvedValue(MED_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/[id]/side-effects/__tests__/route.test.ts b/src/app/api/medications/[id]/side-effects/__tests__/route.test.ts index e30b8372a..ba27f803e 100644 --- a/src/app/api/medications/[id]/side-effects/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/side-effects/__tests__/route.test.ts @@ -31,6 +31,7 @@ vi.mock("@/lib/auth/audit", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -84,6 +85,7 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -298,6 +300,7 @@ describe("POST /api/medications/[id]/side-effects", () => { vi.mocked(prisma.medication.findUnique).mockResolvedValue(MED_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/compliance/__tests__/route.test.ts b/src/app/api/medications/compliance/__tests__/route.test.ts index b947205d6..8490762bb 100644 --- a/src/app/api/medications/compliance/__tests__/route.test.ts +++ b/src/app/api/medications/compliance/__tests__/route.test.ts @@ -30,6 +30,7 @@ vi.mock("@/lib/cache/server-cache", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -106,6 +107,7 @@ beforeEach(() => { ); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -178,6 +180,7 @@ describe("GET /api/medications/compliance", () => { it("returns 429 when the per-user rate limit is exhausted", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/medications/extract/__tests__/route.test.ts b/src/app/api/medications/extract/__tests__/route.test.ts index b80464eb3..c01d578ee 100644 --- a/src/app/api/medications/extract/__tests__/route.test.ts +++ b/src/app/api/medications/extract/__tests__/route.test.ts @@ -110,6 +110,7 @@ beforeEach(() => { vi.clearAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 60_000, }); @@ -152,6 +153,7 @@ describe("POST /api/medications/extract — rate limit", () => { vi.mocked(requireAuth).mockResolvedValue(AUTH_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/mental-health/assessments/__tests__/route.test.ts b/src/app/api/mental-health/assessments/__tests__/route.test.ts index 1d780f051..1d17b58a0 100644 --- a/src/app/api/mental-health/assessments/__tests__/route.test.ts +++ b/src/app/api/mental-health/assessments/__tests__/route.test.ts @@ -96,6 +96,7 @@ beforeEach(() => { vi.mocked(requireModuleEnabled).mockResolvedValue({ enabled: true }); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now(), }); diff --git a/src/app/api/nutrients/water/__tests__/route.test.ts b/src/app/api/nutrients/water/__tests__/route.test.ts index 051a16d14..354ce7cc2 100644 --- a/src/app/api/nutrients/water/__tests__/route.test.ts +++ b/src/app/api/nutrients/water/__tests__/route.test.ts @@ -101,6 +101,7 @@ beforeEach(() => { vi.mocked(requireModuleEnabled).mockResolvedValue({ enabled: true } as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 59, resetAt: Date.now() + 60_000, }); @@ -137,6 +138,7 @@ describe("POST /api/nutrients/water", () => { it("returns 429 when the rate limit is exceeded", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/onboarding/__tests__/step.test.ts b/src/app/api/onboarding/__tests__/step.test.ts index 1700626a3..5856891f4 100644 --- a/src/app/api/onboarding/__tests__/step.test.ts +++ b/src/app/api/onboarding/__tests__/step.test.ts @@ -36,6 +36,7 @@ vi.mock("@/lib/auth/audit", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }), @@ -79,6 +80,7 @@ beforeEach(() => { vi.resetAllMocks(); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 29, resetAt: Date.now() + 60_000, }); @@ -243,6 +245,7 @@ describe("POST /api/onboarding/step — rate limit", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/api/user/avatar/__tests__/route.test.ts b/src/app/api/user/avatar/__tests__/route.test.ts index 0cfb500ee..72e005dce 100644 --- a/src/app/api/user/avatar/__tests__/route.test.ts +++ b/src/app/api/user/avatar/__tests__/route.test.ts @@ -24,6 +24,7 @@ vi.mock("@/lib/db-compat", () => ({ vi.mock("@/lib/rate-limit", () => ({ checkRateLimit: vi.fn().mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3_600_000, }), @@ -89,6 +90,7 @@ beforeEach(() => { // Default rate-limit pass-through; per-test overrides drop in below. vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 9, resetAt: Date.now() + 3_600_000, }); @@ -210,6 +212,7 @@ describe("POST /api/user/avatar", () => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValueOnce({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/api/workouts/__tests__/batch-create.test.ts b/src/app/api/workouts/__tests__/batch-create.test.ts index 05a5dba93..37e971e2a 100644 --- a/src/app/api/workouts/__tests__/batch-create.test.ts +++ b/src/app/api/workouts/__tests__/batch-create.test.ts @@ -145,6 +145,7 @@ beforeEach(() => { vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 60, resetAt: Date.now() + 60_000, }); @@ -218,6 +219,7 @@ describe("POST /api/workouts/batch — auth + size + rate-limit", () => { it("returns 429 when the rate-limit gate blocks the call", async () => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 60_000, }); diff --git a/src/app/mcp/__tests__/route.test.ts b/src/app/mcp/__tests__/route.test.ts index 465939dc1..3780faff5 100644 --- a/src/app/mcp/__tests__/route.test.ts +++ b/src/app/mcp/__tests__/route.test.ts @@ -156,6 +156,7 @@ beforeEach(() => { vi.mocked(isApiGloballyEnabled).mockResolvedValue(true); vi.mocked(checkMcpRateLimit).mockResolvedValue({ allowed: true, + limit: 60, remaining: 119, resetAt: Date.now() + 60_000, }); @@ -352,6 +353,7 @@ describe("/mcp — rate limit", () => { validToken(); vi.mocked(checkMcpRateLimit).mockResolvedValue({ allowed: false, + limit: 60, remaining: 0, resetAt: Date.now() + 30_000, }); diff --git a/src/app/mcp/route.ts b/src/app/mcp/route.ts index ae4a7dceb..c71635dc8 100644 --- a/src/app/mcp/route.ts +++ b/src/app/mcp/route.ts @@ -55,6 +55,7 @@ import { annotate } from "@/lib/logging/context"; import { isModuleEnabled } from "@/lib/modules/gate"; import { isApiGloballyEnabled } from "@/lib/app-settings"; import { checkMcpRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; +import type { RateLimitSnapshot } from "@/lib/rate-limit-context"; import { isMcpOriginConfigured, resolveBaseOrigin, @@ -114,11 +115,9 @@ function moduleDisabled(): Response { } /** 429 with the standard rate-limit headers. */ -function tooManyRequests(resetAt: number, remaining: number): Response { +function tooManyRequests(rl: RateLimitSnapshot): Response { const response = Response.json({ error: "rate_limited" }, { status: 429 }); - for (const [k, v] of Object.entries( - rateLimitHeaders({ allowed: false, remaining, resetAt }), - )) { + for (const [k, v] of Object.entries(rateLimitHeaders(rl))) { response.headers.set(k, v); } return response; @@ -179,7 +178,7 @@ async function handleMcp(request: Request): Promise { const rl = await checkMcpRateLimit(ctx.binding); if (!rl.allowed) { annotate({ action: { name: "mcp.rate_limited" } }); - return tooManyRequests(rl.resetAt, rl.remaining); + return tooManyRequests(rl); } // 3. MODULE GATE — off by default; hide the surface entirely when off. diff --git a/src/lib/__tests__/rate-limit-headers.test.ts b/src/lib/__tests__/rate-limit-headers.test.ts new file mode 100644 index 000000000..6aace7a66 --- /dev/null +++ b/src/lib/__tests__/rate-limit-headers.test.ts @@ -0,0 +1,141 @@ +/** + * The rate-limit response headers and the request-scoped verdict capture. + * + * The headers are what a refused client backs off against, so the shape is + * pinned here rather than left to whichever route happens to be exercised: + * `Retry-After` in whole seconds rounding UP, the cap and the remainder as + * plain integers, and the reset instant in the ISO form it has always carried. + * + * The capture half is what lets `apiHandler` dress a 429 that a handler built + * without headers. Its one interesting rule is that a refusal outranks a pass: + * a handler that clears one bucket and is refused by the next must report the + * bucket that refused it. + */ +import { describe, expect, it } from "vitest"; + +import { + capturedRateLimit, + captureRateLimitResult, + rateLimitResponseHeaders, + runWithRateLimitCapture, +} from "@/lib/rate-limit-context"; + +describe("rateLimitResponseHeaders", () => { + it("emits Retry-After, the cap, the remainder and the reset instant", () => { + const now = Date.UTC(2026, 0, 1, 12, 0, 0); + const headers = rateLimitResponseHeaders( + { limit: 60, remaining: 0, resetAt: now + 30_000 }, + now, + ); + + expect(headers).toEqual({ + "Retry-After": "30", + "X-RateLimit-Limit": "60", + "X-RateLimit-Remaining": "0", + "X-RateLimit-Reset": new Date(now + 30_000).toISOString(), + }); + }); + + it("rounds Retry-After up so the named delay outlasts the window", () => { + const now = Date.UTC(2026, 0, 1, 12, 0, 0); + const headers = rateLimitResponseHeaders( + { limit: 10, remaining: 0, resetAt: now + 1 }, + now, + ); + // One millisecond left still means "wait a second", never "retry now" — + // a floored zero would send the client straight back into the same window. + expect(headers["Retry-After"]).toBe("1"); + }); + + it("never names a delay in the past", () => { + const now = Date.UTC(2026, 0, 1, 12, 0, 0); + const headers = rateLimitResponseHeaders( + { limit: 10, remaining: 0, resetAt: now - 5_000 }, + now, + ); + expect(headers["Retry-After"]).toBe("0"); + }); + + it("keeps X-RateLimit-Reset in the ISO form already on the wire", () => { + const resetAt = Date.UTC(2026, 4, 17, 8, 30, 0); + const headers = rateLimitResponseHeaders({ + limit: 120, + remaining: 7, + resetAt, + }); + expect(headers["X-RateLimit-Reset"]).toBe("2026-05-17T08:30:00.000Z"); + }); +}); + +describe("request-scoped rate-limit capture", () => { + it("reads null outside a request scope", () => { + captureRateLimitResult({ + allowed: false, + limit: 5, + remaining: 0, + resetAt: Date.now(), + }); + expect(capturedRateLimit()).toBeNull(); + }); + + it("reports the last verdict when every bucket passed", () => { + runWithRateLimitCapture(() => { + captureRateLimitResult({ + allowed: true, + limit: 5, + remaining: 4, + resetAt: 1_000, + }); + captureRateLimitResult({ + allowed: true, + limit: 9, + remaining: 8, + resetAt: 2_000, + }); + expect(capturedRateLimit()).toEqual({ + allowed: true, + limit: 9, + remaining: 8, + resetAt: 2_000, + }); + }); + }); + + it("reports the bucket that refused, not one that passed after it", () => { + runWithRateLimitCapture(() => { + captureRateLimitResult({ + allowed: false, + limit: 60, + remaining: 0, + resetAt: 5_000, + }); + captureRateLimitResult({ + allowed: true, + limit: 120, + remaining: 100, + resetAt: 9_000, + }); + expect(capturedRateLimit()).toMatchObject({ limit: 60, resetAt: 5_000 }); + }); + }); + + it("gives each scope its own cell", async () => { + const first = runWithRateLimitCapture(async () => { + captureRateLimitResult({ + allowed: false, + limit: 1, + remaining: 0, + resetAt: 111, + }); + await Promise.resolve(); + return capturedRateLimit(); + }); + const second = runWithRateLimitCapture(async () => { + await Promise.resolve(); + return capturedRateLimit(); + }); + + expect((await first)?.resetAt).toBe(111); + expect(await second).toBeNull(); + }); +}); diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 63f6cdf28..3dc8fb7c7 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -41,6 +41,11 @@ import { assertRecordSessionFence, attachRecordContextEcho, } from "./sharing/record-session-fence"; +import { + capturedRateLimit, + rateLimitResponseHeaders, + runWithRateLimitCapture, +} from "./rate-limit-context"; /** * HTTP methods a read-only credential may use on the REST surface. A request @@ -221,133 +226,163 @@ export function apiHandler Promise>( undefined, }); - return eventStorage.run(evt, async () => { - let response: Response | undefined; - try { - response = await handler(...args); - } catch (error) { - if (error instanceof AssistantDisabledError) { - // v1.4.31 — operator has disabled the assistant surface. - // The 403 + `errorCode: "assistant.disabled."` - // envelope is locked per - // `.planning/RESPONSE-TO-IOS-TEAM-2026-05-16.md` §3 R5. - // Older iOS clients that don't know the errorCode surface - // this as a generic 403; v1.4.31+ clients can branch on the - // errorCode to render an inline operator-disabled notice. - evt.setError(error); - response = NextResponse.json( - { - data: null, - error: error.message, - meta: { errorCode: error.errorCode }, - }, - { status: 403 }, - ); - } else if (error instanceof ConsentRequiredError) { - // v1.12.1 — server-side consent gate before external-LLM PHI - // egress on the operator's server-managed key. Mirrors the - // AssistantDisabledError envelope (403 + meta.errorCode) so the - // iOS client renders an inline "grant consent" notice instead of - // a generic failure. - evt.setError(error); - response = NextResponse.json( - { - data: null, - error: error.message, - meta: { errorCode: error.errorCode }, - }, - { status: 403 }, - ); - } else if (error instanceof StepUpRequiredError) { - // v1.23 — step-up gate not satisfied. Same 401 + meta.errorCode - // envelope shape as the assistant/consent gates so the client can - // branch on the stable code and launch a re-verification flow. - // Checked before the generic HttpError branch because - // StepUpRequiredError extends it. - evt.setError(error); - response = NextResponse.json( - { - data: null, - error: error.message, - meta: { errorCode: error.errorCode }, - }, - { status: error.statusCode }, - ); - } else if (error instanceof SharingAuthError) { - // v1.36.0 — the acting-account resolver refused. Same - // 403 + meta.errorCode envelope as the gates above, so a client - // distinguishes "you may not act as that account" from "this - // endpoint does not support acting on one" without parsing prose. - // Checked before the generic HttpError branch because it extends it. - evt.setError(error); - response = NextResponse.json( - { - data: null, - error: error.message, - meta: { errorCode: error.errorCode }, - }, - { status: error.statusCode }, - ); - } else if (error instanceof HttpError) { - evt.setError(error); - response = NextResponse.json( - { data: null, error: error.message }, - { status: error.statusCode }, - ); - } else if (error instanceof SyntaxError) { - evt.setError(error); - response = NextResponse.json( - { data: null, error: "Invalid JSON body" }, - { status: 400 }, - ); - } else { - evt.setError(error); - // Report to GlitchTip (fire-and-forget) - reportToGlitchtip(error, request, evt).catch(() => {}); - response = NextResponse.json( - { data: null, error: "Interner Serverfehler" }, - { status: 500 }, - ); - } - } finally { - const status = (response as Response | undefined)?.status ?? 500; - evt.finish(status); + return runWithRateLimitCapture(() => + eventStorage.run(evt, async () => { + let response: Response | undefined; try { - emitIfSampled(evt.toJSON()); - } catch { - /* logging must never crash the handler */ + response = await handler(...args); + } catch (error) { + if (error instanceof AssistantDisabledError) { + // v1.4.31 — operator has disabled the assistant surface. + // The 403 + `errorCode: "assistant.disabled."` + // envelope is locked per + // `.planning/RESPONSE-TO-IOS-TEAM-2026-05-16.md` §3 R5. + // Older iOS clients that don't know the errorCode surface + // this as a generic 403; v1.4.31+ clients can branch on the + // errorCode to render an inline operator-disabled notice. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + meta: { errorCode: error.errorCode }, + }, + { status: 403 }, + ); + } else if (error instanceof ConsentRequiredError) { + // v1.12.1 — server-side consent gate before external-LLM PHI + // egress on the operator's server-managed key. Mirrors the + // AssistantDisabledError envelope (403 + meta.errorCode) so the + // iOS client renders an inline "grant consent" notice instead of + // a generic failure. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + meta: { errorCode: error.errorCode }, + }, + { status: 403 }, + ); + } else if (error instanceof StepUpRequiredError) { + // v1.23 — step-up gate not satisfied. Same 401 + meta.errorCode + // envelope shape as the assistant/consent gates so the client can + // branch on the stable code and launch a re-verification flow. + // Checked before the generic HttpError branch because + // StepUpRequiredError extends it. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + meta: { errorCode: error.errorCode }, + }, + { status: error.statusCode }, + ); + } else if (error instanceof SharingAuthError) { + // v1.36.0 — the acting-account resolver refused. Same + // 403 + meta.errorCode envelope as the gates above, so a client + // distinguishes "you may not act as that account" from "this + // endpoint does not support acting on one" without parsing prose. + // Checked before the generic HttpError branch because it extends it. + evt.setError(error); + response = NextResponse.json( + { + data: null, + error: error.message, + meta: { errorCode: error.errorCode }, + }, + { status: error.statusCode }, + ); + } else if (error instanceof HttpError) { + evt.setError(error); + response = NextResponse.json( + { data: null, error: error.message }, + { status: error.statusCode }, + ); + } else if (error instanceof SyntaxError) { + evt.setError(error); + response = NextResponse.json( + { data: null, error: "Invalid JSON body" }, + { status: 400 }, + ); + } else { + evt.setError(error); + // Report to GlitchTip (fire-and-forget) + reportToGlitchtip(error, request, evt).catch(() => {}); + response = NextResponse.json( + { data: null, error: "Interner Serverfehler" }, + { status: 500 }, + ); + } + } finally { + const status = (response as Response | undefined)?.status ?? 500; + evt.finish(status); + try { + emitIfSampled(evt.toJSON()); + } catch { + /* logging must never crash the handler */ + } } - } - const nr = response as NextResponse; - nr.headers.set("x-request-id", evt.getRequestId()); - // v1.37.0 — echo the record context this response was actually served - // under, when one was decided. The value comes from the wide event and - // from nowhere else: the fence stamps it on every call it makes, so - // exactly the responses that resolved a record scope carry it and - // everything else — public routes, actor surfaces, admin, `/me`, static — - // carries nothing. - // - // Deliberately NOT derived here with a `getSession()`. That would put a - // session read on every public route, and it would make the echo a second - // derivation of the context rather than a report of the one that was - // used, which is the difference between "what this response served" and - // "what a later read thinks is true". - // - // Absence is therefore meaningful and safe: the client discards a - // response whose echo CONTRADICTS its adopted context, and serves one - // that carries no echo normally. A discard-on-absence rule would throw - // away the `/api/version` poll on every cycle. - // - // The two header names stay inside the fence module, which is what keeps - // them to four files overall (declared, read, attached client-side, - // published) — see `src/__tests__/record-session-fence-guard.test.ts`. - attachRecordContextEcho(nr.headers, evt.getRecordContext()); - return nr; - }); + const nr = response as NextResponse; + nr.headers.set("x-request-id", evt.getRequestId()); + // v1.37.0 — echo the record context this response was actually served + // under, when one was decided. The value comes from the wide event and + // from nowhere else: the fence stamps it on every call it makes, so + // exactly the responses that resolved a record scope carry it and + // everything else — public routes, actor surfaces, admin, `/me`, static — + // carries nothing. + // + // Deliberately NOT derived here with a `getSession()`. That would put a + // session read on every public route, and it would make the echo a second + // derivation of the context rather than a report of the one that was + // used, which is the difference between "what this response served" and + // "what a later read thinks is true". + // + // Absence is therefore meaningful and safe: the client discards a + // response whose echo CONTRADICTS its adopted context, and serves one + // that carries no echo normally. A discard-on-absence rule would throw + // away the `/api/version` poll on every cycle. + // + // The two header names stay inside the fence module, which is what keeps + // them to four files overall (declared, read, attached client-side, + // published) — see `src/__tests__/record-session-fence-guard.test.ts`. + attachRecordContextEcho(nr.headers, evt.getRecordContext()); + attachRateLimitHeaders(nr); + return nr; + }), + ); }; return wrapped as T; } +/** + * Put the rate-limit headers on a refusal that left the handler without them. + * + * Only ~70 of the 199 route files that consult a limiter attach + * `rateLimitHeaders(rl)` by hand; the rest answer 429 with a bare prose + * envelope, so an offline replay queue that trips the batch ceiling has + * nothing to back off against but a guess. The limiter records its verdict in + * the request-scoped cell, so the headers can be derived here for every one of + * them at once instead of being threaded through each call site — and a route + * added tomorrow gets them without knowing this exists. + * + * `Retry-After` is the presence test rather than a flag: a handler that built + * its own headers already carries it (they come from the same builder), so + * this leaves that response exactly as it was written. + */ +function attachRateLimitHeaders(response: NextResponse): void { + if (response.status !== 429) return; + if (response.headers.has("Retry-After")) return; + const snapshot = capturedRateLimit(); + if (!snapshot) return; + for (const [name, value] of Object.entries( + rateLimitResponseHeaders(snapshot), + )) { + response.headers.set(name, value); + } +} + /** * Authenticated request context. Returned by both session-cookie and Bearer-token * authentication paths. The `session.id` is the session-record id for cookie auth diff --git a/src/lib/documents/__tests__/ai-route-support-limit.test.ts b/src/lib/documents/__tests__/ai-route-support-limit.test.ts index d2a4b1110..f47184ce6 100644 --- a/src/lib/documents/__tests__/ai-route-support-limit.test.ts +++ b/src/lib/documents/__tests__/ai-route-support-limit.test.ts @@ -77,6 +77,7 @@ describe("documentAiRateLimited", () => { const resetAt = Date.now() + 42 * 60_000; const response = documentAiRateLimited({ allowed: false, + limit: 60, remaining: 0, resetAt, }); diff --git a/src/lib/openapi/routes/shared.ts b/src/lib/openapi/routes/shared.ts index 4ce94c266..dad0f7624 100644 --- a/src/lib/openapi/routes/shared.ts +++ b/src/lib/openapi/routes/shared.ts @@ -181,7 +181,8 @@ export const stdResponses = { content: { "application/json": { schema: errorEnvelope } }, }, "429": { - description: "Rate limit exceeded.", + description: + "Rate limit exceeded. The response carries `Retry-After` (whole seconds, rounded up — wait at least that long before retrying), `X-RateLimit-Limit` (the bucket's cap), `X-RateLimit-Remaining` and `X-RateLimit-Reset` (the reset instant, ISO-8601, not epoch seconds). Back off on `Retry-After` rather than guessing.", content: { "application/json": { schema: errorEnvelope } }, }, }; diff --git a/src/lib/rate-limit-context.ts b/src/lib/rate-limit-context.ts new file mode 100644 index 000000000..84293d0dc --- /dev/null +++ b/src/lib/rate-limit-context.ts @@ -0,0 +1,97 @@ +/** + * Request-scoped capture of the rate-limiter's last verdict, plus the one + * builder that turns a verdict into response headers — a LEAF module. + * + * It lives apart from `rate-limit.ts` on purpose. Nearly two hundred route + * tests replace `@/lib/rate-limit` wholesale with a `vi.mock` factory, so + * anything `apiHandler` needs from the limiter has to come from a module those + * factories do not stand in for; otherwise the wrapper calls `undefined()` the + * moment a mocked route is exercised. + * + * Why the capture exists at all: a route that refuses a request answers 429 + * from its own handler body, and most of them build that response with a bare + * `apiError(...)`. Threading the headers through every one of those call sites + * is 129 edits that the next new route immediately forgets. Instead the + * limiter records its verdict here, `apiHandler` runs every handler inside the + * store, and any 429 that leaves without headers gets them on the way out. A + * route that already attaches them keeps exactly the response it built. + */ +import { AsyncLocalStorage } from "node:async_hooks"; + +/** What a rate-limit verdict has to carry to be renderable as headers. */ +export interface RateLimitSnapshot { + /** Requests allowed per window. */ + limit: number; + /** Requests left in the current window; zero once the cap is hit. */ + remaining: number; + /** Epoch milliseconds at which the current window rolls over. */ + resetAt: number; +} + +interface RateLimitCapture { + last: RateLimitSnapshot | null; + denied: RateLimitSnapshot | null; +} + +const storage = new AsyncLocalStorage(); + +/** + * Run `fn` with a fresh capture cell. Called once per request by + * `apiHandler`; outside one, every capture below is a no-op and + * {@link capturedRateLimit} reads null. + */ +export function runWithRateLimitCapture(fn: () => T): T { + return storage.run({ last: null, denied: null }, fn); +} + +/** + * Record a limiter verdict for the request in flight. + * + * A denied verdict wins over an allowed one: a handler that consults two + * buckets and is refused by the second must report the bucket that actually + * refused it, not the one it passed. + */ +export function captureRateLimitResult( + result: RateLimitSnapshot & { allowed: boolean }, +): void { + const store = storage.getStore(); + if (!store) return; + store.last = result; + if (!result.allowed) store.denied = result; +} + +/** The verdict to render for this request, or null when none was taken. */ +export function capturedRateLimit(): RateLimitSnapshot | null { + const store = storage.getStore(); + if (!store) return null; + return store.denied ?? store.last; +} + +/** + * The rate-limit response headers. + * + * `Retry-After` is whole seconds and rounds UP, per RFC 9110 §10.2.3: a client + * that waits the value it is given must find the window already rolled over, + * which flooring cannot promise. It never goes below zero — a window that + * expired between the verdict and the response says "retry now" rather than + * naming a time in the past. + * + * `X-RateLimit-Reset` keeps the ISO-8601 instant it has always carried. It is + * the wrong convention for the header name and every other deployment uses + * epoch or delta seconds, but it is on the wire today and changing it is a + * contract decision of its own; `Retry-After` gives an integer-parsing client + * the number it needs without touching the existing field. + */ +export function rateLimitResponseHeaders( + snapshot: RateLimitSnapshot, + now: number = Date.now(), +): Record { + return { + "Retry-After": String( + Math.max(0, Math.ceil((snapshot.resetAt - now) / 1000)), + ), + "X-RateLimit-Limit": String(snapshot.limit), + "X-RateLimit-Remaining": String(snapshot.remaining), + "X-RateLimit-Reset": new Date(snapshot.resetAt).toISOString(), + }; +} diff --git a/src/lib/rate-limit.ts b/src/lib/rate-limit.ts index b8dccc0a5..d76113579 100644 --- a/src/lib/rate-limit.ts +++ b/src/lib/rate-limit.ts @@ -5,11 +5,14 @@ import { prisma } from "@/lib/db"; import { getClientIpOrTrustWarning } from "@/lib/api-response"; +import { + captureRateLimitResult, + rateLimitResponseHeaders, + type RateLimitSnapshot, +} from "@/lib/rate-limit-context"; -export interface RateLimitResult { +export interface RateLimitResult extends RateLimitSnapshot { allowed: boolean; - remaining: number; - resetAt: number; } /** @@ -45,11 +48,16 @@ export async function checkRateLimit( const resetAt = row.reset_at.getTime(); const count = Number(row.count); - return { + const result: RateLimitResult = { allowed: count <= limit, + limit, remaining: Math.max(0, limit - count), resetAt, }; + // Hand the verdict to the request-scoped cell so `apiHandler` can dress a + // bare 429 on the way out. A no-op outside a request. + captureRateLimitResult(result); + return result; } /** @@ -77,15 +85,13 @@ export async function refundRateLimit(key: string): Promise { } /** - * Rate limit response headers. + * Rate limit response headers: `Retry-After` plus the `X-RateLimit-*` triple. + * See `rateLimitResponseHeaders` for what each one promises. */ export function rateLimitHeaders( - result: RateLimitResult, + result: RateLimitSnapshot, ): Record { - return { - "X-RateLimit-Remaining": String(result.remaining), - "X-RateLimit-Reset": new Date(result.resetAt).toISOString(), - }; + return rateLimitResponseHeaders(result); } /** diff --git a/tests/integration/rate-limit-429-headers.test.ts b/tests/integration/rate-limit-429-headers.test.ts new file mode 100644 index 000000000..8fbb1becc --- /dev/null +++ b/tests/integration/rate-limit-429-headers.test.ts @@ -0,0 +1,135 @@ +/** + * A 429 from a route that attaches nothing itself still carries the headers. + * + * `POST /api/measurements/batch` is the one that matters most: a first-run + * Apple Health import reliably trips its 60-per-minute ceiling, and until now + * the client got a prose sentence and nothing to back off against. The route + * builds its refusal with a bare `apiError(...)`, so this proves the headers + * come from `apiHandler` and the limiter's request-scoped verdict rather than + * from the handler body — which is what makes the other 128 header-less routes + * covered too. + * + * Run against a real Postgres because the limiter's window and remainder come + * out of the atomic upsert; a mocked verdict would prove the plumbing and not + * the numbers. + */ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +const TEST_USER_ID = "user-rate-limit-headers"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + await getPrismaClient().user.create({ + data: { + id: TEST_USER_ID, + username: "rate-limit-headers", + email: "rate-limit-headers@example.test", + }, + }); + const session = await getPrismaClient().session.create({ + data: { + userId: TEST_USER_ID, + expiresAt: new Date(Date.now() + 60 * 60 * 1000), + }, + }); + cookieJar.set("healthlog_session", session.id); +}); + +function batchRequest(): NextRequest { + return new NextRequest("http://localhost/api/measurements/batch", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + entries: [ + { + hkIdentifier: "HKQuantityTypeIdentifierBodyMass", + value: 81.4, + unit: "kg", + startDate: "2026-07-20T06:00:00.000Z", + endDate: "2026-07-20T06:00:00.000Z", + externalId: `rl-${Math.random().toString(36).slice(2)}`, + }, + ], + }), + }); +} + +describe("429 rate-limit headers (real Postgres)", () => { + it("carries Retry-After and the X-RateLimit triple on a route that attaches none", async () => { + const { POST } = await import("@/app/api/measurements/batch/route"); + + // Burn the bucket directly rather than posting sixty batches: the route + // and the pre-charge share one key, so the next call through the handler + // is the one that is refused. + const { checkRateLimit } = await import("@/lib/rate-limit"); + const key = `measurements:batch:${TEST_USER_ID}`; + for (let i = 0; i < 60; i += 1) { + await checkRateLimit(key, 60, 60_000); + } + + const before = Date.now(); + const res = await POST(batchRequest() as never); + expect(res.status).toBe(429); + + const retryAfter = res.headers.get("Retry-After"); + expect(retryAfter).toMatch(/^\d+$/); + const seconds = Number(retryAfter); + expect(seconds).toBeGreaterThan(0); + expect(seconds).toBeLessThanOrEqual(60); + + expect(res.headers.get("X-RateLimit-Limit")).toBe("60"); + expect(res.headers.get("X-RateLimit-Remaining")).toBe("0"); + + const reset = res.headers.get("X-RateLimit-Reset"); + expect(reset).toBeTruthy(); + const resetAt = Date.parse(reset as string); + expect(Number.isNaN(resetAt)).toBe(false); + expect(resetAt).toBeGreaterThan(before); + + // The named delay must actually outlast the window it names. + expect(before + seconds * 1000).toBeGreaterThanOrEqual(resetAt - 1_000); + + // The body is unchanged — the headers are additive. + const body = (await res.json()) as { data: null; error: string }; + expect(body.data).toBeNull(); + expect(typeof body.error).toBe("string"); + }); + + it("leaves a successful response without them", async () => { + const { POST } = await import("@/app/api/measurements/batch/route"); + + const res = await POST(batchRequest() as never); + expect(res.status).toBe(200); + expect(res.headers.get("Retry-After")).toBeNull(); + }); +});