mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(labs): guard partial bound edits against an inverted range
A PUT that moves a single reference bound past the row's stored other bound slipped through the schema refine (which only fires when both bounds arrive together). Both the lab-result and biomarker routes now merge the effective bounds and 422 on low > high. Editing catalog-resolved fields (analyte/unit/panel/range) on a linked reading is also rejected rather than silently no-oped. Shared isInvertedRange/effectiveBound helpers with tests.
This commit is contained in:
1 parent
d1fde96e66
commit
9141b8f74f
4 files changed
+143
-1
No files matched your search
@@ -17,6 +17,7 @@ import {
|
||||
} from "@/lib/labs/biomarker-store";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { updateBiomarkerSchema } from "@/lib/validations/biomarkers";
|
||||
import { effectiveBound, isInvertedRange } from "@/lib/validations/labs";
|
||||
|
||||
/**
|
||||
* v1.18.1 — single Biomarker resource (`/api/biomarkers/{id}`).
|
||||
@@ -136,6 +137,20 @@ export const PUT = apiHandler(
|
||||
if (d.lowerBound !== undefined) data.lowerBound = d.lowerBound;
|
||||
if (d.upperBound !== undefined) data.upperBound = d.upperBound;
|
||||
if (d.panel !== undefined) data.panel = d.panel;
|
||||
|
||||
// Inverted-range guard for a PARTIAL bound update. The schema refine only
|
||||
// fires when both bounds arrive together; moving a single bound past the
|
||||
// row's existing other bound would otherwise persist an inverted window.
|
||||
// Merge effective bounds (parsed when present, else stored) and 422 when
|
||||
// both resolve to concrete numbers with low > high.
|
||||
if (
|
||||
isInvertedRange(
|
||||
effectiveBound(d.lowerBound, existing.lowerBound),
|
||||
effectiveBound(d.upperBound, existing.upperBound),
|
||||
)
|
||||
) {
|
||||
return apiError("lowerBound must not exceed upperBound", 422);
|
||||
}
|
||||
if (d.context !== undefined) {
|
||||
data.contextEncrypted = d.context
|
||||
? encryptContextToBytes(d.context)
|
||||
|
||||
@@ -18,7 +18,11 @@ import {
|
||||
} from "@/lib/labs/serialise";
|
||||
import { decryptNoteFromBytes, encryptNoteToBytes } from "@/lib/labs/store";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { updateLabResultSchema } from "@/lib/validations/labs";
|
||||
import {
|
||||
effectiveBound,
|
||||
isInvertedRange,
|
||||
updateLabResultSchema,
|
||||
} from "@/lib/validations/labs";
|
||||
|
||||
/**
|
||||
* v1.17.1 — single lab-result resource (`/api/labs/{id}`).
|
||||
@@ -125,6 +129,27 @@ export const PUT = apiHandler(
|
||||
|
||||
const d = parsed.data;
|
||||
|
||||
// A reading linked to a catalog marker resolves its name / unit / range
|
||||
// FROM the biomarker (server-authoritative). Editing those catalog-owned
|
||||
// fields on a linked row is meaningless — they would persist as stale
|
||||
// historical columns the serialiser ignores. Reject the attempt with a
|
||||
// 422 rather than silently no-op, so the client never believes an edit
|
||||
// landed. Edit such fields on the Biomarker itself instead.
|
||||
const isLinked = existing.biomarkerId !== null;
|
||||
if (
|
||||
isLinked &&
|
||||
(d.analyte !== undefined ||
|
||||
d.unit !== undefined ||
|
||||
d.panel !== undefined ||
|
||||
d.referenceLow !== undefined ||
|
||||
d.referenceHigh !== undefined)
|
||||
) {
|
||||
return apiError(
|
||||
"analyte / unit / panel / reference range are resolved from the linked biomarker and cannot be edited on a linked reading",
|
||||
422,
|
||||
);
|
||||
}
|
||||
|
||||
// Build `data` field-by-field. `undefined` → leave the column untouched;
|
||||
// an explicit `null` on `panel` / `note` / a reference bound clears it.
|
||||
// A reading linked to a catalog marker is edited via VALUE / takenAt /
|
||||
@@ -137,6 +162,21 @@ export const PUT = apiHandler(
|
||||
if (d.referenceLow !== undefined) data.referenceLow = d.referenceLow;
|
||||
if (d.referenceHigh !== undefined) data.referenceHigh = d.referenceHigh;
|
||||
if (d.takenAt !== undefined) data.takenAt = d.takenAt;
|
||||
|
||||
// Inverted-range guard for a PARTIAL bound update. The schema-level
|
||||
// refine only fires when both bounds arrive in one request; a request
|
||||
// that moves a single bound below/above the row's existing other bound
|
||||
// would otherwise persist an inverted range. Merge the effective bounds
|
||||
// (parsed value when present, else the stored value) and 422 when both
|
||||
// resolve to concrete numbers with low > high.
|
||||
if (
|
||||
isInvertedRange(
|
||||
effectiveBound(d.referenceLow, existing.referenceLow),
|
||||
effectiveBound(d.referenceHigh, existing.referenceHigh),
|
||||
)
|
||||
) {
|
||||
return apiError("referenceLow must not exceed referenceHigh", 422);
|
||||
}
|
||||
if (d.note !== undefined) {
|
||||
data.noteEncrypted = d.note ? encryptNoteToBytes(d.note) : null;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
/**
|
||||
* v1.18.1 — partial-update inverted-range merge guard.
|
||||
*
|
||||
* The lab + biomarker PUT routes merge a parsed bound against the row's stored
|
||||
* bound before checking low ≤ high, so a single-bound edit cannot persist a
|
||||
* transposed range that the schema-level refine (which only fires when both
|
||||
* bounds arrive together) would miss.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { effectiveBound, isInvertedRange } from "@/lib/validations/labs";
|
||||
|
||||
describe("effectiveBound", () => {
|
||||
it("prefers the parsed value when present", () => {
|
||||
expect(effectiveBound(50, 10)).toBe(50);
|
||||
});
|
||||
|
||||
it("uses an explicit null parsed value (clearing the bound)", () => {
|
||||
expect(effectiveBound(null, 10)).toBeNull();
|
||||
});
|
||||
|
||||
it("falls back to the stored value when the field is omitted", () => {
|
||||
expect(effectiveBound(undefined, 10)).toBe(10);
|
||||
expect(effectiveBound(undefined, null)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("isInvertedRange", () => {
|
||||
it("flags low > high", () => {
|
||||
expect(isInvertedRange(120, 100)).toBe(true);
|
||||
});
|
||||
|
||||
it("accepts low == high (inclusive bounds)", () => {
|
||||
expect(isInvertedRange(100, 100)).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts a normal range", () => {
|
||||
expect(isInvertedRange(40, 100)).toBe(false);
|
||||
});
|
||||
|
||||
it("ignores an open-ended range (one bound null)", () => {
|
||||
expect(isInvertedRange(null, 100)).toBe(false);
|
||||
expect(isInvertedRange(40, null)).toBe(false);
|
||||
expect(isInvertedRange(null, null)).toBe(false);
|
||||
});
|
||||
|
||||
it("catches the partial-update case: new low above the stored high", () => {
|
||||
// PUT sets referenceLow=130 on a row whose stored referenceHigh=116.
|
||||
const low = effectiveBound(130, null);
|
||||
const high = effectiveBound(undefined, 116);
|
||||
expect(isInvertedRange(low, high)).toBe(true);
|
||||
});
|
||||
|
||||
it("catches the partial-update case: new high below the stored low", () => {
|
||||
const low = effectiveBound(undefined, 40);
|
||||
const high = effectiveBound(20, null);
|
||||
expect(isInvertedRange(low, high)).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -144,6 +144,34 @@ export const updateLabResultSchema = z
|
||||
|
||||
export type UpdateLabResultInput = z.infer<typeof updateLabResultSchema>;
|
||||
|
||||
/**
|
||||
* Inverted-range merge guard for a PARTIAL bound update.
|
||||
*
|
||||
* The schema-level `referenceLow <= referenceHigh` refine only fires when both
|
||||
* bounds arrive in the SAME request. A partial PUT that moves a single bound
|
||||
* past the row's existing other bound slips through the schema. The route
|
||||
* resolves the EFFECTIVE bounds (the parsed value when present, else the
|
||||
* stored value) and calls this to reject a transposed window with a 422.
|
||||
*
|
||||
* Returns true when both effective bounds are concrete numbers and low > high.
|
||||
*/
|
||||
export function isInvertedRange(
|
||||
effectiveLow: number | null,
|
||||
effectiveHigh: number | null,
|
||||
): boolean {
|
||||
return (
|
||||
effectiveLow !== null && effectiveHigh !== null && effectiveLow > effectiveHigh
|
||||
);
|
||||
}
|
||||
|
||||
/** Resolve an effective bound: the parsed value when present, else the stored. */
|
||||
export function effectiveBound(
|
||||
parsed: number | null | undefined,
|
||||
stored: number | null,
|
||||
): number | null {
|
||||
return parsed !== undefined ? parsed : stored;
|
||||
}
|
||||
|
||||
/** List query: filter by biomarker / analyte (exact) + date range, paginate. */
|
||||
export const listLabResultsSchema = z.object({
|
||||
// v1.18.1 — filter to one catalog marker's readings (the detail chart feed).
|
||||
|
||||
Reference in new issue
Block a user