mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 21:10:59 +00:00
fix(audit): drop the widget-layout breadcrumb that can no longer write
The layout write resolves the caller, so its refusal is always the caller's own and auditLog never files it; the call, its dedup and its tests go, and the refusal stays on the wide event. The series route tests now pin the breadcrumb call instead of a ledger row the caller's own request no longer writes.
This commit is contained in:
1 parent
00dfc5a0c0
commit
43da1739b2
3 files changed
+35
-124
No files matched your search
@@ -4,10 +4,9 @@
|
||||
* Until v1.4.41 this route returned `parsed.error.issues[0].message`,
|
||||
* which dropped every issue past the first. iOS contract debugging
|
||||
* needed one round-trip per wrong field. The route now returns every
|
||||
* issue under `details.issues` AND files a
|
||||
* `dashboard.widgets.validation-failed` breadcrumb through `auditLog()`,
|
||||
* which since v1.43.7 writes it only for a delegate's request (the
|
||||
* decision is tested in `src/lib/auth/__tests__/audit.test.ts`).
|
||||
* issue under `details.issues` and keeps the refusal on the wide event.
|
||||
* Since v1.43.7 it files no audit row: the write resolves the caller, and a
|
||||
* caller's own refusal is telemetry, not an audit event.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
@@ -197,51 +196,14 @@ describe("PUT /api/dashboard/widgets — 422 multi-issue envelope (v1.4.42 W2)",
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("writes one audit-ledger row keyed dashboard.widgets.validation-failed", async () => {
|
||||
it("files no audit row for the caller's own refusal", async () => {
|
||||
const res = await callPut(makeReq({ version: 2, widgets: [] }));
|
||||
expect(res.status).toBe(422);
|
||||
|
||||
// The audit row is fire-and-forget — let the microtask queue drain.
|
||||
await new Promise((r) => setTimeout(r, 5));
|
||||
|
||||
expect(auditLog).toHaveBeenCalledTimes(1);
|
||||
const [action, opts] = vi.mocked(auditLog).mock.calls[0] as unknown as [
|
||||
string,
|
||||
{
|
||||
userId: string;
|
||||
details: { issues: Array<{ path: string; code: string }> };
|
||||
},
|
||||
];
|
||||
expect(opts.userId).toBe("user-1");
|
||||
expect(action).toBe("dashboard.widgets.validation-failed");
|
||||
const details = opts.details;
|
||||
expect(details.issues.length).toBe(2);
|
||||
for (const issue of details.issues) {
|
||||
// v1.4.49 — audit row is the persisted surface. `message`
|
||||
// strips here so a future Zod code that embeds the offending
|
||||
// value cannot leak into the ledger.
|
||||
expect(Object.keys(issue).sort()).toEqual(["code", "path"]);
|
||||
}
|
||||
});
|
||||
|
||||
it("dedups the audit-ledger write across two sequential 422s for the same user (v1.4.43 B2)", async () => {
|
||||
// First 422 writes one row; the second 422 inside the 60 s window
|
||||
// returns the same envelope but skips the audit insert.
|
||||
const res1 = await callPut(makeReq({ version: 2, widgets: [] }));
|
||||
expect(res1.status).toBe(422);
|
||||
const res2 = await callPut(makeReq({ version: 2, widgets: [] }));
|
||||
expect(res2.status).toBe(422);
|
||||
|
||||
await new Promise((r) => setTimeout(r, 5));
|
||||
|
||||
expect(auditLog).toHaveBeenCalledTimes(1);
|
||||
|
||||
// The full multi-issue envelope still rides on every 422 — the
|
||||
// dedup only suppresses the breadcrumb write, never the response.
|
||||
const body2 = (await res2.json()) as {
|
||||
details: { issues: Array<unknown> };
|
||||
};
|
||||
expect(body2.details.issues.length).toBe(2);
|
||||
// The refusal is telemetry on the wide event; the write resolves the
|
||||
// caller, so no delegate's attempt can reach this route.
|
||||
expect(auditLog).not.toHaveBeenCalled();
|
||||
expect(prisma.auditLog.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("surfaces received_keys + received_shape_excerpt + zod_issues in the wide-event meta (v1.4.48 H-iOS-1)", async () => {
|
||||
@@ -304,18 +266,6 @@ describe("PUT /api/dashboard/widgets — 422 multi-issue envelope (v1.4.42 W2)",
|
||||
expect((meta.received_shape_excerpt as string).length).toBe(256);
|
||||
});
|
||||
|
||||
it("does not block the 422 response when the audit-row write rejects", async () => {
|
||||
vi.mocked(auditLog).mockRejectedValueOnce(new Error("db down"));
|
||||
|
||||
const res = await callPut(makeReq({ version: 2, widgets: [] }));
|
||||
// The response is the contract — the audit row is best-effort.
|
||||
expect(res.status).toBe(422);
|
||||
const body = (await res.json()) as {
|
||||
details: { issues: Array<unknown> };
|
||||
};
|
||||
expect(body.details.issues.length).toBe(2);
|
||||
});
|
||||
|
||||
it("redacts sensitive keys before writing the wide-event received_shape_excerpt (v1.4.49)", async () => {
|
||||
// Caller adds a credential-shaped field — must not land in the
|
||||
// wide-event excerpt verbatim. The denylist also covers nested
|
||||
@@ -359,25 +309,6 @@ describe("PUT /api/dashboard/widgets — 422 multi-issue envelope (v1.4.42 W2)",
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("strips `message` from the audit-ledger issues row (v1.4.49)", async () => {
|
||||
// The wide-event meta keeps full message for operator debugging,
|
||||
// but the persisted auditLog row must only carry `path` + `code`
|
||||
// so a future Zod code that embeds the offending value cannot
|
||||
// leak into the ledger.
|
||||
await callPut(makeReq({ version: 2, widgets: [] }));
|
||||
|
||||
expect(auditLog).toHaveBeenCalledTimes(1);
|
||||
const parsed = (
|
||||
vi.mocked(auditLog).mock.calls[0] as unknown as [
|
||||
string,
|
||||
{ details: { issues: Array<Record<string, unknown>> } },
|
||||
]
|
||||
)[1].details;
|
||||
for (const issue of parsed.issues) {
|
||||
expect(Object.keys(issue).sort()).toEqual(["code", "path"]);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("PUT /api/dashboard/widgets — accept-and-ignore unknown ids (v1.7.0 #9)", () => {
|
||||
|
||||
@@ -67,8 +67,6 @@ const widgetIdEnum = z.enum(DASHBOARD_WIDGET_CATALOGUE_IDS);
|
||||
// `/api/dashboard/widgets` and `/api/dashboard/chart-overlay-prefs` share
|
||||
// the same 60 s `(userId, action)` window.
|
||||
export { __resetAuditDedupMemoForTests } from "@/lib/audit-dedup";
|
||||
import { shouldEmitAuditRow } from "@/lib/audit-dedup";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
|
||||
const layoutSchema = z.object({
|
||||
version: z.literal(1),
|
||||
@@ -361,34 +359,10 @@ export const PUT = apiHandler(async (request: NextRequest) => {
|
||||
zod_issues: issues,
|
||||
},
|
||||
});
|
||||
// v1.4.43 B2 — gate the breadcrumb behind a 60 s (userId, action) dedup
|
||||
// so a misbehaving iOS client retrying every second cannot pump
|
||||
// thousands of identical rows into the audit ledger.
|
||||
if (
|
||||
shouldEmitAuditRow(
|
||||
user.id,
|
||||
"dashboard.widgets.validation-failed",
|
||||
Date.now(),
|
||||
)
|
||||
) {
|
||||
// Best-effort breadcrumb — never block the 422 on a write miss.
|
||||
// v1.4.49 — strip `message` from the audit-ledger row so Zod
|
||||
// codes that embed the offending value in their default message
|
||||
// (`invalid_enum_value` and similar) cannot leak user content
|
||||
// through the audit surface. The wide-event excerpt above
|
||||
// already carries the shape signal for operator debugging.
|
||||
const auditIssues = sanitiseZodIssues(parsed.error.issues, {
|
||||
stripValuesFromMessage: true,
|
||||
});
|
||||
// Through `auditLog()`, which files a refused attempt only when a
|
||||
// delegate made it; the caller's own refusal stays on the wide event.
|
||||
void auditLog("dashboard.widgets.validation-failed", {
|
||||
userId: user.id,
|
||||
details: { issues: auditIssues },
|
||||
}).catch(() => {
|
||||
/* swallow — validation response is the contract, audit row is best-effort */
|
||||
});
|
||||
}
|
||||
// No audit row: this write resolves the caller (`requireAuth`), so the
|
||||
// refusal can only ever be the caller's own, which is telemetry and
|
||||
// lives on the wide event above. Only a delegate's refused attempt is
|
||||
// filed (see `isRejectedAttemptAction` in `src/lib/auth/audit.ts`).
|
||||
return returnAllZodIssues(parsed.error, 422);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,12 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
// The breadcrumb's contract is the call; whether `auditLog` writes a row
|
||||
// (only for a delegate's request) is tested in its own suite.
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn(async () => undefined),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
measurement: { findMany: vi.fn(), count: vi.fn() },
|
||||
@@ -58,6 +64,7 @@ import { GET } from "../route";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getSession } from "@/lib/auth/session";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
|
||||
const SESSION_OK = {
|
||||
session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) },
|
||||
@@ -1014,17 +1021,18 @@ describe("GET /api/measurements/series — 422 multi-issue (v1.4.43 W6)", () =>
|
||||
expect(body.details.issues.length).toBeGreaterThanOrEqual(2);
|
||||
});
|
||||
|
||||
it("writes a measurements.series.validation-failed audit row", async () => {
|
||||
it("files a measurements.series.validation-failed breadcrumb through auditLog", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
const res = await GET(req("kind=garbage&days=0"));
|
||||
expect(res.status).toBe(422);
|
||||
await new Promise((r) => setTimeout(r, 5));
|
||||
expect(prisma.auditLog.create).toHaveBeenCalledTimes(1);
|
||||
const call = vi.mocked(prisma.auditLog.create).mock.calls[0]?.[0] as {
|
||||
data: { userId: string; action: string };
|
||||
};
|
||||
expect(call.data.userId).toBe("user-1");
|
||||
expect(call.data.action).toBe("measurements.series.validation-failed");
|
||||
expect(auditLog).toHaveBeenCalledTimes(1);
|
||||
const [action, opts] = vi.mocked(auditLog).mock.calls[0] as unknown as [
|
||||
string,
|
||||
{ userId: string },
|
||||
];
|
||||
expect(opts.userId).toBe("user-1");
|
||||
expect(action).toBe("measurements.series.validation-failed");
|
||||
});
|
||||
|
||||
it("surfaces received_keys + received_shape_excerpt + zod_issues in the wide-event meta (v1.4.48 H-iOS-2)", async () => {
|
||||
@@ -1077,9 +1085,7 @@ describe("GET /api/measurements/series — 422 multi-issue (v1.4.43 W6)", () =>
|
||||
|
||||
it("does not block the 422 when the audit-row write rejects", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
vi.mocked(prisma.auditLog.create).mockRejectedValueOnce(
|
||||
new Error("db down"),
|
||||
);
|
||||
vi.mocked(auditLog).mockRejectedValueOnce(new Error("db down"));
|
||||
const res = await GET(req("kind=garbage&days=0"));
|
||||
expect(res.status).toBe(422);
|
||||
});
|
||||
@@ -1117,13 +1123,13 @@ describe("GET /api/measurements/series — 422 multi-issue (v1.4.43 W6)", () =>
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
await GET(req("kind=garbage&days=0"));
|
||||
|
||||
expect(prisma.auditLog.create).toHaveBeenCalledTimes(1);
|
||||
const call = vi.mocked(prisma.auditLog.create).mock.calls[0]?.[0] as {
|
||||
data: { details: string };
|
||||
};
|
||||
const parsed = JSON.parse(call.data.details) as {
|
||||
issues: Array<Record<string, unknown>>;
|
||||
};
|
||||
expect(auditLog).toHaveBeenCalledTimes(1);
|
||||
const parsed = (
|
||||
vi.mocked(auditLog).mock.calls[0] as unknown as [
|
||||
string,
|
||||
{ details: { issues: Array<Record<string, unknown>> } },
|
||||
]
|
||||
)[1].details;
|
||||
for (const issue of parsed.issues) {
|
||||
expect(Object.keys(issue).sort()).toEqual(["code", "path"]);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user