From 4b36d5ba522f7aa932b4fc215909e76a58f81228 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 5 Jul 2026 15:07:40 +0200 Subject: [PATCH] fix(api): enforce range and source guards on the measurement edit path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PUT /api/measurements/[id] was the one write surface that skipped the per-type plausibility bands — POST, batch, CSV import, Apple export, Telegram and MCP all enforce VALUE_RANGES — so an implausible edited value flowed unchecked into rollups, the health score, the BP gates and the Coach snapshot. The route now validates the new value against the row's own type after the lookup and returns the standard multi-issue 422 envelope on violation. Value edits on server-owned sources (anything outside WRITABLE_MEASUREMENT_SOURCES: connector, import, computed rows) are refused with 409 and the stable errorCode measurement.update.server_owned_source — editing a provider-attributed number forges a reading the server never received. Timestamp and note edits stay allowed, and the edit sheet shows a localised message for the new code. The schema's generic min(0).max(500000) bound is dropped: the per-type band is stricter where it matters, and min(0) blocked legitimate edits of the signed types (ANS_CHARGE, BODY_TEMPERATURE_DEVIATION). Route tests cover out-of-range 422, negative signed-type accept, the server-owned 409, a timestamp-only edit on a server-owned row, and the happy path. The PUT endpoint is not part of the published OpenAPI surface — the generated contract is unchanged. --- messages/de.json | 1 + messages/en.json | 1 + messages/es.json | 1 + messages/fr.json | 1 + messages/it.json | 1 + messages/pl.json | 1 + .../measurements/[id]/__tests__/route.test.ts | 119 ++++++++++++++++-- src/app/api/measurements/[id]/route.ts | 62 ++++++++- .../measurements/measurement-list.tsx | 9 ++ src/lib/validations/measurement.ts | 7 +- 10 files changed, 193 insertions(+), 10 deletions(-) diff --git a/messages/de.json b/messages/de.json index 1193b7d2b..732b16ed2 100644 --- a/messages/de.json +++ b/messages/de.json @@ -517,6 +517,7 @@ "formReset": "Formular zurücksetzen", "saveError": "Fehler beim Speichern", "duplicateTimestamp": "Zum gewählten Zeitpunkt existiert bereits ein Messwert dieses Typs.", + "serverOwnedSource": "Werte aus einer verbundenen Quelle lassen sich nicht bearbeiten – Zeitpunkt und Notiz bleiben änderbar.", "deleteError": "Fehler beim Löschen", "deletedToast": "Messwert gelöscht", "restoredToast": "Messwert wiederhergestellt", diff --git a/messages/en.json b/messages/en.json index 76c774711..dcb9a1849 100644 --- a/messages/en.json +++ b/messages/en.json @@ -517,6 +517,7 @@ "formReset": "Reset form", "saveError": "Error saving", "duplicateTimestamp": "A measurement of this type already exists at the selected timestamp.", + "serverOwnedSource": "Values from a connected source can't be edited — the timestamp and note stay editable.", "deleteError": "Error deleting", "deletedToast": "Measurement deleted", "restoredToast": "Measurement restored", diff --git a/messages/es.json b/messages/es.json index 479aac50e..57186f2e1 100644 --- a/messages/es.json +++ b/messages/es.json @@ -517,6 +517,7 @@ "formReset": "Restablecer formulario", "saveError": "Error al guardar", "duplicateTimestamp": "Ya existe una medición de este tipo en la marca de tiempo seleccionada.", + "serverOwnedSource": "Los valores de una fuente conectada no se pueden editar — la fecha y la nota siguen siendo editables.", "deleteError": "Error al eliminar", "deletedToast": "Medición eliminada", "restoredToast": "Medición restaurada", diff --git a/messages/fr.json b/messages/fr.json index 92a49697a..bdb838530 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -517,6 +517,7 @@ "formReset": "Réinitialiser le formulaire", "saveError": "Erreur lors de l’enregistrement", "duplicateTimestamp": "Une mesure de ce type existe déjà à cet horodatage.", + "serverOwnedSource": "Les valeurs provenant d’une source connectée ne peuvent pas être modifiées — l’horodatage et la note restent modifiables.", "deleteError": "Erreur lors de la suppression", "deletedToast": "Mesure supprimée", "restoredToast": "Mesure restaurée", diff --git a/messages/it.json b/messages/it.json index a8dde4d47..cbc9dd973 100644 --- a/messages/it.json +++ b/messages/it.json @@ -517,6 +517,7 @@ "formReset": "Reimposta modulo", "saveError": "Errore durante il salvataggio", "duplicateTimestamp": "Esiste già una misurazione di questo tipo a questa marca temporale.", + "serverOwnedSource": "I valori provenienti da una fonte collegata non possono essere modificati — data e nota restano modificabili.", "deleteError": "Errore durante l’eliminazione", "deletedToast": "Misurazione eliminata", "restoredToast": "Misurazione ripristinata", diff --git a/messages/pl.json b/messages/pl.json index a772212b5..1b5a953fb 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -517,6 +517,7 @@ "formReset": "Resetuj formularz", "saveError": "Błąd zapisu", "duplicateTimestamp": "Pomiar tego typu już istnieje dla wybranej daty.", + "serverOwnedSource": "Wartości z połączonego źródła nie można edytować — znacznik czasu i notatka pozostają edytowalne.", "deleteError": "Błąd usuwania", "deletedToast": "Pomiar usunięty", "restoredToast": "Pomiar przywrócony", diff --git a/src/app/api/measurements/[id]/__tests__/route.test.ts b/src/app/api/measurements/[id]/__tests__/route.test.ts index 7d7f96a26..6029e657b 100644 --- a/src/app/api/measurements/[id]/__tests__/route.test.ts +++ b/src/app/api/measurements/[id]/__tests__/route.test.ts @@ -28,6 +28,9 @@ vi.mock("@/lib/cache/invalidate", () => ({ vi.mock("@/lib/rollups/measurement-rollups", () => ({ recomputeBucketsForMeasurement: vi.fn().mockResolvedValue(undefined), })); +vi.mock("@/lib/insights/comprehensive-generate", () => ({ + invalidateStatusInsightsForTypes: vi.fn().mockResolvedValue(undefined), +})); vi.mock("next/headers", () => ({ headers: vi.fn(async () => ({ get: () => null })), cookies: vi.fn(async () => ({ @@ -40,6 +43,7 @@ vi.mock("next/headers", () => ({ import { PUT } from "../route"; import { prisma } from "@/lib/db"; import { getSession } from "@/lib/auth/session"; +import { invalidateStatusInsightsForTypes } from "@/lib/insights/comprehensive-generate"; const SESSION_OK = { session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, @@ -56,21 +60,40 @@ function putReq(body: unknown): NextRequest { const ROUTE_CTX = { params: Promise.resolve({ id: "m1" }) }; +const EXISTING = { + id: "m1", + userId: "user-1", + type: "WEIGHT", + source: "MANUAL", + value: 80, + measuredAt: new Date("2026-06-01T08:00:00Z"), + notes: null, + notesEncrypted: null, +}; + beforeEach(() => { vi.resetAllMocks(); vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); vi.mocked(prisma.auditLog.create).mockResolvedValue({} as never); - vi.mocked(prisma.measurement.findFirst).mockResolvedValue({ - id: "m1", - userId: "user-1", - } as never); + // resetAllMocks clears the factory implementation; the route chains + // `.catch()` on this fire-and-forget call, so it must resolve again. + vi.mocked(invalidateStatusInsightsForTypes).mockResolvedValue( + undefined as never, + ); + vi.mocked(prisma.measurement.findFirst).mockResolvedValue(EXISTING as never); + vi.mocked(prisma.measurement.update).mockImplementation((async (args: { + data: Record; + }) => ({ + ...EXISTING, + ...args.data, + })) as never); }); describe("PUT /api/measurements/[id] — 422 multi-issue (v1.4.43 W6)", () => { it("surfaces TWO simultaneous validation errors", async () => { - // `value=-1` (below min 0) + `measuredAt` not a valid ISO. + // `value` not a number + `measuredAt` not a valid ISO. const res = await PUT( - putReq({ value: -1, measuredAt: "not-iso" }), + putReq({ value: "junk", measuredAt: "not-iso" }), ROUTE_CTX, ); expect(res.status).toBe(422); @@ -107,7 +130,10 @@ describe("PUT /api/measurements/[id] — 422 multi-issue (v1.4.43 W6)", () => { }); it("writes a measurements.update.validation-failed audit row", async () => { - const res = await PUT(putReq({ value: -1, measuredAt: "junk" }), ROUTE_CTX); + const res = await PUT( + putReq({ value: "junk", measuredAt: "junk" }), + ROUTE_CTX, + ); expect(res.status).toBe(422); await new Promise((r) => setTimeout(r, 5)); expect(prisma.auditLog.create).toHaveBeenCalledTimes(1); @@ -122,7 +148,84 @@ describe("PUT /api/measurements/[id] — 422 multi-issue (v1.4.43 W6)", () => { vi.mocked(prisma.auditLog.create).mockRejectedValueOnce( new Error("db down"), ); - const res = await PUT(putReq({ value: -1, measuredAt: "junk" }), ROUTE_CTX); + const res = await PUT( + putReq({ value: "junk", measuredAt: "junk" }), + ROUTE_CTX, + ); expect(res.status).toBe(422); }); }); + +describe("PUT /api/measurements/[id] — value guards (di-001)", () => { + it("422s an edit outside the row type's plausibility band", async () => { + // Existing row is WEIGHT (band 1..500 kg) — 9999 must not pass just + // because the edit body carries no type. + const res = await PUT(putReq({ value: 9999 }), ROUTE_CTX); + expect(res.status).toBe(422); + const body = (await res.json()) as { + data: null; + error: string; + details: { issues: Array<{ path: string; message: string }> }; + }; + expect(body.error).toBe("Validation failed"); + expect(body.details.issues).toHaveLength(1); + expect(body.details.issues[0].path).toContain("value"); + expect(prisma.measurement.update).not.toHaveBeenCalled(); + }); + + it("accepts a negative value on a signed type", async () => { + // ANS_CHARGE runs -100..100; the former generic min(0) rejected every + // legitimate negative edit. + vi.mocked(prisma.measurement.findFirst).mockResolvedValue({ + ...EXISTING, + type: "ANS_CHARGE", + value: 12, + } as never); + const res = await PUT(putReq({ value: -40 }), ROUTE_CTX); + expect(res.status).toBe(200); + const call = vi.mocked(prisma.measurement.update).mock.calls[0]?.[0] as { + data: { value?: number }; + }; + expect(call.data.value).toBe(-40); + }); + + it("409s a value edit on a server-owned source with a stable errorCode", async () => { + vi.mocked(prisma.measurement.findFirst).mockResolvedValue({ + ...EXISTING, + source: "WITHINGS", + } as never); + const res = await PUT(putReq({ value: 81 }), ROUTE_CTX); + expect(res.status).toBe(409); + const body = (await res.json()) as { + data: null; + error: string; + meta?: { errorCode?: string }; + }; + expect(body.meta?.errorCode).toBe("measurement.update.server_owned_source"); + expect(prisma.measurement.update).not.toHaveBeenCalled(); + }); + + it("still allows a timestamp-only edit on a server-owned source", async () => { + vi.mocked(prisma.measurement.findFirst).mockResolvedValue({ + ...EXISTING, + source: "WITHINGS", + } as never); + const res = await PUT( + putReq({ measuredAt: "2026-06-01T09:00:00.000Z" }), + ROUTE_CTX, + ); + expect(res.status).toBe(200); + expect(prisma.measurement.update).toHaveBeenCalledTimes(1); + }); + + it("happy path: an in-band edit on a client-owned row persists", async () => { + const res = await PUT(putReq({ value: 82.5 }), ROUTE_CTX); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { value: number } }; + expect(body.data.value).toBe(82.5); + const call = vi.mocked(prisma.measurement.update).mock.calls[0]?.[0] as { + data: { value?: number }; + }; + expect(call.data.value).toBe(82.5); + }); +}); diff --git a/src/app/api/measurements/[id]/route.ts b/src/app/api/measurements/[id]/route.ts index 4354eb3c1..2537dc58a 100644 --- a/src/app/api/measurements/[id]/route.ts +++ b/src/app/api/measurements/[id]/route.ts @@ -10,13 +10,18 @@ import { safeJson, sanitiseZodIssues, } from "@/lib/api-response"; -import { updateMeasurementSchema } from "@/lib/validations/measurement"; +import { + updateMeasurementSchema, + validateMeasurementRange, + WRITABLE_MEASUREMENT_SOURCES, +} from "@/lib/validations/measurement"; import { encryptNote, shapeMeasurementNotes } from "@/lib/crypto/note-cipher"; import { invalidateUserMeasurements } from "@/lib/cache/invalidate"; import { invalidateStatusInsightsForTypes } from "@/lib/insights/comprehensive-generate"; import { recomputeBucketsForMeasurement } from "@/lib/rollups/measurement-rollups"; import { Prisma } from "@/generated/prisma/client"; import { NextRequest } from "next/server"; +import { z } from "zod"; type RouteParams = { params: Promise<{ id: string }> }; @@ -104,6 +109,61 @@ export const PUT = apiHandler( } const data = parsed.data; + + // v1.27.5 (di-001) — the edit path was the ONE write surface that skipped + // the per-type plausibility bands: every other producer (POST, batch, + // CSV import, Apple export, Telegram, MCP) enforces `VALUE_RANGES`, so an + // implausible edited value flowed unchecked into rollups, the health + // score, the BP gates and the Coach snapshot. + if (data.value !== undefined && data.value !== existing.value) { + // Server-owned rows first: a value attributed to a connector / import / + // computed engine is the provider's reading — editing the number would + // forge a source-attributed row the server never received. Mirrors the + // write-side classification (`WRITABLE_MEASUREMENT_SOURCES`): only + // MANUAL and APPLE_HEALTH rows are client-owned. Timestamp and note + // edits stay allowed — annotating a Withings reading is legitimate. + if ( + !(WRITABLE_MEASUREMENT_SOURCES as readonly string[]).includes( + existing.source, + ) + ) { + annotate({ + action: { name: "measurements.update.server-owned-source" }, + meta: { measurement_id: id, source: existing.source }, + }); + return apiError( + "Values from a connected source cannot be edited", + 409, + { errorCode: "measurement.update.server_owned_source" }, + ); + } + + // Range check against the row's OWN type (the edit body carries no + // type). Returned through the standard multi-issue 422 envelope so the + // edit sheet renders it like any other field error. + const rangeCheck = z + .object({ + value: z.number().superRefine((value, ctx) => { + const rangeError = validateMeasurementRange(existing.type, value); + if (rangeError) { + ctx.addIssue({ code: "custom", message: rangeError }); + } + }), + }) + .safeParse({ value: data.value }); + if (!rangeCheck.success) { + annotate({ + action: { name: "measurements.update.validation-failed" }, + meta: { + issue_count: rangeCheck.error.issues.length, + measurement_id: id, + reason: "value_out_of_range", + }, + }); + return returnAllZodIssues(rangeCheck.error, 422); + } + } + let measurement; try { measurement = await prisma.measurement.update({ diff --git a/src/components/measurements/measurement-list.tsx b/src/components/measurements/measurement-list.tsx index 0c11a5d80..d5afe7ffe 100644 --- a/src/components/measurements/measurement-list.tsx +++ b/src/components/measurements/measurement-list.tsx @@ -654,6 +654,15 @@ export function MeasurementList({ setEditError(t("measurements.duplicateTimestamp")); return; } + // v1.27.5 (di-001) — value edits on server-owned sources + // (connector / import / computed rows) are refused with a 409. + if ( + err instanceof ApiError && + err.meta?.errorCode === "measurement.update.server_owned_source" + ) { + setEditError(t("measurements.serverOwnedSource")); + return; + } setEditError( err instanceof Error ? err.message : t("measurements.saveError"), ); diff --git a/src/lib/validations/measurement.ts b/src/lib/validations/measurement.ts index 0266827e6..e78adc2be 100644 --- a/src/lib/validations/measurement.ts +++ b/src/lib/validations/measurement.ts @@ -662,7 +662,12 @@ export const createMeasurementSchema = z ); export const updateMeasurementSchema = z.object({ - value: z.number().min(0).max(500000).optional(), + // No generic magnitude bound here: the route validates the new value + // against the row's OWN type through `validateMeasurementRange` after the + // lookup (the schema cannot know the type — the edit body doesn't carry + // it). The former `min(0)` also blocked legitimate edits of the signed + // types (ANS_CHARGE, BODY_TEMPERATURE_DEVIATION). + value: z.number().optional(), // v1.17 W1b — same plausibility bound on the edit path; an edit cannot // forward-date a reading into the future nor before 1900. measuredAt: validateEntryInstant(