From abbfca2e94664aef72733c8f34194da006acb3a3 Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Sun, 4 Oct 2026 18:40:13 +0800 Subject: [PATCH] fix(audit): store plugin entries with no acting user as a null user_id (#1556) Plugin audit entries written outside a request used the literal "system" as user_id. That column references users.id, so the insert was refused (Postgres logs it as an FK violation) and the entry was silently dropped. Write null instead and keep "system" / plugin: in username. --- src/backend/plugins/ctx.ts | 8 +++--- src/backend/plugins/http.ts | 2 +- src/backend/plugins/permissions.ts | 2 +- src/backend/plugins/ws.ts | 2 +- .../repositories/audit-log-repository.test.ts | 28 +++++++++++++++++++ src/backend/tests/plugins/ctx-audit.test.ts | 3 +- src/backend/utils/audit-logger.ts | 4 ++- 7 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/backend/plugins/ctx.ts b/src/backend/plugins/ctx.ts index 780657d39..ad197877c 100644 --- a/src/backend/plugins/ctx.ts +++ b/src/backend/plugins/ctx.ts @@ -156,7 +156,7 @@ async function writeAudit( const { logAudit } = await import("../utils/audit-logger.js"); await logAudit({ // Attribution comes from the runtime, never from the plugin. - userId: getActor() ?? "system", + userId: getActor() ?? null, username: `plugin:${manifest.id}`, action: `plugin_${options.action}`, resourceType: "plugin", @@ -965,12 +965,12 @@ export function createPluginContext( record: async (entry) => { try { const { logAudit } = await import("../utils/audit-logger.js"); - const actor = getActor() ?? "system"; + const actor = getActor(); const meta = requestMeta(entry.request); await logAudit({ ...meta, - userId: actor, - username: actor, + userId: actor ?? null, + username: actor ?? "system", action: entry.action, resourceType: entry.resourceType ?? "plugin", resourceId: entry.resourceId ?? pluginId, diff --git a/src/backend/plugins/http.ts b/src/backend/plugins/http.ts index ee5f2eeca..daafc078c 100644 --- a/src/backend/plugins/http.ts +++ b/src/backend/plugins/http.ts @@ -351,7 +351,7 @@ async function writePublicRouteAudit( try { const { logAudit } = await import("../utils/audit-logger.js"); await logAudit({ - userId: "system", + userId: null, username: `plugin:${manifest.id}`, action: "plugin_http_public_routes", resourceType: "plugin", diff --git a/src/backend/plugins/permissions.ts b/src/backend/plugins/permissions.ts index 00dab6ad4..e41a72679 100644 --- a/src/backend/plugins/permissions.ts +++ b/src/backend/plugins/permissions.ts @@ -111,7 +111,7 @@ async function auditRefusal( const { logAudit } = await import("../utils/audit-logger.js"); await logAudit({ // Attribution comes from the runtime, never from the plugin. - userId: getActor() ?? "system", + userId: getActor() ?? null, username: `plugin:${pluginId}`, action: `plugin_${action}`, resourceType: "plugin", diff --git a/src/backend/plugins/ws.ts b/src/backend/plugins/ws.ts index 29e4f00dc..d431421d1 100644 --- a/src/backend/plugins/ws.ts +++ b/src/backend/plugins/ws.ts @@ -382,7 +382,7 @@ function auditPublicSocket(pluginId: string, path: string): void { void import("../utils/audit-logger.js") .then(({ logAudit }) => logAudit({ - userId: "system", + userId: null, username: `plugin:${pluginId}`, action: "plugin_ws_public_route", resourceType: "plugin", diff --git a/src/backend/tests/database/repositories/audit-log-repository.test.ts b/src/backend/tests/database/repositories/audit-log-repository.test.ts index 3980bdad9..2de30072b 100644 --- a/src/backend/tests/database/repositories/audit-log-repository.test.ts +++ b/src/backend/tests/database/repositories/audit-log-repository.test.ts @@ -77,6 +77,34 @@ describe("AuditLogRepository", () => { ]); }); + it("stores entries with no acting user, and refuses ids that are not users", async () => { + const repo = await createRepository(); + + await repo.create({ + userId: null, + username: "plugin:example", + action: "plugin_http_public_routes", + resourceType: "plugin", + success: true, + }); + await expect( + repo.create({ + userId: "system", + username: "system", + action: "cleanup", + resourceType: "plugin", + success: true, + }), + ).rejects.toThrow(); + + const page = await repo.listPage({ filters: {}, limit: 10, offset: 0 }); + expect(page.logs).toHaveLength(1); + expect(page.logs[0]).toMatchObject({ + userId: null, + username: "plugin:example", + }); + }); + it("deletes logs by user id and only runs write hook for deleted rows", async () => { let writeCount = 0; const repo = await createRepository(() => { diff --git a/src/backend/tests/plugins/ctx-audit.test.ts b/src/backend/tests/plugins/ctx-audit.test.ts index fbac60d51..7eef84faa 100644 --- a/src/backend/tests/plugins/ctx-audit.test.ts +++ b/src/backend/tests/plugins/ctx-audit.test.ts @@ -73,7 +73,8 @@ describe("ctx.audit.record", () => { const ctx = contextFor("audit-fixture"); await ctx.audit.record({ action: "cleanup", success: false }); expect(auditEntries[0]).toMatchObject({ - userId: "system", + userId: null, + username: "system", resourceType: "plugin", resourceId: "audit-fixture", resourceName: "Audit Fixture", diff --git a/src/backend/utils/audit-logger.ts b/src/backend/utils/audit-logger.ts index 235302c83..58d2f28b1 100644 --- a/src/backend/utils/audit-logger.ts +++ b/src/backend/utils/audit-logger.ts @@ -20,7 +20,9 @@ export async function getAuditUsername(userId: string): Promise { } export interface AuditLogParams { - userId: string; + // Null when no user acted (a plugin starting up, background work): the + // column references users.id, so anything else would be refused. + userId: string | null; username: string; action: string; resourceType: string;