From 893e7e5a4bfd1a35cc3f661012ee250d1d439bba Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Sun, 18 Oct 2026 08:03:01 +0800 Subject: [PATCH] fix(audit): coalesce identical plugin routine audits so they cannot flush the log (#1667) Successful plugin_as_user and plugin_service_* entries written by a plugin with the same user, action, resource and details are recorded once per 10 minutes. Failures and every other action are still written each time. --- .../tests/plugins/security/ctx-guards.test.ts | 3 +- src/backend/tests/utils/audit-logger.test.ts | 97 ++++++++++++++++++- src/backend/utils/audit-logger.ts | 42 ++++++++ 3 files changed, 140 insertions(+), 2 deletions(-) diff --git a/src/backend/tests/plugins/security/ctx-guards.test.ts b/src/backend/tests/plugins/security/ctx-guards.test.ts index a2d83b39b..b0838c38c 100644 --- a/src/backend/tests/plugins/security/ctx-guards.test.ts +++ b/src/backend/tests/plugins/security/ctx-guards.test.ts @@ -75,7 +75,8 @@ const UNGATED: Record = { "schedule.every": "the plugin's own timers", "schedule.after": "the plugin's own timers", "audit.record": "writes under the runtime's actor, never a plugin value", - asUser: "always audited as plugin_as_user", + asUser: + "audited as plugin_as_user (identical successful repeats coalesced for 10 minutes)", currentActor: "reads the actor, cannot set it", "desktop.available": "a yes/no about the environment", "http.baseUrl": "reads the request's public origin", diff --git a/src/backend/tests/utils/audit-logger.test.ts b/src/backend/tests/utils/audit-logger.test.ts index b515154bf..5403462f4 100644 --- a/src/backend/tests/utils/audit-logger.test.ts +++ b/src/backend/tests/utils/audit-logger.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; const createMock = vi.fn().mockResolvedValue(undefined); @@ -60,6 +60,101 @@ describe("logAudit", () => { }); }); +describe("logAudit plugin routine coalescing", () => { + const WINDOW_MS = 10 * 60 * 1000; + let run = 0; + let details = ""; + + const routine = (overrides: Record = {}) => + logAudit({ + userId: null, + username: "plugin:automations", + action: "plugin_as_user", + resourceType: "plugin", + resourceId: "automations", + details, + success: true, + ...overrides, + }); + + beforeEach(() => { + vi.clearAllMocks(); + createMock.mockResolvedValue(undefined); + vi.useFakeTimers(); + // The coalescing memory is module state: a fresh key per test keeps them + // independent. + details = `automations ran background work as user-${++run}`; + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("writes one row for identical successful plugin_as_user repeats in the window", async () => { + await routine(); + vi.advanceTimersByTime(WINDOW_MS - 1); + await routine(); + await routine(); + + expect(createMock).toHaveBeenCalledOnce(); + }); + + it("writes one row for identical successful plugin service calls in the window", async () => { + const service = { + action: "plugin_service_host_metrics_open", + resourceId: "host-metrics", + }; + await routine(service); + await routine(service); + + expect(createMock).toHaveBeenCalledOnce(); + }); + + it("writes again once the window has passed", async () => { + await routine(); + vi.advanceTimersByTime(WINDOW_MS); + await routine(); + + expect(createMock).toHaveBeenCalledTimes(2); + }); + + it("always writes failures", async () => { + await routine(); + await routine({ success: false, errorMessage: "boom" }); + await routine({ success: false, errorMessage: "boom" }); + + expect(createMock).toHaveBeenCalledTimes(3); + }); + + it("never coalesces non-plugin or non-routine actions", async () => { + const login = { + userId: "user-1", + username: "alice", + action: "login", + resourceType: "user", + }; + await routine(login); + await routine(login); + const secretRead = { action: "plugin_secret_shared_read" }; + await routine(secretRead); + await routine(secretRead); + const notPlugin = { username: "alice" }; + await routine(notPlugin); + await routine(notPlugin); + + expect(createMock).toHaveBeenCalledTimes(6); + }); + + it("does not coalesce entries with different details or resourceId", async () => { + await routine(); + await routine({ details: `${details} (other)` }); + await routine({ resourceId: "other-plugin" }); + await routine({ userId: "user-2" }); + + expect(createMock).toHaveBeenCalledTimes(4); + }); +}); + describe("getRequestMeta", () => { it("uses the proxy-validated Express IP", () => { const req = { diff --git a/src/backend/utils/audit-logger.ts b/src/backend/utils/audit-logger.ts index 58d2f28b1..1cc0d1228 100644 --- a/src/backend/utils/audit-logger.ts +++ b/src/backend/utils/audit-logger.ts @@ -35,7 +35,49 @@ export interface AuditLogParams { errorMessage?: string; } +// Plugin routine calls (a headless viewer's heartbeat through ctx.asUser and a +// service) repeat identically every few seconds; written each time they push +// real security entries out of the capped log. Only successful plugin_as_user +// and plugin_service_* lines are coalesced; failures and everything else are +// always written. +const ROUTINE_COALESCE_MS = 10 * 60 * 1000; +const ROUTINE_COALESCE_MAX_KEYS = 5000; +const routineLastWritten = new Map(); + +function isCoalescedRoutine(params: AuditLogParams): boolean { + if ( + !params.success || + !params.username.startsWith("plugin:") || + !( + params.action === "plugin_as_user" || + params.action.startsWith("plugin_service_") + ) + ) { + return false; + } + const key = JSON.stringify([ + params.userId, + params.username, + params.action, + params.resourceType, + params.resourceId, + params.details, + ]); + const now = Date.now(); + const last = routineLastWritten.get(key); + if (last !== undefined && now - last < ROUTINE_COALESCE_MS) return true; + if (routineLastWritten.size >= ROUTINE_COALESCE_MAX_KEYS) { + for (const [k, at] of routineLastWritten) { + if (now - at >= ROUTINE_COALESCE_MS) routineLastWritten.delete(k); + } + } + routineLastWritten.set(key, now); + return false; +} + export async function logAudit(params: AuditLogParams): Promise { + if (isCoalescedRoutine(params)) return; + // Local storage is the source of truth and runs first; forwarding is a copy // and must never delay or fail the audited operation. void forwardAuditEntry(params).catch(() => {});