mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 06:40:50 +00:00
Move the API error classes into a leaf module
api-handler imports the record-session fence and the fence threw RecordSessionChangedError / SharingAccessDeniedError imported back from api-handler — the one real import cycle in the tree, working only by evaluation-order luck. HttpError, the SharingAuthError family and StepUpRequiredError now live in api-errors.ts (importing nothing but the fence contract), api-handler re-exports the identical class objects so no importer moves, and the fence imports the leaf. A guard pins both the identity and the leaf structure.
This commit is contained in:
1 parent
d75034a9f4
commit
b7dfd236ee
4 files changed
+228
-120
No files matched your search
@@ -0,0 +1,72 @@
|
|||||||
|
/**
|
||||||
|
* The error classes live in ONE leaf module — the cycle stays broken.
|
||||||
|
*
|
||||||
|
* `api-handler.ts` imports the record-session fence, and the fence throws
|
||||||
|
* `RecordSessionChangedError` / `SharingAccessDeniedError`. When those
|
||||||
|
* classes lived inside api-handler the two modules imported each other and
|
||||||
|
* the loop worked only by evaluation-order luck. This guard pins the split:
|
||||||
|
*
|
||||||
|
* 1. identity — the class an importer gets from `api-handler` IS the class
|
||||||
|
* the fence throws (from `api-errors`), so `instanceof` across the two
|
||||||
|
* import paths can never diverge;
|
||||||
|
* 2. structure — the fence carries no VALUE import from api-handler
|
||||||
|
* (a type-only import is erased and allowed), and the leaf imports
|
||||||
|
* nothing but the fence contract.
|
||||||
|
*
|
||||||
|
* Watched red: re-declaring `HttpError` inside api-handler (the pre-split
|
||||||
|
* shape) fails the identity leg; adding a value import of api-handler back
|
||||||
|
* into the fence fails the structural leg.
|
||||||
|
*/
|
||||||
|
import { readFileSync } from "node:fs";
|
||||||
|
import { join } from "node:path";
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
|
||||||
|
import * as apiErrors from "@/lib/api-errors";
|
||||||
|
import {
|
||||||
|
HttpError,
|
||||||
|
RecordSessionChangedError,
|
||||||
|
SharingAccessDeniedError,
|
||||||
|
SharingAuthError,
|
||||||
|
SharingNotPermittedError,
|
||||||
|
StepUpRequiredError,
|
||||||
|
} from "@/lib/api-handler";
|
||||||
|
|
||||||
|
const SRC = join(__dirname, "..");
|
||||||
|
|
||||||
|
describe("api-errors leaf module", () => {
|
||||||
|
it("api-handler re-exports the identical class objects", () => {
|
||||||
|
expect(HttpError).toBe(apiErrors.HttpError);
|
||||||
|
expect(SharingAuthError).toBe(apiErrors.SharingAuthError);
|
||||||
|
expect(SharingAccessDeniedError).toBe(apiErrors.SharingAccessDeniedError);
|
||||||
|
expect(SharingNotPermittedError).toBe(apiErrors.SharingNotPermittedError);
|
||||||
|
expect(RecordSessionChangedError).toBe(apiErrors.RecordSessionChangedError);
|
||||||
|
expect(StepUpRequiredError).toBe(apiErrors.StepUpRequiredError);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("the record-session fence carries no value import from api-handler", () => {
|
||||||
|
const source = readFileSync(
|
||||||
|
join(SRC, "lib/sharing/record-session-fence.ts"),
|
||||||
|
"utf8",
|
||||||
|
);
|
||||||
|
const imports = source.match(/^import[^;]+from "@\/lib\/api-handler";/gms);
|
||||||
|
for (const statement of imports ?? []) {
|
||||||
|
expect(
|
||||||
|
statement.startsWith("import type"),
|
||||||
|
`record-session-fence.ts imports a VALUE from api-handler — that is the cycle the api-errors leaf removed:\n${statement}`,
|
||||||
|
).toBe(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("the leaf imports nothing but the fence contract", () => {
|
||||||
|
const source = readFileSync(join(SRC, "lib/api-errors.ts"), "utf8");
|
||||||
|
const imports = source.match(/^import[^;]+;/gms) ?? [];
|
||||||
|
// Self-check: the walker found the one expected import.
|
||||||
|
expect(imports.length).toBeGreaterThan(0);
|
||||||
|
for (const statement of imports) {
|
||||||
|
expect(
|
||||||
|
statement.includes("record-session-fence-contract"),
|
||||||
|
`api-errors.ts must stay a leaf; unexpected import:\n${statement}`,
|
||||||
|
).toBe(true);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,131 @@
|
|||||||
|
/**
|
||||||
|
* Error classes for `apiHandler`-wrapped routes — a LEAF module.
|
||||||
|
*
|
||||||
|
* Moved out of `api-handler.ts` to break the one real import cycle in the
|
||||||
|
* tree: api-handler imports the record-session fence, and the fence threw
|
||||||
|
* `RecordSessionChangedError` / `SharingAccessDeniedError` imported back
|
||||||
|
* from api-handler — a loop that only worked by evaluation-order luck.
|
||||||
|
* This module imports nothing but the fence CONTRACT constants (itself a
|
||||||
|
* leaf), so anything may import it. `api-handler.ts` re-exports every
|
||||||
|
* class, so the ~500 existing importers read exactly as before.
|
||||||
|
*/
|
||||||
|
import { RECORD_FENCE_ERROR_CODE } from "./sharing/record-session-fence-contract";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Custom error class for HTTP errors with status codes.
|
||||||
|
* Throw inside apiHandler-wrapped routes to return a JSON error response.
|
||||||
|
*/
|
||||||
|
export class HttpError extends Error {
|
||||||
|
constructor(
|
||||||
|
public statusCode: number,
|
||||||
|
message: string,
|
||||||
|
) {
|
||||||
|
super(message);
|
||||||
|
this.name = "HttpError";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* v1.36.0 — the two refusals the acting-account resolver can raise.
|
||||||
|
*
|
||||||
|
* Both are 403 with a stable `meta.errorCode`, the same envelope shape the
|
||||||
|
* step-up and consent gates already use, so a client branches on the code
|
||||||
|
* rather than on prose.
|
||||||
|
*/
|
||||||
|
export abstract class SharingAuthError extends HttpError {
|
||||||
|
abstract readonly errorCode: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* "You may not act as that account."
|
||||||
|
*
|
||||||
|
* Deliberately parameterless. The message is a fixed literal and the
|
||||||
|
* constructor takes no arguments, so there is no way for a caller to vary the
|
||||||
|
* response — which is what makes a selector naming an account that does not
|
||||||
|
* exist and one naming an account that granted nothing produce the SAME BYTES.
|
||||||
|
* That is not a formatting nicety: a distinguishable refusal turns this
|
||||||
|
* endpoint into an account-enumeration oracle for anyone with a login.
|
||||||
|
*
|
||||||
|
* The indistinguishability is structural rather than careful, too. Neither
|
||||||
|
* branch ever looks an account up: the resolver asks the grant table for the
|
||||||
|
* (owner, delegate) pair and a missing account and a missing grant are the
|
||||||
|
* same empty result, on the same code path, after the same one query.
|
||||||
|
*
|
||||||
|
* Why the reason is not in the response at all: the caller already knows what
|
||||||
|
* they sent, and the only party who benefits from knowing WHICH condition
|
||||||
|
* failed is someone probing. The reason goes to the audit row and the wide
|
||||||
|
* event, where the owner and the operator can see it.
|
||||||
|
*/
|
||||||
|
export class SharingAccessDeniedError extends SharingAuthError {
|
||||||
|
readonly errorCode = "sharing.access.denied";
|
||||||
|
constructor() {
|
||||||
|
super(403, "Account access denied");
|
||||||
|
this.name = "SharingAccessDeniedError";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* "This endpoint does not act on another account's record."
|
||||||
|
*
|
||||||
|
* A different fact from the one above and so a different code: nothing here is
|
||||||
|
* about whether a grant exists. The route simply never declared that it can be
|
||||||
|
* used under a switch, so it refuses rather than quietly serving the caller's
|
||||||
|
* OWN record — which is the reverse data-mixing failure, and the reason this
|
||||||
|
* class exists at all. A delegate who believes they are reading the owner's
|
||||||
|
* record must never be handed their own instead.
|
||||||
|
*/
|
||||||
|
export class SharingNotPermittedError extends SharingAuthError {
|
||||||
|
readonly errorCode = "sharing.not_permitted";
|
||||||
|
constructor() {
|
||||||
|
super(403, "This endpoint cannot be used while acting on another account");
|
||||||
|
this.name = "SharingNotPermittedError";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* v1.37.0 — "the record this session is on is not the record you asserted."
|
||||||
|
*
|
||||||
|
* A third sibling rather than a branch, so it rides the existing
|
||||||
|
* `SharingAuthError` arm with no new serialisation code. The status is
|
||||||
|
* the one thing that differs and it differs on purpose: 409, not 403, because
|
||||||
|
* this is not a refusal of access. The caller may well be entitled to both
|
||||||
|
* records. What failed is agreement about WHICH one, and the instruction the
|
||||||
|
* code carries is "reconcile through `/api/auth/me` and try again", not "you
|
||||||
|
* have lost this record".
|
||||||
|
*
|
||||||
|
* That distinction is load-bearing for deploy compatibility: the currently
|
||||||
|
* shipped bundle reacts to `sharing.access.denied` by leaving the record and
|
||||||
|
* hard-navigating, which is exactly the wrong response to a transient
|
||||||
|
* disagreement and exactly the right one for a bundle too old to reconcile. See
|
||||||
|
* `src/lib/sharing/record-session-fence-contract.ts` for why the fence hands
|
||||||
|
* each bundle the code it can act on.
|
||||||
|
*
|
||||||
|
* The body carries `meta.errorCode` and nothing else — no current epoch, no
|
||||||
|
* current scope. A client may learn its context from exactly two responses
|
||||||
|
* (`GET /api/auth/me` and `POST /api/account/switch`), and putting the truth in
|
||||||
|
* this refusal would open a third path, one that arrives on the very requests
|
||||||
|
* whose context is in doubt.
|
||||||
|
*/
|
||||||
|
export class RecordSessionChangedError extends SharingAuthError {
|
||||||
|
readonly errorCode = RECORD_FENCE_ERROR_CODE;
|
||||||
|
constructor() {
|
||||||
|
super(409, "The record this session is on has changed");
|
||||||
|
this.name = "RecordSessionChangedError";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Error thrown when a step-up gate is not satisfied. Carries `errorCode` so
|
||||||
|
* the route can surface a stable machine code (`auth.stepup.required`) the
|
||||||
|
* client branches on to launch a re-verification flow rather than parsing
|
||||||
|
* prose.
|
||||||
|
*/
|
||||||
|
export class StepUpRequiredError extends HttpError {
|
||||||
|
constructor(
|
||||||
|
public errorCode: string = "auth.stepup.required",
|
||||||
|
message = "Recent second-factor verification required",
|
||||||
|
) {
|
||||||
|
super(401, message);
|
||||||
|
this.name = "StepUpRequiredError";
|
||||||
|
}
|
||||||
|
}
|
||||||
+19
-118
@@ -37,7 +37,6 @@ import {
|
|||||||
type GrantNeed,
|
type GrantNeed,
|
||||||
} from "./sharing/grants";
|
} from "./sharing/grants";
|
||||||
import type { ShareScope } from "./sharing/scope";
|
import type { ShareScope } from "./sharing/scope";
|
||||||
import { RECORD_FENCE_ERROR_CODE } from "./sharing/record-session-fence-contract";
|
|
||||||
import {
|
import {
|
||||||
assertRecordSessionFence,
|
assertRecordSessionFence,
|
||||||
attachRecordContextEcho,
|
attachRecordContextEcho,
|
||||||
@@ -73,108 +72,26 @@ export function isMcpAudienceToken(permissions: readonly string[]): boolean {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
// The error classes live in the LEAF module `api-errors.ts` (moved to break
|
||||||
* Custom error class for HTTP errors with status codes.
|
// the api-handler ↔ record-session-fence import cycle) and are re-exported
|
||||||
* Throw inside apiHandler-wrapped routes to return a JSON error response.
|
// here so the hundreds of existing importers read exactly as before.
|
||||||
*/
|
import {
|
||||||
export class HttpError extends Error {
|
HttpError,
|
||||||
constructor(
|
SharingAuthError,
|
||||||
public statusCode: number,
|
SharingAccessDeniedError,
|
||||||
message: string,
|
SharingNotPermittedError,
|
||||||
) {
|
RecordSessionChangedError,
|
||||||
super(message);
|
StepUpRequiredError,
|
||||||
this.name = "HttpError";
|
} from "./api-errors";
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
export {
|
||||||
* v1.36.0 — the two refusals the acting-account resolver can raise.
|
HttpError,
|
||||||
*
|
SharingAuthError,
|
||||||
* Both are 403 with a stable `meta.errorCode`, the same envelope shape the
|
SharingAccessDeniedError,
|
||||||
* step-up and consent gates already use, so a client branches on the code
|
SharingNotPermittedError,
|
||||||
* rather than on prose.
|
RecordSessionChangedError,
|
||||||
*/
|
StepUpRequiredError,
|
||||||
export abstract class SharingAuthError extends HttpError {
|
};
|
||||||
abstract readonly errorCode: string;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
|
||||||
* "You may not act as that account."
|
|
||||||
*
|
|
||||||
* Deliberately parameterless. The message is a fixed literal and the
|
|
||||||
* constructor takes no arguments, so there is no way for a caller to vary the
|
|
||||||
* response — which is what makes a selector naming an account that does not
|
|
||||||
* exist and one naming an account that granted nothing produce the SAME BYTES.
|
|
||||||
* That is not a formatting nicety: a distinguishable refusal turns this
|
|
||||||
* endpoint into an account-enumeration oracle for anyone with a login.
|
|
||||||
*
|
|
||||||
* The indistinguishability is structural rather than careful, too. Neither
|
|
||||||
* branch ever looks an account up: the resolver asks the grant table for the
|
|
||||||
* (owner, delegate) pair and a missing account and a missing grant are the
|
|
||||||
* same empty result, on the same code path, after the same one query.
|
|
||||||
*
|
|
||||||
* Why the reason is not in the response at all: the caller already knows what
|
|
||||||
* they sent, and the only party who benefits from knowing WHICH condition
|
|
||||||
* failed is someone probing. The reason goes to the audit row and the wide
|
|
||||||
* event, where the owner and the operator can see it.
|
|
||||||
*/
|
|
||||||
export class SharingAccessDeniedError extends SharingAuthError {
|
|
||||||
readonly errorCode = "sharing.access.denied";
|
|
||||||
constructor() {
|
|
||||||
super(403, "Account access denied");
|
|
||||||
this.name = "SharingAccessDeniedError";
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
|
||||||
* "This endpoint does not act on another account's record."
|
|
||||||
*
|
|
||||||
* A different fact from the one above and so a different code: nothing here is
|
|
||||||
* about whether a grant exists. The route simply never declared that it can be
|
|
||||||
* used under a switch, so it refuses rather than quietly serving the caller's
|
|
||||||
* OWN record — which is the reverse data-mixing failure, and the reason this
|
|
||||||
* class exists at all. A delegate who believes they are reading the owner's
|
|
||||||
* record must never be handed their own instead.
|
|
||||||
*/
|
|
||||||
export class SharingNotPermittedError extends SharingAuthError {
|
|
||||||
readonly errorCode = "sharing.not_permitted";
|
|
||||||
constructor() {
|
|
||||||
super(403, "This endpoint cannot be used while acting on another account");
|
|
||||||
this.name = "SharingNotPermittedError";
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
|
||||||
* v1.37.0 — "the record this session is on is not the record you asserted."
|
|
||||||
*
|
|
||||||
* A third sibling rather than a branch, so it rides the existing
|
|
||||||
* `SharingAuthError` arm below with no new serialisation code. The status is
|
|
||||||
* the one thing that differs and it differs on purpose: 409, not 403, because
|
|
||||||
* this is not a refusal of access. The caller may well be entitled to both
|
|
||||||
* records. What failed is agreement about WHICH one, and the instruction the
|
|
||||||
* code carries is "reconcile through `/api/auth/me` and try again", not "you
|
|
||||||
* have lost this record".
|
|
||||||
*
|
|
||||||
* That distinction is load-bearing for deploy compatibility: the currently
|
|
||||||
* shipped bundle reacts to `sharing.access.denied` by leaving the record and
|
|
||||||
* hard-navigating, which is exactly the wrong response to a transient
|
|
||||||
* disagreement and exactly the right one for a bundle too old to reconcile. See
|
|
||||||
* `src/lib/sharing/record-session-fence-contract.ts` for why the fence hands
|
|
||||||
* each bundle the code it can act on.
|
|
||||||
*
|
|
||||||
* The body carries `meta.errorCode` and nothing else — no current epoch, no
|
|
||||||
* current scope. A client may learn its context from exactly two responses
|
|
||||||
* (`GET /api/auth/me` and `POST /api/account/switch`), and putting the truth in
|
|
||||||
* this refusal would open a third path, one that arrives on the very requests
|
|
||||||
* whose context is in doubt.
|
|
||||||
*/
|
|
||||||
export class RecordSessionChangedError extends SharingAuthError {
|
|
||||||
readonly errorCode = RECORD_FENCE_ERROR_CODE;
|
|
||||||
constructor() {
|
|
||||||
super(409, "The record this session is on has changed");
|
|
||||||
this.name = "RecordSessionChangedError";
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Wraps an API route handler with Wide Event logging.
|
* Wraps an API route handler with Wide Event logging.
|
||||||
@@ -1119,22 +1036,6 @@ export async function requireCookieAuth(): Promise<AuthContext> {
|
|||||||
return { ...sessionData, authMethod: "cookie" };
|
return { ...sessionData, authMethod: "cookie" };
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Error thrown when a step-up gate is not satisfied. Carries `errorCode` so
|
|
||||||
* the route can surface a stable machine code (`auth.stepup.required`) the
|
|
||||||
* client branches on to launch a re-verification flow rather than parsing
|
|
||||||
* prose.
|
|
||||||
*/
|
|
||||||
export class StepUpRequiredError extends HttpError {
|
|
||||||
constructor(
|
|
||||||
public errorCode: string = "auth.stepup.required",
|
|
||||||
message = "Recent second-factor verification required",
|
|
||||||
) {
|
|
||||||
super(401, message);
|
|
||||||
this.name = "StepUpRequiredError";
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Default step-up freshness window (5 minutes) for sensitive mutations.
|
* Default step-up freshness window (5 minutes) for sensitive mutations.
|
||||||
* Within the 5–15 min band OWASP recommends; tight end because the gated
|
* Within the 5–15 min band OWASP recommends; tight end because the gated
|
||||||
|
|||||||
@@ -51,11 +51,15 @@ import {
|
|||||||
recordSessionFenceVerdict,
|
recordSessionFenceVerdict,
|
||||||
type AssertedRecordContext,
|
type AssertedRecordContext,
|
||||||
} from "@/lib/sharing/record-session-fence-contract";
|
} from "@/lib/sharing/record-session-fence-contract";
|
||||||
|
// Runtime error classes come from the LEAF module — importing them from
|
||||||
|
// api-handler recreated the import cycle this split removed. The
|
||||||
|
// `AuthContext` type import stays on api-handler: type-only, erased at
|
||||||
|
// runtime, no cycle.
|
||||||
import {
|
import {
|
||||||
RecordSessionChangedError,
|
RecordSessionChangedError,
|
||||||
SharingAccessDeniedError,
|
SharingAccessDeniedError,
|
||||||
type AuthContext,
|
} from "@/lib/api-errors";
|
||||||
} from "@/lib/api-handler";
|
import type { AuthContext } from "@/lib/api-handler";
|
||||||
import { annotate, getEvent } from "@/lib/logging/context";
|
import { annotate, getEvent } from "@/lib/logging/context";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in new issue
Block a user