mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
Close the way round the delegated-intake refusal
v1.36.1 stopped a delegate changing a dose the owner had already recorded, and it stopped them on one route. The sibling `POST /api/medications/[id]/intake` reaches the same rows through the shared slot upsert, where the only existing protection is against a pending sync echo washing out a recorded dose. An explicit skip is not an echo: it fell through, flipped the outcome, and the inventory the take had consumed was refunded on the way. So the refusal I added was a door with the next one left open. The rule compares OUTCOMES rather than refusing every write onto an actioned slot, and the reason is the third case in the new contract. A double tap, an offline replay and a retry after a partial batch all re-post the same decision, and a caregiver hits all three; refusing those would make the feature unreliable exactly when somebody is standing in a kitchen with a pill box. Same decision applies, a different one is refused. The check rides into `applyCanonicalSlotWrite` rather than sitting at the call site, because that is where the existing row is already loaded: no extra query, and nothing between the read and the update for a concurrent write to slip through. A refused attempt files under the owner with the same action name the sibling route uses, so it reaches the activity panel the same way. Three integration cases, and each asserts the row rather than the status alone — a 403 that had already written the flip would read as protection while being the opposite. Watched fail: with the delegate flag forced off, the flip returns 201 while the replay and the owner's own correction stay green.
This commit is contained in:
1 parent
25e86b36b3
commit
24922b2787
3 files changed
+199
No files matched your search
@@ -52,6 +52,38 @@ async function postIntake(request: NextRequest, { params }: RouteParams) {
|
||||
// `user` is the record the dose belongs to; `actor` is whoever pressed the
|
||||
// button, and is used for nothing but the owner's notification below.
|
||||
const { user, actor, authMethod } = await requireRecordAuth("write");
|
||||
|
||||
// v1.36.1 follow-up — the sibling route `POST /api/medications/intake`
|
||||
// refuses a delegate changing a dose the owner already recorded. This route
|
||||
// reaches the same rows through the slot upsert, so without the same rule it
|
||||
// is simply the way round the other one: an explicit skip posted onto a
|
||||
// taken slot flips the outcome and refunds the inventory the take consumed.
|
||||
// The flag rides into `applyCanonicalSlotWrite`, which is where the existing
|
||||
// row is already loaded, so the check adds no query and cannot race.
|
||||
const isDelegated = actor.id !== user.id;
|
||||
|
||||
/**
|
||||
* The refusal itself, shared by both upsert call sites so the two cannot
|
||||
* drift apart. Files under the OWNER — `auditLog` stamps the actor from the
|
||||
* request's acting context — so a refused attempt reaches the activity panel
|
||||
* the same way the sibling route's does, under the same action name.
|
||||
*/
|
||||
function refuseOutcomeChange(): Response {
|
||||
annotate({
|
||||
action: { name: "medications.intake.update.refused" },
|
||||
meta: { reason: "resolved_event" },
|
||||
});
|
||||
void auditLog("medications.intake.update.refused", {
|
||||
userId: user.id,
|
||||
ipAddress: getClientIp(request),
|
||||
details: { medicationId: id, reason: "resolved_event" },
|
||||
});
|
||||
return apiError(
|
||||
"Changing a dose that is already recorded is not part of shared access",
|
||||
403,
|
||||
{ errorCode: "sharing.not_permitted" },
|
||||
);
|
||||
}
|
||||
// v1.32.8 (iOS #64) — write provenance derived from the transport, never
|
||||
// client-asserted: a Bearer/native call stores `API`, a cookie/browser call
|
||||
// stores `WEB`. The other `IntakeSource` values are producer-owned by OTHER
|
||||
@@ -334,6 +366,7 @@ async function postIntake(request: NextRequest, { params }: RouteParams) {
|
||||
isExplicitSkip,
|
||||
idempotencyKey: idempotencyKey ?? null,
|
||||
createSource: intakeSource,
|
||||
refuseOutcomeChange: isDelegated,
|
||||
// v1.8.5 — resolved + validated site (null unless a tracking-on
|
||||
// injection taken write supplied an allowed site).
|
||||
injectionSite: resolvedInjectionSite,
|
||||
@@ -342,6 +375,7 @@ async function postIntake(request: NextRequest, { params }: RouteParams) {
|
||||
// carried one).
|
||||
doseTaken: resolvedDoseTaken,
|
||||
});
|
||||
if (applied.outcomeChangeRefused) return refuseOutcomeChange();
|
||||
event = applied.row;
|
||||
consumedTransition = applied.consumedTransition;
|
||||
// Reset the snooze when a dose is actually recorded (not on a
|
||||
@@ -400,10 +434,12 @@ async function postIntake(request: NextRequest, { params }: RouteParams) {
|
||||
isExplicitSkip,
|
||||
idempotencyKey: idempotencyKey ?? null,
|
||||
createSource: intakeSource,
|
||||
refuseOutcomeChange: isDelegated,
|
||||
injectionSite: resolvedInjectionSite,
|
||||
attributionSource,
|
||||
doseTaken: resolvedDoseTaken,
|
||||
});
|
||||
if (applied.outcomeChangeRefused) return refuseOutcomeChange();
|
||||
event = applied.row;
|
||||
consumedTransition = applied.consumedTransition;
|
||||
if (!skipped && !applied.noDowngradeNoOp) {
|
||||
|
||||
@@ -61,6 +61,24 @@ export interface ApplyCanonicalSlotWriteInput {
|
||||
* (v1.28) is the bulk route's HealthKit dose-event import.
|
||||
*/
|
||||
createSource: "WEB" | "API" | "REMINDER" | "APPLE_HEALTH";
|
||||
/**
|
||||
* v1.36.x — refuse a write that would change an outcome the owner has
|
||||
* already recorded, instead of applying it last-write-wins.
|
||||
*
|
||||
* Set by the intake routes when the caller is a DELEGATE. Marking a dose
|
||||
* is an admitted contribution; changing a decision the owner already made
|
||||
* is an edit, and edits stay with the owner. The sibling route
|
||||
* `POST /api/medications/intake` refuses this in its own handler, which
|
||||
* left this one as the way round it — the flag is here rather than at the
|
||||
* call site because this is where the existing row is already loaded, so
|
||||
* the check costs no extra query and cannot race the update.
|
||||
*
|
||||
* "Change" means a DIFFERENT outcome. Re-posting the same decision still
|
||||
* applies, because a double tap, an offline replay and a retry after a
|
||||
* partial batch failure all look like that, and refusing them would make
|
||||
* the feature unreliable in exactly the situation a caregiver is in.
|
||||
*/
|
||||
refuseOutcomeChange?: boolean;
|
||||
/**
|
||||
* v1.8.5 — resolved + server-validated injection site to persist on a
|
||||
* taken write. `null` = no site (the column stays / is set NULL). Only
|
||||
@@ -113,6 +131,16 @@ export interface ApplyCanonicalSlotWriteResult {
|
||||
* already-actioned slot and was treated as a no-op (C2 no-downgrade).
|
||||
*/
|
||||
noDowngradeNoOp: boolean;
|
||||
/**
|
||||
* v1.36.x — the write would have CHANGED an outcome the owner already
|
||||
* recorded, and the caller asked for that to be refused rather than
|
||||
* applied. Nothing was written; the caller turns this into a 403.
|
||||
*
|
||||
* Distinct from `noDowngradeNoOp`, which is a silent no-op for a sync
|
||||
* echo that carried no decision. This one means a decision arrived and
|
||||
* disagreed with the one that is already there.
|
||||
*/
|
||||
outcomeChangeRefused: boolean;
|
||||
}
|
||||
|
||||
const SLOT_ROW_SELECT = {
|
||||
@@ -284,6 +312,7 @@ export async function applyCanonicalSlotWrite(
|
||||
// (there was no prior row), so inventory should decrement.
|
||||
consumedTransition: takenAt !== null,
|
||||
noDowngradeNoOp: false,
|
||||
outcomeChangeRefused: false,
|
||||
};
|
||||
} catch (err) {
|
||||
if (!isP2002(err)) throw err;
|
||||
@@ -322,11 +351,35 @@ async function applyToExisting(
|
||||
attributionSource,
|
||||
doseTaken = null,
|
||||
externalId = null,
|
||||
refuseOutcomeChange = false,
|
||||
} = input;
|
||||
|
||||
const existingActioned = existing.takenAt !== null || existing.skipped;
|
||||
const incomingIsPendingEcho = !isExplicitTaken && !isExplicitSkip;
|
||||
|
||||
// v1.36.x — a delegate may record a decision on a slot that is still open;
|
||||
// they may not overturn one the owner already made. Compare the OUTCOMES
|
||||
// rather than refusing every write onto an actioned slot: re-posting the
|
||||
// same decision is a replay (double tap, offline catch-up, a retry after a
|
||||
// partial batch), and those have to keep working.
|
||||
//
|
||||
// Ordered before the C2 echo branch on purpose — an echo carries no
|
||||
// decision, so it can never be an overturn, and letting it fall through
|
||||
// keeps the sync cursor advancing exactly as before.
|
||||
if (refuseOutcomeChange && existingActioned && !incomingIsPendingEcho) {
|
||||
const existingWasSkip = existing.skipped;
|
||||
const incomingIsSkip = isExplicitSkip;
|
||||
if (existingWasSkip !== incomingIsSkip) {
|
||||
return {
|
||||
row: existing,
|
||||
outcome: "updated",
|
||||
consumedTransition: false,
|
||||
noDowngradeNoOp: false,
|
||||
outcomeChangeRefused: true,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// C2 — never clear a recorded dose with a pending projection echo. An
|
||||
// iOS offline re-sync replays a PENDING projection (no takenAt,
|
||||
// skipped=false) for a slot the user already actioned; applying it
|
||||
@@ -341,6 +394,7 @@ async function applyToExisting(
|
||||
outcome: "updated",
|
||||
consumedTransition: false,
|
||||
noDowngradeNoOp: true,
|
||||
outcomeChangeRefused: false,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -385,6 +439,7 @@ async function applyToExisting(
|
||||
outcome: "updated",
|
||||
consumedTransition,
|
||||
noDowngradeNoOp: false,
|
||||
outcomeChangeRefused: false,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -920,6 +920,114 @@ describe("POST /api/medications/intake — what a delegate may NOT do with it",
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The way round the sibling route's refusal, closed.
|
||||
*
|
||||
* `POST /api/medications/intake` refuses a delegate changing a dose the owner
|
||||
* already recorded. This route reaches the same rows through the slot upsert,
|
||||
* so without the same rule it was simply the other door: an explicit skip
|
||||
* posted onto a taken slot flipped the outcome and refunded the inventory the
|
||||
* take had consumed.
|
||||
*
|
||||
* The rule compares OUTCOMES rather than refusing every write onto an actioned
|
||||
* slot, and the third case is why. A double tap, an offline replay and a retry
|
||||
* after a partial batch all re-post the SAME decision, and a caregiver hits all
|
||||
* three; refusing those would make the feature unreliable exactly when somebody
|
||||
* is standing in a kitchen with a pill box.
|
||||
*
|
||||
* Each case asserts the ROW, not only the status. A 403 that had already
|
||||
* written the flip would be worse than no refusal at all, because it would read
|
||||
* as protection.
|
||||
*/
|
||||
describe("POST /api/medications/[id]/intake — the slot upsert is not a way round", () => {
|
||||
async function seedTakenSlot(ownerId: string) {
|
||||
const medication = await seedMedication(ownerId, "Insulin");
|
||||
const slot = new Date();
|
||||
slot.setSeconds(0, 0);
|
||||
const event = await getPrismaClient().medicationIntakeEvent.create({
|
||||
data: {
|
||||
userId: ownerId,
|
||||
medicationId: medication.id,
|
||||
scheduledFor: slot,
|
||||
takenAt: slot,
|
||||
},
|
||||
});
|
||||
return { medicationId: medication.id, slot, eventId: event.id };
|
||||
}
|
||||
|
||||
async function postIntake(
|
||||
medicationId: string,
|
||||
body: Record<string, unknown>,
|
||||
) {
|
||||
const { POST } = await import("@/app/api/medications/[id]/intake/route");
|
||||
return post(
|
||||
POST as Handler,
|
||||
`/api/medications/${medicationId}/intake`,
|
||||
body,
|
||||
{ id: medicationId },
|
||||
);
|
||||
}
|
||||
|
||||
it("refuses a delegate flipping the owner's taken dose to skipped", async () => {
|
||||
const owner = await makeUser("owner");
|
||||
const delegate = await makeUser("delegate");
|
||||
const { medicationId, slot, eventId } = await seedTakenSlot(owner.id);
|
||||
|
||||
await switchInto(owner.id, delegate.id, "WRITE");
|
||||
const response = await postIntake(medicationId, {
|
||||
skipped: true,
|
||||
scheduledFor: slot.toISOString(),
|
||||
});
|
||||
|
||||
expect(response.status).toBe(403);
|
||||
expect((await payload(response)).meta?.errorCode).toBe(
|
||||
"sharing.not_permitted",
|
||||
);
|
||||
|
||||
const row = await getPrismaClient().medicationIntakeEvent.findUniqueOrThrow(
|
||||
{ where: { id: eventId }, select: { takenAt: true, skipped: true } },
|
||||
);
|
||||
expect(row.skipped).toBe(false);
|
||||
expect(row.takenAt).not.toBeNull();
|
||||
});
|
||||
|
||||
it("still accepts a delegate re-posting the same decision", async () => {
|
||||
const owner = await makeUser("owner");
|
||||
const delegate = await makeUser("delegate");
|
||||
const { medicationId, slot, eventId } = await seedTakenSlot(owner.id);
|
||||
|
||||
await switchInto(owner.id, delegate.id, "WRITE");
|
||||
const response = await postIntake(medicationId, {
|
||||
takenAt: new Date().toISOString(),
|
||||
scheduledFor: slot.toISOString(),
|
||||
});
|
||||
|
||||
expect([200, 201]).toContain(response.status);
|
||||
const row = await getPrismaClient().medicationIntakeEvent.findUniqueOrThrow(
|
||||
{ where: { id: eventId }, select: { skipped: true, takenAt: true } },
|
||||
);
|
||||
expect(row.skipped).toBe(false);
|
||||
expect(row.takenAt).not.toBeNull();
|
||||
});
|
||||
|
||||
it("still lets the owner correct their own dose", async () => {
|
||||
const owner = await makeUser("owner");
|
||||
const { medicationId, slot, eventId } = await seedTakenSlot(owner.id);
|
||||
await signIn(owner.id);
|
||||
|
||||
const response = await postIntake(medicationId, {
|
||||
skipped: true,
|
||||
scheduledFor: slot.toISOString(),
|
||||
});
|
||||
|
||||
expect([200, 201]).toContain(response.status);
|
||||
const row = await getPrismaClient().medicationIntakeEvent.findUniqueOrThrow(
|
||||
{ where: { id: eventId }, select: { skipped: true } },
|
||||
);
|
||||
expect(row.skipped).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
writeContract<{ medicationId: string; intakeId: string }>(
|
||||
"POST /api/medications/[id]/intake",
|
||||
{
|
||||
|
||||
Reference in new issue
Block a user