stop deleting audit trails, and say when they are dropped (#1132)

Two ways audit evidence still disappeared silently.

Deleting an account removed its audit entries and session recordings outright.
#1128 relaxed those foreign keys to ON DELETE SET NULL, but deleteUserAndRelatedData
deletes the rows explicitly, so the schema change had no effect on the path that
actually matters. Both repositories gain anonymizeByUserId, which nulls the
reference and leaves the row; username is already denormalised on both tables, so
entries stay attributable to whoever acted.

Separately, the log pruned itself at a hard-coded 10000 rows with no signal.
Entries well inside any retention window were discarded and nothing recorded it.
Retention is now configurable by age via AUDIT_LOG_RETENTION_DAYS, the row cap
via AUDIT_LOG_MAX_ENTRIES, and the two are reported differently: expiring an old
entry is routine and logged at info, while hitting the cap means the ceiling is
too low for how much this install audits and is logged at warn, naming the range
discarded and how to stop it.
This commit is contained in:
ZacharyZcR
2026-07-28 19:23:46 +08:00
committed by GitHub
parent 43e972be84
commit 81b5a6cf01
5 changed files with 348 additions and 17 deletions
@@ -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<number> {
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<number> {
const rows = await this.context.drizzle
.delete(auditLogs)
@@ -105,28 +153,74 @@ export class AuditLogRepository {
}
private async pruneIfNeeded(): Promise<void> {
await this.pruneExpired();
await this.pruneOverflow();
}
/** Drops entries past the configured retention window. */
private async pruneExpired(): Promise<void> {
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<void> {
const max = auditMaxEntries();
const countResult = await this.context.drizzle
.select({ count: sql<number>`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<void> {
@@ -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<number> {
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<number> {
const rows = await this.context.drizzle
.delete(sessionRecordings)