mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
fix(api): enforce range and source guards on the measurement edit path
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.
This commit is contained in:
1 parent
4846dcb478
commit
4b36d5ba52
10 files changed
+193
-10
No files matched your search
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
}) => ({
|
||||
...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);
|
||||
});
|
||||
});
|
||||
@@ -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({
|
||||
|
||||
@@ -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"),
|
||||
);
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in new issue
Block a user