From 2ab66ab2e3c6b9bbaf56e75be522fe1bec8cec73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 25 Jul 2026 17:21:00 +0200 Subject: [PATCH 1/5] fix(ingest): refuse an external id that cannot be a stable identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An external id is half of every ingest dedup key — (userId, source, externalId), (userId, externalSource, externalId), (userId, type, source, externalId). Re-posting the same triple is idempotent and returns the existing row, which holds only while the id is stable across client launches. A client that serialised an opaque platform object with its default object description sent a per-allocation memory address instead, so no sweep ever matched its own earlier row and each one minted a fresh record: a live instance grew 23 phantom medications in a day, none of which could collect a dose, because the dose side resolved through the same rotating string. Add one shared classifier for identifiers that provably cannot be an identity — an angle-bracketed object description carrying a hex pointer, a bare 0x address, and a blank value — and wire it at every surface that accepts a client-supplied externalId. Single-entry creates refuse through the schema, so the client gets the standard 422 multi-issue envelope with a message naming the shape it sent and what to send instead. Batch and bulk surfaces classify per entry and report the offending row as skipped with reason unstable_external_id, leaving the rest of the payload to land: one bad row must not stop a sync. Every refusal carries pinned wide-event meta (count, shape, surface) and never the id itself. A structural guard freezes both routes into the floor: a request-side externalId declaration either applies the field check or carries a marker naming the file that checks it per entry, and the guard verifies that file really does. The backup-restore payload is the one exemption, with its reason recorded — it replays ids the server itself wrote. --- docs/api/openapi.yaml | 18 +- .../external-id-stability-guard.test.ts | 174 +++++++++++++ src/app/api/cycle/day-logs/bulk/route.ts | 33 +++ src/app/api/cycle/day-logs/route.ts | 16 +- src/app/api/cycle/period/route.ts | 10 +- src/app/api/import/route.ts | 43 +++- .../__tests__/unstable-external-id.test.ts | 239 ++++++++++++++++++ src/app/api/measurements/batch/route.ts | 35 +++ .../api/medications/__tests__/route.test.ts | 59 +++++ src/app/api/medications/intake/bulk/route.ts | 35 +++ src/app/api/medications/route.ts | 14 + .../api/mental-health/assessments/route.ts | 12 +- src/app/api/mood-entries/bulk/route.ts | 35 +++ src/app/api/mood-entries/route.ts | 10 +- src/app/api/workouts/batch/route.ts | 53 +++- src/lib/import/csv-measurements.ts | 10 + src/lib/openapi/routes/measurements.ts | 3 +- src/lib/openapi/routes/medications/paths.ts | 2 +- src/lib/openapi/routes/mood.ts | 2 +- .../validations/__tests__/external-id.test.ts | 182 +++++++++++++ src/lib/validations/cycle.ts | 32 ++- src/lib/validations/external-id.ts | 189 ++++++++++++++ .../validations/medication/create-update.ts | 7 +- src/lib/validations/mental-health.ts | 6 +- src/lib/validations/mood.ts | 73 +++--- src/lib/validations/workout.ts | 4 + 26 files changed, 1232 insertions(+), 64 deletions(-) create mode 100644 src/__tests__/external-id-stability-guard.test.ts create mode 100644 src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts create mode 100644 src/lib/validations/__tests__/external-id.test.ts create mode 100644 src/lib/validations/external-id.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 25b8a043c..e0fe37c3e 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -12091,6 +12091,10 @@ components: {00,10,20,30,40,50}, trailing `Z` (e.g. `2026-06-21T14:10:00.000Z`). A re-post OVERWRITES the row and returns `updated`, so iOS uploads ~144 rows/day instead of one per raw HR sample. A row that targets this prefix with a malformed bucket-start suffix is `skipped` with reason `malformed_hr_bucket_id`. + + The id MUST be stable across app restarts. An object description or memory address (``, + `0x12ab34cd`) is a new value on every launch, so it can never match its own earlier row; such an entry is + `skipped` with reason `unstable_external_id` while the rest of the batch still lands. externalSourceVersion: type: string minLength: 1 @@ -12294,7 +12298,10 @@ components: type: string const: APPLE_HEALTH externalId: - description: The HealthKit dose-event UUID. Drives the idempotent re-sync dedup (first-write-wins per Apple dose). + description: "The HealthKit dose-event UUID. Drives the idempotent re-sync dedup (first-write-wins per Apple dose). Must + be stable across app restarts — an object description or memory address (``, `0x12ab34cd`) is a + new value on every launch; such an entry is `skipped` with reason `unstable_external_id` while the rest of + the batch still lands." type: string minLength: 1 maxLength: 128 @@ -12411,7 +12418,9 @@ components: externalId: description: "Stable external identifier of the mirrored medication (the HealthKit `medicationConceptIdentifier`, max 128 chars). Must be supplied together with `externalSource`. Idempotent: re-posting the same pair returns - the existing medication with status 200 instead of minting a duplicate." + the existing medication with status 200 instead of minting a duplicate. The value MUST be identical across + app restarts: an object description or memory address (``, `0x12ab34cd`) is a new value on every + launch, never matches its own earlier row, and is refused with 422 rather than minting a duplicate mirror." type: string minLength: 1 maxLength: 128 @@ -13667,7 +13676,10 @@ components: - TELEGRAM - DAYLIO externalId: - description: iOS-side source-stable id for idempotent dedup on the `(userId, source, externalId)` key. + description: "iOS-side source-stable id for idempotent dedup on the `(userId, source, externalId)` key. Must be stable + across app restarts — an object description or memory address (``, `0x12ab34cd`) is a new value + on every launch; such an entry is `skipped` with reason `unstable_external_id` while the rest of the batch + still lands." type: string minLength: 1 maxLength: 120 diff --git a/src/__tests__/external-id-stability-guard.test.ts b/src/__tests__/external-id-stability-guard.test.ts new file mode 100644 index 000000000..4e59506c5 --- /dev/null +++ b/src/__tests__/external-id-stability-guard.test.ts @@ -0,0 +1,174 @@ +/** + * Structural guard on the external-id stability floor. + * + * A client-supplied `externalId` is half of every ingest dedup key. An id + * that rotates between client launches never matches its own earlier row, + * so each sync sweep mints a fresh record — the failure mode that filled a + * live instance with phantom medications, one per sweep, none of which + * could ever collect a dose. + * + * The fix is only durable if it cannot be bypassed by the NEXT route. + * These guards freeze the two legal ways to accept the field: + * + * 1. the request schema routes it through `assertStableExternalId`, so + * the refusal rides the standard 422 multi-issue envelope; or + * 2. the field is deliberately permissive because a schema-level refusal + * would fail a whole batch on one bad row — in which case the + * declaration carries an `@external-id-checked-per-entry: ` + * marker naming the file that runs `classifyExternalId` per entry, + * and this guard verifies that file really does. + * + * They are tripwires, not proofs. A reviewer who waves through a bad + * addition defeats them, and no test substitutes for that review. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join, sep } from "node:path"; +import { globSync } from "node:fs"; + +const SRC = join(process.cwd(), "src"); +const VALIDATOR_MODULE = "@/lib/validations/external-id"; + +function sourceFiles(): string[] { + return globSync("**/*.{ts,tsx}", { cwd: SRC }) + .filter( + (p) => !p.startsWith(`generated${sep}`) && !p.startsWith("generated/"), + ) + .filter((p) => !p.includes("__tests__")) + .filter((p) => !p.endsWith(".test.ts") && !p.endsWith(".test.tsx")) + .map((p) => p.split(sep).join("/")) + .sort(); +} + +function read(rel: string): string { + return readFileSync(join(SRC, rel), "utf8"); +} + +/** + * A REQUEST-side declaration of a client-supplied external identifier: + * `externalId: z.…` / `externalSourceId: z.…` in a Zod object shape. + * + * Response DTOs under `lib/openapi/` are excluded below — those echo an + * id the server already stored and validating them would be validating + * our own output. + */ +const DECLARATION = /^\s*(externalId|externalSourceId)\s*:\s*z\b/gm; + +/** `// @external-id-checked-per-entry: ` */ +const MARKER = /@external-id-checked-per-entry:\s*(\S+)/; + +/** + * Files that declare the field but are deliberately outside the floor. + * Each entry states WHY; an addition here is a decision, not a shortcut. + */ +const EXEMPT: Record = { + // Backup restore reads a payload the SERVER itself wrote and encrypted. + // It has to reproduce the stored rows byte-for-byte — including ids that + // predate this floor — or a self-hoster with an already-poisoned export + // could never restore it. Nothing client-authored reaches this schema. + "lib/validations/backup.ts": + "restore payload is a server-written backup; refusing a stored id would block a restore", +}; + +/** Lines of `rel` (1-based) on which a request-side declaration appears. */ +function declarationLines(rel: string): number[] { + const source = read(rel); + const lines: number[] = []; + DECLARATION.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = DECLARATION.exec(source)) !== null) { + lines.push(source.slice(0, m.index).split("\n").length); + } + return lines; +} + +function declaringFiles(): string[] { + return sourceFiles() + .filter((rel) => !rel.startsWith("lib/openapi/")) + .filter((rel) => declarationLines(rel).length > 0); +} + +describe("every client-supplied external id routes through the stability floor", () => { + it("finds the declaring files at all (the matcher still matches)", () => { + // A guard whose matcher silently stops matching passes forever. Pin the + // known set so a rename of the field or of the Zod idiom trips here. + const files = declaringFiles(); + expect(files.length).toBeGreaterThanOrEqual(8); + expect(files).toContain("app/api/measurements/batch/route.ts"); + expect(files).toContain("lib/validations/medication/create-update.ts"); + }); + + it.each(declaringFiles())( + "%s either applies the field check or names the per-entry checker", + (rel) => { + if (rel in EXEMPT) { + expect(EXEMPT[rel].length).toBeGreaterThan(20); + return; + } + + const source = read(rel); + const usesFieldCheck = + source.includes(VALIDATOR_MODULE) && + source.includes("assertStableExternalId"); + const marksPerEntry = MARKER.test(source); + expect( + usesFieldCheck || marksPerEntry, + `${rel} declares a client-supplied external id but neither applies ` + + "`assertStableExternalId` to its request schema nor marks the declaration " + + "`@external-id-checked-per-entry: ` naming the route that checks it per entry.", + ).toBe(true); + }, + ); + + it.each(declaringFiles())( + "%s — every per-entry marker points at a file that really checks per entry", + (rel) => { + if (rel in EXEMPT) return; + const source = read(rel); + MARKER.lastIndex = 0; + const targets = [...source.matchAll(new RegExp(MARKER, "g"))].map( + (m) => m[1], + ); + for (const target of targets) { + expect( + target.startsWith("src/"), + `${rel}: marker target "${target}" must be a repo-root-relative src/ path`, + ).toBe(true); + const checker = read(target.replace(/^src\//, "")); + expect( + checker.includes("classifyExternalId"), + `${rel} points its per-entry marker at ${target}, but that file never calls classifyExternalId.`, + ).toBe(true); + } + }, + ); +}); + +describe("the batch surfaces refuse per entry, never whole-batch", () => { + // Each of these drains a client sync queue. A schema-level refusal here + // would stop a whole batch of good rows because of one bad one, which is + // worse than the duplicate it prevents. + const BATCH_ROUTES = [ + "app/api/measurements/batch/route.ts", + "app/api/mood-entries/bulk/route.ts", + "app/api/medications/intake/bulk/route.ts", + "app/api/workouts/batch/route.ts", + "app/api/cycle/day-logs/bulk/route.ts", + "app/api/import/route.ts", + ]; + + it.each(BATCH_ROUTES)("%s classifies each entry itself", (rel) => { + const source = read(rel); + expect(source).toContain("classifyExternalId"); + expect(source).toContain("unstableExternalIdMeta"); + }); + + it.each(BATCH_ROUTES.filter((r) => r !== "app/api/import/route.ts"))( + "%s reports the shared per-entry reason code", + (rel) => { + // The JSON importer has no per-entry status envelope — it reports a + // skipped COUNT — so it is the one batch surface without the code. + expect(read(rel)).toContain("UNSTABLE_EXTERNAL_ID_REASON"); + }, + ); +}); diff --git a/src/app/api/cycle/day-logs/bulk/route.ts b/src/app/api/cycle/day-logs/bulk/route.ts index 5c1b7f225..7a83d30a8 100644 --- a/src/app/api/cycle/day-logs/bulk/route.ts +++ b/src/app/api/cycle/day-logs/bulk/route.ts @@ -32,6 +32,12 @@ import { cycleBulkSchema, MAX_CYCLE_BULK_ENTRIES, } from "@/lib/validations/cycle"; +import { + classifyExternalId, + unstableExternalIdMeta, + UNSTABLE_EXTERNAL_ID_REASON, + type UnstableExternalIdShape, +} from "@/lib/validations/external-id"; import { upsertCycleDayLog } from "@/lib/cycle/day-log-write"; import { DEFAULT_TIMEZONE } from "@/lib/mood/date-key"; @@ -121,9 +127,33 @@ async function postBulk(request: NextRequest): Promise { let updated = 0; let duplicates = 0; let skipped = 0; + const unstableShapes: UnstableExternalIdShape[] = []; for (let i = 0; i < entries.length; i++) { const entry = entries[i]; + + // The `(userId, source, externalId)` upsert key is only idempotent + // while the id is STABLE across client launches. An id that rotates + // per process (an object description carrying a memory address) + // never matches its own earlier row, so every re-post mints another + // day log. Refused per entry — one bad row must not stop the rest of + // the batch landing. + const unstable = + entry.externalId === undefined + ? null + : classifyExternalId(entry.externalId); + if (unstable) { + unstableShapes.push(unstable); + skipped += 1; + results.push({ + index: i, + status: "skipped", + reason: UNSTABLE_EXTERNAL_ID_REASON, + externalId: entry.externalId, + }); + continue; + } + try { const cycleId = owningCycleId(entry.date); const r = await upsertCycleDayLog( @@ -206,6 +236,9 @@ async function postBulk(request: NextRequest): Promise { updated, duplicates, skipped, + ...(unstableShapes.length > 0 + ? unstableExternalIdMeta("cycle.day_log.bulk", unstableShapes) + : {}), }, }); diff --git a/src/app/api/cycle/day-logs/route.ts b/src/app/api/cycle/day-logs/route.ts index 05f48effc..e8ad461da 100644 --- a/src/app/api/cycle/day-logs/route.ts +++ b/src/app/api/cycle/day-logs/route.ts @@ -27,9 +27,13 @@ import { import { withIdempotency } from "@/lib/idempotency"; import { requireCycleEnabled } from "@/lib/cycle/gate"; import { - cycleDayLogInputSchema, + cycleDayLogCreateSchema, cycleDayLogQuerySchema, } from "@/lib/validations/cycle"; +import { + unstableExternalIdMeta, + unstableExternalIdShape, +} from "@/lib/validations/external-id"; import { upsertCycleDayLog } from "@/lib/cycle/day-log-write"; import { findOwningCycleId } from "@/lib/cycle/cycle-attribution"; import { toCycleDayLogDTO, dayLogSymptomInclude } from "@/lib/cycle/dto"; @@ -82,11 +86,17 @@ async function postDayLog(request: NextRequest): Promise { }); if (jsonError) return jsonError; - const parsed = cycleDayLogInputSchema.safeParse(rawBody); + const parsed = cycleDayLogCreateSchema.safeParse(rawBody); if (!parsed.success) { + const shape = unstableExternalIdShape(rawBody); annotate({ action: { name: "cycle.day-log.validation-failed" }, - meta: { issue_count: parsed.error.issues.length }, + meta: { + issue_count: parsed.error.issues.length, + ...(shape + ? unstableExternalIdMeta("cycle.day_log.create", [shape]) + : {}), + }, }); return returnAllZodIssues(parsed.error, 422, { errorCode: "cycle.day-log.invalid", diff --git a/src/app/api/cycle/period/route.ts b/src/app/api/cycle/period/route.ts index 08b176672..fedfa6f61 100644 --- a/src/app/api/cycle/period/route.ts +++ b/src/app/api/cycle/period/route.ts @@ -24,6 +24,10 @@ import { import { requireCycleEnabled } from "@/lib/cycle/gate"; import { withIdempotency } from "@/lib/idempotency"; import { cyclePeriodSchema } from "@/lib/validations/cycle"; +import { + unstableExternalIdMeta, + unstableExternalIdShape, +} from "@/lib/validations/external-id"; import { upsertCycleDayLog } from "@/lib/cycle/day-log-write"; import type { FlowLevel } from "@/lib/cycle/types"; import { @@ -49,9 +53,13 @@ async function postPeriod(request: NextRequest): Promise { const parsed = cyclePeriodSchema.safeParse(rawBody); if (!parsed.success) { + const shape = unstableExternalIdShape(rawBody); annotate({ action: { name: "cycle.period.validation-failed" }, - meta: { issue_count: parsed.error.issues.length }, + meta: { + issue_count: parsed.error.issues.length, + ...(shape ? unstableExternalIdMeta("cycle.period", [shape]) : {}), + }, }); return returnAllZodIssues(parsed.error, 422, { errorCode: "cycle.period.invalid", diff --git a/src/app/api/import/route.ts b/src/app/api/import/route.ts index fba6e2272..f1c32149f 100644 --- a/src/app/api/import/route.ts +++ b/src/app/api/import/route.ts @@ -23,6 +23,11 @@ import { isPlausibleEntryInstant, validateEntryInstant, } from "@/lib/validations/entry-instant"; +import { + classifyExternalId, + unstableExternalIdMeta, + type UnstableExternalIdShape, +} from "@/lib/validations/external-id"; import { encryptNote } from "@/lib/crypto/note-cipher"; import { recomputeUserMoodRollups } from "@/lib/rollups/mood-rollups"; import { @@ -60,6 +65,10 @@ const measurementSchema = z // export is idempotent rather than minting a duplicate set — mirrors the // mood path. Absent → first-write-wins create with a NULL externalId // (distinct in the unique key), which DOES duplicate on re-import. + // Stability is enforced per entry in the import loop, not at the + // field: a schema-level refusal would fail the whole file on one bad + // row. + // @external-id-checked-per-entry: src/app/api/import/route.ts externalId: z.string().min(1).max(120).optional(), }) .refine( @@ -92,6 +101,10 @@ const moodEntrySchema = z // present, the import upserts on `(userId, source, externalId)` so a // re-import of the same export is idempotent rather than minting a // duplicate; absent → the legacy first-write-wins create path. + // Stability is enforced per entry in the import loop, not at the + // field: a schema-level refusal would fail the whole file on one bad + // row. + // @external-id-checked-per-entry: src/app/api/import/route.ts externalId: z.string().min(1).max(120).optional(), }) .superRefine((entry, ctx) => { @@ -170,6 +183,7 @@ export const POST = apiHandler(async (request: NextRequest) => { const userId = user.id; const data = parsed.data; const stats = { measurements: 0, moodEntries: 0, skipped: 0 }; + const unstableShapes: UnstableExternalIdShape[] = []; let writeFailed = false; // Import measurements @@ -189,6 +203,17 @@ export const POST = apiHandler(async (request: NextRequest) => { }> = []; if (data.measurements?.length) { for (const m of data.measurements) { + // The `(userId, type, source, externalId)` upsert key is only + // idempotent while the id is STABLE; an id that rotates between + // exports re-imports as a fresh row every time. Skipped per entry, + // like every other rejected row in this importer. + const unstable = + m.externalId === undefined ? null : classifyExternalId(m.externalId); + if (unstable) { + unstableShapes.push(unstable); + stats.skipped++; + continue; + } try { // `measuredAt` is already a `Date` (validateEntryInstant transform). const measuredAt = m.measuredAt; @@ -299,6 +324,15 @@ export const POST = apiHandler(async (request: NextRequest) => { const fallbackOccurrences = new Map(); const reservedFallbackInstants = new Set(); for (const e of data.moodEntries) { + // Same stability floor as the measurement loop above — the mood + // upsert keys on `(userId, source, externalId)`. + const unstableMood = + e.externalId === undefined ? null : classifyExternalId(e.externalId); + if (unstableMood) { + unstableShapes.push(unstableMood); + stats.skipped++; + continue; + } const identity = e.loggedAt ? null : fallbackMoodIdentity(e); const occurrence = identity === null ? 0 : (fallbackOccurrences.get(identity) ?? 0); @@ -455,7 +489,14 @@ export const POST = apiHandler(async (request: NextRequest) => { } } - annotate({ meta: { import_stats: stats } }); + annotate({ + meta: { + import_stats: stats, + ...(unstableShapes.length > 0 + ? unstableExternalIdMeta("import.json", unstableShapes) + : {}), + }, + }); try { await auditLog("import.upload", { 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 new file mode 100644 index 000000000..cc209dea2 --- /dev/null +++ b/src/app/api/measurements/batch/__tests__/unstable-external-id.test.ts @@ -0,0 +1,239 @@ +/** + * `POST /api/measurements/batch` — external-id stability floor. + * + * `(user_id, type, source, external_id)` is idempotent only while the id + * is stable across client launches. An id that rotates per process — an + * object description carrying a memory address — never matches its own + * earlier row, so every sync sweep mints a fresh measurement. + * + * The refusal is PER ENTRY, matching the range / timestamp guards on this + * route: a client draining a HealthKit queue must not lose 499 good rows + * because one carried a bad id. + */ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { NextRequest } from "next/server"; + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { update: vi.fn() }, + measurement: { + findMany: vi.fn(), + createManyAndReturn: vi.fn(), + updateMany: vi.fn(), + }, + $transaction: vi.fn(async (fn: unknown) => { + if (typeof fn === "function") { + return (fn as (tx: unknown) => unknown)(prisma); + } + }), + }, +})); + +vi.mock("@/lib/measurements/reconcile-external-measurement", () => ({ + reconcileExternalMeasurement: vi.fn( + async (_tx: unknown, input: Record) => ({ + status: "inserted", + row: { id: "row-1", ...input }, + }), + ), +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); + +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn(), +})); + +vi.mock("@/lib/jobs/pr-detection", () => ({ + enqueuePrDetection: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/jobs/reminder-satisfy", () => ({ + enqueueReminderSatisfy: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/cache/invalidate", () => ({ + invalidateUserMeasurements: vi.fn(), +})); + +vi.mock("@/lib/insights/comprehensive-generate", () => ({ + invalidateStatusInsightsForTypes: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/rollups/measurement-rollups", () => ({ + recomputeBucketsForMeasurement: vi.fn().mockResolvedValue(undefined), + collapseToTypeDayKeys: vi.fn(() => []), +})); + +vi.mock("@/lib/daily/morning-refresh-trigger", () => ({ + maybeEnqueueMorningRefresh: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/arrivals/emit-shared", () => ({ + emitDataArrival: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@/lib/logging/context", async (importOriginal) => { + const actual = await importOriginal>(); + return { ...actual, annotate: vi.fn() }; +}); + +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { POST } from "../route"; +import { prisma } from "@/lib/db"; +import { getSession } from "@/lib/auth/session"; +import { checkRateLimit } from "@/lib/rate-limit"; +import { annotate } from "@/lib/logging/context"; +import { reconcileExternalMeasurement } from "@/lib/measurements/reconcile-external-measurement"; + +const SESSION_OK = { + session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: { id: "user-1", username: "testuser", role: "USER" as const }, +}; + +const DAY_MS = 24 * 60 * 60 * 1000; + +function makeRequest(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/measurements/batch", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +/** A plain step entry anchored a day back so it never date-bombs. */ +function stepEntry(externalId: string) { + const at = new Date(Date.now() - DAY_MS).toISOString(); + return { + hkIdentifier: "HKQuantityTypeIdentifierStepCount", + value: 1200, + unit: "count", + startDate: at, + endDate: at, + externalId, + }; +} + +async function statusesFor(entries: unknown[]) { + const res = await POST(makeRequest({ entries })); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { + entries: Array<{ index: number; status: string; reason?: string }>; + }; + }; + return body.data.entries; +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: true, + remaining: 60, + resetAt: Date.now() + 60_000, + }); + vi.mocked(prisma.measurement.findMany).mockResolvedValue([]); + vi.mocked(prisma.measurement.createManyAndReturn).mockResolvedValue([ + { id: "row-1" }, + ] as never); + vi.mocked(prisma.measurement.updateMany).mockResolvedValue({ count: 0 }); +}); + +describe("POST /api/measurements/batch — unstable external ids", () => { + it("skips an object-description id instead of minting a row", async () => { + const results = await statusesFor([ + stepEntry(""), + ]); + expect(results[0]).toMatchObject({ + index: 0, + status: "skipped", + reason: "unstable_external_id", + }); + expect(reconcileExternalMeasurement).not.toHaveBeenCalled(); + }); + + it("skips a bare memory-address id", async () => { + const results = await statusesFor([stepEntry("0x126b25160")]); + expect(results[0]).toMatchObject({ + status: "skipped", + reason: "unstable_external_id", + }); + }); + + it("skips ONLY the offending entries — the rest of the batch still lands", async () => { + const results = await statusesFor([ + stepEntry("hk-uuid-1"), + stepEntry(""), + stepEntry("stats:HKQuantityTypeIdentifierStepCount:2026-07-25"), + stepEntry("0xdeadbeef"), + ]); + expect(results[0]!.status).not.toBe("skipped"); + expect(results[1]).toMatchObject({ reason: "unstable_external_id" }); + expect(results[2]!.status).not.toBe("skipped"); + expect(results[3]).toMatchObject({ reason: "unstable_external_id" }); + // Two good rows still reached the writer. + expect(reconcileExternalMeasurement).toHaveBeenCalledTimes(2); + }); + + it("never turns one bad row into a whole-batch 422", async () => { + const res = await POST( + makeRequest({ + entries: [stepEntry("hk-uuid-1"), stepEntry("0xdeadbeef")], + }), + ); + expect(res.status).toBe(200); + }); + + it("accepts the real id shapes this route receives", async () => { + const results = await statusesFor([ + stepEntry("8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30"), + stepEntry("stats:HKQuantityTypeIdentifierStepCount:2026-07-25"), + stepEntry("hk-uuid-41"), + ]); + for (const r of results) expect(r.status).not.toBe("skipped"); + }); + + it("annotates the refusal with the shape, never the id", async () => { + await statusesFor([ + stepEntry(""), + stepEntry("0xdeadbeef"), + ]); + const calls = vi.mocked(annotate).mock.calls.map(([c]) => c); + const rejection = calls.find( + (c) => c?.meta && "external_id_rejected" in c.meta, + ); + expect(rejection?.meta).toMatchObject({ + external_id_rejected: 2, + external_id_shapes: "object_description,pointer_address", + external_id_surface: "measurement.batch", + }); + expect(JSON.stringify(rejection)).not.toContain("0x12568db80"); + }); + + it("emits no rejection annotation on a clean batch", async () => { + await statusesFor([stepEntry("hk-uuid-1")]); + const calls = vi.mocked(annotate).mock.calls.map(([c]) => c); + expect(calls.some((c) => c?.meta && "external_id_rejected" in c.meta)).toBe( + false, + ); + }); +}); diff --git a/src/app/api/measurements/batch/route.ts b/src/app/api/measurements/batch/route.ts index 8aa5ec5db..77bd7252f 100644 --- a/src/app/api/measurements/batch/route.ts +++ b/src/app/api/measurements/batch/route.ts @@ -62,6 +62,12 @@ import { } from "@/lib/measurements/cross-source-merge"; import { reconcileExternalMeasurement } from "@/lib/measurements/reconcile-external-measurement"; import { isPlausibleEntryInstant } from "@/lib/validations/entry-instant"; +import { + classifyExternalId, + unstableExternalIdMeta, + UNSTABLE_EXTERNAL_ID_REASON, + type UnstableExternalIdShape, +} from "@/lib/validations/external-id"; import { validateMeasurementRange } from "@/lib/validations/measurement"; import { deviceTypeEnum } from "@/lib/validations/source-priority"; import { checkRateLimit } from "@/lib/rate-limit"; @@ -139,6 +145,9 @@ const batchEntrySchema = z.object({ // iOS build omits them and the bucket keeps the avg-only contract. valueMin: z.number().finite().optional(), valueMax: z.number().finite().optional(), + // Stability is enforced per entry in the loop below, not at the field: + // a schema-level refusal would fail the whole batch on one bad row. + // @external-id-checked-per-entry: src/app/api/measurements/batch/route.ts externalId: z.string().min(1).max(120), externalSourceVersion: z.string().min(1).max(120).optional(), // v1.8.6 W6 — optional per-entry source tag. Defaults to @@ -268,9 +277,29 @@ async function postBatch(request: NextRequest): Promise { }; const prepared: Prepared[] = []; const results: EntryResult[] = new Array(entries.length); + const unstableShapes: UnstableExternalIdShape[] = []; for (let index = 0; index < entries.length; index++) { const entry = entries[index]; + + // The `(user_id, type, source, external_id)` dedup key above is only + // idempotent while the id is STABLE across client launches. An id + // that rotates per process (an object description carrying a memory + // address) never matches its own earlier row, so every sync sweep + // mints a fresh measurement. Refused per entry, like the range and + // timestamp guards below — one malformed row must not stop the other + // 499 from landing. + const unstable = classifyExternalId(entry.externalId); + if (unstable) { + unstableShapes.push(unstable); + results[index] = { + index, + status: "skipped", + reason: UNSTABLE_EXTERNAL_ID_REASON, + }; + continue; + } + const mapped = mapAppleHealthEntry({ hkIdentifier: entry.hkIdentifier, value: entry.value, @@ -396,6 +425,12 @@ async function postBatch(request: NextRequest): Promise { }); } + if (unstableShapes.length > 0) { + annotate({ + meta: unstableExternalIdMeta("measurement.batch", unstableShapes), + }); + } + let insertedCount = 0; let updatedCount = 0; let duplicateCount = 0; diff --git a/src/app/api/medications/__tests__/route.test.ts b/src/app/api/medications/__tests__/route.test.ts index c61b568dd..167477f2a 100644 --- a/src/app/api/medications/__tests__/route.test.ts +++ b/src/app/api/medications/__tests__/route.test.ts @@ -622,6 +622,65 @@ describe("POST /api/medications — Apple Health mirror (v1.28)", () => { expect(prisma.medication.create).not.toHaveBeenCalled(); }); + /** + * The mirror's idempotency is only as good as the stability of the + * concept id. A client that serialised an opaque platform object with + * its default description sent `` — a + * memory address that changes on every launch — and a live instance + * grew 23 phantom medications in a day, one per sync sweep, none of + * which could ever collect a dose. The create refuses that shape now. + */ + it("422s on an object-description externalId instead of minting a phantom mirror", async () => { + vi.mocked(prisma.medication.findFirst).mockResolvedValue(null as never); + const res = await POST( + postReq({ + ...MIRROR_BODY, + externalId: "", + }), + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + }; + const issue = body.details.issues.find((i) => i.path === "externalId"); + expect(issue).toBeDefined(); + // The client developer has to be able to act on this without asking. + expect(issue!.message).toContain("stable across app restarts"); + expect(issue!.message).toContain("object description"); + expect(prisma.medication.create).not.toHaveBeenCalled(); + }); + + it("422s on a bare memory-address externalId", async () => { + vi.mocked(prisma.medication.findFirst).mockResolvedValue(null as never); + const res = await POST( + postReq({ ...MIRROR_BODY, externalId: "0xdeadbeef" }), + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + }; + expect( + body.details.issues.some( + (i) => i.path === "externalId" && i.message.includes("memory address"), + ), + ).toBe(true); + expect(prisma.medication.create).not.toHaveBeenCalled(); + }); + + it("still accepts the stable concept-id shapes the mirror really sends", async () => { + for (const externalId of [ + "hk-concept-1", + "8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + "a3f5c9e1b7d2486fa0c3e5b7d9f1a2c4e6b8d0f2a4c6e8b0d2f4a6c8e0b2d4f6", + ]) { + vi.mocked(prisma.medication.create).mockClear(); + vi.mocked(prisma.medication.findFirst).mockResolvedValue(null as never); + const res = await POST(postReq({ ...MIRROR_BODY, externalId })); + expect(res.status).toBe(201); + expect(lastCreateData().externalId).toBe(externalId); + } + }); + it("a plain create keeps today's behavior — no external columns forwarded", async () => { const res = await POST( postReq({ diff --git a/src/app/api/medications/intake/bulk/route.ts b/src/app/api/medications/intake/bulk/route.ts index ac85c7f82..4dcacf96c 100644 --- a/src/app/api/medications/intake/bulk/route.ts +++ b/src/app/api/medications/intake/bulk/route.ts @@ -75,6 +75,12 @@ import { injectionSiteEnum, type InjectionSiteValue, } from "@/lib/validations/medication"; +import { + classifyExternalId, + unstableExternalIdMeta, + UNSTABLE_EXTERNAL_ID_REASON, + type UnstableExternalIdShape, +} from "@/lib/validations/external-id"; import type { InjectionSiteKey } from "@/lib/medications/injection-sites"; import { queueMedicationIntakeSync } from "@/lib/notifications/medication-intake-sync"; import { dispatchMedicationIntakeWebClearBulk } from "@/lib/notifications/web-push-clear"; @@ -128,6 +134,10 @@ const bulkEntrySchema = z // v1.28 — the HealthKit `HKMedicationDoseEvent` UUID. Drives the // idempotent re-sync: an entry whose `(userId, externalId)` already // exists live reports `duplicate` (first-write-wins per Apple dose). + // Stability is enforced per entry in the loop below, not at the + // field: a schema-level refusal would fail the whole batch on one + // bad row. + // @external-id-checked-per-entry: src/app/api/medications/intake/bulk/route.ts externalId: z.string().trim().min(1).max(128).optional(), }) .refine( @@ -318,6 +328,7 @@ async function postBulk(request: NextRequest): Promise { let updated = 0; let duplicates = 0; const skipped: Array<{ index: number; reason: string }> = []; + const unstableShapes: UnstableExternalIdShape[] = []; // v1.4.39 W-MED — collect distinct `(medicationId, dayKey)` pairs // touched by the batch so one rollup recompute fires per pair after // all inserts complete. Per-row recompute would balloon a 500-entry @@ -345,6 +356,27 @@ async function postBulk(request: NextRequest): Promise { continue; } + // The `(userId, externalId)` dose-event dedup below is only + // idempotent while the id is STABLE across client launches. An id + // that rotates per process (an object description carrying a memory + // address) never matches its own earlier row, so every re-sync mints + // another intake event. Refused per entry — one bad row must not + // stop the rest of the batch landing. + const unstable = + entry.externalId === undefined + ? null + : classifyExternalId(entry.externalId); + if (unstable) { + unstableShapes.push(unstable); + skipped.push({ index: i, reason: UNSTABLE_EXTERNAL_ID_REASON }); + results.push({ + index: i, + status: "skipped", + reason: UNSTABLE_EXTERNAL_ID_REASON, + }); + continue; + } + // v1.28 — Apple dose-event replay: the batch pre-check found a live // row already carrying this external UUID (or an earlier entry in // THIS batch inserted it). First-write-wins per Apple dose — report @@ -803,6 +835,9 @@ async function postBulk(request: NextRequest): Promise { updated, duplicates, skipped: skipped.length, + ...(unstableShapes.length > 0 + ? unstableExternalIdMeta("medication.intake.bulk", unstableShapes) + : {}), }, }); diff --git a/src/app/api/medications/route.ts b/src/app/api/medications/route.ts index cb81bd279..b7c7c321e 100644 --- a/src/app/api/medications/route.ts +++ b/src/app/api/medications/route.ts @@ -10,6 +10,10 @@ import { safeJson, } from "@/lib/api-response"; import { createMedicationSchema } from "@/lib/validations/medication"; +import { + unstableExternalIdMeta, + unstableExternalIdShape, +} from "@/lib/validations/external-id"; import { getMedicationCategories, setMedicationCategory, @@ -98,6 +102,16 @@ export const POST = apiHandler(async (request: NextRequest) => { if (jsonError) return jsonError; const parsed = createMedicationSchema.safeParse(body); if (!parsed.success) { + // Surface an unstable concept id as its own wide event: the refusal + // is the visible half of the phantom-mirror class, and a silent 422 + // in the generic bucket is how it stayed invisible the first time. + const shape = unstableExternalIdShape(body); + if (shape) { + annotate({ + action: { name: "medication.external_id.rejected" }, + meta: unstableExternalIdMeta("medication.create", [shape]), + }); + } // v1.4.43 W6 — multi-issue 422. return returnAllZodIssues(parsed.error, 422); } diff --git a/src/app/api/mental-health/assessments/route.ts b/src/app/api/mental-health/assessments/route.ts index 939802cdb..66bf93f20 100644 --- a/src/app/api/mental-health/assessments/route.ts +++ b/src/app/api/mental-health/assessments/route.ts @@ -36,6 +36,10 @@ import { createAssessmentSchema, listAssessmentsSchema, } from "@/lib/validations/mental-health"; +import { + unstableExternalIdMeta, + unstableExternalIdShape, +} from "@/lib/validations/external-id"; import { INSTRUMENTS, INSTRUMENT_MEASUREMENT_TYPE, @@ -183,9 +187,15 @@ async function postAssessment(request: NextRequest): Promise { const parsed = createAssessmentSchema.safeParse(body); if (!parsed.success) { + const shape = unstableExternalIdShape(body); annotate({ action: { name: "mental-health.create.validation-failed" }, - meta: { issue_count: sanitiseZodIssues(parsed.error.issues).length }, + meta: { + issue_count: sanitiseZodIssues(parsed.error.issues).length, + ...(shape + ? unstableExternalIdMeta("mental_health.assessment.create", [shape]) + : {}), + }, }); return returnAllZodIssues(parsed.error, 422); } diff --git a/src/app/api/mood-entries/bulk/route.ts b/src/app/api/mood-entries/bulk/route.ts index b1b1d5940..b9ee78003 100644 --- a/src/app/api/mood-entries/bulk/route.ts +++ b/src/app/api/mood-entries/bulk/route.ts @@ -43,6 +43,12 @@ import { moodLevelEnum, moodSourceEnum, } from "@/lib/validations/mood"; +import { + classifyExternalId, + unstableExternalIdMeta, + UNSTABLE_EXTERNAL_ID_REASON, + type UnstableExternalIdShape, +} from "@/lib/validations/external-id"; import { moodDateKey, DEFAULT_TIMEZONE } from "@/lib/mood/date-key"; import { encryptNote } from "@/lib/crypto/note-cipher"; import { invalidateUserMood } from "@/lib/cache/invalidate"; @@ -95,6 +101,10 @@ const bulkEntrySchema = z.object({ * the measurements batch endpoint. NULL = no dedup hint; the * existing `(userId, date, moodLoggedAt)` unique index still * protects against straight-up duplicates. + * + * Stability is enforced per entry in the loop below, not at the field: + * a schema-level refusal would fail the whole batch on one bad row. + * @external-id-checked-per-entry: src/app/api/mood-entries/bulk/route.ts */ externalId: z.string().min(1).max(120).optional(), }); @@ -187,6 +197,7 @@ async function postBulk(request: NextRequest): Promise { let inserted = 0; let duplicates = 0; const skipped: Array<{ index: number; reason: string }> = []; + const unstableShapes: UnstableExternalIdShape[] = []; // Perf — a single indexed existence read replaces the per-entry // `findUnique` probe (up to 500 sequential round trips pre-fix). Every @@ -241,6 +252,27 @@ async function postBulk(request: NextRequest): Promise { for (let i = 0; i < entries.length; i++) { const entry = entries[i]; + + // The `(userId, source, externalId)` dedup key only holds while the + // id is STABLE across client launches; a per-process value (an object + // description carrying a memory address) mints a fresh entry on every + // re-post. Per-entry refusal, never a whole-batch 422. + const unstable = + entry.externalId === undefined + ? null + : classifyExternalId(entry.externalId); + if (unstable) { + unstableShapes.push(unstable); + skipped.push({ index: i, reason: UNSTABLE_EXTERNAL_ID_REASON }); + results.push({ + index: i, + status: "skipped", + reason: UNSTABLE_EXTERNAL_ID_REASON, + externalId: entry.externalId, + }); + continue; + } + const date = moodDateKey(entry.moodLoggedAt, tz); const score = getScoreForMood(entry.mood); @@ -415,6 +447,9 @@ async function postBulk(request: NextRequest): Promise { inserted, duplicates, skipped: skipped.length, + ...(unstableShapes.length > 0 + ? unstableExternalIdMeta("mood.bulk", unstableShapes) + : {}), }, }); diff --git a/src/app/api/mood-entries/route.ts b/src/app/api/mood-entries/route.ts index 8af6bbb0b..2a98b2ba6 100644 --- a/src/app/api/mood-entries/route.ts +++ b/src/app/api/mood-entries/route.ts @@ -13,6 +13,10 @@ import { listMoodEntriesSchema, getScoreForMood, } from "@/lib/validations/mood"; +import { + unstableExternalIdMeta, + unstableExternalIdShape, +} from "@/lib/validations/external-id"; import { NextRequest } from "next/server"; import { Prisma } from "@/generated/prisma/client"; import { apiHandler, requireAuth } from "@/lib/api-handler"; @@ -160,9 +164,13 @@ async function postMoodEntry(request: NextRequest) { // v1.4.43 W6 — iOS mood log hot path; multi-issue 422 + audit // breadcrumb keyed `mood-entries.create.validation-failed`. const issues = sanitiseZodIssues(parsed.error.issues); + const shape = unstableExternalIdShape(body); annotate({ action: { name: "mood-entries.create.validation-failed" }, - meta: { issue_count: issues.length }, + meta: { + issue_count: issues.length, + ...(shape ? unstableExternalIdMeta("mood.create", [shape]) : {}), + }, }); // v1.4.49 — strip `message` from the audit-ledger row; the // mood-create schema's free-text `note` + `tags` could land in a diff --git a/src/app/api/workouts/batch/route.ts b/src/app/api/workouts/batch/route.ts index 1bf67b8d9..ac9f1c65d 100644 --- a/src/app/api/workouts/batch/route.ts +++ b/src/app/api/workouts/batch/route.ts @@ -70,6 +70,12 @@ import { createBatchWorkoutSchema, MAX_WORKOUTS_PER_BATCH, } from "@/lib/validations/workout"; +import { + classifyExternalId, + unstableExternalIdMeta, + UNSTABLE_EXTERNAL_ID_REASON, + type UnstableExternalIdShape, +} from "@/lib/validations/external-id"; import { dedupeWorkoutBatch } from "@/lib/workouts/canonical-rows"; import { Prisma, type MeasurementSource } from "@/generated/prisma/client"; @@ -281,7 +287,26 @@ async function postBatch(request: NextRequest): Promise { }; const results: EntryResult[] = new Array(workouts.length); - const prepared: Prepared[] = workouts.map((w, index) => { + const unstableShapes: UnstableExternalIdShape[] = []; + const prepared: Prepared[] = workouts.flatMap((w, index) => { + // The `(userId, source, externalId)` dedup key is only idempotent + // while the id is STABLE across client launches. An id that rotates + // per process (an object description carrying a memory address) + // never matches its own earlier row, so every re-sync mints another + // workout. Refused per entry — one bad row must not stop the rest of + // the batch landing. + const unstable = + w.externalId === undefined ? null : classifyExternalId(w.externalId); + if (unstable) { + unstableShapes.push(unstable); + results[index] = { + index, + status: "skipped", + reason: UNSTABLE_EXTERNAL_ID_REASON, + }; + return []; + } + const durationSec = Math.max( 0, Math.round((w.endedAt.getTime() - w.startedAt.getTime()) / 1000), @@ -319,17 +344,25 @@ async function postBatch(request: NextRequest): Promise { ? (w.metadata as Prisma.InputJsonValue) : Prisma.JsonNull, }; - return { - index, - row, - route, - samples, - dedupKey: w.externalId - ? { source: w.source, externalId: w.externalId } - : null, - }; + return [ + { + index, + row, + route, + samples, + dedupKey: w.externalId + ? { source: w.source, externalId: w.externalId } + : null, + }, + ]; }); + if (unstableShapes.length > 0) { + annotate({ + meta: unstableExternalIdMeta("workout.batch", unstableShapes), + }); + } + // v1.4.42 W5 — write-time cross-source dedup. The v1.5 iOS app // drains a HealthKit observer queue that frequently carries the // SAME logical workout from two paired sensors (Apple Watch + diff --git a/src/lib/import/csv-measurements.ts b/src/lib/import/csv-measurements.ts index 3b07b902a..923e2173d 100644 --- a/src/lib/import/csv-measurements.ts +++ b/src/lib/import/csv-measurements.ts @@ -40,6 +40,7 @@ import { MEASUREMENT_NOTES_MAX_LENGTH, } from "@/lib/validations/measurement"; import { isPlausibleEntryInstant } from "@/lib/validations/entry-instant"; +import { isStableExternalId } from "@/lib/validations/external-id"; import { resolveToCanonicalUnit } from "@/lib/measurements/unit-aliases"; /** A normalised, write-ready row in the `/api/import` measurement shape. */ @@ -68,6 +69,7 @@ export type CsvRowReason = | "invalid_glucose_context" | "notes_too_long" | "external_id_too_long" + | "unstable_external_id" | "wrong_column_count"; export type CsvRowStatus = "ok" | "skipped"; @@ -304,6 +306,14 @@ export function parseCsvMeasurements( continue; } + // The import upserts on `(userId, type, source, externalId)`, so an + // id that cannot be stable across exports re-imports as a fresh row + // every time instead of converging on the one it already wrote. + if (rawExternalId.length > 0 && !isStableExternalId(rawExternalId)) { + skip("unstable_external_id"); + continue; + } + rows.push({ line, status: "ok", diff --git a/src/lib/openapi/routes/measurements.ts b/src/lib/openapi/routes/measurements.ts index 8a810c4c3..2621db8c3 100644 --- a/src/lib/openapi/routes/measurements.ts +++ b/src/lib/openapi/routes/measurements.ts @@ -81,7 +81,8 @@ const batchEntrySchema = z "Dedup key for the `(userId, type, source, externalId)` composite index. Three shapes:\n" + "• `HKSample.uuid` — a per-sample reading. First-write-wins: a re-post returns `duplicate` and the stored row is immutable.\n" + "• `stats::` — a per-day cumulative total (Steps, Active Energy, Sleep Duration, Walking/Running Distance, Flights Climbed). A re-post OVERWRITES the row and returns `updated`.\n" + - "• `stats:HKQuantityTypeIdentifierHeartRate:` — v1.19.0 (iOS #34), refined to 10-minute granularity in v1.30.7, heart-rate bucket carrying the bucket's AVERAGE bpm as one PULSE row. `` is the ISO-8601 UTC 10-minute boundary — zeroed seconds/millis, minutes ∈ {00,10,20,30,40,50}, trailing `Z` (e.g. `2026-06-21T14:10:00.000Z`). A re-post OVERWRITES the row and returns `updated`, so iOS uploads ~144 rows/day instead of one per raw HR sample. A row that targets this prefix with a malformed bucket-start suffix is `skipped` with reason `malformed_hr_bucket_id`.", + "• `stats:HKQuantityTypeIdentifierHeartRate:` — v1.19.0 (iOS #34), refined to 10-minute granularity in v1.30.7, heart-rate bucket carrying the bucket's AVERAGE bpm as one PULSE row. `` is the ISO-8601 UTC 10-minute boundary — zeroed seconds/millis, minutes ∈ {00,10,20,30,40,50}, trailing `Z` (e.g. `2026-06-21T14:10:00.000Z`). A re-post OVERWRITES the row and returns `updated`, so iOS uploads ~144 rows/day instead of one per raw HR sample. A row that targets this prefix with a malformed bucket-start suffix is `skipped` with reason `malformed_hr_bucket_id`.\n" + + "The id MUST be stable across app restarts. An object description or memory address (``, `0x12ab34cd`) is a new value on every launch, so it can never match its own earlier row; such an entry is `skipped` with reason `unstable_external_id` while the rest of the batch still lands.", ), externalSourceVersion: z.string().min(1).max(120).optional(), // v1.8.6 W6 — optional per-entry source tag. Defaults to diff --git a/src/lib/openapi/routes/medications/paths.ts b/src/lib/openapi/routes/medications/paths.ts index e7d6fef22..1dd373726 100644 --- a/src/lib/openapi/routes/medications/paths.ts +++ b/src/lib/openapi/routes/medications/paths.ts @@ -104,7 +104,7 @@ const bulkIntakeEntry = z .max(128) .optional() .describe( - "The HealthKit dose-event UUID. Drives the idempotent re-sync dedup (first-write-wins per Apple dose).", + "The HealthKit dose-event UUID. Drives the idempotent re-sync dedup (first-write-wins per Apple dose). Must be stable across app restarts — an object description or memory address (``, `0x12ab34cd`) is a new value on every launch; such an entry is `skipped` with reason `unstable_external_id` while the rest of the batch still lands.", ), }) .meta({ diff --git a/src/lib/openapi/routes/mood.ts b/src/lib/openapi/routes/mood.ts index 0ca5e2f99..93a962000 100644 --- a/src/lib/openapi/routes/mood.ts +++ b/src/lib/openapi/routes/mood.ts @@ -221,7 +221,7 @@ const bulkMoodEntry = z .max(120) .optional() .describe( - "iOS-side source-stable id for idempotent dedup on the `(userId, source, externalId)` key.", + "iOS-side source-stable id for idempotent dedup on the `(userId, source, externalId)` key. Must be stable across app restarts — an object description or memory address (``, `0x12ab34cd`) is a new value on every launch; such an entry is `skipped` with reason `unstable_external_id` while the rest of the batch still lands.", ), }) .meta({ diff --git a/src/lib/validations/__tests__/external-id.test.ts b/src/lib/validations/__tests__/external-id.test.ts new file mode 100644 index 000000000..62d00e29d --- /dev/null +++ b/src/lib/validations/__tests__/external-id.test.ts @@ -0,0 +1,182 @@ +/** + * External-id stability floor. + * + * The rejected cases are the shapes that provably cannot be an identity. + * The accepted cases are every external-id shape this repository actually + * receives — grepped out of the ingest routes, the batch contracts, the + * OpenAPI examples and the existing fixtures. A false positive here costs + * a working client its sync, so the accepted list is the real guard. + */ +import { describe, it, expect } from "vitest"; + +import { + assertStableExternalId, + classifyExternalId, + isStableExternalId, + unstableExternalIdMessage, + unstableExternalIdMeta, + unstableExternalIdShape, + UNSTABLE_EXTERNAL_ID_REASON, +} from "@/lib/validations/external-id"; +import { z } from "zod/v4"; + +describe("classifyExternalId — refused shapes", () => { + it("refuses the object description that caused the live incident", () => { + expect(classifyExternalId("")).toBe( + "object_description", + ); + }); + + it("refuses any description, not just the one class", () => { + for (const value of [ + "", + "", + "", + "", + "apple:", + ]) { + expect(classifyExternalId(value)).toBe("object_description"); + } + }); + + it("refuses a bare pointer value", () => { + expect(classifyExternalId("0x12568db80")).toBe("pointer_address"); + expect(classifyExternalId("0XDEADBEEF")).toBe("pointer_address"); + }); + + it("refuses an empty or whitespace-only id", () => { + expect(classifyExternalId("")).toBe("blank"); + expect(classifyExternalId(" ")).toBe("blank"); + expect(classifyExternalId("\t\n ")).toBe("blank"); + }); + + it("classifies a padded description after trimming", () => { + expect(classifyExternalId(" ")).toBe( + "object_description", + ); + }); +}); + +describe("classifyExternalId — the id shapes this system actually receives", () => { + // Every entry below is a shape that exists in this repository today: + // the iOS batch contract, the Apple Health mirror, the Withings / + // WHOOP / Oura / Fitbit / Polar sync paths, the CSV importer, and the + // web client's own UUID mints. + const accepted = [ + // HealthKit sample UUIDs (upper + lower case, hyphenated) + "8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + "3f0c4e4d-9c1e-4b7e-2a1d-6f30ad2a9cb1", + // Un-hyphenated / compact ids + "8AD2A9CB3F0C4E4D9C1E4B7E2A1D6F30", + // The `stats:` aggregate contract (per-day + hourly bucket) + "stats:HKQuantityTypeIdentifierStepCount:2026-07-25", + "stats:HKQuantityTypeIdentifierActiveEnergyBurned:2026-07-25", + "stats:HKQuantityTypeIdentifierHeartRate:2026-07-25T08:10:00Z", + // Other structured prefix forms minted in this tree today + "assessment:clx1a2b3c4d5e6f7g8h9", + "withings:activity:clx1a2b3c4d5e6f7g8h9:2026-07-25:steps", + "withings:sleep:clx1a2b3c4d5e6f7g8h9:998877:hr_average", + "cycle:123456:strain", + "sleep:0f3a8c2e-2b1d-4f5a-9c8e-1d2f3a4b5c6d:hrv_rmssd", + "spo2:2026-07-25T03:12:00Z:unknown", + "hkcycle:2026-07-25", + "stats:steps:2026-07-25", + "retired:clx1a2b3c4d5e6f7g8h9:hk-uuid-41", + "hk-uuid-41", + // Opaque vendor identifiers + "1234567890", + "clx1a2b3c4d5e6f7g8h9", + "a3f5c9e1b7d2486fa0c3e5b7d9f1a2c4e6b8d0f2a4c6e8b0d2f4a6c8e0b2d4f6", + // Base64-ish blob + "aGVhbHRobG9nOnNhbXBleQ==", + // Platform type identifiers + "HKQuantityTypeIdentifierStepCount", + // Values that merely CONTAIN hex-looking text but are not pointers + "0xygen-sample-1", + "sample-0x12ab-tail", + "notbrackets", + "", + ]; + + it.each(accepted)("accepts %s", (value) => { + expect(classifyExternalId(value)).toBeNull(); + expect(isStableExternalId(value)).toBe(true); + }); + + it("does not reject on length, spaces, or unusual characters alone", () => { + expect(isStableExternalId("My Glucose Meter Export Row 41")).toBe(true); + expect(isStableExternalId("a")).toBe(true); + expect(isStableExternalId("ärztliche-messung-2026-07-25")).toBe(true); + }); +}); + +describe("messages", () => { + it("names what is wrong and what to send instead", () => { + expect(unstableExternalIdMessage("object_description")).toContain( + "stable across app restarts", + ); + expect(unstableExternalIdMessage("object_description")).toContain( + "object description", + ); + expect(unstableExternalIdMessage("pointer_address")).toContain( + "memory address", + ); + expect(unstableExternalIdMessage("blank")).toContain("must not be empty"); + }); +}); + +describe("assertStableExternalId — object-level Zod check", () => { + const schema = z + .object({ externalId: z.string().max(128).optional() }) + .superRefine(assertStableExternalId); + + it("rejects an object description with the actionable message on the field", () => { + const parsed = schema.safeParse({ + externalId: "", + }); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues).toHaveLength(1); + expect(parsed.error.issues[0].path).toEqual(["externalId"]); + expect(parsed.error.issues[0].message).toBe( + unstableExternalIdMessage("object_description"), + ); + }); + + it("accepts a stable id and an absent id alike", () => { + expect( + schema.safeParse({ externalId: "stats:HKFoo:2026-07-25" }).success, + ).toBe(true); + expect(schema.safeParse({}).success).toBe(true); + }); +}); + +describe("wide-event helpers", () => { + it("reads a top-level externalId off an unparsed body", () => { + expect(unstableExternalIdShape({ externalId: "0xdeadbeef" })).toBe( + "pointer_address", + ); + expect(unstableExternalIdShape({ externalId: "uuid-1" })).toBeNull(); + expect(unstableExternalIdShape({ externalId: 42 })).toBeNull(); + expect(unstableExternalIdShape(null)).toBeNull(); + expect(unstableExternalIdShape("not-an-object")).toBeNull(); + }); + + it("pins the meta shape and never carries the id itself", () => { + expect( + unstableExternalIdMeta("measurement.batch", [ + "object_description", + "object_description", + "pointer_address", + ]), + ).toEqual({ + external_id_rejected: 3, + external_id_shapes: "object_description,pointer_address", + external_id_surface: "measurement.batch", + }); + }); + + it("exposes one per-entry reason code for every batch surface", () => { + expect(UNSTABLE_EXTERNAL_ID_REASON).toBe("unstable_external_id"); + }); +}); diff --git a/src/lib/validations/cycle.ts b/src/lib/validations/cycle.ts index ced6da11d..dad5f0ce3 100644 --- a/src/lib/validations/cycle.ts +++ b/src/lib/validations/cycle.ts @@ -11,6 +11,7 @@ */ import { z } from "zod/v4"; import { isPlausibleEntryInstant } from "@/lib/validations/entry-instant"; +import { assertStableExternalId } from "@/lib/validations/external-id"; /** * v1.17 W1b — `loggedAt` instant on the cycle wire stays a string (the iOS @@ -138,11 +139,28 @@ export const cycleDayLogInputSchema = z.object({ note: z.string().max(500).optional(), loggedAt: boundedLoggedAt, source: cycleSourceEnum.optional().default("MANUAL"), + // Shared with the bulk ingest, which enforces external-id stability per + // entry rather than at the field (a schema-level refusal would fail the + // whole batch on one bad row). The single-entry POST parses through + // `cycleDayLogCreateSchema` below, which adds the field check. + // @external-id-checked-per-entry: src/app/api/cycle/day-logs/bulk/route.ts externalId: z.string().min(1).max(120).optional(), }); export type CycleDayLogInput = z.infer; +/** + * Single-entry POST body. Same shape as the bulk entry plus the + * external-id stability floor: a rotating id defeats the + * `(userId, source, externalId)` upsert and mints a fresh day log on every + * re-post. The bulk path deliberately keeps the permissive entry schema — + * it reports the offending entry as `skipped` rather than failing the + * whole batch — and applies the same rule per entry in the route. + */ +export const cycleDayLogCreateSchema = cycleDayLogInputSchema.superRefine( + assertStableExternalId, +); + /** PATCH body — every field optional; `date`/`source`/`externalId` are immutable on update. */ export const cycleDayLogPatchSchema = z.object({ flow: flowLevelEnum.nullable().optional(), @@ -180,12 +198,14 @@ export const cycleBulkSchema = z.object({ /* ── period-boundary shortcut ────────────────────────────────────── */ -export const cyclePeriodSchema = z.object({ - action: z.enum(["start", "end"]), - date: dateString, - externalId: z.string().min(1).max(120).optional(), - loggedAt: boundedLoggedAt, -}); +export const cyclePeriodSchema = z + .object({ + action: z.enum(["start", "end"]), + date: dateString, + externalId: z.string().min(1).max(120).optional(), + loggedAt: boundedLoggedAt, + }) + .superRefine(assertStableExternalId); /* ── calendar + history queries ──────────────────────────────────── */ diff --git a/src/lib/validations/external-id.ts b/src/lib/validations/external-id.ts new file mode 100644 index 000000000..baee1f8a1 --- /dev/null +++ b/src/lib/validations/external-id.ts @@ -0,0 +1,189 @@ +/** + * Client-supplied external identifiers — stability floor. + * + * Every ingest surface that accepts an `externalId` uses it as one half of + * a dedup key: `(userId, source, externalId)`, `(userId, externalSource, + * externalId)`, `(userId, type, source, externalId)`. Re-posting the same + * triple is idempotent and returns the existing row. That idempotency is + * only as good as the STABILITY of the identifier — an id that changes + * between two runs of the same client never matches its own earlier row, + * so every sync sweep mints a fresh record and the user's history fills + * with phantom rows that carry no doses, no schedule, no follow-up data. + * + * That is not hypothetical. A native client serialised an opaque platform + * object with the language's default object description and sent the + * result as `externalId`. The default description of an object with no + * description contract is `` — a per-allocation + * MEMORY ADDRESS. A live instance accumulated 23 duplicate medications in + * a single day, one per sync sweep, each with a different address in its + * id, and the dose events that should have attached to them resolved + * through the same rotating string and were never stored at all. + * + * This module is the one place that recognises an identifier which CANNOT + * be a stable identity. It is deliberately narrow: the cost of a false + * negative is a duplicate row, the cost of a false positive is a + * legitimate client losing the ability to sync at all. Only shapes that + * are provably not identities are rejected: + * + * 1. `blank` — empty or whitespace-only after trimming. + * Carries no identity by construction. + * 2. `pointer_address` — a bare `0x…` hex value and nothing else. + * A raw address, valid only inside the + * process that printed it. + * 3. `object_description` — an angle-bracketed description containing + * an `0x…` hex token, e.g. `` + * or ``. This is the general + * form of a default object description, not a + * match on one platform class: no identity + * format wraps itself in angle brackets around + * a hex pointer. + * + * Everything else passes. In particular the identifier shapes this system + * actually receives all survive: + * + * - UUIDs, upper or lower case, hyphenated or not + * (`8AD2A9CB-...`, HealthKit sample UUIDs, SwiftData row ids) + * - structured prefix forms + * (`stats:HKQuantityTypeIdentifierStepCount:2026-07-25`, + * `stats:HKQuantityTypeIdentifierHeartRate:2026-07-25T08:10:00Z`, + * `assessment:`, `whoop:`, `oura:`) + * - opaque vendor identifiers (numeric ids, base64-ish blobs, hashes — + * including a 64-char SHA-256 hex digest, which has no `0x` prefix) + * - platform type identifiers (`HKQuantityTypeIdentifierStepCount`) + * + * There is deliberately NO rule on length, on the presence of spaces, on + * character class, or on "looks random enough". Those would reject real + * vendor identifiers, and a rule that cannot be justified against a shape + * this system actually receives does not belong here. + */ +import { z } from "zod/v4"; + +/** The closed set of shapes that cannot be a stable identity. */ +export type UnstableExternalIdShape = + "blank" | "pointer_address" | "object_description"; + +/** + * A bare hex pointer and nothing else — `0x12568db80`. Anchored on both + * ends so an opaque vendor id that merely CONTAINS "0x" is untouched. + */ +const BARE_POINTER = /^0x[0-9a-f]+$/i; + +/** + * An angle-bracketed description carrying a hex pointer, searched + * anywhere in the value so a client that prefixes its own namespace + * (`apple:`) is caught too. `[^<>]*` keeps the bracket pair + * adjacent — the token has to be one description, not two unrelated + * angle brackets spanning half the string. + */ +const POINTER_DESCRIPTION = /<[^<>]*0x[0-9a-f]+[^<>]*>/i; + +/** + * Classify an external identifier. Returns `null` when the value is + * usable as an identity, or the shape that disqualifies it. + */ +export function classifyExternalId( + value: string, +): UnstableExternalIdShape | null { + const trimmed = value.trim(); + if (trimmed.length === 0) return "blank"; + if (BARE_POINTER.test(trimmed)) return "pointer_address"; + if (POINTER_DESCRIPTION.test(trimmed)) return "object_description"; + return null; +} + +/** Convenience predicate over {@link classifyExternalId}. */ +export function isStableExternalId(value: string): boolean { + return classifyExternalId(value) === null; +} + +const SHAPE_MESSAGES: Record = { + blank: "external identifier must not be empty", + pointer_address: + "external identifier must be stable across app restarts; received a memory address (0x…), which is a new value on every launch and mints a duplicate record on every sync", + object_description: + "external identifier must be stable across app restarts; received an object description (), which embeds a memory address and is a new value on every launch — serialise the identity itself, not the object's description", +}; + +/** The 422 message a client developer reads. Names the shape received. */ +export function unstableExternalIdMessage( + shape: UnstableExternalIdShape, +): string { + return SHAPE_MESSAGES[shape]; +} + +/** + * Object-level Zod check for a single-entry request schema that carries a + * top-level `externalId`. Used as `schema.superRefine(assertStableExternalId)` + * so the refusal rides the standard 422 multi-issue envelope + * (`returnAllZodIssues` / `sanitiseZodIssues`) with `path: ["externalId"]` + * instead of a hand-rolled error. + * + * Batch / bulk surfaces deliberately do NOT use this: a schema-level + * refusal fails the whole payload, and one bad row must not stop the other + * 499 from landing. Those routes call {@link classifyExternalId} per entry + * and report the entry through their own `skipped` contract. + */ +export function assertStableExternalId( + value: { externalId?: string | null }, + ctx: z.RefinementCtx, +): void { + const raw = value.externalId; + if (typeof raw !== "string") return; + const shape = classifyExternalId(raw); + if (shape === null) return; + ctx.addIssue({ + code: "custom", + path: ["externalId"], + message: unstableExternalIdMessage(shape), + }); +} + +/** + * Read a top-level `externalId` off an unparsed request body and classify + * it. Single-entry routes use this on the 422 path to annotate the wide + * event with the SHAPE that was refused — never the id itself, which is + * client free-text and can carry anything. + */ +export function unstableExternalIdShape( + body: unknown, +): UnstableExternalIdShape | null { + if (typeof body !== "object" || body === null) return null; + const raw = (body as { externalId?: unknown }).externalId; + if (typeof raw !== "string") return null; + return classifyExternalId(raw); +} + +/** + * Pinned wide-event meta for a refusal. Keys are fixed so dashboards stay + * stable; the payload carries the count, the classification, and the + * surface — never the identifier itself, which is client free-text. + * + * `external_id_shapes` is the sorted, de-duplicated shape set as a single + * comma-joined string — a scalar rather than an array so it groups + * cleanly in the event store. + * + * The surface rides the META rather than only the action name because a + * wide event carries exactly one action, and most of these refusals + * happen on a path that already names its own (`…​.validation-failed`, + * `measurement.batch.ingest`). Overwriting that action would cost more + * than it buys, so `external_id_rejected > 0` is the one query that finds + * every occurrence on every surface; routes whose failure path claims no + * action of its own additionally set `.external_id.rejected`. + */ +export function unstableExternalIdMeta( + surface: string, + shapes: UnstableExternalIdShape[], +): { + external_id_rejected: number; + external_id_shapes: string; + external_id_surface: string; +} { + return { + external_id_rejected: shapes.length, + external_id_shapes: [...new Set(shapes)].sort().join(","), + external_id_surface: surface, + }; +} + +/** Per-entry `reason` code every batch / bulk surface reports. */ +export const UNSTABLE_EXTERNAL_ID_REASON = "unstable_external_id"; diff --git a/src/lib/validations/medication/create-update.ts b/src/lib/validations/medication/create-update.ts index 053edcb8e..6f74798dc 100644 --- a/src/lib/validations/medication/create-update.ts +++ b/src/lib/validations/medication/create-update.ts @@ -12,6 +12,7 @@ import { rxNormCodeField, } from "./base"; import { scheduleSchema } from "./schedule"; +import { assertStableExternalId } from "@/lib/validations/external-id"; /** * v1.5 — medication-level course window + one-shot flag. The fields @@ -187,7 +188,7 @@ export const createMedicationSchema = z .max(128) .optional() .describe( - "Stable external identifier of the mirrored medication (the HealthKit `medicationConceptIdentifier`, max 128 chars). Must be supplied together with `externalSource`. Idempotent: re-posting the same pair returns the existing medication with status 200 instead of minting a duplicate.", + "Stable external identifier of the mirrored medication (the HealthKit `medicationConceptIdentifier`, max 128 chars). Must be supplied together with `externalSource`. Idempotent: re-posting the same pair returns the existing medication with status 200 instead of minting a duplicate. The value MUST be identical across app restarts: an object description or memory address (``, `0x12ab34cd`) is a new value on every launch, never matches its own earlier row, and is refused with 422 rather than minting a duplicate mirror.", ), ...courseWindowFields, /** @@ -206,6 +207,10 @@ export const createMedicationSchema = z path: ["externalId"], }, ) + // The concept id has to be STABLE across client launches or the + // idempotent-mirror pre-query above never matches its own earlier row + // and every sync sweep mints another phantom medication. + .superRefine(assertStableExternalId) .refine((b) => b.oneShot !== true || !!b.startsOn, { message: "startsOn is required when oneShot is true", path: ["startsOn"], diff --git a/src/lib/validations/mental-health.ts b/src/lib/validations/mental-health.ts index 8415afe60..9331f9f85 100644 --- a/src/lib/validations/mental-health.ts +++ b/src/lib/validations/mental-health.ts @@ -11,6 +11,7 @@ import { INSTRUMENTS, type InstrumentId, } from "@/lib/mental-health/instruments"; +import { assertStableExternalId } from "@/lib/validations/external-id"; export const assessmentInstrumentEnum = z.enum(["PHQ9", "GAD7", "WHO5", "SCI"]); @@ -69,7 +70,10 @@ export const createAssessmentSchema = z }); } }); - }); + }) + // The replay anchor only protects against a duplicate administration + // while the id is stable across client launches. + .superRefine(assertStableExternalId); export const listAssessmentsSchema = z.object({ instrument: assessmentInstrumentEnum.optional(), diff --git a/src/lib/validations/mood.ts b/src/lib/validations/mood.ts index 2d3a5ce30..117e98a17 100644 --- a/src/lib/validations/mood.ts +++ b/src/lib/validations/mood.ts @@ -8,6 +8,7 @@ */ import { z } from "zod/v4"; import { validateEntryInstant } from "@/lib/validations/entry-instant"; +import { assertStableExternalId } from "@/lib/validations/external-id"; // --- CRUD schemas for mood entries --- @@ -68,39 +69,45 @@ const ratedFactor = z.object({ }); const ratedFactors = z.array(ratedFactor).max(30); -export const createMoodEntrySchema = z.object({ - mood: moodLevelEnum, - tags: z.array(z.string().max(50)).max(20).optional(), - // v1.8.5 — structured-tag keys from the taxonomy. Server resolves each - // key to a `MoodTag` row and writes the `MoodEntryTagLink` join; - // unknown keys are dropped silently (the catalog is the source of - // truth, a stale client can't mint a tag). - tagKeys: structuredTagKeys.optional(), - // v1.12.0 — rated factors scored 1..5 (or the factor's own scale). - // Parallel to the binary `tagKeys`; persisted on - // `MoodEntryTagLink.rating`. Out-of-scale or non-RATED keys are - // rejected (422) / dropped server-side per the catalog. - ratedFactors: ratedFactors.optional(), - // v1.4.30 H-5 — first-class free-text note. Replaces the - // `tags: ["note:"]` workaround. Capped at 500 chars so the - // Coach evidence shelf renders cleanly without truncating chips. - note: z.string().max(500).optional(), - // v1.17 W1b — plausibility bound (shared `validateEntryInstant`): no - // future instants beyond a 5-min clock-skew tolerance, no instant before - // 1900. Mirrors the measurement + medication-intake bound. - moodLoggedAt: validateEntryInstant( - z.iso.datetime({ offset: true }).transform((s) => new Date(s)), - ), - source: moodSourceEnum.optional().default("MANUAL"), - // v1.12.1 — optional source-stable id (e.g. an iOS SwiftData row UUID). - // When present, the create upserts on `(userId, source, externalId)` - // so a re-post with the same id updates the existing row in place - // instead of either 409-ing or minting a duplicate — the idempotent - // re-import iOS drives over Bearer. NULL keeps the legacy - // `(userId, date, moodLoggedAt)` behaviour. Bound matches the bulk - // `externalId` so one path can't accept an id the other rejects. - externalId: z.string().min(1).max(120).optional(), -}); +export const createMoodEntrySchema = z + .object({ + mood: moodLevelEnum, + tags: z.array(z.string().max(50)).max(20).optional(), + // v1.8.5 — structured-tag keys from the taxonomy. Server resolves each + // key to a `MoodTag` row and writes the `MoodEntryTagLink` join; + // unknown keys are dropped silently (the catalog is the source of + // truth, a stale client can't mint a tag). + tagKeys: structuredTagKeys.optional(), + // v1.12.0 — rated factors scored 1..5 (or the factor's own scale). + // Parallel to the binary `tagKeys`; persisted on + // `MoodEntryTagLink.rating`. Out-of-scale or non-RATED keys are + // rejected (422) / dropped server-side per the catalog. + ratedFactors: ratedFactors.optional(), + // v1.4.30 H-5 — first-class free-text note. Replaces the + // `tags: ["note:"]` workaround. Capped at 500 chars so the + // Coach evidence shelf renders cleanly without truncating chips. + note: z.string().max(500).optional(), + // v1.17 W1b — plausibility bound (shared `validateEntryInstant`): no + // future instants beyond a 5-min clock-skew tolerance, no instant before + // 1900. Mirrors the measurement + medication-intake bound. + moodLoggedAt: validateEntryInstant( + z.iso.datetime({ offset: true }).transform((s) => new Date(s)), + ), + source: moodSourceEnum.optional().default("MANUAL"), + // v1.12.1 — optional source-stable id (e.g. an iOS SwiftData row UUID). + // When present, the create upserts on `(userId, source, externalId)` + // so a re-post with the same id updates the existing row in place + // instead of either 409-ing or minting a duplicate — the idempotent + // re-import iOS drives over Bearer. NULL keeps the legacy + // `(userId, date, moodLoggedAt)` behaviour. Bound matches the bulk + // `externalId` so one path can't accept an id the other rejects. + externalId: z.string().min(1).max(120).optional(), + }) + // The upsert key is only idempotent while the id is stable; an id that + // rotates per client launch mints a fresh entry on every re-post. The + // bulk twin carries the same rule per entry (it cannot fail the whole + // batch on one bad row). + .superRefine(assertStableExternalId); export const updateMoodEntrySchema = z.object({ mood: moodLevelEnum.optional(), diff --git a/src/lib/validations/workout.ts b/src/lib/validations/workout.ts index dfd25584e..8352a8d4d 100644 --- a/src/lib/validations/workout.ts +++ b/src/lib/validations/workout.ts @@ -173,6 +173,10 @@ export const createWorkoutSchema = z // schema, so a client cannot forge a row attributed to an integration it // does not own. source: writableMeasurementSourceEnum.optional().default("MANUAL"), + // The only writer of this schema is the batch ingest, which enforces + // external-id stability per entry rather than at the field: a + // schema-level refusal would fail the whole batch on one bad row. + // @external-id-checked-per-entry: src/app/api/workouts/batch/route.ts externalId: z.string().max(128).optional(), externalSourceVersion: z.string().max(64).optional(), metadata: z.record(z.string(), z.unknown()).optional(), From 6e4bcffadfe45f869b596334ebf77781f77ed25e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 25 Jul 2026 17:31:39 +0200 Subject: [PATCH 2/5] test(ingest): prove the external-id floor cannot be reintroduced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harden the structural guard so it reads the file body with its imports stripped: grepping the raw source cannot tell a file that APPLIES the check from one that merely imports it, so deleting the call while leaving the import slipped straight through. Add the partial-success case on a second batch envelope — the mood bulk ingest echoes the offending entry back with its index, status and reason while the good rows still upsert. --- .../external-id-stability-guard.test.ts | 26 ++++-- .../mood-entries/bulk/__tests__/route.test.ts | 86 +++++++++++++++++++ 2 files changed, 105 insertions(+), 7 deletions(-) diff --git a/src/__tests__/external-id-stability-guard.test.ts b/src/__tests__/external-id-stability-guard.test.ts index 4e59506c5..47f942f04 100644 --- a/src/__tests__/external-id-stability-guard.test.ts +++ b/src/__tests__/external-id-stability-guard.test.ts @@ -44,6 +44,19 @@ function read(rel: string): string { return readFileSync(join(SRC, rel), "utf8"); } +/** + * The file with its `import … from "…";` statements removed. + * + * A guard that greps the raw source cannot tell "this file APPLIES the + * check" from "this file merely imports it" — deleting the call while + * leaving the import would slip straight through. Every usage assertion + * below therefore runs against the body, and the module import is + * asserted separately where it matters. + */ +function body(rel: string): string { + return read(rel).replace(/import\s+[\s\S]*?from\s+"[^"]+";/g, ""); +} + /** * A REQUEST-side declaration of a client-supplied external identifier: * `externalId: z.…` / `externalSourceId: z.…` in a Zod object shape. @@ -106,11 +119,10 @@ describe("every client-supplied external id routes through the stability floor", return; } - const source = read(rel); const usesFieldCheck = - source.includes(VALIDATOR_MODULE) && - source.includes("assertStableExternalId"); - const marksPerEntry = MARKER.test(source); + read(rel).includes(VALIDATOR_MODULE) && + body(rel).includes("assertStableExternalId"); + const marksPerEntry = MARKER.test(read(rel)); expect( usesFieldCheck || marksPerEntry, `${rel} declares a client-supplied external id but neither applies ` + @@ -134,7 +146,7 @@ describe("every client-supplied external id routes through the stability floor", target.startsWith("src/"), `${rel}: marker target "${target}" must be a repo-root-relative src/ path`, ).toBe(true); - const checker = read(target.replace(/^src\//, "")); + const checker = body(target.replace(/^src\//, "")); expect( checker.includes("classifyExternalId"), `${rel} points its per-entry marker at ${target}, but that file never calls classifyExternalId.`, @@ -158,7 +170,7 @@ describe("the batch surfaces refuse per entry, never whole-batch", () => { ]; it.each(BATCH_ROUTES)("%s classifies each entry itself", (rel) => { - const source = read(rel); + const source = body(rel); expect(source).toContain("classifyExternalId"); expect(source).toContain("unstableExternalIdMeta"); }); @@ -168,7 +180,7 @@ describe("the batch surfaces refuse per entry, never whole-batch", () => { (rel) => { // The JSON importer has no per-entry status envelope — it reports a // skipped COUNT — so it is the one batch surface without the code. - expect(read(rel)).toContain("UNSTABLE_EXTERNAL_ID_REASON"); + expect(body(rel)).toContain("UNSTABLE_EXTERNAL_ID_REASON"); }, ); }); diff --git a/src/app/api/mood-entries/bulk/__tests__/route.test.ts b/src/app/api/mood-entries/bulk/__tests__/route.test.ts index bfbe3f9b6..081284866 100644 --- a/src/app/api/mood-entries/bulk/__tests__/route.test.ts +++ b/src/app/api/mood-entries/bulk/__tests__/route.test.ts @@ -461,6 +461,92 @@ describe("POST /api/mood-entries/bulk — structured tagKeys (v1.12.0)", () => { }); }); +describe("POST /api/mood-entries/bulk — unstable external ids", () => { + /** + * `(userId, source, externalId)` dedups only while the id is stable + * across client launches. A per-process value — an object description + * carrying a memory address — never matches its own earlier row, so + * every re-post mints another entry. The refusal is PER ENTRY: a client + * draining an offline queue must not lose its good rows to one bad one. + */ + beforeEach(() => { + vi.mocked(prisma.moodEntry.findMany).mockResolvedValue([]); + vi.mocked(prisma.moodEntry.upsert).mockResolvedValue({ + id: "entry-1", + } as never); + }); + + it("skips the offending entry and still writes the rest of the batch", async () => { + const res = await POST( + postReq({ + entries: [ + { + mood: "GUT", + moodLoggedAt: "2026-05-16T08:00:00.000Z", + externalId: "8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + }, + { + mood: "OKAY", + moodLoggedAt: "2026-05-16T09:00:00.000Z", + externalId: "", + }, + { + mood: "SUPER_GUT", + moodLoggedAt: "2026-05-16T10:00:00.000Z", + externalId: "0xdeadbeef", + }, + ], + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { + processed: number; + inserted: number; + skipped: Array<{ index: number; reason: string }>; + entries: Array<{ + index: number; + status: string; + reason?: string; + externalId?: string; + }>; + }; + }; + expect(body.data.processed).toBe(3); + expect(body.data.entries[0].status).toBe("inserted"); + expect(body.data.entries[1]).toMatchObject({ + index: 1, + status: "skipped", + reason: "unstable_external_id", + externalId: "", + }); + expect(body.data.entries[2]).toMatchObject({ + index: 2, + status: "skipped", + reason: "unstable_external_id", + }); + expect(body.data.skipped).toEqual([ + { index: 1, reason: "unstable_external_id" }, + { index: 2, reason: "unstable_external_id" }, + ]); + // Only the good entry reached the writer. + expect(prisma.moodEntry.upsert).toHaveBeenCalledTimes(1); + }); + + it("leaves an entry without an externalId untouched", async () => { + const res = await POST( + postReq({ + entries: [{ mood: "OKAY", moodLoggedAt: "2026-05-16T08:00:00.000Z" }], + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { entries: Array<{ status: string }> }; + }; + expect(body.data.entries[0].status).toBe("inserted"); + }); +}); + describe("POST /api/mood-entries/bulk — tombstone suppression", () => { it("keeps a tombstoned match deleted: true no-op reported as duplicate", async () => { // A soft-deleted row under the batched pre-read: the sync feed already From 15b5f6f92f658cdc539956ef73b78e6b84ff9875 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 25 Jul 2026 18:07:13 +0200 Subject: [PATCH 3/5] fix(ingest): hold the stability floor for identities no request schema sees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dedup key on the medication intake surfaces is not only `externalId`. `MedicationIntakeEvent.idempotency_key` is a `@unique` column persisted with the dose row forever, with no TTL and no sweep, matched by plain equality on the per-medication POST, the bulk drain and the external ingest bridge. That makes it an identity, not a replay token, and it carries the same failure mode: a key that rotates per client launch never matches its own earlier post, so every retry logs the dose again. The blank case is sharper still — the column is unique table-wide, so an empty key resolves to whichever unrelated row stored it first. Refuse both shapes at the schema on the single-entry paths and per entry on the bulk drain. The MCP write path hashes a host-supplied key into the `externalId` half of its dedup key, so the shape is laundered before anything downstream could see it and the check has to run on the input. That closes a live one: every tool handler falls back to `""` when the host omits the argument, and `""` digests to a constant, so each keyless write after the first reported `already_logged` and was silently dropped. The HTTP `Idempotency-Key` header stays out, and now says why in the guard: `IdempotencyKey.expiresAt` bounds it and the cleanup job sweeps it, so it is a windowed replay token and rotation is its design. Extend the structural guard with a provenance rule for identities that never pass a Zod request schema — a spreadsheet column, an MCP tool argument. Inside the import and MCP trees, a module handling an identity must apply the floor itself or import a module that does. That is a real chain rather than a list of known offenders: a new importer that mints an `externalId` from file input and imports nothing checked fails it, with no allowlist to escape into. --- .../external-id-stability-guard.test.ts | 160 ++++++++++++- .../cycle/__tests__/external-id-floor.test.ts | 213 +++++++++++++++++ .../day-logs/bulk/__tests__/route.test.ts | 109 +++++++++ src/app/api/import/__tests__/route.test.ts | 120 ++++++++++ .../ingest/medication/__tests__/route.test.ts | 84 +++++++ src/app/api/ingest/medication/route.ts | 10 +- .../[id]/intake/__tests__/route.test.ts | 92 ++++++++ src/app/api/medications/[id]/intake/route.ts | 13 +- .../intake/bulk/__tests__/route.test.ts | 128 ++++++++++ src/app/api/medications/intake/bulk/route.ts | 25 +- .../assessments/__tests__/route.test.ts | 51 ++++ .../api/mood-entries/__tests__/route.test.ts | 68 ++++++ .../__tests__/unstable-external-id.test.ts | 220 ++++++++++++++++++ .../import/__tests__/csv-measurements.test.ts | 56 +++++ src/lib/mcp/__tests__/writes.test.ts | 76 ++++++ src/lib/mcp/write-tools.ts | 21 ++ src/lib/mcp/writes.ts | 59 +++++ src/lib/validations/external-id.ts | 88 +++++-- src/lib/validations/medication/intake.ts | 24 +- 19 files changed, 1574 insertions(+), 43 deletions(-) create mode 100644 src/app/api/cycle/__tests__/external-id-floor.test.ts create mode 100644 src/app/api/workouts/batch/__tests__/unstable-external-id.test.ts diff --git a/src/__tests__/external-id-stability-guard.test.ts b/src/__tests__/external-id-stability-guard.test.ts index 47f942f04..88b279fb1 100644 --- a/src/__tests__/external-id-stability-guard.test.ts +++ b/src/__tests__/external-id-stability-guard.test.ts @@ -58,14 +58,23 @@ function body(rel: string): string { } /** - * A REQUEST-side declaration of a client-supplied external identifier: - * `externalId: z.…` / `externalSourceId: z.…` in a Zod object shape. + * A REQUEST-side declaration of a client-supplied identity string: + * `externalId: z.…` / `externalSourceId: z.…` / `idempotencyKey: z.…` in a + * Zod object shape. * - * Response DTOs under `lib/openapi/` are excluded below — those echo an - * id the server already stored and validating them would be validating + * `idempotencyKey` is here because `MedicationIntakeEvent.idempotency_key` + * is NOT a windowed replay token — it is a `@unique` column persisted with + * the dose row forever and matched by plain equality, which makes it an + * identity with the same failure mode. The HTTP `Idempotency-Key` HEADER + * is a different thing and stays out: `IdempotencyKey.expiresAt` bounds it + * and `lib/jobs/idempotency-cleanup.ts` sweeps it. + * + * Response DTOs under `lib/openapi/` are excluded below — those echo a + * value the server already stored and validating them would be validating * our own output. */ -const DECLARATION = /^\s*(externalId|externalSourceId)\s*:\s*z\b/gm; +const DECLARATION = + /^\s*(externalId|externalSourceId|idempotencyKey)\s*:\s*z\b/gm; /** `// @external-id-checked-per-entry: ` */ const MARKER = /@external-id-checked-per-entry:\s*(\S+)/; @@ -83,6 +92,59 @@ const EXEMPT: Record = { "restore payload is a server-written backup; refusing a stored id would block a restore", }; +/** + * Trees where a stored identity is derived from input that never passes + * through a Zod request schema — an uploaded file's row, or an MCP host's + * tool argument. The declaration rule above cannot see those, so this one + * works on provenance instead: inside these trees, any module that touches + * an identity must either apply the floor itself or import a module that + * does. + * + * That is a real chain, not a list of known offenders: a NEW importer that + * mints an `externalId` from file input and imports nothing checked fails + * here, with no allowlist to add itself to. + */ +const DERIVED_IDENTITY_TREES = ["lib/import/", "app/api/import/", "lib/mcp/"]; + +const IDENTITY_MENTION = /\b(externalId|idempotencyKey)\b/; + +/** Floor symbols that count as "this module applies the check itself". */ +const FLOOR_SYMBOLS = [ + "classifyExternalId", + "isStableExternalId", + "assertStableExternalId", + "assertStableIdempotencyKey", +]; + +/** Local `@/…` and relative modules this file imports, as src-relative paths. */ +function importedLocalModules(rel: string): string[] { + const source = read(rel); + const dir = rel.split("/").slice(0, -1).join("/"); + const out: string[] = []; + for (const m of source.matchAll(/from\s+"([^"]+)"/g)) { + const spec = m[1]; + let target: string | null = null; + if (spec.startsWith("@/")) target = spec.slice(2); + else if (spec.startsWith("./")) target = `${dir}/${spec.slice(2)}`; + else if (spec.startsWith("../")) { + const parts = dir.split("/"); + let rest = spec; + while (rest.startsWith("../")) { + parts.pop(); + rest = rest.slice(3); + } + target = [...parts, rest].join("/"); + } + if (target) out.push(target.replace(/\.tsx?$/, "")); + } + return out; +} + +function appliesFloor(rel: string): boolean { + const b = body(rel); + return FLOOR_SYMBOLS.some((sym) => b.includes(sym)); +} + /** Lines of `rel` (1-based) on which a request-side declaration appears. */ function declarationLines(rel: string): number[] { const source = read(rel); @@ -121,13 +183,15 @@ describe("every client-supplied external id routes through the stability floor", const usesFieldCheck = read(rel).includes(VALIDATOR_MODULE) && - body(rel).includes("assertStableExternalId"); + (body(rel).includes("assertStableExternalId") || + body(rel).includes("assertStableIdempotencyKey")); const marksPerEntry = MARKER.test(read(rel)); expect( usesFieldCheck || marksPerEntry, - `${rel} declares a client-supplied external id but neither applies ` + - "`assertStableExternalId` to its request schema nor marks the declaration " + - "`@external-id-checked-per-entry: ` naming the route that checks it per entry.", + `${rel} declares a client-supplied identity string but neither applies ` + + "`assertStableExternalId` / `assertStableIdempotencyKey` to its request schema " + + "nor marks the declaration `@external-id-checked-per-entry: ` naming the " + + "route that checks it per entry.", ).toBe(true); }, ); @@ -156,6 +220,84 @@ describe("every client-supplied external id routes through the stability floor", ); }); +describe("identities derived outside a Zod schema stay inside the floor", () => { + /** + * The CSV importer builds an `externalId` from a spreadsheet column, and + * the MCP write path hashes a host-supplied key into one. Neither goes + * through a Zod request field, so the declaration rule above is blind to + * both — which is exactly the gap this rule closes. + */ + function derivedIdentityFiles(): string[] { + return sourceFiles() + .filter((rel) => DERIVED_IDENTITY_TREES.some((t) => rel.startsWith(t))) + .filter((rel) => IDENTITY_MENTION.test(read(rel))); + } + + it("finds the derived-identity modules at all (the matcher still matches)", () => { + const files = derivedIdentityFiles(); + expect(files).toContain("lib/import/csv-measurements.ts"); + expect(files).toContain("lib/mcp/writes.ts"); + }); + + it.each(derivedIdentityFiles())( + "%s applies the floor itself or imports a module that does", + (rel) => { + if (appliesFloor(rel)) return; + const sources = importedLocalModules(rel).filter((target) => { + try { + return appliesFloor(`${target}.ts`); + } catch { + return false; + } + }); + expect( + sources.length, + `${rel} handles an external identity but neither applies the stability floor ` + + "nor imports a module that does. It derives an identity from input no Zod " + + "request schema sees (a file row, an MCP tool argument), so nothing else " + + "will catch an unstable value here.", + ).toBeGreaterThan(0); + }, + ); + + it("the CSV importer refuses an unstable id with its own row reason", () => { + // The importer has no per-entry Zod field to hang a refine on; its + // contract is a per-row `reason` code. Pin that the floor is wired to + // that contract and not merely imported. + const b = body("lib/import/csv-measurements.ts"); + expect(b).toContain("isStableExternalId"); + expect(b).toContain('skip("unstable_external_id")'); + expect(read("lib/import/csv-measurements.ts")).toContain( + '| "unstable_external_id"', + ); + }); + + it("the MCP write path checks the key BEFORE it is hashed into an identity", () => { + // A SHA-256 of a pointer address is clean hex, so the externalId floor + // downstream can never see the shape. The check has to run on the input. + const b = body("lib/mcp/writes.ts"); + const guardAt = b.indexOf("checkMcpIdempotencyKey"); + const hashAt = b.indexOf("mcpExternalId"); + expect(guardAt).toBeGreaterThan(-1); + expect(hashAt).toBeGreaterThan(-1); + expect(b).toContain("classifyExternalId"); + // Every entry point returns the refusal before reaching the hash. + for (const fn of [ + "logMcpMeasurement", + "logMcpMood", + "logMcpBloodPressure", + ]) { + const start = b.indexOf(`export async function ${fn}`); + expect(start, `${fn} not found`).toBeGreaterThan(-1); + const scope = b.slice(start, start + 2000); + expect( + scope.indexOf("checkMcpIdempotencyKey"), + `${fn} does not check the idempotency key before writing`, + ).toBeGreaterThan(-1); + } + }); +}); + describe("the batch surfaces refuse per entry, never whole-batch", () => { // Each of these drains a client sync queue. A schema-level refusal here // would stop a whole batch of good rows because of one bad one, which is diff --git a/src/app/api/cycle/__tests__/external-id-floor.test.ts b/src/app/api/cycle/__tests__/external-id-floor.test.ts new file mode 100644 index 000000000..9d313003b --- /dev/null +++ b/src/app/api/cycle/__tests__/external-id-floor.test.ts @@ -0,0 +1,213 @@ +/** + * The two single-entry cycle writes — `POST /api/cycle/day-logs` and + * `POST /api/cycle/period` — against the external-id stability floor. + * + * Both upsert on `(userId, source, externalId)`, so an id that changes + * between two launches of the same client never matches its own earlier + * row: the mirror mints a fresh day log (or a fresh period marker) on + * every sweep. Unlike their bulk twin these are single-entry, so the + * refusal is the standard 422 multi-issue envelope and nothing is written. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest } from "next/server"; + +vi.mock("@/lib/db", () => ({ + prisma: { + auditLog: { create: vi.fn() }, + cycleDayLog: { + findUniqueOrThrow: vi.fn(), + findFirst: vi.fn(), + findFirstOrThrow: vi.fn(), + }, + menstrualCycle: { + findMany: vi.fn().mockResolvedValue([]), + findUniqueOrThrow: vi.fn(), + }, + $transaction: vi.fn(), + }, +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn(), + rateLimitHeaders: () => ({}), +})); +vi.mock("@/lib/idempotency", () => ({ + withIdempotency: + (fn: (...args: Args) => Promise) => + (...args: Args) => + fn(...args), +})); +vi.mock("@/lib/cycle/gate", () => ({ + requireCycleEnabled: vi.fn(), +})); +vi.mock("@/lib/cycle/day-log-write", () => ({ + upsertCycleDayLog: vi.fn(), +})); +vi.mock("@/lib/cycle/cycle-attribution", () => ({ + findOwningCycleId: vi.fn().mockResolvedValue(null), +})); +vi.mock("@/lib/cycle/dto", async (importOriginal) => ({ + ...(await importOriginal()), + toCycleDayLogDTO: vi.fn(() => ({ id: "day-1" })), + toMenstrualCycleDTO: vi.fn(() => ({ id: "cycle-1" })), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { POST as postDayLog } from "../day-logs/route"; +import { POST as postPeriod } from "../period/route"; +import { prisma } from "@/lib/db"; +import { getSession } from "@/lib/auth/session"; +import { checkRateLimit } from "@/lib/rate-limit"; +import { requireCycleEnabled } from "@/lib/cycle/gate"; +import { upsertCycleDayLog } from "@/lib/cycle/day-log-write"; + +const SESSION_OK = { + session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: { id: "user-1", username: "tester", role: "USER" as const }, +}; + +function req(path: string, body: unknown): NextRequest { + return new NextRequest(`http://localhost${path}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +const POISONED = ""; +/** What the HealthKit cycle mirror really writes (`import-accumulator.ts`). */ +const REAL_ID = "hkcycle:2026-01-01"; + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true } as never); + vi.mocked(requireCycleEnabled).mockResolvedValue({ + enabled: true, + profile: { sensitiveCategoryEncryption: false }, + } as never); + vi.mocked(upsertCycleDayLog).mockResolvedValue({ + id: "day-1", + existed: false, + changed: true, + } as never); + vi.mocked(prisma.cycleDayLog.findUniqueOrThrow).mockResolvedValue({ + id: "day-1", + symptoms: [], + } as never); + vi.mocked(prisma.cycleDayLog.findFirst).mockResolvedValue(null as never); + vi.mocked(prisma.cycleDayLog.findFirstOrThrow).mockResolvedValue({ + id: "day-1", + symptoms: [], + } as never); + vi.mocked(prisma.menstrualCycle.findUniqueOrThrow).mockResolvedValue({ + id: "cycle-1", + startDate: "2026-01-01", + } as never); +}); + +describe("POST /api/cycle/day-logs — external-id stability floor", () => { + it("422s on an object-description externalId and writes nothing", async () => { + const res = await postDayLog( + req("/api/cycle/day-logs", { + date: "2026-01-01", + loggedAt: "2026-01-01T08:00:00.000Z", + source: "APPLE_HEALTH", + externalId: POISONED, + }), + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + meta?: { errorCode?: string }; + }; + const issue = body.details.issues.find((i) => i.path === "externalId"); + expect(issue).toBeDefined(); + expect(issue!.message).toContain("stable across app restarts"); + expect(body.meta?.errorCode).toBe("cycle.day-log.invalid"); + expect(upsertCycleDayLog).not.toHaveBeenCalled(); + }); + + it("422s on a bare memory-address externalId", async () => { + const res = await postDayLog( + req("/api/cycle/day-logs", { + date: "2026-01-01", + loggedAt: "2026-01-01T08:00:00.000Z", + externalId: "0x126b25160", + }), + ); + expect(res.status).toBe(422); + expect(upsertCycleDayLog).not.toHaveBeenCalled(); + }); + + it("accepts the HealthKit cycle-day id the mirror really sends", async () => { + const res = await postDayLog( + req("/api/cycle/day-logs", { + date: "2026-01-01", + loggedAt: "2026-01-01T08:00:00.000Z", + source: "APPLE_HEALTH", + externalId: REAL_ID, + }), + ); + expect(res.status).toBe(201); + expect(upsertCycleDayLog).toHaveBeenCalledTimes(1); + }); +}); + +describe("POST /api/cycle/period — external-id stability floor", () => { + it("422s on an object-description externalId and opens no cycle", async () => { + const res = await postPeriod( + req("/api/cycle/period", { + action: "start", + date: "2026-01-01", + loggedAt: "2026-01-01T08:00:00.000Z", + externalId: POISONED, + }), + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + meta?: { errorCode?: string }; + }; + expect( + body.details.issues.some( + (i) => + i.path === "externalId" && + i.message.includes("stable across app restarts"), + ), + ).toBe(true); + expect(body.meta?.errorCode).toBe("cycle.period.invalid"); + expect(prisma.$transaction).not.toHaveBeenCalled(); + }); + + it("accepts the real HealthKit id and reaches the write transaction", async () => { + vi.mocked(prisma.$transaction).mockResolvedValue({ + cycleId: "cycle-1", + } as never); + const res = await postPeriod( + req("/api/cycle/period", { + action: "start", + date: "2026-01-01", + loggedAt: "2026-01-01T08:00:00.000Z", + externalId: REAL_ID, + }), + ); + expect(res.status).toBeLessThan(400); + expect(prisma.$transaction).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/app/api/cycle/day-logs/bulk/__tests__/route.test.ts b/src/app/api/cycle/day-logs/bulk/__tests__/route.test.ts index bc177e247..d75526e8e 100644 --- a/src/app/api/cycle/day-logs/bulk/__tests__/route.test.ts +++ b/src/app/api/cycle/day-logs/bulk/__tests__/route.test.ts @@ -11,6 +11,7 @@ import { NextRequest } from "next/server"; vi.mock("@/lib/db", () => ({ prisma: { auditLog: { create: vi.fn() }, + menstrualCycle: { findMany: vi.fn().mockResolvedValue([]) }, }, })); @@ -48,6 +49,7 @@ vi.mock("next/headers", () => ({ })); import { POST } from "../route"; +import { prisma } from "@/lib/db"; import { getSession } from "@/lib/auth/session"; import { checkRateLimit } from "@/lib/rate-limit"; import { requireCycleEnabled } from "@/lib/cycle/gate"; @@ -72,7 +74,9 @@ beforeEach(() => { vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true } as never); vi.mocked(requireCycleEnabled).mockResolvedValue({ enabled: true, + profile: { sensitiveCategoryEncryption: false }, } as never); + vi.mocked(prisma.menstrualCycle.findMany).mockResolvedValue([] as never); }); describe("POST /api/cycle/day-logs/bulk — body cap", () => { @@ -94,3 +98,108 @@ describe("POST /api/cycle/day-logs/bulk — body cap", () => { expect(upsertCycleDayLog).not.toHaveBeenCalled(); }); }); + +describe("POST /api/cycle/day-logs/bulk — external-id stability floor", () => { + /** + * The bulk drain upserts on `(userId, source, externalId)`. A per-launch + * id never matches its own earlier row, so every re-post mints another + * day log. Refused PER ENTRY — the client's other days must still land. + */ + beforeEach(() => { + let n = 0; + vi.mocked(upsertCycleDayLog).mockImplementation( + async () => + ({ id: `day-${++n}`, existed: false, changed: true }) as never, + ); + }); + + it("skips the poisoned entry and still writes the two good ones", async () => { + const res = await POST( + postReq({ + entries: [ + { + date: "2026-01-01", + loggedAt: "2026-01-01T08:00:00.000Z", + source: "APPLE_HEALTH", + externalId: "hkcycle:2026-01-01", + }, + { + date: "2026-01-02", + loggedAt: "2026-01-02T08:00:00.000Z", + source: "APPLE_HEALTH", + externalId: "", + }, + { + date: "2026-01-03", + loggedAt: "2026-01-03T08:00:00.000Z", + source: "APPLE_HEALTH", + externalId: "8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + }, + ], + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { + processed: number; + inserted: number; + skipped: number; + entries: Array<{ + index: number; + status: string; + reason?: string; + externalId?: string; + }>; + }; + }; + expect(body.data.processed).toBe(3); + expect(body.data.inserted).toBe(2); + expect(body.data.skipped).toBe(1); + expect(body.data.entries[0].status).toBe("inserted"); + expect(body.data.entries[1]).toMatchObject({ + index: 1, + status: "skipped", + reason: "unstable_external_id", + externalId: "", + }); + expect(body.data.entries[2].status).toBe("inserted"); + // Only the two good entries reached the writer. + expect(upsertCycleDayLog).toHaveBeenCalledTimes(2); + }); + + it("never turns one poisoned entry into a whole-batch 422", async () => { + const res = await POST( + postReq({ + entries: [ + { + date: "2026-01-01", + loggedAt: "2026-01-01T08:00:00.000Z", + externalId: "0xdeadbeef", + }, + ], + }), + ); + expect(res.status).toBe(200); + }); + + it("accepts the HealthKit cycle-day id shape the mirror really sends", async () => { + const res = await POST( + postReq({ + entries: [ + { + date: "2026-01-01", + loggedAt: "2026-01-01T08:00:00.000Z", + source: "APPLE_HEALTH", + externalId: "hkcycle:2026-01-01", + }, + ], + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { entries: Array<{ status: string }> }; + }; + expect(body.data.entries[0].status).toBe("inserted"); + expect(upsertCycleDayLog).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/app/api/import/__tests__/route.test.ts b/src/app/api/import/__tests__/route.test.ts index f616d3c12..096b72f8c 100644 --- a/src/app/api/import/__tests__/route.test.ts +++ b/src/app/api/import/__tests__/route.test.ts @@ -1163,3 +1163,123 @@ describe("POST /api/import — write failure classification", () => { expect(auditLog).toHaveBeenCalledTimes(1); }); }); + +describe("POST /api/import — external-id stability floor", () => { + /** + * The importer upserts on `(userId, type, source=IMPORT, externalId)` so + * a re-import of the same export converges instead of duplicating. An id + * that cannot be stable between two exports defeats that. The importer + * has no per-entry status envelope — its vocabulary is a skipped COUNT — + * so the offending row lands there and every other row still imports. + */ + beforeEach(() => { + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: true, + remaining: 4, + resetAt: new Date(Date.now() + 1000), + } as never); + vi.mocked(prisma.measurement.createManyAndReturn).mockResolvedValue([ + { id: "m-1", type: "WEIGHT", measuredAt: new Date() }, + ] as never); + vi.mocked(prisma.measurement.create).mockResolvedValue({ + id: "m-2", + type: "WEIGHT", + measuredAt: new Date(), + } as never); + vi.mocked(prisma.moodEntry.upsert).mockResolvedValue({} as never); + vi.mocked(prisma.moodEntry.create).mockResolvedValue({} as never); + }); + + function importReq(body: unknown) { + return new NextRequest("http://localhost/api/import", { + method: "POST", + body: JSON.stringify(body), + headers: { "content-type": "application/json" }, + }); + } + + function measurement(externalId?: string) { + return { + type: "WEIGHT", + value: 80, + unit: "kg", + measuredAt: "2026-05-01T08:00:00.000Z", + ...(externalId ? { externalId } : {}), + }; + } + + it("skips the poisoned row, imports the two good ones, and never writes the bad id", async () => { + const response = await POST( + importReq({ + measurements: [ + measurement("scale-row-42"), + measurement(""), + measurement("scale-row-43"), + ], + }), + ); + expect(response.status).toBeLessThan(400); + const body = (await response.json()) as { + data: { measurements: number; skipped: number }; + }; + expect(body.data.measurements).toBe(2); + expect(body.data.skipped).toBe(1); + const written = vi + .mocked(prisma.measurement.createManyAndReturn) + .mock.calls.flatMap( + (call) => (call[0] as { data: Array<{ externalId?: string }> }).data, + ); + expect(written).toHaveLength(2); + expect(written.map((r) => r.externalId)).toEqual([ + "scale-row-42", + "scale-row-43", + ]); + }); + + it("skips a poisoned mood row and still imports the good one", async () => { + const response = await POST( + importReq({ + moodEntries: [ + { + date: "2026-05-01", + mood: "GUT", + score: 4, + loggedAt: "2026-05-01T08:00:00.000Z", + externalId: "daylio-1041", + }, + { + date: "2026-05-02", + mood: "OKAY", + score: 3, + loggedAt: "2026-05-02T08:00:00.000Z", + externalId: "0xdeadbeef", + }, + ], + }), + ); + expect(response.status).toBeLessThan(400); + const body = (await response.json()) as { + data: { moodEntries: number; skipped: number }; + }; + expect(body.data.moodEntries).toBe(1); + expect(body.data.skipped).toBe(1); + expect(vi.mocked(prisma.moodEntry.upsert)).toHaveBeenCalledTimes(1); + }); + + it("accepts the exporter row ids a real re-import carries", async () => { + const response = await POST( + importReq({ + measurements: [ + measurement("scale-row-42"), + measurement("8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30"), + ], + }), + ); + expect(response.status).toBeLessThan(400); + const body = (await response.json()) as { + data: { measurements: number; skipped: number }; + }; + expect(body.data.measurements).toBe(2); + expect(body.data.skipped).toBe(0); + }); +}); diff --git a/src/app/api/ingest/medication/__tests__/route.test.ts b/src/app/api/ingest/medication/__tests__/route.test.ts index 1b79f4226..cee53c4a1 100644 --- a/src/app/api/ingest/medication/__tests__/route.test.ts +++ b/src/app/api/ingest/medication/__tests__/route.test.ts @@ -349,3 +349,87 @@ describe("POST /api/ingest/medication — slot attribution + convergence (v1.16. expect(consumeForIntake).not.toHaveBeenCalled(); }); }); + +describe("POST /api/ingest/medication — idempotency-key stability floor", () => { + /** + * `MedicationIntakeEvent.idempotency_key` is not a windowed replay + * token: it is a `@unique` column persisted with the dose row forever + * and matched by plain equality, which makes it an identity. A bridge + * that mints a per-process key never matches its own earlier post, so + * every retry logs the dose again. + * + * The blank case is the sharper one here — the column is unique + * table-wide, so an empty key would resolve to whichever unrelated row + * stored "" first rather than to this caller's own earlier post. + */ + it("422s on an object-description key and logs nothing", async () => { + const res = await POST( + postReq({ + medicationName: "Metformin", + idempotencyKey: "", + }), + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + }; + const issue = body.details.issues.find((i) => i.path === "idempotencyKey"); + expect(issue).toBeDefined(); + expect(issue!.message).toContain("idempotency key"); + expect(issue!.message).toContain("stable across app restarts"); + expect(prisma.medicationIntakeEvent.create).not.toHaveBeenCalled(); + }); + + it("422s on a whitespace-only key rather than matching a stranger's row", async () => { + const res = await POST( + postReq({ medicationName: "Metformin", idempotencyKey: " " }), + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + }; + expect( + body.details.issues.some( + (i) => + i.path === "idempotencyKey" && + i.message.includes("must not be empty"), + ), + ).toBe(true); + expect(prisma.medicationIntakeEvent.create).not.toHaveBeenCalled(); + }); + + it("accepts the timestamped key the documented bridge example sends", async () => { + // Ad-hoc take: no schedules on the medication, so the write goes + // straight to a create instead of converging onto a slot row. + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + timezone: "Europe/Berlin", + } as never); + vi.mocked(prisma.medication.findFirst).mockResolvedValue({ + id: "med-1", + name: "Metformin", + startsOn: null, + endsOn: null, + oneShot: false, + createdAt: new Date("2026-01-01T00:00:00.000Z"), + schedules: [], + scheduleRevisions: [], + } as never); + vi.mocked(prisma.medicationIntakeEvent.findFirst).mockResolvedValue(null); + vi.mocked(prisma.medicationIntakeEvent.findMany).mockResolvedValue( + [] as never, + ); + vi.mocked(prisma.medicationIntakeEvent.create).mockResolvedValue({ + id: "evt-new", + } as never); + + const res = await POST( + postReq({ + medicationName: "Metformin", + takenAt: "2026-06-10T06:42:00.000Z", + idempotencyKey: "intake-202602191230", + }), + ); + expect(res.status).toBe(201); + expect(prisma.medicationIntakeEvent.create).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/app/api/ingest/medication/route.ts b/src/app/api/ingest/medication/route.ts index 30e6c77b7..4e1845184 100644 --- a/src/app/api/ingest/medication/route.ts +++ b/src/app/api/ingest/medication/route.ts @@ -13,6 +13,10 @@ import { import { hashToken } from "@/lib/auth/hmac"; import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; import { externalIntakeSchema } from "@/lib/validations/medication"; +import { + unstableExternalIdMeta, + unstableExternalIdShape, +} from "@/lib/validations/external-id"; import { isApiGloballyEnabled } from "@/lib/app-settings"; import { recomputeMedicationComplianceForEvent } from "@/lib/rollups/medication-compliance-rollups"; import { @@ -108,9 +112,13 @@ export const POST = apiHandler(async (request: NextRequest) => { // integration sees the full validation shape at once. Audit // breadcrumb keyed `ingest.medication.validation-failed`. const issues = sanitiseZodIssues(parsed.error.issues); + const shape = unstableExternalIdShape(body, "idempotencyKey"); annotate({ action: { name: "ingest.medication.validation-failed" }, - meta: { issue_count: issues.length }, + meta: { + issue_count: issues.length, + ...(shape ? unstableExternalIdMeta("medication.ingest", [shape]) : {}), + }, }); // v1.4.49 — strip `message` from the audit-ledger row; external // ingest carries free-text `medicationName` + opaque diff --git a/src/app/api/medications/[id]/intake/__tests__/route.test.ts b/src/app/api/medications/[id]/intake/__tests__/route.test.ts index f47647000..06dbfa686 100644 --- a/src/app/api/medications/[id]/intake/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/intake/__tests__/route.test.ts @@ -698,3 +698,95 @@ describe("POST /api/medications/[id]/intake — as-needed (schedule-less) medica expect(restoreForIntake).toHaveBeenCalledTimes(1); }); }); + +describe("POST /api/medications/[id]/intake — idempotency-key stability floor", () => { + function postReq(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/medications/med-1/intake", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + } + + /** + * `MedicationIntakeEvent.idempotency_key` is a `@unique` column + * persisted with the dose row forever and matched by plain equality — + * an identity, not a windowed replay token. A caller minting a + * per-process key never matches its own earlier post, so a retried + * submission logs the dose a second time. + */ + it("422s on an object-description key and records no dose", async () => { + const res = await POST( + postReq({ + takenAt: new Date(Date.now() - 60 * 60 * 1000).toISOString(), + idempotencyKey: "", + }), + ROUTE_PARAMS, + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + }; + const issue = body.details.issues.find((i) => i.path === "idempotencyKey"); + expect(issue).toBeDefined(); + expect(issue!.message).toContain("idempotency key"); + expect(issue!.message).toContain("stable across app restarts"); + expect(prisma.$transaction).not.toHaveBeenCalled(); + }); + + it("422s on a bare memory-address key", async () => { + const res = await POST( + postReq({ + takenAt: new Date(Date.now() - 60 * 60 * 1000).toISOString(), + idempotencyKey: "0x126b25160", + }), + ROUTE_PARAMS, + ); + expect(res.status).toBe(422); + expect(prisma.$transaction).not.toHaveBeenCalled(); + }); + + it("accepts a stable caller-issued key and records the dose", async () => { + const takenAt = new Date(Date.now() - 60 * 60 * 1000); + vi.mocked(prisma.$transaction).mockResolvedValue([ + { + id: "evt-idem-1", + userId: "user-1", + medicationId: "med-1", + scheduledFor: takenAt, + takenAt, + skipped: false, + }, + ] as never); + vi.mocked(prisma.medication.findFirst).mockResolvedValueOnce({ + id: "med-1", + startsOn: null, + endsOn: null, + oneShot: false, + createdAt: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000), + schedules: [], + } as never); + vi.mocked(prisma.medicationIntakeEvent.findFirst).mockResolvedValueOnce( + null as never, + ); + vi.mocked(prisma.medication.findUnique).mockResolvedValue({ + id: "med-1", + userId: "user-1", + oneShot: false, + active: true, + } as never); + vi.mocked(prisma.medication.updateMany).mockResolvedValue({ + count: 0, + } as never); + + const res = await POST( + postReq({ + takenAt: takenAt.toISOString(), + idempotencyKey: "ios-outbox-8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + }), + ROUTE_PARAMS, + ); + expect(res.status).toBe(201); + expect(prisma.$transaction).toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/medications/[id]/intake/route.ts b/src/app/api/medications/[id]/intake/route.ts index 5fe7aed25..99d027529 100644 --- a/src/app/api/medications/[id]/intake/route.ts +++ b/src/app/api/medications/[id]/intake/route.ts @@ -14,6 +14,10 @@ import { intakeSchema, listIntakeEventsSchema, } from "@/lib/validations/medication"; +import { + unstableExternalIdMeta, + unstableExternalIdShape, +} from "@/lib/validations/external-id"; import { resolveInjectionSiteForWrite } from "@/lib/medications/injection-site-write"; import type { InjectionSiteKey } from "@/lib/medications/injection-sites"; import { withIdempotency } from "@/lib/idempotency"; @@ -72,9 +76,16 @@ async function postIntake(request: NextRequest, { params }: RouteParams) { // v1.4.43 W6 — per-med intake POST hot path; multi-issue 422 + // audit breadcrumb keyed `medications.intake.create.validation-failed`. const issues = sanitiseZodIssues(parsed.error.issues); + const shape = unstableExternalIdShape(body, "idempotencyKey"); annotate({ action: { name: "medications.intake.create.validation-failed" }, - meta: { issue_count: issues.length, medication_id: id }, + meta: { + issue_count: issues.length, + medication_id: id, + ...(shape + ? unstableExternalIdMeta("medication.intake.create", [shape]) + : {}), + }, }); // v1.4.49 — strip `message` from the audit-ledger row; the // intake payload carries `idempotencyKey` (opaque caller string). diff --git a/src/app/api/medications/intake/bulk/__tests__/route.test.ts b/src/app/api/medications/intake/bulk/__tests__/route.test.ts index 84fd54a0e..8f1632bde 100644 --- a/src/app/api/medications/intake/bulk/__tests__/route.test.ts +++ b/src/app/api/medications/intake/bulk/__tests__/route.test.ts @@ -1358,3 +1358,131 @@ describe("POST /api/medications/intake/bulk — v1.28 Apple Health dose events", expect("externalId" in createArgs.data).toBe(false); }); }); + +describe("POST /api/medications/intake/bulk — identity stability floor", () => { + /** + * Two dedup keys ride this route and BOTH are identities matched by + * equality forever: `(userId, externalId)` for the Apple dose event, and + * the `@unique` `idempotency_key` column for the replay probe. A value + * that rotates per client launch matches neither, so every re-sync logs + * the dose again. Refused PER ENTRY — a client draining an offline queue + * must not lose its good doses to one bad row. + */ + beforeEach(() => { + vi.mocked(prisma.medication.findMany).mockResolvedValue([ + { id: "med-1", externalSource: "APPLE_HEALTH" }, + ] as never); + vi.mocked(prisma.medication.findFirst).mockResolvedValue(null as never); + vi.mocked(prisma.medicationIntakeEvent.findMany).mockResolvedValue( + [] as never, + ); + vi.mocked(prisma.medicationIntakeEvent.findFirst).mockResolvedValue( + null as never, + ); + vi.mocked(prisma.medicationIntakeEvent.findUnique).mockResolvedValue( + null as never, + ); + vi.mocked(prisma.medicationIntakeEvent.create).mockResolvedValue({ + id: "evt-new", + } as never); + }); + + function entry(overrides: Record) { + return { + medicationId: "med-1", + takenAt: "2026-06-15T05:01:00.000Z", + ...overrides, + }; + } + + it("skips the poisoned dose event and still logs the two good ones", async () => { + const res = await POST( + postReq({ + entries: [ + entry({ + source: "APPLE_HEALTH", + externalId: "8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + }), + entry({ + source: "APPLE_HEALTH", + externalId: "", + }), + entry({ + source: "APPLE_HEALTH", + externalId: "3f0c4e4d-9c1e-4b7e-2a1d-6f30ad2a9cb1", + }), + ], + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { + processed: number; + skipped: Array<{ index: number; reason: string }>; + entries: Array<{ index: number; status: string; reason?: string }>; + }; + }; + expect(body.data.processed).toBe(3); + expect(body.data.entries[0].status).not.toBe("skipped"); + expect(body.data.entries[1]).toMatchObject({ + index: 1, + status: "skipped", + reason: "unstable_external_id", + }); + expect(body.data.entries[2].status).not.toBe("skipped"); + expect(body.data.skipped).toEqual([ + { index: 1, reason: "unstable_external_id" }, + ]); + expect(prisma.medicationIntakeEvent.create).toHaveBeenCalledTimes(2); + }); + + it("skips a rotating idempotencyKey — the column is a permanent identity, not a windowed token", async () => { + const res = await POST( + postReq({ + entries: [ + entry({ idempotencyKey: "intake-2026-06-15T05:01:00Z-med-1" }), + entry({ idempotencyKey: "" }), + ], + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { + entries: Array<{ index: number; status: string; reason?: string }>; + }; + }; + expect(body.data.entries[0].status).not.toBe("skipped"); + expect(body.data.entries[1]).toMatchObject({ + index: 1, + status: "skipped", + reason: "unstable_external_id", + }); + expect(prisma.medicationIntakeEvent.create).toHaveBeenCalledTimes(1); + }); + + it("never turns one poisoned entry into a whole-batch 422", async () => { + const res = await POST( + postReq({ entries: [entry({ idempotencyKey: "0xdeadbeef" })] }), + ); + expect(res.status).toBe(200); + }); + + it("accepts the HealthKit dose-event UUID the mirror really sends", async () => { + const res = await POST( + postReq({ + entries: [ + entry({ + source: "APPLE_HEALTH", + externalId: "8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + }), + ], + }), + ); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { entries: Array<{ status: string }> }; + }; + expect(body.data.entries[0].status).not.toBe("skipped"); + expect(prisma.medicationIntakeEvent.create).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/app/api/medications/intake/bulk/route.ts b/src/app/api/medications/intake/bulk/route.ts index 4dcacf96c..c969f17da 100644 --- a/src/app/api/medications/intake/bulk/route.ts +++ b/src/app/api/medications/intake/bulk/route.ts @@ -101,6 +101,11 @@ const bulkEntrySchema = z // 5-year floor), matching the single-intake create + edit paths. takenAt: boundedTakenAtSchema.optional(), skipped: z.boolean().optional().default(false), + // Persisted to the `@unique` `idempotency_key` column and matched by + // equality forever, so it is an identity and carries the same + // stability floor. Checked per entry for the same reason as + // `externalId` above. + // @external-id-checked-per-entry: src/app/api/medications/intake/bulk/route.ts idempotencyKey: z.string().min(1).max(128).optional(), // v1.8.5 — optional per-entry injection site. Validated against the // medication's effective allowed set; a disallowed site marks the @@ -356,16 +361,20 @@ async function postBulk(request: NextRequest): Promise { continue; } - // The `(userId, externalId)` dose-event dedup below is only - // idempotent while the id is STABLE across client launches. An id - // that rotates per process (an object description carrying a memory - // address) never matches its own earlier row, so every re-sync mints - // another intake event. Refused per entry — one bad row must not - // stop the rest of the batch landing. + // Both dedup keys on this route are only idempotent while the value + // is STABLE across client launches: `(userId, externalId)` for the + // Apple dose event, and the `@unique` `idempotency_key` column for + // the replay probe below. A value that rotates per process (an + // object description carrying a memory address) never matches its + // own earlier row, so every re-sync logs the dose again. Refused per + // entry — one bad row must not stop the rest of the batch landing. const unstable = - entry.externalId === undefined + (entry.externalId === undefined ? null - : classifyExternalId(entry.externalId); + : classifyExternalId(entry.externalId)) ?? + (entry.idempotencyKey === undefined + ? null + : classifyExternalId(entry.idempotencyKey)); if (unstable) { unstableShapes.push(unstable); skipped.push({ index: i, reason: UNSTABLE_EXTERNAL_ID_REASON }); 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 24f2ba941..1d780f051 100644 --- a/src/app/api/mental-health/assessments/__tests__/route.test.ts +++ b/src/app/api/mental-health/assessments/__tests__/route.test.ts @@ -397,3 +397,54 @@ describe("POST /api/mental-health/assessments", () => { expect(prisma.mentalHealthAssessment.create).not.toHaveBeenCalled(); }); }); + +describe("POST /api/mental-health/assessments — external-id stability floor", () => { + /** + * `externalId` is the durable replay anchor for the native client's + * outbox — the only thing standing between a retried submission and a + * second recorded administration of the same screener. It only holds + * while the value survives an app restart. + */ + it("422s on an object-description externalId and records nothing", async () => { + const res = await callPost( + makeReq({ + instrument: "PHQ9", + items: [1, 1, 1, 1, 1, 1, 1, 1, 0], + externalId: "", + }), + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + }; + const issue = body.details.issues.find((i) => i.path === "externalId"); + expect(issue).toBeDefined(); + expect(issue!.message).toContain("stable across app restarts"); + expect(prisma.mentalHealthAssessment.create).not.toHaveBeenCalled(); + }); + + it("accepts the outbox UUID the native client actually sends", async () => { + vi.mocked(prisma.mentalHealthAssessment.create).mockResolvedValue({ + id: "mha_2", + instrument: "PHQ9", + locale: "en", + version: "standard", + totalScore: 8, + severityBand: "mild", + item9Flagged: false, + crisisShownAt: null, + takenAt: new Date("2026-06-28T00:00:00.000Z"), + createdAt: new Date("2026-06-28T00:00:00.000Z"), + } as never); + + const res = await callPost( + makeReq({ + instrument: "PHQ9", + items: [1, 1, 1, 1, 1, 1, 1, 1, 0], + externalId: "8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + }), + ); + expect(res.status).toBe(201); + expect(prisma.mentalHealthAssessment.create).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/app/api/mood-entries/__tests__/route.test.ts b/src/app/api/mood-entries/__tests__/route.test.ts index 2ddf87a90..1b54d3e0c 100644 --- a/src/app/api/mood-entries/__tests__/route.test.ts +++ b/src/app/api/mood-entries/__tests__/route.test.ts @@ -563,3 +563,71 @@ describe("POST /api/mood-entries — externalId idempotent re-import (v1.12.1)", expect(txClient.moodEntry.create).not.toHaveBeenCalled(); }); }); + +describe("POST /api/mood-entries — external-id stability floor", () => { + const BASE = { + mood: "GUT", + moodLoggedAt: "2026-06-01T08:00:00.000Z", + }; + + /** + * The create upserts on `(userId, source, externalId)` so a re-post with + * the same id updates in place. A per-launch id defeats that and mints a + * new entry every sync. + */ + it("422s on an object-description externalId and writes nothing", async () => { + const res = await POST( + postReq({ + ...BASE, + externalId: "", + }), + ); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + meta?: { errorCode?: string }; + }; + const issue = body.details.issues.find((i) => i.path === "externalId"); + expect(issue).toBeDefined(); + expect(issue!.message).toContain("stable across app restarts"); + expect(issue!.message).toContain("object description"); + expect(body.meta?.errorCode).toBe("mood.create.invalid"); + expect(txClient.moodEntry.upsert).not.toHaveBeenCalled(); + expect(txClient.moodEntry.create).not.toHaveBeenCalled(); + }); + + it("422s on a bare memory-address externalId", async () => { + const res = await POST(postReq({ ...BASE, externalId: "0x126b25160" })); + expect(res.status).toBe(422); + const body = (await res.json()) as { + details: { issues: Array<{ path: string; message: string }> }; + }; + expect( + body.details.issues.some( + (i) => i.path === "externalId" && i.message.includes("memory address"), + ), + ).toBe(true); + expect(txClient.moodEntry.upsert).not.toHaveBeenCalled(); + }); + + it("accepts the SwiftData row UUID the iOS client actually sends", async () => { + txClient.moodEntry.upsert.mockResolvedValue({ + id: "mood-ext-1", + tags: null, + moodLoggedAt: new Date(BASE.moodLoggedAt), + mood: "GUT", + note: null, + source: "MANUAL", + date: "2026-06-01", + }); + + const res = await POST( + postReq({ + ...BASE, + externalId: "8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + }), + ); + expect(res.status).toBe(201); + expect(txClient.moodEntry.upsert).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/app/api/workouts/batch/__tests__/unstable-external-id.test.ts b/src/app/api/workouts/batch/__tests__/unstable-external-id.test.ts new file mode 100644 index 000000000..1e2cd9cd1 --- /dev/null +++ b/src/app/api/workouts/batch/__tests__/unstable-external-id.test.ts @@ -0,0 +1,220 @@ +/** + * `POST /api/workouts/batch` — external-id stability floor. + * + * `(userId, source, externalId)` is idempotent only while the id survives + * an app restart. A per-process value — an object description carrying a + * memory address — never matches its own earlier row, so every re-sync + * mints another workout for the same session. + * + * The refusal is PER ENTRY. A cold-start HealthKit backfill is "every + * workout I have ever recorded"; losing the whole batch to one bad row + * would be worse than the duplicate it prevents. + */ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { NextRequest } from "next/server"; + +vi.mock("@/lib/db", () => ({ + prisma: { + user: { findUnique: vi.fn() }, + workout: { findMany: vi.fn(), createManyAndReturn: vi.fn() }, + $transaction: vi.fn(async (fn: unknown) => { + if (typeof fn === "function") { + return (fn as (tx: unknown) => unknown)(txClient); + } + }), + }, +})); + +const txClient = { + workout: { createManyAndReturn: vi.fn() }, + workoutRoute: { createMany: vi.fn() }, + workoutSamples: { createMany: vi.fn() }, +}; + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn(), + rateLimitHeaders: () => ({}), +})); +vi.mock("@/lib/idempotency", () => ({ + withIdempotency: + (fn: (...args: Args) => Promise) => + (...args: Args) => + fn(...args), +})); +vi.mock("@/lib/jobs/pr-detection", () => ({ + enqueuePrDetection: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/cache/invalidate", () => ({ + invalidateUserMeasurements: vi.fn(), +})); +vi.mock("@/lib/arrivals/emit-shared", () => ({ + emitDataArrival: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/context", async (importOriginal) => { + const actual = await importOriginal>(); + return { ...actual, annotate: vi.fn() }; +}); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { POST } from "../route"; +import { prisma } from "@/lib/db"; +import { getSession } from "@/lib/auth/session"; +import { checkRateLimit } from "@/lib/rate-limit"; +import { annotate } from "@/lib/logging/context"; + +const SESSION_OK = { + session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: { id: "user-1", username: "tester", role: "USER" as const }, +}; + +function postReq(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/workouts/batch", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +/** A minimal well-formed run; only the id varies per case. */ +function workout(externalId: string, offsetMin = 0) { + const start = new Date(Date.UTC(2026, 5, 15, 6, offsetMin, 0)); + const end = new Date(start.getTime() + 30 * 60 * 1000); + return { + sportType: "running", + startedAt: start.toISOString(), + endedAt: end.toISOString(), + source: "APPLE_HEALTH", + externalId, + }; +} + +async function entriesFor(workouts: unknown[]) { + const res = await POST(postReq({ workouts })); + expect(res.status).toBe(200); + const body = (await res.json()) as { + data: { + entries: Array<{ index: number; status: string; reason?: string }>; + skipped: Array<{ index: number; reason: string }>; + }; + }; + return body.data; +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(checkRateLimit).mockResolvedValue({ allowed: true } as never); + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + sourcePriorityJson: null, + } as never); + vi.mocked(prisma.workout.findMany).mockResolvedValue([] as never); + // Every row handed to the writer comes back inserted, keyed by its id. + txClient.workout.createManyAndReturn.mockImplementation( + async (args: { + data: Array> | Record; + }) => { + const rows = Array.isArray(args.data) ? args.data : [args.data]; + return rows.map((row, i) => ({ + id: `w-${i}-${String(row.externalId ?? "null")}`, + source: row.source, + externalId: row.externalId ?? null, + })); + }, + ); + txClient.workoutRoute.createMany.mockResolvedValue({ count: 0 }); + txClient.workoutSamples.createMany.mockResolvedValue({ count: 0 }); +}); + +describe("POST /api/workouts/batch — unstable external ids", () => { + it("skips the poisoned workout and still writes the two good ones", async () => { + const data = await entriesFor([ + workout("8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", 0), + workout("", 40), + workout("3f0c4e4d-9c1e-4b7e-2a1d-6f30ad2a9cb1", 80), + ]); + expect(data.entries[0].status).toBe("inserted"); + expect(data.entries[1]).toMatchObject({ + index: 1, + status: "skipped", + reason: "unstable_external_id", + }); + expect(data.entries[2].status).toBe("inserted"); + expect(data.skipped).toEqual([ + { index: 1, reason: "unstable_external_id" }, + ]); + // The writer only ever saw the two good rows. + const written = txClient.workout.createManyAndReturn.mock.calls.flatMap( + (call) => { + const arg = call[0] as { data: unknown }; + return Array.isArray(arg.data) ? arg.data : [arg.data]; + }, + ) as Array<{ externalId: string | null }>; + expect(written).toHaveLength(2); + expect(written.map((r) => r.externalId)).not.toContain( + "", + ); + }); + + it("skips a bare memory-address id", async () => { + const data = await entriesFor([workout("0x126b25160")]); + expect(data.entries[0]).toMatchObject({ + status: "skipped", + reason: "unstable_external_id", + }); + expect(txClient.workout.createManyAndReturn).not.toHaveBeenCalled(); + }); + + it("never turns one poisoned entry into a whole-batch refusal", async () => { + const res = await POST( + postReq({ workouts: [workout("0xdeadbeef"), workout("hk-uuid-41", 40)] }), + ); + expect(res.status).toBe(200); + }); + + it("accepts the HealthKit workout UUID the iOS sync really sends", async () => { + const data = await entriesFor([ + workout("8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30"), + ]); + expect(data.entries[0].status).toBe("inserted"); + expect(txClient.workout.createManyAndReturn).toHaveBeenCalled(); + }); + + it("leaves a workout with no externalId alone — NULL is legitimately distinct", async () => { + const noId: Record = { ...workout("unused") }; + delete noId.externalId; + const data = await entriesFor([noId]); + expect(data.entries[0].status).toBe("inserted"); + }); + + it("annotates the refusal with the shape, never the id", async () => { + await entriesFor([ + workout("", 0), + workout("0xdeadbeef", 40), + ]); + const calls = vi.mocked(annotate).mock.calls.map(([c]) => c); + const rejection = calls.find( + (c) => c?.meta && "external_id_rejected" in c.meta, + ); + expect(rejection?.meta).toMatchObject({ + external_id_rejected: 2, + external_id_shapes: "object_description,pointer_address", + external_id_surface: "workout.batch", + }); + expect(JSON.stringify(rejection)).not.toContain("0x12568db80"); + }); +}); diff --git a/src/lib/import/__tests__/csv-measurements.test.ts b/src/lib/import/__tests__/csv-measurements.test.ts index bfc1f7104..e161af279 100644 --- a/src/lib/import/__tests__/csv-measurements.test.ts +++ b/src/lib/import/__tests__/csv-measurements.test.ts @@ -202,3 +202,59 @@ describe("parseCsvMeasurements — optional columns", () => { expect(out.rows[0].row?.externalId).toBeUndefined(); }); }); + +describe("parseCsvMeasurements — external-id stability floor", () => { + /** + * The CSV route upserts on `(userId, type, source=IMPORT, externalId)`, + * so an id that cannot be stable between two exports re-imports as a + * fresh row every time instead of converging on the one it already + * wrote. This importer has no Zod field to hang a refine on — its + * contract is a per-row `reason`, so the refusal lands there and every + * other row still parses. + */ + it("skips a row whose externalId is an object description", () => { + const out = parse([ + "WEIGHT,80,kg,2026-05-01T08:00:00Z,,,", + ]); + expect(out.rows[0]).toMatchObject({ + status: "skipped", + reason: "unstable_external_id", + }); + expect(out.rows[0].row).toBeUndefined(); + }); + + it("skips a row whose externalId is a bare memory address", () => { + expect( + parse(["WEIGHT,80,kg,2026-05-01T08:00:00Z,,,0x126b25160"]).rows[0], + ).toMatchObject({ status: "skipped", reason: "unstable_external_id" }); + }); + + it("skips only the offending row — the rest of the file still imports", () => { + const out = parse([ + "WEIGHT,80,kg,2026-05-01T08:00:00Z,,,scale-row-42", + "WEIGHT,81,kg,2026-05-02T08:00:00Z,,,", + "WEIGHT,82,kg,2026-05-03T08:00:00Z,,,scale-row-43", + ]); + expect(out.rows.map((r) => r.status)).toEqual(["ok", "skipped", "ok"]); + expect(out.rows[1].reason).toBe("unstable_external_id"); + expect(out.rows[0].row?.externalId).toBe("scale-row-42"); + expect(out.rows[2].row?.externalId).toBe("scale-row-43"); + expect(out.fatal).toBeUndefined(); + }); + + it("accepts the spreadsheet / meter-export id shapes a real file carries", () => { + const out = parse([ + "WEIGHT,80,kg,2026-05-01T08:00:00Z,,,scale-row-42", + "WEIGHT,81,kg,2026-05-02T08:00:00Z,,,8AD2A9CB-3F0C-4E4D-9C1E-4B7E2A1D6F30", + "WEIGHT,82,kg,2026-05-03T08:00:00Z,,,stats:HKQuantityTypeIdentifierBodyMass:2026-05-03", + "WEIGHT,83,kg,2026-05-04T08:00:00Z,,,Morning Weigh-In 4", + ]); + expect(out.rows.every((r) => r.status === "ok")).toBe(true); + }); + + it("leaves a row with no externalId alone — an absent id is not a blank one", () => { + const out = parse(["WEIGHT,80,kg,2026-05-01T08:00:00Z,,,"]); + expect(out.rows[0].status).toBe("ok"); + expect(out.rows[0].row?.externalId).toBeUndefined(); + }); +}); diff --git a/src/lib/mcp/__tests__/writes.test.ts b/src/lib/mcp/__tests__/writes.test.ts index 78f7e7213..6772d4f77 100644 --- a/src/lib/mcp/__tests__/writes.test.ts +++ b/src/lib/mcp/__tests__/writes.test.ts @@ -389,3 +389,79 @@ describe("logMcpBloodPressure", () => { } }); }); + +describe("MCP writes — idempotency-key stability floor", () => { + /** + * The MCP key is hashed into the `externalId` half of + * `(userId, type, MCP, externalId)` and matched forever, so it is an + * identity. The hash launders the shape — a pointer address and a real + * key both digest to clean hex — so the check has to run on the INPUT, + * before `mcpExternalId`. + * + * The blank case is the live one: every tool handler falls back to `""` + * when the host omits the argument, and `""` digests to a CONSTANT, so + * without this floor every keyless write after the first would report + * `already_logged` and be silently dropped. + */ + it.each([ + ["blank", ""], + ["whitespace", " "], + ["pointer address", "0x12568db80"], + ["object description", ""], + ])("refuses a %s measurement key before it is hashed", async (_l, key) => { + const result = await logMcpMeasurement({ + userId: "u-1", + type: "WEIGHT", + value: 80, + idempotencyKey: key, + }); + expect(result.status).toBe("unstable_idempotency_key"); + expect(measurement.findUnique).not.toHaveBeenCalled(); + expect(measurement.create).not.toHaveBeenCalled(); + }); + + it("refuses a blank mood key instead of collapsing onto one row", async () => { + const result = await logMcpMood({ + userId: "u-1", + score: 4, + idempotencyKey: "", + }); + expect(result.status).toBe("unstable_idempotency_key"); + expect(moodEntry.create).not.toHaveBeenCalled(); + }); + + it("refuses a blank blood-pressure key instead of collapsing onto one row", async () => { + const result = await logMcpBloodPressure({ + userId: "u-1", + systolic: 120, + diastolic: 80, + idempotencyKey: "", + }); + expect(result.status).toBe("unstable_idempotency_key"); + expect($transaction).not.toHaveBeenCalled(); + }); + + it("carries a message naming the field a host has to fix", async () => { + const result = await logMcpMeasurement({ + userId: "u-1", + type: "WEIGHT", + value: 80, + idempotencyKey: "", + }); + expect(result).toMatchObject({ status: "unstable_idempotency_key" }); + if (result.status !== "unstable_idempotency_key") return; + expect(result.reason).toContain("idempotency key"); + expect(result.reason).toContain("stable across app restarts"); + }); + + it("still accepts a normal host-chosen key", async () => { + const result = await logMcpMeasurement({ + userId: "u-1", + type: "WEIGHT", + value: 80, + idempotencyKey: "weight-2026-07-25-morning", + }); + expect(result.status).toBe("written"); + expect(measurement.create).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/mcp/write-tools.ts b/src/lib/mcp/write-tools.ts index 76e1e49c0..f6ae24c4e 100644 --- a/src/lib/mcp/write-tools.ts +++ b/src/lib/mcp/write-tools.ts @@ -268,6 +268,13 @@ const WRITE_TOOL_DEFINITIONS: McpToolDefinition[] = [ idempotencyKey, }); + if (result.status === "unstable_idempotency_key") { + return { + written: false, + error: "unstable_idempotency_key", + reason: result.reason, + }; + } if (result.status === "unsupported_type") { return { written: false, error: "unsupported_type" }; } @@ -344,6 +351,13 @@ const WRITE_TOOL_DEFINITIONS: McpToolDefinition[] = [ idempotencyKey, }); + if (result.status === "unstable_idempotency_key") { + return { + written: false, + error: "unstable_idempotency_key", + reason: result.reason, + }; + } if (result.status === "invalid_score") { return { written: false, error: "invalid_score" }; } @@ -429,6 +443,13 @@ const WRITE_TOOL_DEFINITIONS: McpToolDefinition[] = [ idempotencyKey, }); + if (result.status === "unstable_idempotency_key") { + return { + written: false, + error: "unstable_idempotency_key", + reason: result.reason, + }; + } if (result.status === "out_of_range") { return { written: false, error: "out_of_range", reason: result.reason }; } diff --git a/src/lib/mcp/writes.ts b/src/lib/mcp/writes.ts index f5ef982cb..83183d2db 100644 --- a/src/lib/mcp/writes.ts +++ b/src/lib/mcp/writes.ts @@ -41,6 +41,12 @@ import { recomputeMoodBucketsForEntry } from "@/lib/rollups/mood-rollups"; import { MOOD_ENUM_BY_SCORE } from "@/lib/mood/labels"; import { getScoreForMood } from "@/lib/validations/mood"; import { moodDateKey, DEFAULT_TIMEZONE } from "@/lib/mood/date-key"; +import { + classifyExternalId, + unstableExternalIdMessage, + unstableExternalIdMeta, + IDEMPOTENCY_KEY_LABEL, +} from "@/lib/validations/external-id"; /** The MCP write provenance — distinct from MANUAL / TELEGRAM. */ const MCP_SOURCE = "MCP" as const; @@ -52,6 +58,41 @@ const MCP_SOURCE = "MCP" as const; * `(userId, type, MCP, externalId)` so a key can never collide with a * Telegram or manual row. */ +/** + * The MCP write key is an IDENTITY, not a windowed replay token: it is + * hashed into the `externalId` half of `(userId, type, MCP, externalId)` + * and matched forever. A key that changes between the preview call and + * the commit call — or between two commits of the same logical write — + * mints a second row instead of converging. + * + * The hash launders the shape, so the floor has to run on the INPUT: a + * pointer-shaped key and a stable one both digest to clean hex, and the + * `externalId` guard downstream could never tell them apart. The blank + * case is the live one — every tool handler falls back to `""` when the + * host omits the argument, and `""` digests to a CONSTANT, so every + * keyless write after the first would report `already_logged` and be + * silently dropped. + */ +function checkMcpIdempotencyKey( + tool: string, + idempotencyKey: string, +): { status: "unstable_idempotency_key"; reason: string } | null { + const shape = classifyExternalId(idempotencyKey); + if (shape === null) return null; + annotate({ + action: { name: "mcp.tool.write" }, + meta: { + tool, + status: "unstable_idempotency_key", + ...unstableExternalIdMeta(`mcp.${tool}`, [shape]), + }, + }); + return { + status: "unstable_idempotency_key", + reason: unstableExternalIdMessage(shape, IDEMPOTENCY_KEY_LABEL), + }; +} + function mcpExternalId( prefix: "measure" | "mood" | "bp", idempotencyKey: string, @@ -186,6 +227,7 @@ export type McpMeasurementResult = | { status: "unsupported_type" } | { status: "unsupported_unit" } | { status: "out_of_range"; reason: string } + | { status: "unstable_idempotency_key"; reason: string } | { status: "written"; measurement: McpMeasurementRecord } | { status: "already_logged"; measurement: McpMeasurementRecord }; @@ -205,6 +247,12 @@ export async function logMcpMeasurement(input: { measuredAt?: Date; idempotencyKey: string; }): Promise { + const unstableKey = checkMcpIdempotencyKey( + "log_measurement", + input.idempotencyKey, + ); + if (unstableKey) return unstableKey; + // Normalise the caller's unit hint to canonical (converting the value) or // refuse it — a non-canonical unit must never be persisted verbatim. Runs // BEFORE the range check so the plausibility gate sees the canonical value @@ -337,6 +385,7 @@ export interface McpBloodPressureRecord { export type McpBloodPressureResult = | { status: "out_of_range"; reason: string } + | { status: "unstable_idempotency_key"; reason: string } | { status: "written"; bloodPressure: McpBloodPressureRecord } | { status: "already_logged"; bloodPressure: McpBloodPressureRecord }; @@ -359,6 +408,12 @@ export async function logMcpBloodPressure(input: { measuredAt?: Date; idempotencyKey: string; }): Promise { + const unstableKey = checkMcpIdempotencyKey( + "log_blood_pressure", + input.idempotencyKey, + ); + if (unstableKey) return unstableKey; + // Same validation the confirm-gate preview runs (both ranges, the systolic > // diastolic guard, and the measuredAt instant bound) — one verdict for both. const check = checkMcpBloodPressure( @@ -514,6 +569,7 @@ export interface McpMoodRecord { export type McpMoodResult = | { status: "invalid_score" } + | { status: "unstable_idempotency_key"; reason: string } | { status: "written"; moodEntry: McpMoodRecord } | { status: "already_logged"; moodEntry: McpMoodRecord }; @@ -531,6 +587,9 @@ export async function logMcpMood(input: { tz?: string | null; idempotencyKey: string; }): Promise { + const unstableKey = checkMcpIdempotencyKey("log_mood", input.idempotencyKey); + if (unstableKey) return unstableKey; + const mood = MOOD_ENUM_BY_SCORE[input.score]; if (!mood) { annotate({ diff --git a/src/lib/validations/external-id.ts b/src/lib/validations/external-id.ts index baee1f8a1..f4331085c 100644 --- a/src/lib/validations/external-id.ts +++ b/src/lib/validations/external-id.ts @@ -96,19 +96,43 @@ export function isStableExternalId(value: string): boolean { return classifyExternalId(value) === null; } -const SHAPE_MESSAGES: Record = { - blank: "external identifier must not be empty", - pointer_address: - "external identifier must be stable across app restarts; received a memory address (0x…), which is a new value on every launch and mints a duplicate record on every sync", - object_description: - "external identifier must be stable across app restarts; received an object description (), which embeds a memory address and is a new value on every launch — serialise the identity itself, not the object's description", -}; +/** + * How the offending field is named back to the caller. The rule is about + * the VALUE, so it applies to every client-supplied string this system + * later matches on as an identity — not only `externalId`. + */ +export const EXTERNAL_ID_LABEL = "external identifier"; +export const IDEMPOTENCY_KEY_LABEL = "idempotency key"; /** The 422 message a client developer reads. Names the shape received. */ export function unstableExternalIdMessage( shape: UnstableExternalIdShape, + label: string = EXTERNAL_ID_LABEL, ): string { - return SHAPE_MESSAGES[shape]; + switch (shape) { + case "blank": + return `${label} must not be empty`; + case "pointer_address": + return `${label} must be stable across app restarts; received a memory address (0x…), which is a new value on every launch and mints a duplicate record on every sync`; + case "object_description": + return `${label} must be stable across app restarts; received an object description (), which embeds a memory address and is a new value on every launch — serialise the identity itself, not the object's description`; + } +} + +function assertStableIdentityField( + raw: unknown, + ctx: z.RefinementCtx, + field: string, + label: string, +): void { + if (typeof raw !== "string") return; + const shape = classifyExternalId(raw); + if (shape === null) return; + ctx.addIssue({ + code: "custom", + path: [field], + message: unstableExternalIdMessage(shape, label), + }); } /** @@ -127,15 +151,42 @@ export function assertStableExternalId( value: { externalId?: string | null }, ctx: z.RefinementCtx, ): void { - const raw = value.externalId; - if (typeof raw !== "string") return; - const shape = classifyExternalId(raw); - if (shape === null) return; - ctx.addIssue({ - code: "custom", - path: ["externalId"], - message: unstableExternalIdMessage(shape), - }); + assertStableIdentityField( + value.externalId, + ctx, + "externalId", + EXTERNAL_ID_LABEL, + ); +} + +/** + * The same floor for a client-supplied `idempotencyKey`. + * + * The HTTP `Idempotency-Key` header is a replay token inside a TTL and is + * deliberately out of scope — `IdempotencyKey.expiresAt` bounds it and a + * cron sweeps it. `MedicationIntakeEvent.idempotency_key` is NOT that: it + * is a `@unique` column persisted with the dose row forever, with no TTL + * and no sweep, matched by plain equality on the per-medication intake + * POST, the bulk intake ingest and the external ingest surface. That makes + * it an identity, and it carries the same failure mode — a value that + * rotates per client launch never matches its own earlier row, so every + * re-sync logs the dose again. + * + * The blank arm matters more here than anywhere else: the column is + * `@unique` across the table, so a second row carrying "" cannot exist, + * and an empty key would otherwise resolve to whichever unrelated row got + * there first. + */ +export function assertStableIdempotencyKey( + value: { idempotencyKey?: string | null }, + ctx: z.RefinementCtx, +): void { + assertStableIdentityField( + value.idempotencyKey, + ctx, + "idempotencyKey", + IDEMPOTENCY_KEY_LABEL, + ); } /** @@ -146,9 +197,10 @@ export function assertStableExternalId( */ export function unstableExternalIdShape( body: unknown, + field: "externalId" | "idempotencyKey" = "externalId", ): UnstableExternalIdShape | null { if (typeof body !== "object" || body === null) return null; - const raw = (body as { externalId?: unknown }).externalId; + const raw = (body as Record)[field]; if (typeof raw !== "string") return null; return classifyExternalId(raw); } diff --git a/src/lib/validations/medication/intake.ts b/src/lib/validations/medication/intake.ts index 5e3a03a49..5a4e1fc3f 100644 --- a/src/lib/validations/medication/intake.ts +++ b/src/lib/validations/medication/intake.ts @@ -1,6 +1,7 @@ import { z } from "zod/v4"; import { boundedTakenAtSchema, injectionSiteEnum } from "./base"; +import { assertStableIdempotencyKey } from "@/lib/validations/external-id"; export const intakeSchema = z .object({ @@ -82,18 +83,29 @@ export const intakeSchema = z "Dose actually consumed for THIS intake when it deviates from (or documents) the medication's configured dose, e.g. a half tablet or a titration step. Free text, max 50 characters. Omit to record the take under the medication's configured dose.", ), }) + // `MedicationIntakeEvent.idempotency_key` is a `@unique` column persisted + // with the dose row forever and matched by equality — an identity, not a + // windowed replay token. A key that rotates per client launch never + // matches its own earlier row, so every re-sync logs the dose again. + .superRefine(assertStableIdempotencyKey) .meta({ id: "MedicationIntakeRequest", description: "Per-medication intake log body. Idempotent via `idempotencyKey`; the server also dedupes by a 60-second sliding window when the key is absent. Non-skipped intakes auto-decrement pen inventory (best-effort), refresh the per-day compliance rollup, and — for one-shot medications — flip `active` to false. The optional `injectionSite` is persisted only for an INJECTION medication with site-tracking enabled and is validated against the medication's effective allowed set (422 on a disallowed value).", }); -export const externalIntakeSchema = z.object({ - medicationName: z.string().min(1).max(200), - // v1.16.9 — same plausibility bounds as the interactive create paths. - takenAt: boundedTakenAtSchema.optional(), - idempotencyKey: z.string().max(128), -}); +export const externalIntakeSchema = z + .object({ + medicationName: z.string().min(1).max(200), + // v1.16.9 — same plausibility bounds as the interactive create paths. + takenAt: boundedTakenAtSchema.optional(), + idempotencyKey: z.string().max(128), + }) + // Same identity floor as the interactive twin. The blank arm carries real + // weight on this surface: the column is `@unique` table-wide, so an empty + // key resolves to whichever unrelated row happened to store "" first + // instead of to this caller's own earlier post. + .superRefine(assertStableIdempotencyKey); export const listIntakeEventsSchema = z.object({ limit: z.coerce.number().int().min(1).max(500).optional().default(25), From 18691d455029d430159a755f3a91e6478f71a6e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 25 Jul 2026 18:41:40 +0200 Subject: [PATCH 4/5] fix(medications): scope the intake idempotency key to its owner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `medication_intake_events.idempotency_key` carried a GLOBAL unique from 0001_init. The value is client-supplied, so two people running the same client can mint the same key — and on a global unique the second one is simply refused by the constraint. Their dose is not written and nothing tells them so. The bulk drain made it worse: its replay probe read the key with no user scope, resolved the first user's row, and reported the second user's entry as a duplicate carrying a stranger's row id. Migration 0273 replaces the global unique with `(user_id, idempotency_key)`, the grain every other client-supplied identity on the table already uses. Both unscoped reads in the bulk drain now key on the composite; the other four reads were already narrowed by user. On existing data this is a relaxation, so nothing needs reconciling: the old constraint is strictly stronger than the new one, and no pair of rows can violate an index the previous one already forbade. The migration still carries a reconciliation step for a database whose global index was dropped by hand at some point. It never deletes or rewrites a dose — it clears the dedup HINT on the later row of a colliding pair and keeps every intake event, timestamp, attribution and inventory stamp intact. The new index is created before the old one is dropped, so a failure aborts with the old protection still in place. A structural guard freezes the read side: no `medicationIntakeEvent` query may filter on the key without narrowing by `userId`. --- .../migration.sql | 47 +++ prisma/schema.prisma | 8 +- .../external-id-stability-guard.test.ts | 76 +++++ src/app/api/medications/intake/bulk/route.ts | 17 +- .../medications-intake-bulk.test.ts | 283 ++++++++++++++++++ 5 files changed, 428 insertions(+), 3 deletions(-) create mode 100644 prisma/migrations/0273_scope_intake_idempotency_key_per_user/migration.sql diff --git a/prisma/migrations/0273_scope_intake_idempotency_key_per_user/migration.sql b/prisma/migrations/0273_scope_intake_idempotency_key_per_user/migration.sql new file mode 100644 index 000000000..c4aba2094 --- /dev/null +++ b/prisma/migrations/0273_scope_intake_idempotency_key_per_user/migration.sql @@ -0,0 +1,47 @@ +-- Scope the medication-intake idempotency key per user. +-- +-- `medication_intake_events.idempotency_key` has carried a GLOBAL unique +-- index since 0001_init. The key is client-supplied, so a global unique +-- means one user's key can block another user's: the second user's write +-- is refused by the constraint and the dose is silently lost, and the +-- bulk route's unscoped replay probe resolves the key to the FIRST user's +-- row and hands its id back to the second user. +-- +-- The correct grain is per user, matching every other client-supplied +-- identity on this table (`(user_id, external_id)`, migration 0239). +-- +-- On existing data this is a RELAXATION: the old constraint is strictly +-- stronger than the new one — if no two rows anywhere share a key, then +-- no two rows of the SAME user share one either — so no row can violate +-- the new index and nothing needs reconciling. Step 1 below is a safety +-- net for a database whose global index was dropped by hand at some +-- point; against an intact schema it matches zero rows. +-- +-- Step 1 never deletes or rewrites a dose. It only clears the dedup HINT +-- on the strictly later row of a colliding pair, keeping every intake +-- event, its timestamps, its attribution and its inventory stamp intact. +-- The cost is that one row loses replay protection it never really had. + +-- 1. Safety net: break any pre-existing same-user key collision by +-- clearing the key on every row but the earliest of its group. +UPDATE "medication_intake_events" AS e +SET "idempotency_key" = NULL +WHERE e."idempotency_key" IS NOT NULL + AND EXISTS ( + SELECT 1 + FROM "medication_intake_events" AS earlier + WHERE earlier."user_id" = e."user_id" + AND earlier."idempotency_key" = e."idempotency_key" + AND ( + earlier."created_at" < e."created_at" + OR (earlier."created_at" = e."created_at" AND earlier."id" < e."id") + ) + ); + +-- 2. Create the per-user unique BEFORE dropping the global one, so a +-- failure here aborts the migration with the old protection intact. +CREATE UNIQUE INDEX "medication_intake_events_user_id_idempotency_key_key" + ON "medication_intake_events" ("user_id", "idempotency_key"); + +-- 3. Retire the global unique. +DROP INDEX IF EXISTS "medication_intake_events_idempotency_key_key"; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index e2d69c31f..fd31c7cf6 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -2757,7 +2757,12 @@ model MedicationIntakeEvent { /// nightly dedup leaves both standalone. attributionSource IntakeAttributionSource @default(AUTO) @map("attribution_source") source IntakeSource @default(WEB) - idempotencyKey String? @unique @map("idempotency_key") + /// v1.32.37 — client-supplied replay anchor, unique PER USER (migration + /// 0273). It carried a GLOBAL unique from 0001_init, which let one + /// user's key refuse another user's write outright and let an unscoped + /// lookup resolve to a stranger's row. Same grain as `(user_id, + /// external_id)` below: every read must key on `(userId, idempotencyKey)`. + idempotencyKey String? @map("idempotency_key") createdAt DateTime @default(now()) @map("created_at") /// v1.4.25 W4d — only populated for GLP-1 intake events (the picker /// is the only surface that writes this column). Existing rows stay @@ -2829,6 +2834,7 @@ model MedicationIntakeEvent { /// unique above, migration 0121), so the schema keeps the full /// `@@unique` for the `userId_externalId` compound where-key. @@unique([userId, externalId]) + @@unique([userId, idempotencyKey]) @@index([userId, medicationId, scheduledFor]) // v1.7.0 sync — keyset support for the `/api/sync/changes` intake feed // (`where userId` ordered by `(updatedAt asc, id asc)`). diff --git a/src/__tests__/external-id-stability-guard.test.ts b/src/__tests__/external-id-stability-guard.test.ts index 88b279fb1..8e3abe525 100644 --- a/src/__tests__/external-id-stability-guard.test.ts +++ b/src/__tests__/external-id-stability-guard.test.ts @@ -298,6 +298,82 @@ describe("identities derived outside a Zod schema stay inside the floor", () => }); }); +describe("the intake idempotency key is never read across users", () => { + /** + * `medication_intake_events.idempotency_key` carried a GLOBAL unique + * from 0001_init until migration 0273. Client-supplied keys on a global + * unique meant one user's key blocked another user's write outright, + * and an unscoped lookup resolved a stranger's row and handed back its + * id. The column is now unique per user; every read has to key on + * `(userId, idempotencyKey)` or the leak comes straight back. + */ + function intakeQueriesFilteringOnTheKey(): Array<{ + file: string; + line: number; + scoped: boolean; + }> { + const out: Array<{ file: string; line: number; scoped: boolean }> = []; + for (const rel of sourceFiles()) { + const source = read(rel); + if (!source.includes("medicationIntakeEvent.")) continue; + const call = /medicationIntakeEvent\.(\w+)\(\s*\{/g; + let m: RegExpExecArray | null; + while ((m = call.exec(source)) !== null) { + // Walk to the matching brace so the window is the call, not a + // fixed slice that could bleed into the next statement. + let depth = 0; + let j = source.indexOf("{", m.index); + for (; j < source.length; j++) { + if (source[j] === "{") depth++; + else if (source[j] === "}" && --depth === 0) break; + } + const body = source.slice(m.index, j + 1); + const whereAt = body.indexOf("where"); + if (whereAt === -1) continue; + const where = body.slice(whereAt, whereAt + 400); + if (!where.includes("idempotencyKey")) continue; + out.push({ + file: rel, + line: source.slice(0, m.index).split("\n").length, + // Keyed on the row's own primary id is scoped by construction — + // that is an update-by-id that happens to WRITE the key. + scoped: where.includes("userId") || /where:\s*\{\s*id:/.test(where), + }); + } + } + return out; + } + + it("finds the reads at all (the matcher still matches)", () => { + const found = intakeQueriesFilteringOnTheKey(); + expect(found.length).toBeGreaterThanOrEqual(6); + expect(found.map((f) => f.file)).toContain( + "app/api/medications/intake/bulk/route.ts", + ); + }); + + it("every query filtering on the key also narrows by userId", () => { + const unscoped = intakeQueriesFilteringOnTheKey().filter((f) => !f.scoped); + expect( + unscoped, + "a medicationIntakeEvent query filters on idempotencyKey without a userId. " + + "The key is client-supplied and unique per USER (migration 0273); an " + + "unscoped read resolves another user's row.", + ).toEqual([]); + }); + + it("the schema keeps the key unique per user, not globally", () => { + const schema = readFileSync( + join(process.cwd(), "prisma", "schema.prisma"), + "utf8", + ); + expect(schema).toContain("@@unique([userId, idempotencyKey])"); + expect(schema).not.toMatch( + /idempotencyKey\s+String\?\s+@unique\s+@map\("idempotency_key"\)/, + ); + }); +}); + describe("the batch surfaces refuse per entry, never whole-batch", () => { // Each of these drains a client sync queue. A schema-level refusal here // would stop a whole batch of good rows because of one bad one, which is diff --git a/src/app/api/medications/intake/bulk/route.ts b/src/app/api/medications/intake/bulk/route.ts index c969f17da..996cab697 100644 --- a/src/app/api/medications/intake/bulk/route.ts +++ b/src/app/api/medications/intake/bulk/route.ts @@ -616,8 +616,16 @@ async function postBulk(request: NextRequest): Promise { // by `source`. The partial unique index carries `source`, so it // cannot catch this duplicate; the probe must. if (entry.idempotencyKey) { + // Scoped to the caller. The key is client-supplied, so an + // unscoped read would resolve another user's row and hand its + // id back here (migration 0273 made the unique per user). const replay = await prisma.medicationIntakeEvent.findUnique({ - where: { idempotencyKey: entry.idempotencyKey }, + where: { + userId_idempotencyKey: { + userId: user.id, + idempotencyKey: entry.idempotencyKey, + }, + }, select: { id: true }, }); if (replay) { @@ -793,7 +801,12 @@ async function postBulk(request: NextRequest): Promise { ) { const existing = entry.idempotencyKey ? await prisma.medicationIntakeEvent.findUnique({ - where: { idempotencyKey: entry.idempotencyKey }, + where: { + userId_idempotencyKey: { + userId: user.id, + idempotencyKey: entry.idempotencyKey, + }, + }, select: { id: true }, }) : // v1.28 — an Apple entry that raced the partial diff --git a/tests/integration/medications-intake-bulk.test.ts b/tests/integration/medications-intake-bulk.test.ts index 8feac204a..886e2dc90 100644 --- a/tests/integration/medications-intake-bulk.test.ts +++ b/tests/integration/medications-intake-bulk.test.ts @@ -483,4 +483,287 @@ describe("POST /api/medications/intake/bulk (real Postgres)", () => { }); expect(stored).toHaveLength(1); }); + + /** + * Migration 0273 — the idempotency key is unique PER USER. + * + * It carried a GLOBAL unique from 0001_init. Because the key is + * client-supplied, two people running the same client could mint the + * same one, and then the global constraint refused the second person's + * write outright: their dose was silently lost. The unscoped replay + * probe made it worse by resolving the key to the FIRST user's row and + * handing that stranger's row id back. + * + * These two cases are the whole contract: different users must not + * collide, and one user replaying their own key must still dedup. + */ + it("lets two different users post the SAME idempotency key — both doses land", async () => { + const { POST } = await import("@/app/api/medications/intake/bulk/route"); + const prisma = getPrismaClient(); + const SHARED_KEY = "ios-outbox-2026-05-16T08:00"; + + // User A posts first. + const first = await POST( + makeRequest({ + entries: [ + { + medicationId: ownedMedId, + scheduledFor: "2026-05-16T08:00:00.000Z", + takenAt: "2026-05-16T08:02:00.000Z", + idempotencyKey: SHARED_KEY, + }, + ], + }), + ); + expect(first.status).toBe(200); + expect( + ((await first.json()) as { data: { inserted: number } }).data.inserted, + ).toBe(1); + + // Now the OTHER user, with their own session and their own medication, + // posts the very same key. + const otherSession = await prisma.session.create({ + data: { + userId: OTHER_USER_ID, + expiresAt: new Date(Date.now() + 365 * 24 * 60 * 60 * 1000), + }, + }); + cookieJar.set("healthlog_session", otherSession.id); + + const second = await POST( + makeRequest({ + entries: [ + { + medicationId: foreignMedId, + scheduledFor: "2026-05-16T08:00:00.000Z", + takenAt: "2026-05-16T08:03:00.000Z", + idempotencyKey: SHARED_KEY, + }, + ], + }), + ); + expect(second.status).toBe(200); + const secondJson = (await second.json()) as { + data: { + inserted: number; + duplicates: number; + entries: Array<{ status: string; id?: string }>; + }; + }; + // The second user's dose LANDS. Pre-0273 it was reported `duplicate` + // against the first user's row and never written. + expect(secondJson.data.inserted).toBe(1); + expect(secondJson.data.duplicates).toBe(0); + + const mine = await prisma.medicationIntakeEvent.findMany({ + where: { userId: OTHER_USER_ID }, + select: { id: true, userId: true, idempotencyKey: true }, + }); + const theirs = await prisma.medicationIntakeEvent.findMany({ + where: { userId: TEST_USER_ID }, + select: { id: true }, + }); + expect(mine).toHaveLength(1); + expect(theirs).toHaveLength(1); + expect(mine[0].idempotencyKey).toBe(SHARED_KEY); + // And no id belonging to the first user was echoed to the second. + const echoed = secondJson.data.entries[0]?.id; + if (echoed) expect(echoed).not.toBe(theirs[0].id); + }); + + it("still dedups when the SAME user replays their own key", async () => { + const { POST } = await import("@/app/api/medications/intake/bulk/route"); + const prisma = getPrismaClient(); + const KEY = "ios-outbox-replay-001"; + const body = { + entries: [ + { + medicationId: ownedMedId, + scheduledFor: "2026-05-18T08:00:00.000Z", + takenAt: "2026-05-18T08:02:00.000Z", + idempotencyKey: KEY, + }, + ], + }; + + const first = await POST(makeRequest(body)); + expect( + ((await first.json()) as { data: { inserted: number } }).data.inserted, + ).toBe(1); + + const replay = await POST(makeRequest(body)); + const replayJson = (await replay.json()) as { + data: { + inserted: number; + duplicates: number; + entries: Array<{ status: string; id?: string }>; + }; + }; + expect(replayJson.data.inserted).toBe(0); + expect(replayJson.data.duplicates).toBe(1); + + const stored = await prisma.medicationIntakeEvent.findMany({ + where: { userId: TEST_USER_ID, idempotencyKey: KEY }, + select: { id: true }, + }); + expect(stored).toHaveLength(1); + // The replay resolves to the caller's OWN row. + expect(replayJson.data.entries[0]?.id).toBe(stored[0].id); + }); + + it("holds the per-user unique at the database, not only in the route", async () => { + const prisma = getPrismaClient(); + const KEY = "db-level-shared-key"; + + await prisma.medicationIntakeEvent.create({ + data: { + userId: TEST_USER_ID, + medicationId: ownedMedId, + scheduledFor: new Date("2026-05-19T08:00:00.000Z"), + takenAt: new Date("2026-05-19T08:00:00.000Z"), + source: "WEB", + idempotencyKey: KEY, + }, + }); + + // Same key, different user — allowed now, refused before 0273. + await expect( + prisma.medicationIntakeEvent.create({ + data: { + userId: OTHER_USER_ID, + medicationId: foreignMedId, + scheduledFor: new Date("2026-05-19T08:00:00.000Z"), + takenAt: new Date("2026-05-19T08:00:00.000Z"), + source: "WEB", + idempotencyKey: KEY, + }, + }), + ).resolves.toBeTruthy(); + + // Same key, SAME user — still refused. + await expect( + prisma.medicationIntakeEvent.create({ + data: { + userId: TEST_USER_ID, + medicationId: ownedMedId, + scheduledFor: new Date("2026-05-20T08:00:00.000Z"), + takenAt: new Date("2026-05-20T08:00:00.000Z"), + source: "WEB", + idempotencyKey: KEY, + }, + }), + ).rejects.toMatchObject({ code: "P2002" }); + }); + + /** + * Migration 0273 step 1 — the reconciliation safety net. + * + * Against an intact database it can never fire: the global unique it + * replaces is strictly stronger than the per-user one, so no colliding + * pair can exist. It is there for a database whose global index was + * dropped by hand at some point. That means nothing else in this suite + * exercises it, so this case builds the collision on purpose and runs + * the SHIPPED SQL — read from the migration file, not a copy that could + * drift — against it. + * + * The whole thing runs inside a transaction that is rolled back, so the + * index surgery never escapes into the rest of the run. + */ + it("migration 0273 reconciles a pre-existing same-user collision without losing a dose", async () => { + const prisma = getPrismaClient(); + const { readFileSync } = await import("node:fs"); + const { join } = await import("node:path"); + + const sql = readFileSync( + join( + process.cwd(), + "prisma", + "migrations", + "0273_scope_intake_idempotency_key_per_user", + "migration.sql", + ), + "utf8", + ); + // Strip comments BEFORE splitting: the prose contains semicolons of + // its own, and splitting first would slice a statement in half. + const statements = sql + .split("\n") + .filter((line) => !line.trim().startsWith("--")) + .join("\n") + .split(";") + .map((part) => part.trim()) + .filter(Boolean); + const safetyNet = statements.find((st) => st.startsWith("UPDATE")); + const createIndex = statements.find((st) => + st.startsWith("CREATE UNIQUE INDEX"), + ); + expect( + safetyNet, + "migration must carry the reconciliation UPDATE", + ).toBeTruthy(); + expect(createIndex, "migration must carry the per-user index").toBeTruthy(); + + const COLLIDING_KEY = "legacy-collision-key"; + let observed: Array<{ id: string; idempotencyKey: string | null }> = []; + + class Rollback extends Error {} + await expect( + prisma.$transaction(async (tx) => { + // Simulate the only database on which the collision is possible: + // one whose unique index was dropped by hand. + await tx.$executeRawUnsafe( + 'DROP INDEX "medication_intake_events_user_id_idempotency_key_key"', + ); + + const earlier = await tx.medicationIntakeEvent.create({ + data: { + id: "collide-earlier", + userId: TEST_USER_ID, + medicationId: ownedMedId, + scheduledFor: new Date("2026-05-21T08:00:00.000Z"), + takenAt: new Date("2026-05-21T08:00:00.000Z"), + source: "WEB", + idempotencyKey: COLLIDING_KEY, + createdAt: new Date("2026-05-21T08:00:00.000Z"), + }, + }); + const later = await tx.medicationIntakeEvent.create({ + data: { + id: "collide-later", + userId: TEST_USER_ID, + medicationId: ownedMedId, + scheduledFor: new Date("2026-05-22T08:00:00.000Z"), + takenAt: new Date("2026-05-22T08:00:00.000Z"), + source: "WEB", + idempotencyKey: COLLIDING_KEY, + createdAt: new Date("2026-05-22T08:00:00.000Z"), + }, + }); + + // Run the migration's own reconciliation, then its own index. + await tx.$executeRawUnsafe(safetyNet!); + await tx.$executeRawUnsafe(createIndex!); + + observed = await tx.medicationIntakeEvent.findMany({ + where: { id: { in: [earlier.id, later.id] } }, + orderBy: { createdAt: "asc" }, + select: { id: true, idempotencyKey: true }, + }); + throw new Rollback(); + }), + ).rejects.toBeInstanceOf(Rollback); + + // Both doses survive — the migration never deletes or rewrites a row. + expect(observed).toHaveLength(2); + expect(observed[0]).toEqual({ + id: "collide-earlier", + idempotencyKey: COLLIDING_KEY, + }); + // Only the dedup HINT on the later row is cleared, which is what makes + // the unique index creatable. + expect(observed[1]).toEqual({ + id: "collide-later", + idempotencyKey: null, + }); + }); }); From 132238fd05855117e7970084a1324446f89c588a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 25 Jul 2026 19:14:30 +0200 Subject: [PATCH 5/5] =?UTF-8?q?chore(release):=20v1.32.37=20=E2=80=94=20re?= =?UTF-8?q?fuse=20an=20identity=20that=20cannot=20be=20stable,=20and=20sco?= =?UTF-8?q?pe=20the=20intake=20replay=20key=20to=20its=20owner?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 30 ++++++++++++++++++++++++++++++ docs/api/openapi.yaml | 2 +- package.json | 2 +- public/sw.js | 2 +- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01e58b020..1e658029f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ ## [Unreleased] +## [1.32.37] — 2026-07-25 + +Two ways a record could quietly lose data, both closed. + +An identifier that a client sends along with a reading is meant to say "this is +the same reading you already have". One client was sending a value that changed +every time the app restarted, so nothing ever matched, and every sync created a +fresh copy. On one instance that produced 23 medications nobody had ever taken, +none of them with a dose or a single logged intake, because the doses were +looking for a medication under an identifier that no longer existed. + +The server now refuses an identifier that cannot possibly be stable, with a 422 +that says what is wrong instead of quietly making duplicates. A batch rejects +only the offending entry and still accepts the rest, so one bad row cannot stop +a sync. Twelve entry points check it. The identifier shapes that real apps and +devices actually send are pinned by tests, so a future rule cannot get greedy +and start dropping valid data. + +The second one was worse and had no symptom at all. The repeat-protection key +on a medication intake was unique across the whole database rather than per +account. Two people generating the same key independently meant the second one +could not save their dose at all. The database refused it, and the intake was +gone. The key is now unique per account, and every lookup asks who it belongs +to. Recording an intake through the automation interface without sending a key +had a related problem: the first one saved and every one after it was discarded +as a repeat. Also fixed. + +The migration does not touch any existing intake. It relaxes a rule rather than +tightening one, so nothing that was allowed before becomes disallowed now. + ## [1.32.36] — 2026-07-25 Nothing here changes what you see. This release is about the checks that run diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index e0fe37c3e..227c94387 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: HealthLog API - version: 1.32.36 + version: 1.32.37 description: >- Self-hosted personal-health-tracking PWA — public API surface for the iOS native client and external ingest. diff --git a/package.json b/package.json index 84acc9630..374ad5de4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "healthlog", - "version": "1.32.36", + "version": "1.32.37", "description": "Self-hosted personal-health-tracking PWA with Withings integration, AI insights, and doctor-report PDF export.", "license": "PolyForm-Noncommercial-1.0.0", "homepage": "https://healthlog.dev", diff --git a/public/sw.js b/public/sw.js index c153bf4ea..90b4ee924 100644 --- a/public/sw.js +++ b/public/sw.js @@ -36,7 +36,7 @@ try { // v1.4.38.4 → v1.4.42. Do not hand-edit; bump `package.json` and rebuild. const CACHE_VERSION = (typeof self !== "undefined" && self.__APP_VERSION__) || - /* @sw-version-fallback */ "v1.32.36"; + /* @sw-version-fallback */ "v1.32.37"; const STATIC_CACHE = `healthlog-static-${CACHE_VERSION}`; const PAGE_CACHE = `healthlog-pages-${CACHE_VERSION}`; // v1.18.6 — read-only data cache for a curated allowlist of safe GET `/api/*`