From 796d009c07a2255bc5d5c3a335af49619bc20ae0 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Thu, 27 Aug 2026 18:58:14 +0200 Subject: [PATCH] fix(housekeeping): harden audited command dispatch --- src/actions/housekeeping-command.test.ts | 118 +++++-- src/actions/housekeeping-command.ts | 63 +++- .../foundation/commands/confirmation.ts | 4 +- .../foundation/commands/dispatcher.test.ts | 317 +++++++++++++++++- .../foundation/commands/dispatcher.ts | 199 ++++++++--- .../foundation/commands/registry.test.ts | 87 ++++- .../foundation/commands/registry.ts | 111 ++++-- 7 files changed, 775 insertions(+), 124 deletions(-) diff --git a/src/actions/housekeeping-command.test.ts b/src/actions/housekeeping-command.test.ts index f2adf81b..f82fc3ce 100644 --- a/src/actions/housekeeping-command.test.ts +++ b/src/actions/housekeeping-command.test.ts @@ -7,8 +7,18 @@ import { } from "@/features/housekeeping/foundation/contracts"; import type { AuditEntry } from "@/lib/services/audit"; -const { auditEntries, context, rateLimitCalls } = vi.hoisted(() => ({ +const { + auditEntries, + auditWriteMock, + commandExecutions, + context, + getContextMock, + getIpMock, + rateLimitCalls, +} = vi.hoisted(() => ({ auditEntries: [] as AuditEntry[], + auditWriteMock: vi.fn(), + commandExecutions: [] as string[], context: { actor: { id: 71, username: "server-operator", rank: 4 }, isSuperAdmin: false, @@ -17,23 +27,21 @@ const { auditEntries, context, rateLimitCalls } = vi.hoisted(() => ({ hasAll: (...slugs: string[]) => slugs.every((slug) => slug === "admin.settings.edit"), }, + getContextMock: vi.fn(), + getIpMock: vi.fn(), rateLimitCalls: [] as Array<[string, number, number]>, })); vi.mock("@/features/housekeeping/foundation/server-capability-context", () => ({ - getHousekeepingCapabilityContext: async () => context, + getHousekeepingCapabilityContext: getContextMock, })); vi.mock("@/lib/services/audit", () => ({ - housekeepingAuditWriter: { - write: async (entry: AuditEntry) => { - auditEntries.push({ ...entry }); - }, - }, + housekeepingAuditWriter: { write: auditWriteMock }, })); vi.mock("@/lib/rate-limit", () => ({ - clientIp: async () => "203.0.113.7", + clientIp: getIpMock, rateLimit: async (key: string, attempts: number, windowMs: number) => { rateLimitCalls.push([key, attempts, windowMs]); return { ok: true, retryAfter: 0 }; @@ -50,37 +58,36 @@ registerHousekeepingCommand({ input: z.object({ value: z.string() }), requiresReason: false, rateLimit: { attempts: 5, windowMs: 120_000 }, - execute: async (commandContext, input) => - ok( + execute: async (commandContext, input) => { + commandExecutions.push(input.value); + return ok( { value: input.value, actorId: commandContext.capability.actor.id, ipAddress: commandContext.ipAddress, }, commandContext.correlationId, - ), + ); + }, }); beforeEach(() => { auditEntries.length = 0; + commandExecutions.length = 0; rateLimitCalls.length = 0; + getContextMock.mockReset().mockResolvedValue(context); + getIpMock.mockReset().mockResolvedValue("203.0.113.7"); + auditWriteMock.mockReset().mockImplementation(async (entry: AuditEntry) => { + auditEntries.push({ ...entry }); + }); }); describe("executeHousekeepingCommand", () => { - it("accepts a plain request and ignores spoofed server-owned metadata", async () => { - const request = { + it("accepts a plain request and derives all policy metadata server-side", async () => { + const result = await executeHousekeepingCommand({ commandId: "system.server-action.serializable", input: { value: "saved" }, - risk: "sensitive", - owner: "people", - capability: { mode: "any", slugs: ["forged.permission"] }, - actor: { id: 999 }, - ipAddress: "198.51.100.9", - rateLimit: { attempts: 999, windowMs: 1 }, - audit: { action: "forged.action", target: "forged-target" }, - }; - - const result = await executeHousekeepingCommand(request); + }); expect(result).toMatchObject({ ok: true, @@ -109,4 +116,69 @@ describe("executeHousekeepingCommand", () => { ]); expect(auditEntries[0]?.correlationId).toBe(result.correlationId); }); + + it("strictly rejects spoofed server-owned metadata before execution", async () => { + const result = await executeHousekeepingCommand({ + commandId: "system.server-action.serializable", + input: { value: "forged" }, + risk: "sensitive", + owner: "people", + capability: { mode: "any", slugs: ["forged.permission"] }, + actor: { id: 999 }, + ipAddress: "198.51.100.9", + rateLimit: { attempts: 999, windowMs: 1 }, + audit: { action: "forged.action", target: "forged-target" }, + } as never); + + expect(result).toMatchObject({ + ok: false, + error: { code: "VALIDATION" }, + }); + expect(commandExecutions).toEqual([]); + expect(rateLimitCalls).toEqual([]); + expect(auditEntries).toEqual([]); + }); + + it("sanitizes server context acquisition failures into typed results", async () => { + getContextMock.mockRejectedValue( + new Error("session database secret exposed"), + ); + + const result = await executeHousekeepingCommand({ + commandId: "system.server-action.serializable", + input: { value: "blocked" }, + }); + + expect(result).toMatchObject({ + ok: false, + error: { code: "INTERNAL", messageKey: "errors.housekeeping.internal" }, + }); + expect(JSON.stringify(result)).not.toContain("secret exposed"); + expect(commandExecutions).toEqual([]); + }); + + it("maps completed-operation audit failures to a typed partial result", async () => { + auditWriteMock.mockImplementation(async (entry: AuditEntry) => { + if (entry.outcome === "success") { + throw new Error("success audit unavailable"); + } + auditEntries.push({ ...entry }); + }); + + const result = await executeHousekeepingCommand({ + commandId: "system.server-action.serializable", + input: { value: "changed" }, + }); + + expect(commandExecutions).toEqual(["changed"]); + expect(result).toMatchObject({ + ok: false, + error: { + code: "INTERNAL", + messageKey: "errors.housekeeping.partial", + }, + }); + expect(auditEntries.map((entry) => entry.outcome)).toEqual(["partial"]); + expect(auditEntries[0]?.correlationId).toBe(result.correlationId); + }); }); diff --git a/src/actions/housekeeping-command.ts b/src/actions/housekeeping-command.ts index e699f01a..fec43c6d 100644 --- a/src/actions/housekeeping-command.ts +++ b/src/actions/housekeeping-command.ts @@ -1,27 +1,58 @@ "use server"; +import { AuditOutcomePersistenceError } from "@/features/housekeeping/foundation/commands/audit-envelope"; +import { dispatchHousekeepingCommand } from "@/features/housekeeping/foundation/commands/dispatcher"; +import { sealHousekeepingCommandRegistry } from "@/features/housekeeping/foundation/commands/registry"; import { - dispatchHousekeepingCommand, - type HousekeepingCommandRequest, -} from "@/features/housekeeping/foundation/commands/dispatcher"; -import type { HousekeepingResult } from "@/features/housekeeping/foundation/contracts"; + fail, + type HousekeepingResult, + mapUnknownError, +} from "@/features/housekeeping/foundation/contracts"; import { getHousekeepingCapabilityContext } from "@/features/housekeeping/foundation/server-capability-context"; import { clientIp, rateLimit } from "@/lib/rate-limit"; import { housekeepingAuditWriter } from "@/lib/services/audit"; export async function executeHousekeepingCommand( - request: HousekeepingCommandRequest, + request: unknown, ): Promise> { - const [context, ipAddress] = await Promise.all([ - getHousekeepingCapabilityContext(), - clientIp(), - ]); + sealHousekeepingCommandRegistry(); + try { + const [context, ipAddress] = await Promise.all([ + getHousekeepingCapabilityContext(), + clientIp(), + ]); - return dispatchHousekeepingCommand(request, { - context, - ipAddress, - audit: housekeepingAuditWriter, - rateLimit: async (key, attempts, windowMs) => - (await rateLimit(key, attempts, windowMs)).ok, - }); + return await dispatchHousekeepingCommand(request, { + context, + ipAddress, + audit: housekeepingAuditWriter, + rateLimit: async (key, attempts, windowMs) => + (await rateLimit(key, attempts, windowMs)).ok, + }); + } catch (error) { + if ( + error instanceof AuditOutcomePersistenceError && + isHousekeepingResult(error.operationResult) + ) { + return fail( + "INTERNAL", + "errors.housekeeping.partial", + error.operationResult.correlationId, + ); + } + return mapUnknownError(error); + } +} + +function isHousekeepingResult( + value: unknown, +): value is HousekeepingResult { + return ( + typeof value === "object" && + value !== null && + "ok" in value && + typeof (value as { ok?: unknown }).ok === "boolean" && + "correlationId" in value && + typeof (value as { correlationId?: unknown }).correlationId === "string" + ); } diff --git a/src/features/housekeeping/foundation/commands/confirmation.ts b/src/features/housekeeping/foundation/commands/confirmation.ts index 0ffed083..4ea06a62 100644 --- a/src/features/housekeeping/foundation/commands/confirmation.ts +++ b/src/features/housekeeping/foundation/commands/confirmation.ts @@ -8,8 +8,8 @@ export interface HousekeepingCommandConfirmation { reason?: string; } -export function confirmHousekeepingCommand( - command: HousekeepingCommand, +export function confirmHousekeepingCommand( + command: HousekeepingCommand, reason: string | undefined, correlationId: string, ): HousekeepingResult { diff --git a/src/features/housekeeping/foundation/commands/dispatcher.test.ts b/src/features/housekeeping/foundation/commands/dispatcher.test.ts index 2ef5dbbb..f13787d4 100644 --- a/src/features/housekeeping/foundation/commands/dispatcher.test.ts +++ b/src/features/housekeeping/foundation/commands/dispatcher.test.ts @@ -122,12 +122,8 @@ describe("dispatchHousekeepingCommand", () => { error: { code: "FORBIDDEN" }, }); expect(executed).toBe(false); - expect(audit.entries.map((entry) => entry.outcome)).toEqual([ - "intent", - "denied", - ]); + expect(audit.entries.map((entry) => entry.outcome)).toEqual(["denied"]); expect(audit.entries[0]?.correlationId).toBe(result.correlationId); - expect(audit.entries[1]?.correlationId).toBe(result.correlationId); }); it("rejects invalid input before the command can execute", async () => { @@ -182,10 +178,7 @@ describe("dispatchHousekeepingCommand", () => { }, }); expect(executed).toBe(false); - expect(audit.entries.map((entry) => entry.outcome)).toEqual([ - "intent", - "denied", - ]); + expect(audit.entries.map((entry) => entry.outcome)).toEqual(["denied"]); }); it("uses actor, IP, command ID, and the approved command limit", async () => { @@ -392,4 +385,310 @@ describe("dispatchHousekeepingCommand", () => { ]); expect(audit.entries[1]?.correlationId).toBe(result.correlationId); }); + it("runs sensitive validation and rate preflight before intent and execution", async () => { + const events: string[] = []; + const audit = auditRecorder(events); + const tracedContext = { + ...capabilityContext(), + hasAny: (...slugs: string[]) => { + events.push("capability"); + return slugs.includes("admin.settings.edit"); + }, + }; + register( + baseCommand("system.dispatch.preflight-order", { + risk: "sensitive", + requiresReason: true, + input: z.object({ enabled: z.boolean() }).superRefine(() => { + events.push("input"); + }), + execute: async (context) => { + events.push("execute"); + return ok(null, context.correlationId); + }, + }), + ); + + await dispatchHousekeepingCommand( + { + commandId: "system.dispatch.preflight-order", + input: { enabled: true }, + reason: "Approved change", + }, + dependencies({ + context: tracedContext, + audit: audit.writer, + rateLimit: async () => { + events.push("rate"); + return true; + }, + }), + ); + + expect(events).toEqual([ + "capability", + "input", + "rate", + "audit:intent", + "execute", + "audit:success", + ]); + }); + + it.each([ + ["capability", "system.dispatch.preflight-denied"], + ["input", "system.dispatch.preflight-invalid"], + ["rate", "system.dispatch.preflight-limited"], + ] as const)( + "writes only denied evidence when sensitive %s preflight rejects", + async (boundary, commandId) => { + const audit = auditRecorder(); + let executed = false; + register( + baseCommand(commandId, { + risk: "sensitive", + input: z.object({ count: z.number().positive() }), + execute: async (context) => { + executed = true; + return ok(null, context.correlationId); + }, + }), + ); + + const result = await dispatchHousekeepingCommand( + { + commandId, + input: { count: boundary === "input" ? -1 : 1 }, + }, + dependencies({ + context: + boundary === "capability" + ? capabilityContext([]) + : capabilityContext(), + audit: audit.writer, + rateLimit: async () => boundary !== "rate", + }), + ); + + expect(result).toMatchObject({ ok: false }); + expect(executed).toBe(false); + expect(audit.entries.map((entry) => entry.outcome)).toEqual(["denied"]); + expect(audit.entries[0]?.correlationId).toBe(result.correlationId); + }, + ); + + it("writes generic server-owned evidence for a validated unknown command", async () => { + const audit = auditRecorder(); + + const result = await dispatchHousekeepingCommand( + { commandId: "system.dispatch.unknown-audited", input: {} }, + dependencies({ audit: audit.writer }), + ); + + expect(result).toMatchObject({ + ok: false, + error: { code: "NOT_FOUND" }, + }); + expect(audit.entries).toEqual([ + { + userId: 42, + action: "housekeeping.command.dispatch", + target: "system.dispatch.unknown-audited", + correlationId: result.correlationId, + outcome: "denied", + ipAddress: "198.51.100.8", + }, + ]); + }); + + it.each([ + null, + [], + Object.assign(Object.create({ inherited: true }), { + commandId: "system.dispatch.unknown", + input: {}, + }), + { commandId: "system.dispatch.unknown", input: {}, risk: "safe" }, + { commandId: `system.${"x".repeat(200)}`, input: {} }, + { + commandId: "system.dispatch.unknown", + input: {}, + reason: "x".repeat(1001), + }, + ])( + "returns a correlated validation result for malformed runtime request %#", + async (request) => { + const result = await dispatchHousekeepingCommand( + request as never, + dependencies(), + ); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "VALIDATION", + messageKey: "errors.housekeeping.validation", + }, + }); + expect(result.correlationId).toMatch(/^[0-9a-f-]{36}$/i); + }, + ); + + it("sanitizes a capability exception and writes failure evidence", async () => { + const audit = auditRecorder(); + register( + baseCommand("system.dispatch.capability-exception", { + input: z.object({}), + execute: async (context) => ok(null, context.correlationId), + }), + ); + const brokenContext = { + ...capabilityContext(), + hasAny: () => { + throw new Error("capability secret exposed"); + }, + }; + + const result = await dispatchHousekeepingCommand( + { commandId: "system.dispatch.capability-exception", input: {} }, + dependencies({ context: brokenContext, audit: audit.writer }), + ); + + expect(result).toMatchObject({ + ok: false, + error: { code: "INTERNAL", messageKey: "errors.housekeeping.internal" }, + }); + expect(JSON.stringify(result)).not.toContain("secret exposed"); + expect(audit.entries.map((entry) => entry.outcome)).toEqual(["failure"]); + }); + + it("sanitizes a schema exception and writes failure evidence", async () => { + const audit = auditRecorder(); + register( + baseCommand("system.dispatch.schema-exception", { + input: z.preprocess(() => { + throw new Error("schema secret exposed"); + }, z.object({})), + execute: async (context) => ok(null, context.correlationId), + }), + ); + + const result = await dispatchHousekeepingCommand( + { commandId: "system.dispatch.schema-exception", input: {} }, + dependencies({ audit: audit.writer }), + ); + + expect(result).toMatchObject({ + ok: false, + error: { code: "INTERNAL", messageKey: "errors.housekeeping.internal" }, + }); + expect(JSON.stringify(result)).not.toContain("secret exposed"); + expect(audit.entries.map((entry) => entry.outcome)).toEqual(["failure"]); + }); + + it("throws completed-operation evidence and persists partial when success audit rejects", async () => { + const attempts: string[] = []; + const persisted: string[] = []; + register( + baseCommand("system.dispatch.success-audit-rejection", { + input: z.object({}), + execute: async (context) => + ok({ changed: true }, context.correlationId), + }), + ); + + const dispatch = dispatchHousekeepingCommand( + { commandId: "system.dispatch.success-audit-rejection", input: {} }, + dependencies({ + audit: { + write: async (entry) => { + attempts.push(entry.outcome ?? "missing"); + if (entry.outcome === "success") { + throw new Error("success audit unavailable"); + } + persisted.push(entry.outcome ?? "missing"); + }, + }, + }), + ); + + await expect(dispatch).rejects.toMatchObject({ + name: "AuditOutcomePersistenceError", + operationCompleted: true, + operationResult: { ok: true, data: { changed: true } }, + }); + expect(attempts).toEqual(["success", "partial"]); + expect(persisted).toEqual(["partial"]); + }); + + it("preserves a returned command failure when failure audit rejects", async () => { + const attempts: string[] = []; + register( + baseCommand("system.dispatch.returned-failure-audit-rejection", { + input: z.object({}), + execute: async () => + fail( + "DEPENDENCY_UNAVAILABLE", + "errors.housekeeping.dependencyUnavailable", + "wrong-correlation", + ), + }), + ); + + const result = await dispatchHousekeepingCommand( + { + commandId: "system.dispatch.returned-failure-audit-rejection", + input: {}, + }, + dependencies({ + audit: { + write: async (entry) => { + attempts.push(entry.outcome ?? "missing"); + throw new Error("failure audit unavailable"); + }, + }, + }), + ); + + expect(result).toMatchObject({ + ok: false, + error: { code: "DEPENDENCY_UNAVAILABLE" }, + }); + expect(result.correlationId).not.toBe("wrong-correlation"); + expect(attempts).toEqual(["failure"]); + }); + + it("preserves a sanitized throwing-command failure when failure audit rejects", async () => { + const attempts: string[] = []; + register( + baseCommand("system.dispatch.thrown-failure-audit-rejection", { + input: z.object({}), + execute: async () => { + throw new Error("command database secret exposed"); + }, + }), + ); + + const result = await dispatchHousekeepingCommand( + { + commandId: "system.dispatch.thrown-failure-audit-rejection", + input: {}, + }, + dependencies({ + audit: { + write: async (entry) => { + attempts.push(entry.outcome ?? "missing"); + throw new Error("audit replacement secret exposed"); + }, + }, + }), + ); + + expect(result).toMatchObject({ + ok: false, + error: { code: "INTERNAL", messageKey: "errors.housekeeping.internal" }, + }); + expect(JSON.stringify(result)).not.toContain("secret exposed"); + expect(attempts).toEqual(["failure"]); + }); }); diff --git a/src/features/housekeeping/foundation/commands/dispatcher.ts b/src/features/housekeeping/foundation/commands/dispatcher.ts index 1bd61393..292ff88a 100644 --- a/src/features/housekeeping/foundation/commands/dispatcher.ts +++ b/src/features/housekeeping/foundation/commands/dispatcher.ts @@ -1,3 +1,4 @@ +import { z } from "zod"; import type { AuditEntry, HousekeepingAuditWriter } from "@/lib/services/audit"; import { satisfiesCapability } from "../capability-context"; import { @@ -7,7 +8,11 @@ import { mapUnknownError, } from "../contracts"; import { createCorrelationId } from "../correlation"; -import { writeIntent, writeOutcome } from "./audit-envelope"; +import { + AuditOutcomePersistenceError, + writeIntent, + writeOutcome, +} from "./audit-envelope"; import { confirmHousekeepingCommand } from "./confirmation"; import { getHousekeepingCommand, @@ -15,6 +20,20 @@ import { type HousekeepingCommandContext, } from "./registry"; +const normalizedCommandId = z + .string() + .transform((value) => value.normalize("NFC").trim()) + .pipe(z.string().min(1).max(160)); +const normalizedReason = z + .string() + .transform((value) => value.normalize("NFC").trim()) + .pipe(z.string().max(1000)); +const commandRequestSchema = z.strictObject({ + commandId: normalizedCommandId, + input: z.unknown(), + reason: normalizedReason.optional(), +}); + export interface HousekeepingCommandRequest { commandId: string; input: unknown; @@ -29,54 +48,86 @@ export interface HousekeepingCommandDependencies { } export async function dispatchHousekeepingCommand( - request: HousekeepingCommandRequest, + request: unknown, dependencies: HousekeepingCommandDependencies, ): Promise> { const correlationId = createCorrelationId(); - const command = getHousekeepingCommand(request.commandId); + let parsedRequest: ReturnType; + try { + parsedRequest = commandRequestSchema.safeParse(request); + } catch (error) { + return mapUnknownError(error, correlationId); + } + if (!parsedRequest.success) { + return fail("VALIDATION", "errors.housekeeping.validation", correlationId); + } + + const commandRequest = parsedRequest.data; + const command = getHousekeepingCommand(commandRequest.commandId); if (!command) { - return fail("NOT_FOUND", "errors.housekeeping.notFound", correlationId); + const result = fail( + "NOT_FOUND", + "errors.housekeeping.notFound", + correlationId, + ); + return persistReturnedOutcome( + dependencies.audit, + createUnknownCommandAuditEntry( + commandRequest.commandId, + dependencies, + correlationId, + ), + "denied", + result, + ); } - const reason = - typeof request.reason === "string" - ? request.reason.normalize("NFC").trim() || undefined - : undefined; const auditEntry = createAuditEntry( command, dependencies.context, dependencies.ipAddress, correlationId, - reason, + commandRequest.reason || undefined, ); - if (command.risk === "sensitive") { - try { - await writeIntent(dependencies.audit, auditEntry); - } catch (error) { - return mapUnknownError(error, correlationId); - } + let authorized: boolean; + try { + authorized = satisfiesCapability(dependencies.context, command.capability); + } catch (error) { + return persistReturnedOutcome( + dependencies.audit, + auditEntry, + "failure", + mapUnknownError(error, correlationId), + ); } - - if (!satisfiesCapability(dependencies.context, command.capability)) { - return finishWithAudit( + if (!authorized) { + return persistReturnedOutcome( dependencies.audit, auditEntry, "denied", fail("FORBIDDEN", "errors.housekeeping.forbidden", correlationId), - correlationId, ); } - const parsedInput = command.input.safeParse(request.input); + let parsedInput: ReturnType; + try { + parsedInput = command.input.safeParse(commandRequest.input); + } catch (error) { + return persistReturnedOutcome( + dependencies.audit, + auditEntry, + "failure", + mapUnknownError(error, correlationId), + ); + } if (!parsedInput.success) { const fieldErrors: Record = {}; for (const issue of parsedInput.error.issues) { const field = String(issue.path[0] ?? "input"); fieldErrors[field] = ["errors.validation.invalid"]; } - - return finishWithAudit( + return persistReturnedOutcome( dependencies.audit, auditEntry, "denied", @@ -86,22 +137,30 @@ export async function dispatchHousekeepingCommand( correlationId, fieldErrors, ), - correlationId, ); } - const confirmation = confirmHousekeepingCommand( - command, - reason, - correlationId, - ); + let confirmation: HousekeepingResult; + try { + confirmation = confirmHousekeepingCommand( + command, + commandRequest.reason || undefined, + correlationId, + ); + } catch (error) { + return persistReturnedOutcome( + dependencies.audit, + auditEntry, + "failure", + mapUnknownError(error, correlationId), + ); + } if (!confirmation.ok) { - return finishWithAudit( + return persistReturnedOutcome( dependencies.audit, auditEntry, "denied", confirmation, - correlationId, ); } @@ -117,25 +176,30 @@ export async function dispatchHousekeepingCommand( command.rateLimit.windowMs, ); } catch (error) { - return finishWithAudit( + return persistReturnedOutcome( dependencies.audit, auditEntry, "failure", mapUnknownError(error, correlationId), - correlationId, ); } - if (!allowed) { - return finishWithAudit( + return persistReturnedOutcome( dependencies.audit, auditEntry, "denied", fail("RATE_LIMITED", "errors.housekeeping.rateLimited", correlationId), - correlationId, ); } + if (command.risk === "sensitive") { + try { + await writeIntent(dependencies.audit, auditEntry); + } catch (error) { + return mapUnknownError(error, correlationId); + } + } + const commandContext: HousekeepingCommandContext = { capability: dependencies.context, correlationId, @@ -145,22 +209,27 @@ export async function dispatchHousekeepingCommand( try { result = await command.execute(commandContext, parsedInput.data); } catch (error) { - return finishWithAudit( + return persistReturnedOutcome( dependencies.audit, auditEntry, "failure", mapUnknownError(error, correlationId), - correlationId, ); } const correlatedResult = withCorrelation(result, correlationId); - return finishWithAudit( + if (!correlatedResult.ok) { + return persistReturnedOutcome( + dependencies.audit, + auditEntry, + outcomeForFailure(correlatedResult), + correlatedResult, + ); + } + return persistSuccessfulOutcome( dependencies.audit, auditEntry, - outcomeFor(correlatedResult), correlatedResult, - correlationId, ); } @@ -182,10 +251,24 @@ function createAuditEntry( }; } +function createUnknownCommandAuditEntry( + commandId: string, + dependencies: HousekeepingCommandDependencies, + correlationId: string, +): AuditEntry { + return { + userId: dependencies.context.actor.id, + action: "housekeeping.command.dispatch", + target: commandId, + correlationId, + ipAddress: dependencies.ipAddress, + }; +} + function createRateLimitKey( commandId: string, context: HousekeepingCapabilityContext, - ipAddress = "0.0.0.0", + ipAddress: string, ): string { return `housekeeping-command:${context.actor.id}:${ipAddress}:${commandId}`; } @@ -197,10 +280,9 @@ function withCorrelation( return { ...result, correlationId }; } -function outcomeFor( - result: HousekeepingResult, -): "success" | "failure" | "denied" { - if (result.ok) return "success"; +function outcomeForFailure( + result: Extract, { ok: false }>, +): "failure" | "denied" { return result.error.code === "FORBIDDEN" || result.error.code === "VALIDATION" || result.error.code === "RATE_LIMITED" @@ -208,17 +290,34 @@ function outcomeFor( : "failure"; } -async function finishWithAudit( +async function persistReturnedOutcome( writer: HousekeepingAuditWriter, entry: AuditEntry, - outcome: "success" | "failure" | "denied", + outcome: "failure" | "denied", result: HousekeepingResult, - correlationId: string, ): Promise> { try { await writeOutcome(writer, entry, outcome); + } catch { + // The command/preflight failure remains authoritative if evidence is down. + } + return result; +} + +async function persistSuccessfulOutcome( + writer: HousekeepingAuditWriter, + entry: AuditEntry, + result: HousekeepingResult, +): Promise> { + try { + await writeOutcome(writer, entry, "success"); return result; - } catch (error) { - return mapUnknownError(error, correlationId); + } catch (auditOutcomeError) { + try { + await writeOutcome(writer, entry, "partial"); + } catch { + // The completed-operation error below remains the primary evidence. + } + throw new AuditOutcomePersistenceError(result, auditOutcomeError); } } diff --git a/src/features/housekeeping/foundation/commands/registry.test.ts b/src/features/housekeeping/foundation/commands/registry.test.ts index 3ab522bf..929dd5f1 100644 --- a/src/features/housekeeping/foundation/commands/registry.test.ts +++ b/src/features/housekeeping/foundation/commands/registry.test.ts @@ -5,6 +5,7 @@ import { getHousekeepingCommand, type HousekeepingCommand, registerHousekeepingCommand, + sealHousekeepingCommandRegistry, } from "./registry"; function command( @@ -24,12 +25,19 @@ function command( } describe("housekeeping command registry", () => { - it("returns the exact command registered under its global ID", () => { + it("returns the validated snapshot registered under its global ID", () => { const registered = command("people.registry.lookup"); registerHousekeepingCommand(registered); - expect(getHousekeepingCommand(registered.id)).toBe(registered); + expect(getHousekeepingCommand(registered.id)).toMatchObject({ + id: "people.registry.lookup", + owner: "people", + risk: "safe", + requiresReason: false, + rateLimit: { attempts: 4, windowMs: 30_000 }, + }); + expect(getHousekeepingCommand(registered.id)).not.toBe(registered); }); it("rejects a duplicate global command ID before it can shadow its owner", () => { @@ -47,4 +55,79 @@ describe("housekeeping command registry", () => { ), ).toThrow("command owner mismatch: people.registry.owner-mismatch"); }); + it("stores a frozen snapshot that resists mutation through the original definition", () => { + const original = command("people.registry.immutable"); + const callerOwnedInput = original.input; + registerHousekeepingCommand(original); + const registered = getHousekeepingCommand(original.id); + if (!registered) throw new Error("registered command missing"); + + const registeredInput = registered.input; + const registeredExecute = registered.execute; + (original as { risk: "safe" | "sensitive" }).risk = "sensitive"; + (original as { owner: "people" | "system" }).owner = "system"; + (original as { input: z.ZodType<{ id: number }> }).input = z.object({ + id: z.literal(999), + }); + (original.capability.slugs as string[]).push("admin.settings.edit"); + (original.rateLimit as { attempts: number }).attempts = 999; + ( + original as { + execute: HousekeepingCommand<{ id: number }, { id: number }>["execute"]; + } + ).execute = async (context) => ok({ id: 999 }, context.correlationId); + + expect(registered).toMatchObject({ + risk: "safe", + owner: "people", + capability: { slugs: ["admin.users.view"] }, + rateLimit: { attempts: 4, windowMs: 30_000 }, + }); + expect(registered.input).toBe(registeredInput); + expect(registered.input).not.toBe(callerOwnedInput); + expect(Object.isFrozen(callerOwnedInput)).toBe(false); + expect(registered.execute).toBe(registeredExecute); + expect(Object.isFrozen(registered)).toBe(true); + expect(Object.isFrozen(registered.input)).toBe(true); + expect(Object.isFrozen(registered.capability)).toBe(true); + expect(Object.isFrozen(registered.capability.slugs)).toBe(true); + expect(Object.isFrozen(registered.rateLimit)).toBe(true); + }); + + it.each([ + ["risk", { risk: "dangerous" }], + ["requiresReason", { requiresReason: "yes" }], + [ + "capability mode", + { capability: { mode: "none", slugs: ["admin.users.view"] } }, + ], + ["empty capability", { capability: { mode: "any", slugs: [] } }], + [ + "unknown capability", + { capability: { mode: "any", slugs: ["forged.permission"] } }, + ], + ["execute", { execute: null }], + [ + "Zod schema", + { input: { safeParse: () => ({ success: true, data: {} }) } }, + ], + ["normalized ID", { id: " people.registry.invalid-id " }], + ] as const)("rejects an invalid %s definition", (label, override) => { + const invalid = { + ...command( + `people.registry.invalid-${label.toLowerCase().replaceAll(" ", "-")}`, + ), + ...override, + } as unknown as HousekeepingCommand<{ id: number }, { id: number }>; + + expect(() => registerHousekeepingCommand(invalid)).toThrow(); + }); + + it("seals the global registry after deterministic bootstrap", () => { + sealHousekeepingCommandRegistry(); + + expect(() => + registerHousekeepingCommand(command("people.registry.after-seal")), + ).toThrow("command registry is sealed"); + }); }); diff --git a/src/features/housekeeping/foundation/commands/registry.ts b/src/features/housekeeping/foundation/commands/registry.ts index 079cf7ed..cb6678ac 100644 --- a/src/features/housekeeping/foundation/commands/registry.ts +++ b/src/features/housekeeping/foundation/commands/registry.ts @@ -1,4 +1,5 @@ -import type { z } from "zod"; +import { z } from "zod"; +import { PERMS } from "@/lib/permission-slugs"; import { HOUSEKEEPING_DOMAIN_IDS, type HousekeepingDomainId, @@ -10,39 +11,61 @@ import type { } from "../contracts"; export interface HousekeepingCommandContext { - capability: HousekeepingCapabilityContext; - correlationId: string; - ipAddress: string; + readonly capability: HousekeepingCapabilityContext; + readonly correlationId: string; + readonly ipAddress: string; } export interface HousekeepingCommand { - id: string; - owner: HousekeepingDomainId; - risk: "safe" | "sensitive"; - capability: CapabilityRequirement; - input: z.ZodType; - requiresReason: boolean; - rateLimit: { attempts: number; windowMs: number }; - execute( + readonly id: string; + readonly owner: HousekeepingDomainId; + readonly risk: "safe" | "sensitive"; + readonly capability: CapabilityRequirement; + readonly input: z.ZodType; + readonly requiresReason: boolean; + readonly rateLimit: Readonly<{ attempts: number; windowMs: number }>; + readonly execute: ( context: HousekeepingCommandContext, input: I, - ): Promise>; + ) => Promise>; } type RegisteredHousekeepingCommand = HousekeepingCommand; const approvedOwners = new Set(HOUSEKEEPING_DOMAIN_IDS); +const knownCapabilitySlugs = new Set(Object.values(PERMS)); +const commandIdPattern = /^[a-z][a-z0-9-]*(?:\.[a-z0-9][a-z0-9-]*)+$/; const commands = new Map(); +let sealed = false; export function registerHousekeepingCommand( command: HousekeepingCommand, ): void { + if (sealed) throw new Error("command registry is sealed"); validateCommand(command); if (commands.has(command.id)) { throw new Error(`duplicate command id: ${command.id}`); } - commands.set(command.id, command as RegisteredHousekeepingCommand); + const capability = Object.freeze({ + mode: command.capability.mode, + slugs: Object.freeze([...command.capability.slugs]), + }) satisfies CapabilityRequirement; + const snapshot = Object.freeze({ + id: command.id, + owner: command.owner, + risk: command.risk, + capability, + input: Object.freeze(command.input.clone()), + requiresReason: command.requiresReason, + rateLimit: Object.freeze({ ...command.rateLimit }), + execute: command.execute, + }) as RegisteredHousekeepingCommand; + commands.set(snapshot.id, snapshot); +} + +export function sealHousekeepingCommandRegistry(): void { + sealed = true; } export function getHousekeepingCommand( @@ -52,22 +75,37 @@ export function getHousekeepingCommand( } function validateCommand(command: HousekeepingCommand): void { - if (typeof command.id !== "string" || !command.id.trim()) { - throw new Error("empty command id"); + if ( + !isNormalizedIdentifier(command.id) || + !commandIdPattern.test(command.id) + ) { + throw new Error(`invalid command id: ${String(command.id)}`); } - if (!approvedOwners.has(command.owner)) { - throw new Error(`unknown command owner: ${command.owner}`); + if ( + !isNormalizedIdentifier(command.owner) || + !approvedOwners.has(command.owner) + ) { + throw new Error(`unknown command owner: ${String(command.owner)}`); } if (!command.id.startsWith(`${command.owner}.`)) { throw new Error(`command owner mismatch: ${command.id}`); } - if ( - !command.input || - typeof (command.input as { safeParse?: unknown }).safeParse !== "function" - ) { + if (command.risk !== "safe" && command.risk !== "sensitive") { + throw new Error(`invalid command risk: ${command.id}`); + } + if (typeof command.requiresReason !== "boolean") { + throw new Error(`invalid command reason policy: ${command.id}`); + } + validateCapability(command.capability, command.id); + if (!(command.input instanceof z.ZodType)) { throw new Error(`command input schema missing: ${command.id}`); } + if (typeof command.execute !== "function") { + throw new Error(`command execute missing: ${command.id}`); + } if ( + typeof command.rateLimit !== "object" || + command.rateLimit === null || !Number.isInteger(command.rateLimit.attempts) || command.rateLimit.attempts <= 0 || !Number.isInteger(command.rateLimit.windowMs) || @@ -76,3 +114,32 @@ function validateCommand(command: HousekeepingCommand): void { throw new Error(`invalid command rate limit: ${command.id}`); } } + +function validateCapability( + requirement: CapabilityRequirement, + commandId: string, +): void { + if ( + typeof requirement !== "object" || + requirement === null || + (requirement.mode !== "any" && requirement.mode !== "all") || + !Array.isArray(requirement.slugs) || + requirement.slugs.length === 0 + ) { + throw new Error(`invalid command capability: ${commandId}`); + } + for (const slug of requirement.slugs) { + if (!isNormalizedIdentifier(slug) || !knownCapabilitySlugs.has(slug)) { + throw new Error(`invalid command capability: ${commandId}`); + } + } +} + +function isNormalizedIdentifier(value: unknown): value is string { + return ( + typeof value === "string" && + value.length > 0 && + value.length <= 160 && + value === value.normalize("NFC").trim() + ); +}