diff --git a/src/backend/database/repositories/audit-log-repository.ts b/src/backend/database/repositories/audit-log-repository.ts index 6e9c86a9..c7682546 100644 --- a/src/backend/database/repositories/audit-log-repository.ts +++ b/src/backend/database/repositories/audit-log-repository.ts @@ -1,6 +1,8 @@ -import { and, asc, desc, eq, gte, inArray, lte, sql } from "drizzle-orm"; +import { and, asc, desc, eq, gte, inArray, lt, lte, sql } from "drizzle-orm"; import { auditLogs } from "../db/schema.js"; import type { DatabaseContext } from "./database-context.js"; +import { sqlTimestampDaysAgo } from "./sql-timestamp.js"; +import { databaseLogger } from "../../utils/logger.js"; export type AuditLogRecord = typeof auditLogs.$inferSelect; export type NewAuditLogRecord = typeof auditLogs.$inferInsert; @@ -19,8 +21,31 @@ export type AuditLogPage = { total: number; }; -const PRUNE_MAX = 10000; -const PRUNE_TARGET = 9000; +export const AUDIT_RETENTION_DAYS_ENV = "AUDIT_LOG_RETENTION_DAYS"; +export const AUDIT_MAX_ENTRIES_ENV = "AUDIT_LOG_MAX_ENTRIES"; + +const DEFAULT_MAX_ENTRIES = 10000; +const PRUNE_TARGET_RATIO = 0.9; + +function positiveIntEnv(key: string, env: NodeJS.ProcessEnv): number | null { + const raw = Number(env[key]); + return Number.isFinite(raw) && raw > 0 ? Math.floor(raw) : null; +} + +/** + * How long entries are kept. Unset means "no time limit", in which case only + * the row cap applies. + */ +export function auditRetentionDays( + env: NodeJS.ProcessEnv = process.env, +): number | null { + return positiveIntEnv(AUDIT_RETENTION_DAYS_ENV, env); +} + +/** Hard ceiling on stored entries, so a busy install cannot fill the disk. */ +export function auditMaxEntries(env: NodeJS.ProcessEnv = process.env): number { + return positiveIntEnv(AUDIT_MAX_ENTRIES_ENV, env) ?? DEFAULT_MAX_ENTRIES; +} export class AuditLogRepository { constructor( @@ -70,6 +95,29 @@ export class AuditLogRepository { return rows.map((row) => row.action); } + /** + * Detaches entries from a user being deleted instead of removing them. + * + * The schema already relaxed this foreign key to ON DELETE SET NULL, but the + * account-deletion path deletes the rows explicitly, which undoes that. An + * audit trail that vanishes with the account it recorded cannot answer the + * question it exists for, and offboarding is exactly when that question gets + * asked. `username` is denormalised, so the entry stays attributable. + */ + async anonymizeByUserId(userId: string): Promise { + const rows = await this.context.drizzle + .update(auditLogs) + .set({ userId: null }) + .where(eq(auditLogs.userId, userId)) + .returning({ id: auditLogs.id }); + + if (rows.length > 0) { + await this.afterWrite(); + } + + return rows.length; + } + async deleteByUserId(userId: string): Promise { const rows = await this.context.drizzle .delete(auditLogs) @@ -105,28 +153,74 @@ export class AuditLogRepository { } private async pruneIfNeeded(): Promise { + await this.pruneExpired(); + await this.pruneOverflow(); + } + + /** Drops entries past the configured retention window. */ + private async pruneExpired(): Promise { + const days = auditRetentionDays(); + if (days === null) return; + + const cutoff = sqlTimestampDaysAgo(days); + const rows = await this.context.drizzle + .delete(auditLogs) + .where(lt(auditLogs.timestamp, cutoff)) + .returning({ id: auditLogs.id }); + + if (rows.length > 0) { + databaseLogger.info( + `Pruned ${rows.length} audit entries past retention`, + { + operation: "audit_retention_prune", + removed: rows.length, + retentionDays: days, + cutoff, + }, + ); + } + } + + /** + * Enforces the row cap. Unlike retention this discards entries that are still + * within the window, so it is reported as a warning: it means the ceiling is + * too low for how much this install audits, and evidence is being lost. + */ + private async pruneOverflow(): Promise { + const max = auditMaxEntries(); const countResult = await this.context.drizzle .select({ count: sql`COUNT(*)` }) .from(auditLogs); const count = countResult[0]?.count ?? 0; - if (count < PRUNE_MAX) { - return; - } + if (count < max) return; - const deleteCount = count - PRUNE_TARGET; + const deleteCount = count - Math.floor(max * PRUNE_TARGET_RATIO); const rows = await this.context.drizzle - .select({ id: auditLogs.id }) + .select({ id: auditLogs.id, timestamp: auditLogs.timestamp }) .from(auditLogs) .orderBy(asc(auditLogs.timestamp)) .limit(deleteCount); - const ids = rows.map((row) => row.id); + if (rows.length === 0) return; - if (ids.length > 0) { - await this.context.drizzle - .delete(auditLogs) - .where(inArray(auditLogs.id, ids)); - } + await this.context.drizzle.delete(auditLogs).where( + inArray( + auditLogs.id, + rows.map((row) => row.id), + ), + ); + + databaseLogger.warn( + `Audit log hit its ${max}-entry cap; discarded ${rows.length} entries`, + { + operation: "audit_overflow_prune", + removed: rows.length, + maxEntries: max, + oldestRemoved: rows[0]?.timestamp, + newestRemoved: rows[rows.length - 1]?.timestamp, + hint: `Raise ${AUDIT_MAX_ENTRIES_ENV}, or set ${AUDIT_RETENTION_DAYS_ENV} and export older entries before they are dropped.`, + }, + ); } private async afterWrite(): Promise { diff --git a/src/backend/database/repositories/session-recording-repository.ts b/src/backend/database/repositories/session-recording-repository.ts index fa3efc10..9259d108 100644 --- a/src/backend/database/repositories/session-recording-repository.ts +++ b/src/backend/database/repositories/session-recording-repository.ts @@ -197,6 +197,25 @@ export class SessionRecordingRepository { return rows.length > 0; } + /** + * Detaches recordings from a user being deleted instead of removing them. + * A recording is evidence about the host as much as about the person, and the + * file stays on disk regardless — deleting only the row would orphan it. + */ + async anonymizeByUserId(userId: string): Promise { + const rows = await this.context.drizzle + .update(sessionRecordings) + .set({ userId: null }) + .where(eq(sessionRecordings.userId, userId)) + .returning({ id: sessionRecordings.id }); + + if (rows.length > 0) { + await this.afterWrite(); + } + + return rows.length; + } + async deleteByUserId(userId: string): Promise { const rows = await this.context.drizzle .delete(sessionRecordings) diff --git a/src/backend/database/routes/delete-user-data.ts b/src/backend/database/routes/delete-user-data.ts index b3ec33e1..268cd92d 100644 --- a/src/backend/database/routes/delete-user-data.ts +++ b/src/backend/database/routes/delete-user-data.ts @@ -44,7 +44,9 @@ export async function deleteUserAndRelatedData(userId: string): Promise { userId, ); - await createCurrentSessionRecordingRepository().deleteByUserId(userId); + // Retained rather than deleted: these outlive the account by design. + // See anonymizeByUserId on each repository. + await createCurrentSessionRecordingRepository().anonymizeByUserId(userId); await createCurrentRbacAccessRepository().deleteHostAccessForUserReferences( userId, @@ -56,7 +58,7 @@ export async function deleteUserAndRelatedData(userId: string): Promise { await createCurrentRoleRepository().removeAllRolesFromUser(userId); await createCurrentAlertRepository().deleteByUserId(userId); - await createCurrentAuditLogRepository().deleteByUserId(userId); + await createCurrentAuditLogRepository().anonymizeByUserId(userId); await createCurrentSshCredentialUsageRepository().deleteByUserId(userId); 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 e113e89e..91d80261 100644 --- a/src/backend/tests/database/repositories/audit-log-repository.test.ts +++ b/src/backend/tests/database/repositories/audit-log-repository.test.ts @@ -28,7 +28,7 @@ describe("AuditLogRepository", () => { CREATE TABLE audit_logs ( id INTEGER PRIMARY KEY AUTOINCREMENT, - user_id TEXT NOT NULL, + user_id TEXT REFERENCES users(id) ON DELETE SET NULL, username TEXT NOT NULL, action TEXT NOT NULL, resource_type TEXT NOT NULL, @@ -129,4 +129,46 @@ describe("AuditLogRepository", () => { ).logs.map((log) => log.userId), ).toEqual(["user-2"]); }); + + it("keeps entries when their user is deleted, detaching instead of removing", async () => { + const repo = await createRepository(); + + await repo.create({ + userId: "user-1", + username: "alice", + action: "delete_host", + resourceType: "host", + resourceId: "9", + success: true, + timestamp: "2026-07-01T00:00:00.000Z", + }); + await repo.create({ + userId: "user-2", + username: "bob", + action: "create_host", + resourceType: "host", + resourceId: "8", + success: true, + timestamp: "2026-07-02T00:00:00.000Z", + }); + + expect(await repo.anonymizeByUserId("user-1")).toBe(1); + + const { logs } = await repo.listPage({ filters: {}, limit: 10, offset: 0 }); + expect(logs).toHaveLength(2); + + const detached = logs.find((log) => log.action === "delete_host"); + // The account is gone; the entry and its actor name are not. + expect(detached?.userId).toBeNull(); + expect(detached?.username).toBe("alice"); + expect(logs.find((log) => log.action === "create_host")?.userId).toBe( + "user-2", + ); + }); + + it("reports nothing to detach for a user with no entries", async () => { + const repo = await createRepository(); + + expect(await repo.anonymizeByUserId("user-2")).toBe(0); + }); }); diff --git a/src/backend/tests/database/repositories/audit-log-retention.test.ts b/src/backend/tests/database/repositories/audit-log-retention.test.ts new file mode 100644 index 00000000..a8d768f4 --- /dev/null +++ b/src/backend/tests/database/repositories/audit-log-retention.test.ts @@ -0,0 +1,174 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const logs = vi.hoisted(() => ({ info: vi.fn(), warn: vi.fn() })); + +vi.mock("../../../utils/logger.js", () => ({ + databaseLogger: logs, +})); + +const { TestSqliteDatabase } = await import("./test-support.js"); +const { + AuditLogRepository, + auditRetentionDays, + auditMaxEntries, + AUDIT_RETENTION_DAYS_ENV, + AUDIT_MAX_ENTRIES_ENV, +} = await import("../../../database/repositories/audit-log-repository.js"); + +let adapter: InstanceType | null = null; +const savedEnv: Record = {}; + +beforeEach(() => { + logs.info.mockReset(); + logs.warn.mockReset(); + for (const key of [AUDIT_RETENTION_DAYS_ENV, AUDIT_MAX_ENTRIES_ENV]) { + savedEnv[key] = process.env[key]; + delete process.env[key]; + } +}); + +afterEach(async () => { + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + if (adapter) { + await adapter.close(); + adapter = null; + } +}); + +async function createRepository() { + adapter = new TestSqliteDatabase(); + const context = await adapter.connect(); + adapter.exec(` + CREATE TABLE audit_logs ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + user_id TEXT, + username TEXT NOT NULL, + action TEXT NOT NULL, + resource_type TEXT NOT NULL, + resource_id TEXT, + resource_name TEXT, + details TEXT, + ip_address TEXT, + user_agent TEXT, + success INTEGER NOT NULL, + error_message TEXT, + timestamp TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP + ); + `); + return new AuditLogRepository(context); +} + +function daysAgo(days: number): string { + const d = new Date(Date.now() - days * 24 * 60 * 60 * 1000); + return d.toISOString().slice(0, 19).replace("T", " "); +} + +async function seed( + repo: Awaited>, + timestamp: string, + action = "create_host", +) { + await repo.create({ + userId: "u-1", + username: "alice", + action, + resourceType: "host", + success: true, + timestamp, + }); +} + +describe("audit retention configuration", () => { + it("has no time limit unless one is configured", () => { + expect(auditRetentionDays({})).toBeNull(); + expect(auditRetentionDays({ [AUDIT_RETENTION_DAYS_ENV]: "90" })).toBe(90); + }); + + it("ignores values that are not a positive count", () => { + for (const bad of ["0", "-5", "", "abc"]) { + expect( + auditRetentionDays({ [AUDIT_RETENTION_DAYS_ENV]: bad }), + ).toBeNull(); + } + }); + + it("falls back to the built-in cap", () => { + expect(auditMaxEntries({})).toBe(10000); + expect(auditMaxEntries({ [AUDIT_MAX_ENTRIES_ENV]: "250" })).toBe(250); + expect(auditMaxEntries({ [AUDIT_MAX_ENTRIES_ENV]: "-1" })).toBe(10000); + }); +}); + +describe("audit retention pruning", () => { + it("keeps everything when no retention is set", async () => { + const repo = await createRepository(); + + await seed(repo, daysAgo(400)); + await seed(repo, daysAgo(1)); + + const { total } = await repo.listPage({ + filters: {}, + limit: 10, + offset: 0, + }); + expect(total).toBe(2); + expect(logs.info).not.toHaveBeenCalled(); + }); + + it("drops entries past the retention window and says so", async () => { + process.env[AUDIT_RETENTION_DAYS_ENV] = "30"; + const repo = await createRepository(); + + await seed(repo, daysAgo(90), "old_action"); + await seed(repo, daysAgo(5), "recent_action"); + + const { logs: rows } = await repo.listPage({ + filters: {}, + limit: 10, + offset: 0, + }); + expect(rows.map((r) => r.action)).toEqual(["recent_action"]); + + expect(logs.info).toHaveBeenCalledWith( + expect.stringContaining("past retention"), + expect.objectContaining({ operation: "audit_retention_prune" }), + ); + }); + + it("warns when the row cap discards entries still inside the window", async () => { + process.env[AUDIT_MAX_ENTRIES_ENV] = "5"; + const repo = await createRepository(); + + for (let i = 0; i < 6; i++) { + await seed(repo, daysAgo(10 - i), `action_${i}`); + } + + // The cap is not a retention policy: these entries were still current. + expect(logs.warn).toHaveBeenCalledWith( + expect.stringContaining("cap"), + expect.objectContaining({ + operation: "audit_overflow_prune", + maxEntries: 5, + }), + ); + + const { total } = await repo.listPage({ + filters: {}, + limit: 20, + offset: 0, + }); + expect(total).toBeLessThan(6); + }); + + it("stays quiet while under the cap", async () => { + process.env[AUDIT_MAX_ENTRIES_ENV] = "100"; + const repo = await createRepository(); + + await seed(repo, daysAgo(1)); + + expect(logs.warn).not.toHaveBeenCalled(); + }); +});