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(),