diff --git a/src/app/api/labs/restore/__tests__/route.test.ts b/src/app/api/labs/restore/__tests__/route.test.ts new file mode 100644 index 000000000..5a4e3bcaf --- /dev/null +++ b/src/app/api/labs/restore/__tests__/route.test.ts @@ -0,0 +1,149 @@ +/** + * `POST /api/labs/restore` — the un-tombstone behind the lab-history + * delete-Undo toast. + * + * The route had no test of its own. It writes health data back into the + * record and, until now, wrote no audit row for it: the trail showed a + * deletion that appeared to have been reversed by nobody. With a delegate + * able to act on a shared record that question has a second possible answer, + * so the row is what makes it answerable. + * + * Also pins the ownership scope the affordance depends on — a forged, + * foreign or never-deleted id is a silent no-op rather than a 404 existence + * leak — because that scope lives in the mutation `where` and nothing else + * asserted it. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest } from "next/server"; + +vi.mock("@/lib/db", () => ({ + prisma: { + labResult: { updateMany: vi.fn() }, + }, +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn(), + rateLimitHeaders: () => ({}), +})); +vi.mock("@/lib/idempotency", () => ({ + withIdempotency: + (fn: (...args: Args) => Promise) => + (...args: Args) => + fn(...args), +})); +vi.mock("@/lib/cache/invalidate", () => ({ + invalidateUserHealthScore: vi.fn(), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { POST } from "../route"; +import { prisma } from "@/lib/db"; +import { getSession } from "@/lib/auth/session"; +import { auditLog } from "@/lib/auth/audit"; +import { checkRateLimit } from "@/lib/rate-limit"; +import { invalidateUserHealthScore } from "@/lib/cache/invalidate"; + +const SESSION_OK = { + session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: { id: "user-1", username: "tester", role: "USER" as const }, +}; + +function postReq(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/labs/restore", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: true, + remaining: 59, + reset: Date.now() + 60_000, + } as never); + vi.mocked(prisma.labResult.updateMany).mockResolvedValue({ + count: 2, + } as never); +}); + +describe("POST /api/labs/restore", () => { + it("clears the tombstone on owned, currently-deleted rows only", async () => { + const res = await POST(postReq({ ids: ["lab-1", "lab-2"] })); + expect(res.status).toBe(200); + + const call = vi.mocked(prisma.labResult.updateMany).mock.calls[0][0] as { + where: Record; + data: Record; + }; + expect(call.where).toEqual({ + id: { in: ["lab-1", "lab-2"] }, + userId: "user-1", + deletedAt: { not: null }, + }); + expect(call.data).toEqual({ deletedAt: null }); + }); + + it("writes an audit row naming how many readings came back", async () => { + await POST(postReq({ ids: ["lab-1", "lab-2"] })); + + expect(auditLog).toHaveBeenCalledTimes(1); + const [action, payload] = vi.mocked(auditLog).mock.calls[0] as [ + string, + { userId: string; details: Record }, + ]; + expect(action).toBe("labResult.restore"); + expect(payload.userId).toBe("user-1"); + expect(payload.details).toMatchObject({ count: 2 }); + }); + + it("reports zero and leaves the health score alone when nothing matched", async () => { + vi.mocked(prisma.labResult.updateMany).mockResolvedValue({ + count: 0, + } as never); + + const res = await POST(postReq({ ids: ["forged"] })); + expect(res.status).toBe(200); + const json = (await res.json()) as { data: { restored: number } }; + expect(json.data.restored).toBe(0); + expect(invalidateUserHealthScore).not.toHaveBeenCalled(); + }); + + it("rejects an empty id list with 422 and touches nothing", async () => { + const res = await POST(postReq({ ids: [] })); + expect(res.status).toBe(422); + expect(prisma.labResult.updateMany).not.toHaveBeenCalled(); + expect(auditLog).not.toHaveBeenCalled(); + }); + + it("returns 429 without restoring when the rate limit is exceeded", async () => { + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: false, + remaining: 0, + reset: Date.now() + 60_000, + } as never); + + const res = await POST(postReq({ ids: ["lab-1"] })); + expect(res.status).toBe(429); + expect(prisma.labResult.updateMany).not.toHaveBeenCalled(); + expect(auditLog).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/labs/restore/route.ts b/src/app/api/labs/restore/route.ts index af4c853dc..24c734675 100644 --- a/src/app/api/labs/restore/route.ts +++ b/src/app/api/labs/restore/route.ts @@ -14,9 +14,11 @@ import { NextRequest } from "next/server"; import { z } from "zod/v4"; import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { auditLog } from "@/lib/auth/audit"; import { apiError, apiSuccess, + getClientIp, returnAllZodIssues, safeJson, } from "@/lib/api-response"; @@ -69,6 +71,16 @@ async function postRestore(request: NextRequest): Promise { }); if (count > 0) invalidateUserHealthScore(user.id); + // Bringing a reading back is a health-data write and earns the same durable + // row its delete already writes (`labResult.delete`). Without this the trail + // showed a deletion that appeared to be reversed by nobody. Mirrors + // `measurement.restore`, which is the same affordance one domain over. + await auditLog("labResult.restore", { + userId: user.id, + ipAddress: getClientIp(request), + details: { count }, + }); + annotate({ action: { name: "labs.restore" }, meta: { restored: count } }); return apiSuccess({ restored: count }); diff --git a/src/app/api/medications/[id]/intake/purge/__tests__/route.test.ts b/src/app/api/medications/[id]/intake/purge/__tests__/route.test.ts index 9a4a76a03..d0aa31c67 100644 --- a/src/app/api/medications/[id]/intake/purge/__tests__/route.test.ts +++ b/src/app/api/medications/[id]/intake/purge/__tests__/route.test.ts @@ -15,7 +15,7 @@ import { NextRequest } from "next/server"; vi.mock("@/lib/db", () => ({ prisma: { medication: { findUnique: vi.fn() }, - medicationIntakeEvent: { deleteMany: vi.fn() }, + medicationIntakeEvent: { updateMany: vi.fn() }, medicationComplianceRollup: { deleteMany: vi.fn() }, }, })); @@ -82,7 +82,7 @@ beforeEach(() => { vi.mocked(prisma.medication.findUnique).mockResolvedValue({ name: "Test", } as never); - vi.mocked(prisma.medicationIntakeEvent.deleteMany).mockResolvedValue({ + vi.mocked(prisma.medicationIntakeEvent.updateMany).mockResolvedValue({ count: 7, } as never); vi.mocked(prisma.medicationComplianceRollup.deleteMany).mockResolvedValue({ @@ -116,7 +116,36 @@ describe("DELETE /api/medications/[id]/intake/purge", () => { ); const res = await DELETE(deleteReq(), ROUTE_PARAMS); expect(res.status).toBe(404); - expect(prisma.medicationIntakeEvent.deleteMany).not.toHaveBeenCalled(); + expect(prisma.medicationIntakeEvent.updateMany).not.toHaveBeenCalled(); expect(invalidateUserMedications).not.toHaveBeenCalled(); }); + + // The purge was the one destructive verb in the intake family that removed + // rows outright: a mis-tap was final, and a client offline at purge time + // kept every row forever because a row that no longer exists produces no + // tombstone. Assert the CALL SHAPE rather than a count — a count comes back + // identical from a `deleteMany`, which is exactly the regression this pins. + it("tombstones the events instead of removing them", async () => { + await DELETE(deleteReq(), ROUTE_PARAMS); + + expect(prisma.medicationIntakeEvent.updateMany).toHaveBeenCalledTimes(1); + const call = vi.mocked(prisma.medicationIntakeEvent.updateMany).mock + .calls[0][0] as { + where: Record; + data: Record; + }; + + // Scoped to the caller's own rows for this medication, and to LIVE rows + // only, so a second purge counts zero rather than re-stamping tombstones + // and re-bumping every syncVersion the clients have already seen. + expect(call.where).toEqual({ + medicationId: "med-1", + userId: "user-1", + deletedAt: null, + }); + + // A tombstone the sync feed can carry: a stamp plus a monotonic bump. + expect(call.data.deletedAt).toBeInstanceOf(Date); + expect(call.data.syncVersion).toEqual({ increment: 1 }); + }); }); diff --git a/src/app/api/medications/[id]/intake/purge/route.ts b/src/app/api/medications/[id]/intake/purge/route.ts index d7f342851..34af5204b 100644 --- a/src/app/api/medications/[id]/intake/purge/route.ts +++ b/src/app/api/medications/[id]/intake/purge/route.ts @@ -30,8 +30,27 @@ export const DELETE = apiHandler( select: { name: true }, }); - const { count } = await prisma.medicationIntakeEvent.deleteMany({ - where: { medicationId: id, userId: user.id }, + // Tombstone, like both siblings (single-event DELETE, bulk-delete). This + // was the one destructive verb in the intake family with nothing to + // reconstruct from: `deleteMany` removed the rows outright, so a purge + // fired by mistake was final, and it was invisible to `/api/sync/changes` + // — a client offline at purge time kept every row forever, because a row + // that no longer exists produces no tombstone to send. + // + // Setting `deletedAt` (+ bumping `syncVersion`) makes the rows invisible + // to every today / compliance / list read (all of which filter + // `deletedAt: null`) while the sync feed emits one tombstone per row. The + // nightly tombstone purge collects them later, so nothing grows without + // bound. Scope `deletedAt: null` so a second purge counts zero rather than + // re-bumping rows already gone. + // + // No inventory refund, deliberately: the single-event and bulk deletes + // refund because "I did not take that dose" puts the pill back in the box. + // A purge wipes an entire history as bookkeeping, and refunding every dose + // ever taken would inflate the current container count instead. + const { count } = await prisma.medicationIntakeEvent.updateMany({ + where: { medicationId: id, userId: user.id, deletedAt: null }, + data: { deletedAt: new Date(), syncVersion: { increment: 1 } }, }); // v1.4.39 W-MED — drop every rollup row for this medication so the diff --git a/src/app/api/nutrients/water/__tests__/route.test.ts b/src/app/api/nutrients/water/__tests__/route.test.ts index 9139fc7e8..443fe2b55 100644 --- a/src/app/api/nutrients/water/__tests__/route.test.ts +++ b/src/app/api/nutrients/water/__tests__/route.test.ts @@ -38,6 +38,9 @@ vi.mock("@/lib/idempotency", () => ({ fn(...args), })); vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); vi.mock("@/lib/db-compat", () => ({ ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), })); @@ -58,6 +61,7 @@ import { checkRateLimit } from "@/lib/rate-limit"; import { requireModuleEnabled } from "@/lib/modules/gate"; import { invalidateUserDashboardSnapshot } from "@/lib/cache/invalidate"; import { NUTRIENT_CATALOG } from "@/lib/nutrients/catalog"; +import { auditLog } from "@/lib/auth/audit"; const SESSION_OK = { session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, @@ -217,6 +221,37 @@ describe("POST /api/nutrients/water", () => { unit: "ml", }); }); + + // A health-data write earns a durable row. This one reached only the wide + // event, which expires, so months later nothing could answer who put a + // day's total there — a question with two possible answers once a delegate + // can act on a shared record. + it("writes an audit row naming the day, the mode and the amount", async () => { + await POST(postReq({ amountMl: 250, mode: "add", day: "2026-07-16" })); + + expect(auditLog).toHaveBeenCalledTimes(1); + const [action, payload] = vi.mocked(auditLog).mock.calls[0] as [ + string, + { userId: string; details: Record }, + ]; + expect(action).toBe("nutrient.water.write"); + expect(payload.userId).toBe("user-1"); + expect(payload.details).toMatchObject({ + day: "2026-07-16", + mode: "add", + amountMl: 250, + resultingAmount: 500, + }); + }); + + it("writes no audit row when the module gate refuses", async () => { + vi.mocked(requireModuleEnabled).mockResolvedValue({ + enabled: false, + response: apiError("Module disabled", 403), + } as never); + await POST(postReq({ amountMl: 250, mode: "add", day: "2026-07-16" })); + expect(auditLog).not.toHaveBeenCalled(); + }); }); describe("POST /api/nutrients/water — day bound and increment cap", () => { diff --git a/src/app/api/nutrients/water/route.ts b/src/app/api/nutrients/water/route.ts index 84044ffba..1ac6e63fa 100644 --- a/src/app/api/nutrients/water/route.ts +++ b/src/app/api/nutrients/water/route.ts @@ -19,9 +19,11 @@ import { NextRequest } from "next/server"; import { prisma } from "@/lib/db"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; +import { auditLog } from "@/lib/auth/audit"; import { apiError, apiSuccess, + getClientIp, returnAllZodIssues, safeJson, } from "@/lib/api-response"; @@ -135,6 +137,17 @@ async function postWater(request: NextRequest): Promise { // matching the mood / medication / measurement posture. invalidateUserDashboardSnapshot(user.id); + // A health-data write earns a durable row, like every other one. This was + // the last manual write in the nutrients family that only reached the wide + // event, which expires; the audit trail is what answers "who put this here" + // months later, and with a delegate able to act on a shared record that + // question has a second possible answer. + await auditLog("nutrient.water.write", { + userId: user.id, + ipAddress: getClientIp(request), + details: { day, mode, amountMl, resultingAmount: row.amount }, + }); + annotate({ action: { name: "nutrient.water.write" }, meta: { mode, day, amount_ml: amountMl }, diff --git a/src/components/labs/biomarker-manager.tsx b/src/components/labs/biomarker-manager.tsx index f2f024393..699a5d06d 100644 --- a/src/components/labs/biomarker-manager.tsx +++ b/src/components/labs/biomarker-manager.tsx @@ -26,8 +26,14 @@ import type { BiomarkerDto, BiomarkerListResponse } from "./types"; * Lives on the Labs page (not Settings) because the catalog IS the Labs * feature's primary object. Each marker renders as a single compact row * (name + unit inline, reference range / panel on a muted sub-line) with a - * hide toggle, an edit button (reusing the define/edit sheet), and a delete - * (the `onDelete: SetNull` FK keeps existing readings, just unlinks them). + * hide toggle, an edit button (reusing the define/edit sheet), and a delete. + * + * Delete removes the marker AND every reading under it, in one transaction — + * this line used to claim the `onDelete: SetNull` FK left the readings behind + * as unlinked rows, which stopped being true when delete became a "drop this + * and its history" action and the confirmation copy was rewritten in all six + * locales to say so. Hide is the reversible path and is one row above it, so + * the destructive one is deliberately the destructive one. * * "Hide" is a soft remove: a marker the user no longer needs leaves the * active list and the lab-entry pickers but keeps its readings + canonical diff --git a/src/components/labs/lab-biomarker-detail.tsx b/src/components/labs/lab-biomarker-detail.tsx index d57b42bd7..16442a653 100644 --- a/src/components/labs/lab-biomarker-detail.tsx +++ b/src/components/labs/lab-biomarker-detail.tsx @@ -266,9 +266,12 @@ export function LabBiomarkerDetail({ biomarkerId }: { biomarkerId: string }) { } // v1.18.9 (#41/#3) — delete the biomarker directly from its detail page so - // a stray marker is removable without a Settings detour. `onDelete: SetNull` - // keeps the readings and unlinks them. On success, invalidate the catalog + - // result list and return to the labs surface. + // a stray marker is removable without a Settings detour. This removes every + // reading under the marker along with it; the line here used to claim + // `onDelete: SetNull` kept them, which stopped being true when delete became + // a "drop this and its history" action. The confirmation dialog states it in + // all six locales, and Hide is the reversible alternative. On success, + // invalidate the catalog + result list and return to the labs surface. const deleteMarker = useMutation({ mutationFn: () => apiDelete(`/api/biomarkers/${biomarkerId}`), onSuccess: () => { diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index 187b362f5..5c711ad16 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -648,10 +648,23 @@ async function readActingCarrier(auth: AuthContext): Promise { /** * Record a refused delegation. * - * Written for the OWNER's benefit as much as the operator's: "someone tried to - * open my record and was turned away" is the question the sharing panel exists - * to answer. The row names the actor (the caller), the account they aimed at, - * and why it failed — none of which reaches the response. + * The row is filed under the CALLER, with no actor, and it is the operator's + * trail rather than the owner's. It names who called, the account they claimed, + * and why the claim was refused — none of which reaches the response. + * + * It is deliberately not filed under the claimed account, and that decision is + * worth stating because the opposite reads as the obvious one. A row is only + * written for a HEADER refusal, and a header is an unverified per-request + * assertion: the value is whatever the caller typed. Filing under it would let + * any authenticated caller write rows into any account's activity feed by + * naming it, which is a spam vector aimed at exactly the panel that exists to + * be trustworthy. Verifying the claim first would mean a grant lookup on a path + * whose whole job is to refuse without touching the record. + * + * What the owner does see is the other half: `recordDelegatedAccess` files a + * per-day row under the OWNER, naming the actor, for every delegated read that + * succeeded. "Who reached my record" is answered there. "Whose client sent a + * malformed claim" is an operator question and stays here. * * Fire-and-forget: an audit write that fails must not convert a 403 into a 500, * because the 403 is the part that protects the record.