From b55850967dab9f7c8dff53175f25170e48a73483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 4 Oct 2026 10:14:22 +0200 Subject: [PATCH] fix(idempotency): replay a key only for the same request body The replay cell was keyed on (user, key, method, path). A client that sends two different writes under one key got the first write's cached response for the second, and the second was never saved. Native clients up to 1.1.0 post the two halves of a manual blood pressure exactly that way, so the diastolic value was silently dropped. Each cell now stores a SHA-256 fingerprint of the request body (JSON with recursively sorted keys, otherwise the raw bytes; scoped by method and path; at most the first MiB is read). Lookup replays, or refuses as in flight, only when the fingerprint matches. A different body, or a row written before the column existed, runs the handler without claiming or caching and leaves the existing cell for its own retries; a lost claim race re-checks the winner's fingerprint the same way. The body itself is never stored. Migration 0371 adds the nullable idempotency_keys.request_fingerprint column. The OpenAPI parameter description, the client notes and the Home Assistant guide say what a replay now requires. docs/ops gains a read-only query that lists systolic readings left without their diastolic half, held to the schema by an integration test. --- CHANGELOG.md | 15 ++ docs/CONTRIBUTING-CLIENTS.md | 2 +- docs/api/openapi.yaml | 14 +- docs/integrations/home-assistant.md | 5 +- docs/ops/blood-pressure-missing-diastolic.md | 60 +++++ .../migration.sql | 8 + prisma/schema.prisma | 22 +- .../__tests__/idempotency-conflict.test.ts | 11 +- src/lib/__tests__/idempotency.test.ts | 208 +++++++++++++++ src/lib/idempotency.ts | 184 ++++++++++++- src/lib/openapi/routes/shared.ts | 5 +- .../idempotency-body-fingerprint.test.ts | 249 ++++++++++++++++++ 12 files changed, 761 insertions(+), 22 deletions(-) create mode 100644 docs/ops/blood-pressure-missing-diastolic.md create mode 100644 prisma/migrations/0371_idempotency_request_fingerprint/migration.sql create mode 100644 tests/integration/idempotency-body-fingerprint.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 53cb335cb..5654e7f10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## [Unreleased] + +### Fixed + +- **Blood pressure saved without its diastolic value.** The iPhone app (up to + 1.0.3, and TestFlight 1.1.0) sends the two halves of a manual reading as two + requests under one `Idempotency-Key`, and the server answered the second + with the first one's cached response instead of saving it. The replay cache + now stores a SHA-256 fingerprint of the request body (migration 0371, + `idempotency_keys.request_fingerprint`) and replays only the same body; a + different body under the same key is saved normally. Readings saved before + this fix stay incomplete: delete the incomplete reading and enter it again + with both values and the original time. Operators can list affected + readings with the query in `docs/ops/blood-pressure-missing-diastolic.md`. + ## [1.39.9] — 2026-10-03 Workouts can be logged by hand, the body fat tile opens its own page, and diff --git a/docs/CONTRIBUTING-CLIENTS.md b/docs/CONTRIBUTING-CLIENTS.md index 2f5660022..e2f627e8c 100644 --- a/docs/CONTRIBUTING-CLIENTS.md +++ b/docs/CONTRIBUTING-CLIENTS.md @@ -19,7 +19,7 @@ Every JSON route answers the same shape: `{ data, error }`, with `meta` on the e - Refresh tokens rotate per device and are one-time-use. Reusing an old one revokes that device's whole token family, on purpose. Persist the newest pair atomically and never retry a refresh with a token that already succeeded once. - Bearer token scopes are fail-closed. A token minted for one purpose does not open unrelated routes, and a route that does not declare a scope refuses narrow tokens outright. Expect `403` to be a normal, recoverable answer, not an anomaly. - Admin surfaces are cookie-session only, by construction. A native client cannot be an admin client, whatever scopes its token carries. Do not build admin features. -- Mutations accept an `Idempotency-Key` header. Replays answer with the original result plus `X-Idempotent-Replay`, so retrying on a dropped connection is safe. Use it for anything a user would notice twice. +- Mutations accept an `Idempotency-Key` header. A repeat of the same key with the same body answers with the original result plus `X-Idempotent-Replay`, so retrying on a dropped connection is safe. A different body under the same key is treated as a new request, not a replay; still, give each distinct write its own key. Use it for anything a user would notice twice. ## Data semantics worth knowing before you sync diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 25431a0bd..50a2926cb 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -162,12 +162,14 @@ paths: pattern: ^[A-Za-z0-9_\-:.]{8,128}$ minLength: 8 maxLength: 128 - description: "Makes this write safe to retry. Send the same key again and the first response is replayed rather than the - work repeated, marked with `X-Idempotent-Replay: true`. A key that does not match the pattern is IGNORED - rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no - key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different - route is a different key. While a first request under this key is still in flight, a second is refused with - 409 rather than queued." + description: "Makes this write safe to retry. Send the same key with the same body again and the first response is + replayed rather than the work repeated, marked with `X-Idempotent-Replay: true`. The body is compared as + JSON with key order ignored; a different body under the same key is not a retry and runs as a new write, + while the first response stays stored for retries of the first request. A key that does not match the + pattern is IGNORED rather than refused — the write then proceeds unprotected and looks exactly like a + request that carried no key, so validate the key on your side. Scoped per user and per (method, path): the + same key on a different route is a different key. While a first request under this key is still in flight, a + second with the same body is refused with 409 rather than queued." requestBody: required: true content: diff --git a/docs/integrations/home-assistant.md b/docs/integrations/home-assistant.md index c521a8c74..71d679ccc 100644 --- a/docs/integrations/home-assistant.md +++ b/docs/integrations/home-assistant.md @@ -171,8 +171,9 @@ tells you which rows to retry. ## Retries and duplicates Send an `Idempotency-Key` header with a value your automation can reproduce for -the same reading. A retry with the same key returns the original response -instead of writing a second row: +the same reading. A retry with the same key and the same body returns the +original response instead of writing a second row. A different body under the +same key is a different reading and is written: ```yaml headers: diff --git a/docs/ops/blood-pressure-missing-diastolic.md b/docs/ops/blood-pressure-missing-diastolic.md new file mode 100644 index 000000000..1509a6ade --- /dev/null +++ b/docs/ops/blood-pressure-missing-diastolic.md @@ -0,0 +1,60 @@ +# Blood pressure readings missing their diastolic half + +## What happened + +The iPhone app (every App Store build up to 1.0.3 and TestFlight 1.1.0) +saves a manually entered blood pressure as two requests to +`POST /api/measurements`: one `BLOOD_PRESSURE_SYS`, one +`BLOOD_PRESSURE_DIA`, both carrying the same `Idempotency-Key`. Until the +replay cache learned to compare request bodies, the server treated the second +request as a retry of the first. It answered with the first request's cached +`201` (header `X-Idempotent-Replay: true`) and never wrote the diastolic +value. The app showed success, so nobody noticed. + +The fix makes the cache replay only a request with the same body. A different +body under the same key now runs normally. Nothing recovers the values that +were never sent to the database: they exist only on the person's phone, if at +all. + +## Finding the affected readings + +Read-only. Run it against the HealthLog database (with the bundled compose +file: `docker compose exec db psql -U healthlog healthlog`). It lists every live systolic reading that has no +live diastolic reading for the same account at the same instant. + + + +```sql +SELECT u.username, + m.id, + m.measured_at, + m.value AS systolic, + m.source +FROM measurements m +JOIN users u ON u.id = m.user_id +WHERE m.type = 'BLOOD_PRESSURE_SYS' + AND m.deleted_at IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM measurements d + WHERE d.user_id = m.user_id + AND d.measured_at = m.measured_at + AND d.type = 'BLOOD_PRESSURE_DIA' + AND d.deleted_at IS NULL + ) +ORDER BY u.username, m.measured_at; +``` + + + +Readings from the phone app carry `source = 'MANUAL'`. Rows from a synced +source (Withings, Apple Health) that show up here are a different gap, not +this one: those sources write both halves themselves. + +## What to tell users + +There is nothing the server can repair: the diastolic value never reached +it. A person whose reading appears in the list can re-enter it: delete the +incomplete reading, then add the blood pressure again with both values and +the original date and time. From this release on, both halves are saved on +the first try, from any app version. diff --git a/prisma/migrations/0371_idempotency_request_fingerprint/migration.sql b/prisma/migrations/0371_idempotency_request_fingerprint/migration.sql new file mode 100644 index 000000000..dcb360852 --- /dev/null +++ b/prisma/migrations/0371_idempotency_request_fingerprint/migration.sql @@ -0,0 +1,8 @@ +-- What request a replay-cache cell answered, as a SHA-256 of the canonical +-- request body scoped by method and path. A request that reuses a key with a +-- different body is executed instead of being handed the first body's cached +-- response. The body itself is never stored. Nullable: rows written before +-- this release carry none, and a null never matches, so they stop replaying +-- for anything but their own retries already in flight (they expire within +-- 24 hours either way). +ALTER TABLE "idempotency_keys" ADD COLUMN IF NOT EXISTS "request_fingerprint" TEXT; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 6a05a5659..9250aef3f 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -5545,15 +5545,19 @@ model ProviderHealth { /// (userId, key, method, path) so retries within the TTL replay the /// original outcome instead of producing a duplicate side-effect. model IdempotencyKey { - id String @id @default(cuid()) - userId String @map("user_id") - key String - method String - path String - responseStatus Int @map("response_status") - responseBody String @map("response_body") // serialized JSON envelope - expiresAt DateTime @map("expires_at") - createdAt DateTime @default(now()) @map("created_at") + id String @id @default(cuid()) + userId String @map("user_id") + key String + method String + path String + responseStatus Int @map("response_status") + responseBody String @map("response_body") // serialized JSON envelope + /// SHA-256 of the canonical request body, scoped by method and path. A + /// lookup replays only when it matches; a different body under the same key + /// runs normally. Null on rows written before it existed, which never match. + requestFingerprint String? @map("request_fingerprint") + expiresAt DateTime @map("expires_at") + createdAt DateTime @default(now()) @map("created_at") user User @relation(fields: [userId], references: [id], onDelete: Cascade) diff --git a/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts b/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts index adb31bbdc..dc93fd8e2 100644 --- a/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts +++ b/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts @@ -58,10 +58,13 @@ vi.mock("@/lib/profile/health-facts", () => ({ import { NextRequest } from "next/server"; import { POST } from "../route"; +import { requestFingerprint } from "@/lib/idempotency"; /** The pending sentinel `findCached` recognises as "another call is running". */ const PENDING_STATUS = 0; +const BODY = JSON.stringify({ kind: "SMOKING_STATUS", value: "FORMER" }); + function request(): NextRequest { return new NextRequest("http://localhost/api/anamnesis/facts", { method: "POST", @@ -69,7 +72,7 @@ function request(): NextRequest { "content-type": "application/json", "idempotency-key": "key-in-flight", }, - body: JSON.stringify({ kind: "SMOKING_STATUS", value: "FORMER" }), + body: BODY, }); } @@ -99,6 +102,12 @@ describe("idempotency 409 envelope, over the real route", () => { it("serves `error` as a string when a request is already in flight", async () => { mocks.findUnique.mockResolvedValue({ id: "idem-1", + // The same request in flight: a different body would run instead. + requestFingerprint: requestFingerprint( + "POST", + "/api/anamnesis/facts", + new TextEncoder().encode(BODY), + ), responseStatus: PENDING_STATUS, responseBody: "", expiresAt: new Date(Date.now() + 60_000), diff --git a/src/lib/__tests__/idempotency.test.ts b/src/lib/__tests__/idempotency.test.ts index 61494372e..a93a23ee1 100644 --- a/src/lib/__tests__/idempotency.test.ts +++ b/src/lib/__tests__/idempotency.test.ts @@ -49,6 +49,8 @@ import { withIdempotency, defaultUserIdResolver, isCachableStatus, + canonicalJson, + requestFingerprint, } from "../idempotency"; import { prisma } from "@/lib/db"; import { getSession } from "@/lib/auth/session"; @@ -81,6 +83,13 @@ function makeRequest( }); } +/** The fingerprint `makeRequest("POST", …)` produces. */ +const OK_BODY_FINGERPRINT = requestFingerprint( + "POST", + "/api/example", + new TextEncoder().encode(JSON.stringify({ ok: true })), +); + beforeEach(() => { vi.resetAllMocks(); vi.mocked(prisma.idempotencyKey.findUnique).mockResolvedValue(null); @@ -144,6 +153,11 @@ describe("withIdempotency", () => { data: { responseBody: string; responseStatus: number }; } ).data; + const claimedFingerprint = ( + vi.mocked(prisma.idempotencyKey.create).mock.calls[0][0] as { + data: { requestFingerprint: string }; + } + ).data.requestFingerprint; // Second call: cache hit returns persisted envelope. vi.mocked(prisma.idempotencyKey.findUnique).mockResolvedValueOnce({ @@ -152,6 +166,7 @@ describe("withIdempotency", () => { key: "abc-12345678", method: "POST", path: "/api/example", + requestFingerprint: claimedFingerprint, responseStatus: persistedBody.responseStatus, responseBody: persistedBody.responseBody, expiresAt: new Date(Date.now() + 86_400_000), @@ -296,6 +311,7 @@ describe("withIdempotency concurrency claim (A3)", () => { key: "abc-12345678", method: "POST", path: "/api/example", + requestFingerprint: OK_BODY_FINGERPRINT, responseStatus: 0, responseBody: "", expiresAt: new Date(Date.now() + 60_000), @@ -928,3 +944,195 @@ describe("the record-session fence sits above the replay cache", () => { expect(handler).toHaveBeenCalledTimes(1); }); }); + +describe("request fingerprint", () => { + const enc = (text: string) => new TextEncoder().encode(text); + + it("sorts object keys recursively and keeps array order", () => { + expect(canonicalJson({ b: 1, a: { d: [3, 1], c: null } })).toBe( + '{"a":{"c":null,"d":[3,1]},"b":1}', + ); + expect(canonicalJson([{ y: 1, x: 2 }, "s"])).toBe('[{"x":2,"y":1},"s"]'); + }); + + it("treats the same JSON object in a different key order as one request", () => { + const a = requestFingerprint("POST", "/p", enc('{"type":"X","value":1}')); + const b = requestFingerprint( + "POST", + "/p", + enc('{ "value": 1, "type": "X" }'), + ); + expect(a).toBe(b); + expect(a).toMatch(/^[0-9a-f]{64}$/); + }); + + it("tells nested values and array order apart", () => { + const base = requestFingerprint("POST", "/p", enc('{"a":{"b":[1,2]}}')); + expect(requestFingerprint("POST", "/p", enc('{"a":{"b":[2,1]}}'))).not.toBe( + base, + ); + expect(requestFingerprint("POST", "/p", enc('{"a":{"b":[1,3]}}'))).not.toBe( + base, + ); + }); + + it("tells the two halves of a blood pressure apart", () => { + const sys = requestFingerprint( + "POST", + "/api/measurements", + enc('{"type":"BLOOD_PRESSURE_SYS","value":120}'), + ); + const dia = requestFingerprint( + "POST", + "/api/measurements", + enc('{"type":"BLOOD_PRESSURE_DIA","value":80}'), + ); + expect(sys).not.toBe(dia); + }); + + it("hashes a non-JSON or empty body as its raw bytes", () => { + const text = requestFingerprint("POST", "/p", enc("not json")); + expect(text).toBe(requestFingerprint("POST", "/p", enc("not json"))); + expect(text).not.toBe(requestFingerprint("POST", "/p", enc("not json!"))); + const empty = requestFingerprint("DELETE", "/p", new Uint8Array()); + expect(empty).toBe(requestFingerprint("DELETE", "/p", new Uint8Array())); + expect(empty).not.toBe(text); + // Invalid UTF-8 is raw bytes, not an error. + expect(() => + requestFingerprint("POST", "/p", new Uint8Array([0xff, 0xfe])), + ).not.toThrow(); + }); + + it("is scoped by method and path", () => { + const body = enc('{"a":1}'); + const base = requestFingerprint("POST", "/p", body); + expect(requestFingerprint("PUT", "/p", body)).not.toBe(base); + expect(requestFingerprint("POST", "/q", body)).not.toBe(base); + }); +}); + +describe("withIdempotency with a different body under one key", () => { + function postWith(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/example", { + method: "POST", + headers: { + "content-type": "application/json", + "idempotency-key": "abc-12345678", + }, + body: JSON.stringify(body), + }); + } + + function liveRow(fingerprint: string | null, responseStatus: number) { + return { + id: "idem-1", + userId: "u-1", + key: "abc-12345678", + method: "POST", + path: "/api/example", + requestFingerprint: fingerprint, + responseStatus, + responseBody: "", + expiresAt: new Date(Date.now() + 60_000), + createdAt: new Date(), + } as never; + } + + it.each([ + ["a completed row for another body", "f".repeat(64), 201], + ["an in-flight claim for another body", "f".repeat(64), 0], + ["a legacy row without a fingerprint", null, 201], + ])( + "runs the handler without claiming or caching over %s", + async (_label, fingerprint, status) => { + vi.mocked(prisma.idempotencyKey.findUnique).mockResolvedValueOnce( + liveRow(fingerprint, status), + ); + const handler = vi.fn(async () => + NextResponse.json({ data: "dia", error: null }, { status: 201 }), + ); + const wrapped = withIdempotency<[NextRequest]>( + handler, + async () => "u-1", + ); + const res = await wrapped(postWith({ type: "BLOOD_PRESSURE_DIA" })); + + expect(res.status).toBe(201); + expect(res.headers.get("X-Idempotent-Replay")).toBeNull(); + expect(handler).toHaveBeenCalledTimes(1); + expect(prisma.idempotencyKey.create).not.toHaveBeenCalled(); + expect(prisma.idempotencyKey.updateMany).not.toHaveBeenCalled(); + expect(prisma.idempotencyKey.deleteMany).not.toHaveBeenCalled(); + expect(prisma.idempotencyKey.delete).not.toHaveBeenCalled(); + }, + ); + + it("runs the handler when a racing claim for another body wins the insert", async () => { + vi.mocked(prisma.idempotencyKey.create).mockRejectedValueOnce( + Object.assign(new Error("unique"), { code: "P2002" }), + ); + // First lookup: nothing yet. Second, after the collision: the other body. + vi.mocked(prisma.idempotencyKey.findUnique) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(liveRow("f".repeat(64), 0)); + const handler = vi.fn(async () => + NextResponse.json({ data: "dia", error: null }, { status: 201 }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + const res = await wrapped(postWith({ type: "BLOOD_PRESSURE_DIA" })); + + expect(res.status).toBe(201); + expect(handler).toHaveBeenCalledTimes(1); + expect(prisma.idempotencyKey.updateMany).not.toHaveBeenCalled(); + }); + + it("stores the fingerprint, never the body, on the claim", async () => { + const handler = vi.fn(async () => + NextResponse.json({ data: "ok", error: null }, { status: 201 }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + await wrapped(postWith({ type: "BLOOD_PRESSURE_SYS", value: 123 })); + + const claim = ( + vi.mocked(prisma.idempotencyKey.create).mock.calls[0][0] as { + data: Record; + } + ).data; + expect(claim.requestFingerprint).toBe( + requestFingerprint( + "POST", + "/api/example", + new TextEncoder().encode( + JSON.stringify({ type: "BLOOD_PRESSURE_SYS", value: 123 }), + ), + ), + ); + expect(JSON.stringify(claim)).not.toContain("BLOOD_PRESSURE_SYS"); + }); + + it("leaves the body readable for the handler", async () => { + const handler = vi.fn(async (req: NextRequest) => + NextResponse.json({ data: await req.json(), error: null }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + const res = await wrapped(postWith({ type: "WEIGHT", value: 70 })); + expect((await res.json()).data).toEqual({ type: "WEIGHT", value: 70 }); + }); + + it("reads at most a bounded prefix of a large body and still hands the whole body on", async () => { + const big = "x".repeat(3 * 1024 * 1024); + const handler = vi.fn(async (req: NextRequest) => + NextResponse.json({ data: (await req.text()).length, error: null }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + const res = await wrapped( + new NextRequest("http://localhost/api/example", { + method: "POST", + headers: { "idempotency-key": "abc-12345678" }, + body: big, + }), + ); + expect((await res.json()).data).toBe(big.length); + expect(prisma.idempotencyKey.create).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/idempotency.ts b/src/lib/idempotency.ts index 70e2ddcbc..82d2cc0b6 100644 --- a/src/lib/idempotency.ts +++ b/src/lib/idempotency.ts @@ -11,6 +11,19 @@ * the record the request claims is folded into `key` (see `cellKey`), so the * tuple identifies a request to one person's record rather than a request by * one person. A caller acting only as themselves is unaffected, byte for byte. + * + * A cell also remembers WHICH request it answered: a SHA-256 of the canonical + * request body (`requestFingerprint`). A key names one request, not a slot, and + * a client that reuses a key for a second, different request must not be + * handed the first request's response. Native clients up to 1.1.0 sent the two + * halves of a manual blood pressure as two POSTs under one key; the second half + * replayed the first's 201 and was never written. A lookup therefore replays, + * or reports in-flight, only for the same body. A different body — or a row + * written before fingerprints existed — runs the handler normally without + * claiming or caching anything, and the existing cell stays for its own + * retries. This is deliberately not the IETF draft's 422: refusing would turn + * every affected client's second write into a visible error with no way for + * the person to recover the value. */ import type { NextRequest } from "next/server"; import { headers } from "next/headers"; @@ -109,6 +122,122 @@ export interface IdempotencyContext { key: string; method: string; path: string; + /** See `requestFingerprint`. Never the body itself. */ + fingerprint: string; +} + +/** + * How much of a body the fingerprint reads before it stops. The wrapper runs + * before the handler's own bounded readers and upload slots, so it must not + * buffer an arbitrarily large body on their behalf: reading the clone holds + * what it has read in the original's queue until the handler consumes it. + * Every JSON write the cache fronts is far below this; the one multipart route + * (document upload) is above it, and there the prefix plus the declared length + * identifies a retry of the same upload, with the route's own content hash as + * the duplicate check behind it. + */ +const FINGERPRINT_MAX_BYTES = 1024 * 1024; + +/** + * JSON with every object's keys sorted, recursively. Arrays keep their order — + * it is part of the request. Exported for its unit tests. + */ +export function canonicalJson(value: unknown): string { + if (Array.isArray(value)) { + return `[${value.map((v) => canonicalJson(v)).join(",")}]`; + } + if (value !== null && typeof value === "object") { + const entries = Object.keys(value) + .sort() + .map( + (k) => + `${JSON.stringify(k)}:${canonicalJson((value as Record)[k])}`, + ); + return `{${entries.join(",")}}`; + } + return JSON.stringify(value); +} + +/** + * The fingerprint of a request: SHA-256 hex over method, path and the body in + * canonical form. A body that parses as JSON is hashed as `canonicalJson`, so + * the same object serialised with a different key order is the same request; + * anything else (multipart, plain text, an empty body) is hashed as its raw + * bytes. `truncatedAt` marks a body cut at `FINGERPRINT_MAX_BYTES`, hashed as + * that raw prefix together with the declared length. Exported for unit tests. + */ +export function requestFingerprint( + method: string, + path: string, + body: Uint8Array, + truncatedAt: string | null = null, +): string { + const hash = createHash("sha256") + .update(method) + .update("\0") + .update(path) + .update("\0"); + if (truncatedAt !== null) { + return hash.update(`partial:${truncatedAt}\0`).update(body).digest("hex"); + } + if (body.length > 0) { + try { + const text = new TextDecoder("utf-8", { fatal: true }).decode(body); + return hash + .update("json:") + .update(canonicalJson(JSON.parse(text))) + .digest("hex"); + } catch { + // Not UTF-8 or not JSON — fall through to the raw bytes. + } + } + return hash.update("raw:").update(body).digest("hex"); +} + +/** + * Read the request's body from a clone, at most `FINGERPRINT_MAX_BYTES`, and + * fingerprint it. The original request keeps its body for the handler. + */ +async function fingerprintRequest( + request: Request | NextRequest, + path: string, +): Promise { + const stream = request.clone().body; + if (!stream) + return requestFingerprint(request.method, path, new Uint8Array()); + const reader = stream.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + let truncated = false; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + if (total + value.byteLength > FINGERPRINT_MAX_BYTES) { + chunks.push(value.subarray(0, FINGERPRINT_MAX_BYTES - total)); + total = FINGERPRINT_MAX_BYTES; + truncated = true; + // Cancelling this branch of the tee stops it from queueing the rest of + // the body, and leaves the handler's branch intact. Not awaited: a tee + // branch's cancel settles only once BOTH branches are done, which is + // after the handler has run. + void reader.cancel().catch(() => {}); + break; + } + chunks.push(value); + total += value.byteLength; + } + const body = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + body.set(chunk, offset); + offset += chunk.byteLength; + } + return requestFingerprint( + request.method, + path, + body, + truncated ? (request.headers.get("content-length") ?? "unknown") : null, + ); } function getIdempotencyKey(request: Request | NextRequest): string | null { @@ -125,10 +254,16 @@ function getIdempotencyKey(request: Request | NextRequest): string | null { * - `{ kind: "pending" }` — another request holds an in-flight claim * for this key; the caller must NOT run the * handler again. + * - `{ kind: "mismatch" }` — the live row answered a different body (or + * predates fingerprints); the caller runs the + * handler without claiming or caching. * - `null` — no live row; the caller may claim + run. */ type CacheLookup = - { kind: "replay"; response: NextResponse } | { kind: "pending" } | null; + | { kind: "replay"; response: NextResponse } + | { kind: "pending" } + | { kind: "mismatch" } + | null; /** * Look up the state of a (userId, key, method, path) tuple. Distinguishes @@ -157,6 +292,13 @@ async function findCached(ctx: IdempotencyContext): Promise { return null; } + // Checked before both the pending and the replay arm: a different request + // under the same key is neither in flight nor answered. A legacy row's null + // never equals a fingerprint. + if (row.requestFingerprint !== ctx.fingerprint) { + return { kind: "mismatch" }; + } + if (row.responseStatus === PENDING_STATUS) { // A concurrent request claimed this key and is still running its // side-effect. Signal the caller to refuse rather than double-execute. @@ -208,6 +350,7 @@ async function claimKey(ctx: IdempotencyContext): Promise { key: ctx.key, method: ctx.method, path: ctx.path, + requestFingerprint: ctx.fingerprint, responseStatus: PENDING_STATUS, responseBody: "", expiresAt: new Date(Date.now() + PENDING_TTL_MS), @@ -233,6 +376,7 @@ async function releaseClaim(ctx: IdempotencyContext): Promise { key: ctx.key, method: ctx.method, path: ctx.path, + requestFingerprint: ctx.fingerprint, responseStatus: PENDING_STATUS, }, }) @@ -312,6 +456,7 @@ async function persistCached( key: ctx.key, method: ctx.method, path: ctx.path, + requestFingerprint: ctx.fingerprint, }, data: { responseStatus: response.status, @@ -502,6 +647,23 @@ async function narrowTokenFacet(): Promise { return apiToken.permissions.includes("*") ? null : `t:${apiToken.id}`; } +/** + * Run a request whose key is already taken by a different body: the handler + * runs exactly as it would without a key, and nothing is claimed or cached, so + * the existing cell keeps answering its own retries. + */ +function runUncached( + ctx: IdempotencyContext, + handler: (...args: Args) => Promise, + args: Args, +): Promise { + annotate({ + action: { name: "idempotency.key.body_mismatch" }, + meta: { method: ctx.method, path: ctx.path }, + }); + return handler(...args); +} + export function withIdempotency< Args extends [Request | NextRequest, ...unknown[]], >( @@ -589,14 +751,27 @@ export function withIdempotency< if (tokenFacet !== null) facets.push(tokenFacet); const url = new URL(request.url); + const fingerprint = await fingerprintRequest(request, url.pathname).catch( + () => null, + ); + if (fingerprint === null) { + // The body could not be read for the fingerprint (already consumed, or + // the stream failed). Without knowing which request this is, no cell + // may answer for it; the handler meets the body problem itself. + return handler(...args); + } const ctx: IdempotencyContext = { userId, key: cellKey(key, claimedRecord, facets), method: request.method, path: url.pathname, + fingerprint, }; const cached = await findCached(ctx); + if (cached?.kind === "mismatch") { + return runUncached(ctx, handler, args); + } if (cached?.kind === "replay") { return cached.response; } @@ -617,6 +792,13 @@ export function withIdempotency< // one caller may execute the side-effect for a given key. const won = await claimKey(ctx); if (!won) { + // The racing claim may be a different request under the same key — + // the two halves of one reading posted together. That one is not a + // duplicate of this one and must still run. + const raced = await findCached(ctx); + if (raced?.kind === "mismatch") { + return runUncached(ctx, handler, args); + } annotate({ action: { name: "idempotency.inflight_conflict" }, meta: { method: ctx.method, path: ctx.path }, diff --git a/src/lib/openapi/routes/shared.ts b/src/lib/openapi/routes/shared.ts index c96613bc0..831933630 100644 --- a/src/lib/openapi/routes/shared.ts +++ b/src/lib/openapi/routes/shared.ts @@ -391,7 +391,8 @@ export const profileEmailRateLimitResponse = { * * Scope, so nobody reads more into it than is there: only POST, PUT, PATCH and * DELETE are covered (`SUPPORTED_METHODS`), the key is scoped per user and per - * `(method, path)`, and `isCachableStatus` refuses to cache 5xx or the + * `(method, path)`, a replay needs the same body (a different body under the + * same key runs as a new write and is neither replayed nor refused), and `isCachableStatus` refuses to cache 5xx or the * transient 401 / 403 / 408 / 429 — a retry after one of those runs for real. * * `src/__tests__/idempotency-contract-publication.test.ts` holds this to the @@ -410,7 +411,7 @@ export const idempotencyKeyParameter = { maxLength: 128, }, description: - "Makes this write safe to retry. Send the same key again and the first response is replayed rather than the work repeated, marked with `X-Idempotent-Replay: true`. A key that does not match the pattern is IGNORED rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different route is a different key. While a first request under this key is still in flight, a second is refused with 409 rather than queued.", + "Makes this write safe to retry. Send the same key with the same body again and the first response is replayed rather than the work repeated, marked with `X-Idempotent-Replay: true`. The body is compared as JSON with key order ignored; a different body under the same key is not a retry and runs as a new write, while the first response stays stored for retries of the first request. A key that does not match the pattern is IGNORED rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different route is a different key. While a first request under this key is still in flight, a second with the same body is refused with 409 rather than queued.", }; /** diff --git a/tests/integration/idempotency-body-fingerprint.test.ts b/tests/integration/idempotency-body-fingerprint.test.ts new file mode 100644 index 000000000..c0d04d93c --- /dev/null +++ b/tests/integration/idempotency-body-fingerprint.test.ts @@ -0,0 +1,249 @@ +/** + * One `Idempotency-Key`, two different requests. + * + * Native clients up to 1.1.0 post a manual blood pressure as two + * `POST /api/measurements` calls — the systolic half, then the diastolic half — + * under the same key. While the replay cell was keyed on + * `(user, key, method, path)` alone, the second call was answered with the + * first call's cached 201 and the diastolic value was never written. The cell + * now carries a fingerprint of the request body: only the same body replays. + * + * Everything here is real — the shipped route, the shipped wrapper, the + * session resolver and the `idempotency_keys` unique index. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { POST as postMeasurement } from "@/app/api/measurements/route"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +process.env.ENCRYPTION_KEY ??= + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + +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_A = "idem-fingerprint-a"; +const USER_B = "idem-fingerprint-b"; +const KEY = "bp-entry-5c1e2a7f"; +const MEASURED_AT = "2026-09-30T07:15:00.000Z"; + +const SYS = { type: "BLOOD_PRESSURE_SYS", value: 128, measuredAt: MEASURED_AT }; +const DIA = { type: "BLOOD_PRESSURE_DIA", value: 84, measuredAt: MEASURED_AT }; + +const sessions = new Map(); + +function post(body: unknown, key = KEY): Promise { + return postMeasurement( + new NextRequest("http://localhost/api/measurements", { + method: "POST", + headers: { + "content-type": "application/json", + "idempotency-key": key, + }, + body: JSON.stringify(body), + }), + ); +} + +function signInAs(userId: string) { + cookieJar.set("healthlog_session", sessions.get(userId)!); +} + +async function rowsOf(userId: string) { + const rows = await getPrismaClient().measurement.findMany({ + where: { userId, deletedAt: null }, + select: { type: true, value: true }, + }); + // By name, not by the enum's declaration order. + return rows.sort((a, b) => a.type.localeCompare(b.type)); +} + +beforeEach(async () => { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + cookieJar.clear(); + headerJar.clear(); + sessions.clear(); + for (const id of [USER_A, USER_B]) { + await prisma.user.create({ + data: { + id, + username: id, + email: `${id}@example.test`, + timezone: "UTC", + }, + }); + const session = await prisma.session.create({ + data: { userId: id, expiresAt: new Date(Date.now() + 60 * 60 * 1000) }, + }); + sessions.set(id, session.id); + } + signInAs(USER_A); +}); + +describe("Idempotency-Key reused for a different body (real Postgres)", () => { + it("writes both halves of a blood pressure posted under one key", async () => { + const first = await post(SYS); + expect(first.status).toBe(201); + + const second = await post(DIA); + expect(second.status).toBe(201); + expect(second.headers.get("X-Idempotent-Replay")).toBeNull(); + + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_DIA", value: 84 }, + { type: "BLOOD_PRESSURE_SYS", value: 128 }, + ]); + }); + + it("still replays a retry of the same body and writes one row", async () => { + const first = await post(SYS); + expect(first.status).toBe(201); + const firstBody = await first.json(); + + // Same object, different key order: the same request. + const retry = await post({ + measuredAt: MEASURED_AT, + value: 128, + type: "BLOOD_PRESSURE_SYS", + }); + expect(retry.status).toBe(201); + expect(retry.headers.get("X-Idempotent-Replay")).toBe("true"); + expect(await retry.json()).toEqual(firstBody); + + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_SYS", value: 128 }, + ]); + }); + + it("keeps the first cell answering its own retries after a different body ran", async () => { + await post(SYS); + await post(DIA); + const retry = await post(SYS); + expect(retry.headers.get("X-Idempotent-Replay")).toBe("true"); + expect(await rowsOf(USER_A)).toHaveLength(2); + const cells = await getPrismaClient().idempotencyKey.findMany({ + where: { userId: USER_A }, + }); + expect(cells).toHaveLength(1); + expect(cells[0]?.requestFingerprint).toMatch(/^[0-9a-f]{64}$/); + }); + + it("writes both halves when they are in flight at the same time", async () => { + const [a, b] = await Promise.all([post(SYS), post(DIA)]); + expect([a.status, b.status]).toEqual([201, 201]); + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_DIA", value: 84 }, + { type: "BLOOD_PRESSURE_SYS", value: 128 }, + ]); + }); + + it("still lets only one of two identical bodies in flight run", async () => { + const [a, b] = await Promise.all([post(SYS), post(SYS)]); + expect([a.status, b.status].sort()).toEqual([201, 409]); + expect(await rowsOf(USER_A)).toHaveLength(1); + }); + + it("does not replay a row written before fingerprints existed", async () => { + await getPrismaClient().idempotencyKey.create({ + data: { + userId: USER_A, + key: KEY, + method: "POST", + path: "/api/measurements", + responseStatus: 201, + responseBody: JSON.stringify({ data: { id: "legacy" }, error: null }), + expiresAt: new Date(Date.now() + 60 * 60 * 1000), + }, + }); + + const res = await post(DIA); + expect(res.status).toBe(201); + expect(res.headers.get("X-Idempotent-Replay")).toBeNull(); + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_DIA", value: 84 }, + ]); + // The legacy row is left alone, not claimed over. + const cell = await getPrismaClient().idempotencyKey.findFirstOrThrow({ + where: { userId: USER_A, key: KEY }, + }); + expect(cell.requestFingerprint).toBeNull(); + }); + + it("never hands one user's cached response to another using the same key", async () => { + const first = await post(SYS); + const firstBody = await first.json(); + + signInAs(USER_B); + const other = await post(SYS); + expect(other.status).toBe(201); + expect(other.headers.get("X-Idempotent-Replay")).toBeNull(); + expect(await other.json()).not.toEqual(firstBody); + + expect(await rowsOf(USER_A)).toHaveLength(1); + expect(await rowsOf(USER_B)).toHaveLength(1); + }); + + it("the documented operator query finds a systolic reading left without its diastolic half", async () => { + const doc = readFileSync( + join(process.cwd(), "docs/ops/blood-pressure-missing-diastolic.md"), + "utf8", + ); + const block = + /([\s\S]*?)/.exec( + doc, + ); + const sql = block?.[1] + .replace(/```sql|```/g, "") + .trim() + .replace(/;$/, ""); + expect(sql).toBeTruthy(); + + // A complete reading through the fixed path, and the damage the old + // cache left behind: a systolic half on its own. + await post(SYS); + await post(DIA); + const lone = await getPrismaClient().measurement.create({ + data: { + userId: USER_A, + type: "BLOOD_PRESSURE_SYS", + value: 141, + unit: "mmHg", + measuredAt: new Date("2026-09-29T19:40:00.000Z"), + }, + }); + + const found = await getPrismaClient().$queryRawUnsafe< + Array<{ id: string; systolic: number }> + >(sql!); + expect(found.map((r) => [r.id, r.systolic])).toEqual([[lone.id, 141]]); + }); +});