mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 05:12:14 +00:00
fix(medications): scope the intake idempotency key to its owner
`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`.
This commit is contained in:
1 parent
15b5f6f92f
commit
18691d4550
5 files changed
+428
-3
No files matched your search
@@ -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";
|
||||
@@ -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)`).
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -616,8 +616,16 @@ async function postBulk(request: NextRequest): Promise<Response> {
|
||||
// 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<Response> {
|
||||
) {
|
||||
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
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user