From 3c23c6e61de6b29d30ee6919bedb63a0a7663c10 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Sat, 5 Sep 2026 17:19:32 +0200 Subject: [PATCH] fix(housekeeping): complete system audit outcomes --- .../services/mutations-production.test.ts | 79 +++++- .../domains/system/services/mutations.ts | 258 ++++++++++++++---- .../rank-mutations-production.test.ts | 96 +++++++ .../system/services/system-atomic.test.ts | 65 +++++ src/lib/services/permission-ranks.test.ts | 52 ++++ src/lib/services/permission-ranks.ts | 9 +- 6 files changed, 506 insertions(+), 53 deletions(-) diff --git a/src/features/housekeeping/domains/system/services/mutations-production.test.ts b/src/features/housekeeping/domains/system/services/mutations-production.test.ts index b9678dfd..93af5ef9 100644 --- a/src/features/housekeeping/domains/system/services/mutations-production.test.ts +++ b/src/features/housekeeping/domains/system/services/mutations-production.test.ts @@ -2,10 +2,20 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { PERMS } from "@/lib/permission-slugs"; import type { HousekeepingCapabilityContext } from "../../../foundation/contracts"; -const { logAudit, markReadWhere, rconSend } = vi.hoisted(() => ({ +const { + logAudit, + markReadWhere, + rconExecuteCommand, + rconGiveCredits, + rconSend, + rconUpdateCatalog, +} = vi.hoisted(() => ({ logAudit: vi.fn(), markReadWhere: vi.fn(), + rconExecuteCommand: vi.fn(), + rconGiveCredits: vi.fn(), rconSend: vi.fn(), + rconUpdateCatalog: vi.fn(), })); vi.mock("@/lib/services/audit", () => ({ logAudit })); @@ -27,7 +37,13 @@ vi.mock("@/lib/services/rcon", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - rcon: { send: rconSend }, + rcon: { + ...actual.rcon, + executeCommand: rconExecuteCommand, + giveCredits: rconGiveCredits, + send: rconSend, + updateCatalog: rconUpdateCatalog, + }, }; }); @@ -50,6 +66,9 @@ describe("System production mutation failures", () => { beforeEach(() => { markReadWhere.mockReset(); rconSend.mockReset(); + rconExecuteCommand.mockReset(); + rconGiveCredits.mockReset(); + rconUpdateCatalog.mockReset(); logAudit.mockReset(); logAudit.mockResolvedValue(undefined); }); @@ -121,8 +140,64 @@ describe("System production mutation failures", () => { "intent", "success", ]); + expect(logAudit.mock.calls.map(([entry]) => entry.after)).toEqual([ + { message: "Hotel notice" }, + { message: "Hotel notice", delivered: true }, + ]); }); + it.each([ + [ + "rcon.give-credits", + { userId: 17, amount: 50 }, + rconGiveCredits, + { userId: 17, amount: 50 }, + ], + [ + "rcon.execute-command", + { userId: 17, command: "dance" }, + rconExecuteCommand, + { userId: 17, command: "dance" }, + ], + [ + "rcon.update-catalog", + {}, + rconUpdateCatalog, + { request: "update-catalog" }, + ], + ] as const)( + "audits bounded target details for %s intent and outcome", + async (operation, input, transport, expectedDetails) => { + transport.mockResolvedValueOnce(true); + + const result = await systemMutationService.execute( + { + capability: capabilityContext([PERMS.RCON_EXECUTE]), + correlationId: `audit-${operation}`, + reason: "Approved command-center operation", + }, + operation, + input, + ); + + expect(result).toMatchObject({ ok: true }); + expect(logAudit).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + outcome: "intent", + after: expectedDetails, + }), + ); + expect(logAudit).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + outcome: "success", + after: { ...expectedDetails, delivered: true }, + }), + ); + }, + ); + it("maps mark-read persistence failure to dependency unavailable", async () => { markReadWhere.mockRejectedValue(new Error("database unavailable")); diff --git a/src/features/housekeeping/domains/system/services/mutations.ts b/src/features/housekeeping/domains/system/services/mutations.ts index c79b3917..953af731 100644 --- a/src/features/housekeeping/domains/system/services/mutations.ts +++ b/src/features/housekeeping/domains/system/services/mutations.ts @@ -123,6 +123,7 @@ class SystemCommittedExternalFailure extends Error { readonly data: unknown, readonly external: "completed" | "failed", readonly audit: "persisted" | "unavailable", + readonly cache?: "invalidated" | "unavailable", ) { super( "System database change committed but external synchronization is incomplete", @@ -194,6 +195,7 @@ export function createSystemMutationService( status: "partial", external: error.external, audit: error.audit, + ...(error.cache ? { cache: error.cache } : {}), }); } if (error instanceof SystemCommittedCacheFailure) { @@ -386,6 +388,7 @@ async function completeExternalSynchronization( if (error instanceof SystemMutationFailure) throw error; } + let audit: "persisted" | "unavailable" = "persisted"; if (!synchronized) { try { await logAudit( @@ -394,37 +397,50 @@ async function completeExternalSynchronization( } catch { // The transactionally persisted intent is sufficient for recovery. } + } else + try { + const entry = syncAuditEntry( + currentIntent, + context, + recoveryId, + "success", + ); + await logAudit({ + ...entry, + after: { ...entry.after, ...(superseded ? { superseded: true } : {}) }, + }); + } catch { + audit = "unavailable"; + } + + const synchronization = synchronized ? "completed" : "pending"; + const result = { + ...synchronizationData(currentIntent, recoveryId, synchronization, { + ...extra, + ...(superseded ? { superseded: true } : {}), + }), + ...(audit === "unavailable" ? { pending: ["completion-audit"] } : {}), + }; + let cache: "invalidated" | "unavailable" | undefined; + if (currentIntent.kind === "rank-permission-cache") { + try { + await invalidatePermissionsCache(result); + cache = "invalidated"; + } catch { + cache = "unavailable"; + } + } + + if (!synchronized || audit === "unavailable" || cache === "unavailable") { throw new SystemCommittedExternalFailure( - synchronizationData(currentIntent, recoveryId, "pending", extra), - "failed", - "persisted", + result, + synchronized ? "completed" : "failed", + audit, + cache, ); } - try { - const entry = syncAuditEntry(currentIntent, context, recoveryId, "success"); - await logAudit({ - ...entry, - after: { ...entry.after, ...(superseded ? { superseded: true } : {}) }, - }); - } catch { - throw new SystemCommittedExternalFailure( - { - ...synchronizationData(currentIntent, recoveryId, "completed", { - ...extra, - ...(superseded ? { superseded: true } : {}), - }), - pending: ["completion-audit"], - }, - "completed", - "unavailable", - ); - } - - return synchronizationData(currentIntent, recoveryId, "completed", { - ...extra, - ...(superseded ? { superseded: true } : {}), - }); + return result; } function parseExternalSyncIntent( @@ -573,6 +589,92 @@ function mutationAuditEntry( }; } +function externalOperationAuditEntry( + operation: SystemMutationOperation, + context: SystemMutationContext, + details: Readonly>, + outcome: "intent" | "success" | "failure", + separateSyncAction = false, +) { + return { + ...mutationAuditEntry(operation, context, undefined, { ...details }), + ...(separateSyncAction + ? { + action: `system.${operation}.sync`, + target: `${operation}.sync`, + } + : {}), + outcome, + }; +} + +function normalizedRconAuditDetails( + operation: Extract, + data: Record, +): Readonly> { + switch (operation) { + case "rcon.update-catalog": + return { request: "update-catalog" }; + case "rcon.update-word-filter": + return { request: "update-word-filter" }; + case "rcon.update-navigator": + return { request: "update-navigator" }; + case "rcon.hotel-alert": + return { message: text(data.message, 512) }; + case "rcon.disconnect-user": + return { + userId: positiveInteger(data.userId), + username: text(data.username, 255), + }; + case "rcon.alert-user": + return { + userId: positiveInteger(data.userId), + message: text(data.message, 512), + }; + case "rcon.forward-user": + return { + userId: positiveInteger(data.userId), + roomId: positiveInteger(data.roomId), + }; + case "rcon.give-credits": + case "rcon.give-duckets": + case "rcon.give-diamonds": + return { + userId: positiveInteger(data.userId), + amount: positiveInteger(data.amount), + }; + case "rcon.give-badge": + return { + userId: positiveInteger(data.userId), + badge: text(data.badge, 32), + }; + case "rcon.set-motto": + return { + userId: positiveInteger(data.userId), + motto: text(data.motto, 127), + }; + case "rcon.set-rank": + return { + userId: positiveInteger(data.userId), + rank: positiveInteger(data.rank), + }; + case "rcon.set-rank.retry": + return { recoveryId: recoveryIdentifier(data.recoveryId) }; + case "rcon.execute-command": + return { + userId: positiveInteger(data.userId), + command: text(data.command, 100), + }; + case "rcon.send-gift": + return { + userId: positiveInteger(data.userId), + itemId: positiveInteger(data.itemId), + message: + text(data.message || "Here is a gift.", 255) || "Here is a gift.", + }; + } +} + async function invalidatePermissionsCache(data: unknown): Promise { try { revalidateTag("permissions", { expire: 0 }); @@ -1045,6 +1147,10 @@ async function executeConfigurationMutation( const entries = Object.entries(settings) .map(([key, value]) => [text(key, 100), text(value, 512, false)] as const) .filter(([key]) => Boolean(key)); + const syncDetails = { + request: "update-config", + saved: entries.length, + } as const; await db.transaction(async (tx) => { for (const [key, value] of entries) { await tx @@ -1058,16 +1164,47 @@ async function executeConfigurationMutation( }), tx, ); + await logAudit( + externalOperationAuditEntry( + operation, + context, + syncDetails, + "intent", + true, + ), + tx, + ); }); let synchronized = false; try { synchronized = await rcon.updateConfig(); } catch {} + let outcomeAudit: "persisted" | "unavailable" = "persisted"; + try { + await logAudit( + externalOperationAuditEntry( + operation, + context, + { ...syncDetails, synchronized }, + synchronized ? "success" : "failure", + true, + ), + ); + } catch { + outcomeAudit = "unavailable"; + } if (!synchronized) { throw new SystemCommittedExternalFailure( { saved: entries.length }, "failed", - "persisted", + outcomeAudit, + ); + } + if (outcomeAudit === "unavailable") { + throw new SystemCommittedExternalFailure( + { saved: entries.length }, + "completed", + "unavailable", ); } return { saved: entries.length }; @@ -1166,11 +1303,13 @@ async function executeRconMutation( const data = record(input); const usesDurableDatabaseIntent = operation === "rcon.set-rank" || operation === "rcon.set-rank.retry"; + const auditDetails = usesDurableDatabaseIntent + ? {} + : normalizedRconAuditDetails(operation, data); if (!usesDurableDatabaseIntent) { - await logAudit({ - ...mutationAuditEntry(operation, context), - outcome: "intent", - }); + await logAudit( + externalOperationAuditEntry(operation, context, auditDetails, "intent"), + ); } let externalCompleted = false; try { @@ -1326,9 +1465,15 @@ async function executeRconMutation( if (!usesDurableDatabaseIntent) { try { await logAudit( - mutationAuditEntry(operation, context, undefined, { - delivered: true, - }), + externalOperationAuditEntry( + operation, + context, + { + ...auditDetails, + delivered: true, + }, + "success", + ), ); } catch { throw new SystemCommittedExternalFailure( @@ -1346,10 +1491,14 @@ async function executeRconMutation( !(error instanceof SystemCommittedExternalFailure) ) { try { - await logAudit({ - ...mutationAuditEntry(operation, context), - outcome: "failure", - }); + await logAudit( + externalOperationAuditEntry( + operation, + context, + { ...auditDetails, delivered: false }, + "failure", + ), + ); } catch { // Preserve the authoritative external failure. } @@ -1411,19 +1560,24 @@ const systemProductionMutationAdapter: SystemMutationAdapter = { "errors.housekeeping.validation", ); } - await logAudit({ - ...mutationAuditEntry(operation, context), - outcome: "intent", - }); + await logAudit( + externalOperationAuditEntry(operation, context, { message }, "intent"), + ); let delivered = false; try { await requireRcon(await rcon.send("hotelalert", { message })); delivered = true; try { await logAudit( - mutationAuditEntry(operation, context, undefined, { - delivered: true, - }), + externalOperationAuditEntry( + operation, + context, + { + message, + delivered: true, + }, + "success", + ), ); } catch { throw new SystemCommittedExternalFailure( @@ -1436,10 +1590,14 @@ const systemProductionMutationAdapter: SystemMutationAdapter = { } catch (error) { if (!delivered && !(error instanceof SystemCommittedExternalFailure)) { try { - await logAudit({ - ...mutationAuditEntry(operation, context), - outcome: "failure", - }); + await logAudit( + externalOperationAuditEntry( + operation, + context, + { message, delivered: false }, + "failure", + ), + ); } catch {} } throw error; diff --git a/src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts b/src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts index 94427f58..9040a90b 100644 --- a/src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts +++ b/src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts @@ -24,6 +24,7 @@ const doubles = vi.hoisted(() => ({ logStaffActivity: vi.fn(), logStaffActivityInTransaction: vi.fn(), prepareEmulatorRankCreation: vi.fn(), + revalidateTag: vi.fn(), rconSend: vi.fn(), rconSetRank: vi.fn(), updateEmulatorRank: vi.fn(), @@ -57,6 +58,8 @@ vi.mock("@/lib/services/audit", () => ({ logAudit: doubles.logAudit, })); +vi.mock("next/cache", () => ({ revalidateTag: doubles.revalidateTag })); + vi.mock("@/lib/services/permission-ranks", () => ({ RANK_GENERAL_FIELDS: new Set([ "rank_name", @@ -65,6 +68,15 @@ vi.mock("@/lib/services/permission-ranks", () => ({ "prefix", "prefix_color", "hidden_rank", + "job_description", + "staff_color", + "staff_background", + "log_commands", + "room_effect", + "auto_credits_amount", + "auto_pixels_amount", + "auto_gotw_amount", + "auto_points_amount", ]), createEmulatorRank: doubles.createEmulatorRank, createEmulatorRankInTransaction: doubles.createEmulatorRankInTransaction, @@ -160,6 +172,9 @@ beforeEach(() => { prefix: "", prefix_color: "", hidden_rank: "0", + job_description: "Keeps the hotel safe", + staff_color: "#123456", + staff_background: "#abcdef", log_commands: "1", room_effect: 0, auto_credits_amount: 0, @@ -173,6 +188,7 @@ beforeEach(() => { doubles.logAudit.mockResolvedValue(undefined); doubles.logStaffActivity.mockResolvedValue(undefined); doubles.logStaffActivityInTransaction.mockResolvedValue(undefined); + doubles.revalidateTag.mockReturnValue(undefined); doubles.dbDelete.mockImplementation(deleteChain); doubles.dbExecute.mockResolvedValue([[{ id: 7 }]]); doubles.dbInsert.mockImplementation(insertChain); @@ -201,6 +217,7 @@ function expectPendingSynchronization( status: "partial", external: "failed", audit: "persisted", + ...(operation.startsWith("access.rank.") ? { cache: "invalidated" } : {}), }, }); if (typeof result === "object" && result !== null && "ok" in result) { @@ -358,6 +375,9 @@ describe("durable rank synchronization", () => { }), transactionToken, ); + expect(doubles.revalidateTag).toHaveBeenCalledWith("permissions", { + expire: 0, + }); }); it.each([ @@ -396,9 +416,85 @@ describe("durable rank synchronization", () => { 7, ...(operation === "access.rank.update" ? [{ badge: "ADM" }] : []), ); + expect(doubles.revalidateTag).toHaveBeenCalledWith("permissions", { + expire: 0, + }); }, ); + it("combines failed rank synchronization and cache invalidation statuses", async () => { + doubles.rconSend.mockResolvedValue(false); + doubles.revalidateTag.mockImplementationOnce(() => { + throw new Error("cache unavailable"); + }); + + const result = await systemMutationService.execute( + serviceContext(PERMS.PERMISSIONS_MANAGE), + "access.rank.update", + { id: 7, fields: { badge: "ADM" } }, + ); + + expect(result).toMatchObject({ + ok: true, + completion: { + status: "partial", + external: "failed", + audit: "persisted", + cache: "unavailable", + }, + }); + }); + + it("audits changed values for every editable general rank field", async () => { + const before = { + ...(await doubles.fetchEmulatorRankForEdit()), + job_description: "Old role", + staff_color: "#111111", + staff_background: "#222222", + }; + const after = { + ...before, + job_description: "New role", + staff_color: "#333333", + staff_background: "#444444", + }; + doubles.fetchEmulatorRankForEdit + .mockResolvedValueOnce(before) + .mockResolvedValueOnce(after); + doubles.rconSend.mockResolvedValue(true); + + const result = await systemMutationService.execute( + serviceContext(PERMS.PERMISSIONS_MANAGE), + "access.rank.update", + { + id: 7, + fields: { + job_description: "New role", + staff_color: "#333333", + staff_background: "#444444", + }, + }, + ); + + expect(result).toMatchObject({ ok: true }); + expect(doubles.logAudit).toHaveBeenCalledWith( + expect.objectContaining({ + action: "system.access.rank.update", + before: expect.objectContaining({ + job_description: "Old role", + staff_color: "#111111", + staff_background: "#222222", + }), + after: expect.objectContaining({ + job_description: "New role", + staff_color: "#333333", + staff_background: "#444444", + }), + }), + transactionToken, + ); + }); + it("never calls RCON when set-rank database persistence rolls back", async () => { doubles.dbSelect.mockImplementationOnce(() => limitedSelection([{ rank: 3 }]), diff --git a/src/features/housekeeping/domains/system/services/system-atomic.test.ts b/src/features/housekeeping/domains/system/services/system-atomic.test.ts index b3c0c6bd..0a88f504 100644 --- a/src/features/housekeeping/domains/system/services/system-atomic.test.ts +++ b/src/features/housekeeping/domains/system/services/system-atomic.test.ts @@ -7,6 +7,7 @@ const doubles = vi.hoisted(() => ({ insertValues: vi.fn(), logAudit: vi.fn(), reload: vi.fn(), + rconUpdateConfig: vi.fn(), selectRows: [] as unknown[][], transaction: vi.fn(), txExecute: vi.fn(), @@ -33,6 +34,13 @@ vi.mock("@/lib/services/audit", () => ({ logAudit: doubles.logAudit })); vi.mock("@/lib/services/site-settings", () => ({ siteSettings: { reload: doubles.reload }, })); +vi.mock("@/lib/services/rcon", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + rcon: { ...actual.rcon, updateConfig: doubles.rconUpdateConfig }, + }; +}); import { systemMutationService } from "./mutations"; @@ -63,6 +71,7 @@ beforeEach(() => { onDuplicateKeyUpdate: vi.fn().mockResolvedValue(undefined), }); doubles.reload.mockResolvedValue({ invalidated: true, redis: "invalidated" }); + doubles.rconUpdateConfig.mockResolvedValue(true); doubles.logAudit.mockResolvedValue(undefined); }); @@ -136,4 +145,60 @@ describe("System atomic database mutations", () => { completion: { status: "partial", cache: "unavailable" }, }); }); + + it("persists emulator-config sync intent with the database and audits success afterward", async () => { + const result = await systemMutationService.execute( + context(PERMS.SETTINGS_EDIT), + "configuration.emulator-settings.save", + { settings: { hotel_name: "Epic", max_users: "500" } }, + ); + + expect(result).toMatchObject({ ok: true, data: { saved: 2 } }); + expect(doubles.logAudit).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + action: "system.configuration.emulator-settings.save.sync", + outcome: "intent", + after: { request: "update-config", saved: 2 }, + }), + tx, + ); + expect(doubles.logAudit).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + action: "system.configuration.emulator-settings.save.sync", + outcome: "success", + after: { request: "update-config", saved: 2, synchronized: true }, + }), + ); + }); + + it("audits emulator-config synchronization failure after the committed intent", async () => { + doubles.rconUpdateConfig.mockResolvedValueOnce(false); + + const result = await systemMutationService.execute( + context(PERMS.SETTINGS_EDIT), + "configuration.emulator-settings.save", + { settings: { hotel_name: "Epic" } }, + ); + + expect(result).toMatchObject({ + ok: true, + data: { saved: 1 }, + completion: { status: "partial", external: "failed" }, + }); + expect(doubles.logAudit).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ outcome: "intent" }), + tx, + ); + expect(doubles.logAudit).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + action: "system.configuration.emulator-settings.save.sync", + outcome: "failure", + after: { request: "update-config", saved: 1, synchronized: false }, + }), + ); + }); }); diff --git a/src/lib/services/permission-ranks.test.ts b/src/lib/services/permission-ranks.test.ts index f4df7e77..46c3f3d8 100644 --- a/src/lib/services/permission-ranks.test.ts +++ b/src/lib/services/permission-ranks.test.ts @@ -163,4 +163,56 @@ describe("fetchEmulatorRankForEdit", () => { permissionMaxValues: { cmd_ban: 1, cmd_owner: 2 }, }); }); + + it("returns every editable general field for transactional audit snapshots", async () => { + const queries: string[] = []; + const db = { + execute: async (query: SQL | string) => { + const text = queryText(query); + queries.push(text); + if (text.includes("FROM permission_ranks")) { + return [ + [ + { + id: 11, + rank_name: "Developer", + badge: "DEV", + level: 11, + prefix: "[DEV]", + prefix_color: "#111111", + hidden_rank: "0", + job_description: "Builds hotel features", + staff_color: "#222222", + staff_background: "#333333", + log_commands: "1", + room_effect: 4, + auto_credits_amount: 10, + auto_pixels_amount: 20, + auto_gotw_amount: 30, + auto_points_amount: 40, + }, + ], + [], + ]; + } + if (text.includes("FROM information_schema.columns")) return [[], []]; + if (text.includes("FROM permissions")) return [[{ id: 11 }], []]; + throw new Error(`Unexpected query: ${text}`); + }, + transaction: async () => { + throw new Error("Unexpected transaction"); + }, + }; + + await expect( + fetchEmulatorRankForEdit(db as never, 11), + ).resolves.toMatchObject({ + job_description: "Builds hotel features", + staff_color: "#222222", + staff_background: "#333333", + }); + expect(queries[0]).toContain("job_description"); + expect(queries[0]).toContain("staff_color"); + expect(queries[0]).toContain("staff_background"); + }); }); diff --git a/src/lib/services/permission-ranks.ts b/src/lib/services/permission-ranks.ts index 46ee6b70..a0a82691 100644 --- a/src/lib/services/permission-ranks.ts +++ b/src/lib/services/permission-ranks.ts @@ -33,6 +33,9 @@ export interface EmulatorRankSummary { } export interface EmulatorRankForEdit extends EmulatorRankSummary { + job_description: string; + staff_color: string; + staff_background: string; log_commands: string; room_effect: number; auto_credits_amount: number; @@ -117,7 +120,8 @@ export async function fetchEmulatorRankForEdit( db, sql` SELECT id, rank_name, badge, level, prefix, prefix_color, hidden_rank, - log_commands, room_effect, auto_credits_amount, auto_pixels_amount, + job_description, staff_color, staff_background, log_commands, + room_effect, auto_credits_amount, auto_pixels_amount, auto_gotw_amount, auto_points_amount FROM permission_ranks WHERE id = ${rankId} @@ -182,6 +186,9 @@ export async function fetchEmulatorRankForEdit( prefix: String(row.prefix ?? ""), prefix_color: String(row.prefix_color ?? ""), hidden_rank: String(row.hidden_rank ?? "0"), + job_description: String(row.job_description ?? ""), + staff_color: String(row.staff_color ?? ""), + staff_background: String(row.staff_background ?? ""), log_commands: String(row.log_commands ?? "0"), room_effect: Number(row.room_effect ?? 0), auto_credits_amount: Number(row.auto_credits_amount ?? 0),