mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 06:52:04 +00:00
Stop the API switch hiding live tokens, and cap the unit-preference body
Four defects the registry work surfaced, each with a check that fails without
the fix.
`GET /api/tokens` no longer consults the instance-wide API switch. That switch
gates the surfaces a token is FOR — external ingest, the MCP bridge — and not a
token's ability to authenticate: nothing on the `requireAuth` path reads it, so
a token minted before an operator flipped it stays live while it is off, and
refusing the list hid exactly the credentials their owner most needed to see,
their own signed-in phone among them. The revoke keeps the gate; the asymmetry
is stated in both route comments and pinned by the test, because widening it is
a decision about what the switch means rather than a fix to the read.
`PATCH /api/auth/me/unit-preference` reads its body through `safeJson` with a
1 KB cap, like the scalar beside it. It used a bare `request.json()` and was the
one endpoint in the family with no body-size limit at all — the new test proves
it by sending 2 KB and watching the old handler answer 200. Malformed JSON now
answers 400 like every sibling instead of 422; the only caller checks `res.ok`
and never the status, so nothing depends on the old code. The existing
malformed-JSON assertion was changed deliberately: it pinned the inconsistency,
not a contract.
`PATCH /api/auth/passkeys/{id}` returns the multi-issue envelope. It answered a
flat "Invalid request", so an empty name and a 200-character name produced
byte-identical bodies and the form had nothing to show beside the field.
`GET /api/user/thresholds` has a header that describes the handler. It promised
`{ defaults, overrides, effective }` and has always returned
`{ effective, overrides }`. The guard compares the two, so the comment cannot
drift again in either direction.
The two published operations whose contracts moved are updated with them.
This commit is contained in:
1 parent
b6efc6cb5a
commit
63f791120a
11 files changed
+507
-28
No files matched your search
@@ -148,7 +148,15 @@ describe("PATCH /api/auth/me/unit-preference", () => {
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects malformed JSON with 422", async () => {
|
||||
// The three cases below pin the capped `safeJson` read. Before it, the
|
||||
// handler called `request.json()` bare: malformed JSON came back 422 rather
|
||||
// than the 400 every sibling scalar answers, a wrong content type was parsed
|
||||
// rather than refused, and there was NO body-size limit — this was the one
|
||||
// endpoint in the family that would materialise whatever it was handed. The
|
||||
// malformed-JSON assertion used to say 422 and was changed deliberately: it
|
||||
// pinned the inconsistency rather than a contract, and the only caller
|
||||
// branches on `res.ok` alone.
|
||||
it("rejects malformed JSON with 400, like its sibling scalars", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
const req = new Request("http://localhost/api/auth/me/unit-preference", {
|
||||
method: "PATCH",
|
||||
@@ -156,7 +164,36 @@ describe("PATCH /api/auth/me/unit-preference", () => {
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
const res = await (PATCH as (r: Request) => Promise<Response>)(req);
|
||||
expect(res.status).toBe(422);
|
||||
expect(res.status).toBe(400);
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("refuses a body over the 1 KB cap with 413", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
// Valid JSON, valid enum value, and far too big: the cap has to be what
|
||||
// rejects it, not the schema.
|
||||
const req = new Request("http://localhost/api/auth/me/unit-preference", {
|
||||
method: "PATCH",
|
||||
body: JSON.stringify({
|
||||
unitPreference: "imperial",
|
||||
padding: "x".repeat(2048),
|
||||
}),
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
const res = await (PATCH as (r: Request) => Promise<Response>)(req);
|
||||
expect(res.status).toBe(413);
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("refuses a non-JSON content type with 415", async () => {
|
||||
vi.mocked(getSession).mockResolvedValue(SESSION_OK as never);
|
||||
const req = new Request("http://localhost/api/auth/me/unit-preference", {
|
||||
method: "PATCH",
|
||||
body: "unitPreference=imperial",
|
||||
headers: { "Content-Type": "application/x-www-form-urlencoded" },
|
||||
});
|
||||
const res = await (PATCH as (r: Request) => Promise<Response>)(req);
|
||||
expect(res.status).toBe(415);
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -15,13 +15,22 @@
|
||||
* Mirrors the `disable-coach` per-user-scalar pattern: 60/min rate
|
||||
* limit, Zod safeParse → 422 via `returnAllZodIssues`, audit-log row,
|
||||
* field-by-field write (no mass assignment).
|
||||
*
|
||||
* The body goes through `safeJson` with a 1 KB cap, like the
|
||||
* `documents-auto-ai-read` scalar beside it. It used to read a bare
|
||||
* `request.json()`, which applied no size limit at all — the one endpoint in
|
||||
* this family that would materialise whatever it was sent — and answered a
|
||||
* malformed body with 422 where every sibling answers 400. Both were
|
||||
* accidents of the original copy, not a contract: the only caller checks
|
||||
* `res.ok` and never the status.
|
||||
*/
|
||||
import { apiHandler, requireAuth, HttpError } from "@/lib/api-handler";
|
||||
import { apiHandler, requireAuth } from "@/lib/api-handler";
|
||||
import {
|
||||
apiError,
|
||||
apiSuccess,
|
||||
getClientIp,
|
||||
returnAllZodIssues,
|
||||
safeJson,
|
||||
} from "@/lib/api-response";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
@@ -73,12 +82,12 @@ export const PATCH = apiHandler(async (req: Request) => {
|
||||
return response;
|
||||
}
|
||||
|
||||
let body: unknown;
|
||||
try {
|
||||
body = await req.json();
|
||||
} catch {
|
||||
throw new HttpError(422, "unit-preference.body.invalid_json");
|
||||
}
|
||||
// The body is a single enum value — bound the parse so a malformed or
|
||||
// oversized payload is rejected before it is materialised.
|
||||
const { data: body, error: jsonError } = await safeJson(req, {
|
||||
maxBytes: 1024,
|
||||
});
|
||||
if (jsonError) return jsonError;
|
||||
|
||||
const parsed = unitPreferencePatchSchema.safeParse(body);
|
||||
if (!parsed.success) {
|
||||
|
||||
@@ -0,0 +1,157 @@
|
||||
/**
|
||||
* `PATCH /api/auth/passkeys/{id}` — the rename's refusal has to name the field.
|
||||
*
|
||||
* The handler used to answer `apiError("Invalid request", 422)` and throw the
|
||||
* Zod issues away, which is the one thing every sibling body-parsing route in
|
||||
* this tree does not do. A person renaming a passkey to an empty string and a
|
||||
* person pasting 200 characters got the same three words, so the form had
|
||||
* nothing to put beside the field and no way to say which rule was broken.
|
||||
*
|
||||
* These cases pin the multi-issue envelope by its CONTENT — the issue list has
|
||||
* to be present and has to point at `name` — rather than by status alone. A
|
||||
* status-only assertion would have passed against the flat error it replaced.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: (fn: unknown) => fn,
|
||||
requireAuth: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
passkey: {
|
||||
findUnique: vi.fn(),
|
||||
update: vi.fn(),
|
||||
count: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() }));
|
||||
|
||||
vi.mock("@/lib/auth/audit", () => ({
|
||||
auditLog: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
import { PATCH } from "../route";
|
||||
import { requireAuth } from "@/lib/api-handler";
|
||||
import { prisma } from "@/lib/db";
|
||||
|
||||
const USER = { id: "user-1", username: "u", passwordHash: "argon2id$…" };
|
||||
|
||||
const ROW = {
|
||||
id: "pk-1",
|
||||
name: "Phone",
|
||||
credentialDeviceType: "multiDevice",
|
||||
credentialBackedUp: true,
|
||||
createdAt: new Date("2026-01-01T00:00:00Z"),
|
||||
lastUsedAt: null,
|
||||
};
|
||||
|
||||
type IssueEnvelope = {
|
||||
error: string;
|
||||
details?: { issues?: Array<{ path?: unknown; message?: string }> };
|
||||
};
|
||||
|
||||
function mkPatch(body: unknown): Request {
|
||||
return new Request("http://localhost/api/auth/passkeys/pk-1", {
|
||||
method: "PATCH",
|
||||
body: JSON.stringify(body),
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
}
|
||||
|
||||
const params = Promise.resolve({ id: "pk-1" });
|
||||
|
||||
type Handler = (
|
||||
r: Request,
|
||||
ctx: { params: Promise<{ id: string }> },
|
||||
) => Promise<Response>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(requireAuth).mockResolvedValue({ user: USER } as never);
|
||||
});
|
||||
|
||||
describe("PATCH /api/auth/passkeys/{id} — the rename refusal", () => {
|
||||
it("returns the multi-issue envelope for an empty name", async () => {
|
||||
const res = await (PATCH as unknown as Handler)(mkPatch({ name: "" }), {
|
||||
params,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(422);
|
||||
const env = (await res.json()) as IssueEnvelope;
|
||||
// The flat refusal this replaces said "Invalid request" and carried no
|
||||
// `details` at all, so both halves matter.
|
||||
expect(env.error).toBe("Validation failed");
|
||||
expect(env.details?.issues?.length).toBeGreaterThan(0);
|
||||
expect(JSON.stringify(env.details?.issues)).toContain("name");
|
||||
expect(prisma.passkey.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns the multi-issue envelope for a name over 64 characters", async () => {
|
||||
const res = await (PATCH as unknown as Handler)(
|
||||
mkPatch({ name: "x".repeat(65) }),
|
||||
{ params },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(422);
|
||||
const env = (await res.json()) as IssueEnvelope;
|
||||
expect(env.details?.issues?.length).toBeGreaterThan(0);
|
||||
expect(JSON.stringify(env.details?.issues)).toContain("name");
|
||||
expect(prisma.passkey.update).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("distinguishes the two refusals from each other", async () => {
|
||||
// The point of the change: a client can tell WHICH rule was broken. Under
|
||||
// the flat error both bodies were byte-identical.
|
||||
const tooShort = await (PATCH as unknown as Handler)(
|
||||
mkPatch({ name: "" }),
|
||||
{ params },
|
||||
);
|
||||
const tooLong = await (PATCH as unknown as Handler)(
|
||||
mkPatch({ name: "x".repeat(65) }),
|
||||
{ params },
|
||||
);
|
||||
|
||||
expect(JSON.stringify(await tooShort.json())).not.toBe(
|
||||
JSON.stringify(await tooLong.json()),
|
||||
);
|
||||
});
|
||||
|
||||
it("still renames on a valid name, trimming it first", async () => {
|
||||
vi.mocked(prisma.passkey.findUnique).mockResolvedValue({
|
||||
userId: USER.id,
|
||||
} as never);
|
||||
vi.mocked(prisma.passkey.update).mockResolvedValue({
|
||||
...ROW,
|
||||
name: "Work laptop",
|
||||
} as never);
|
||||
|
||||
const res = await (PATCH as unknown as Handler)(
|
||||
mkPatch({ name: " Work laptop " }),
|
||||
{ params },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(prisma.passkey.update).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ data: { name: "Work laptop" } }),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not leak another account's passkey through the rename", async () => {
|
||||
vi.mocked(prisma.passkey.findUnique).mockResolvedValue({
|
||||
userId: "someone-else",
|
||||
} as never);
|
||||
|
||||
const res = await (PATCH as unknown as Handler)(
|
||||
mkPatch({ name: "Mine now" }),
|
||||
{ params },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(prisma.passkey.update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
apiSuccess,
|
||||
apiError,
|
||||
getClientIp,
|
||||
returnAllZodIssues,
|
||||
safeJson,
|
||||
} from "@/lib/api-response";
|
||||
import { NextRequest } from "next/server";
|
||||
@@ -26,7 +27,12 @@ export const PATCH = apiHandler(
|
||||
|
||||
const parsed = passkeyRenameSchema.safeParse(body);
|
||||
if (!parsed.success) {
|
||||
return apiError("Invalid request", 422);
|
||||
// Multi-issue, like every sibling that parses a body. The flat
|
||||
// `apiError("Invalid request", 422)` this replaces discarded the issues,
|
||||
// so a person renaming a passkey could not tell an empty name from one
|
||||
// over the 64-character limit — the form had nothing to put beside the
|
||||
// field.
|
||||
return returnAllZodIssues(parsed.error, 422);
|
||||
}
|
||||
|
||||
const passkey = await prisma.passkey.findUnique({
|
||||
|
||||
Reference in new issue
Block a user