fix(housekeeping): harden audited command dispatch
This commit is contained in:
1 parent
6aed71a8b2
commit
796d009c07
7 files changed
+775
-124
No files matched your search
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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<HousekeepingResult<unknown>> {
|
||||
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<unknown> {
|
||||
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"
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user