keep audit trails and recordings when a user is deleted (#1128)

audit_logs and session_recordings both referenced users with ON DELETE CASCADE,
so removing an account erased everything it had ever done. An audit trail that
disappears with the account it recorded cannot answer the question it exists for,
and a recording is evidence about a host as much as about a person.

Both foreign keys become ON DELETE SET NULL. audit_logs already denormalises
username, so an entry still names who acted once the reference is gone.
session_recordings did not, so the column is added and backfilled first —
otherwise relaxing the constraint would only trade deleted evidence for
anonymous evidence.

SQLite cannot alter a foreign key in place, so existing databases are migrated
by copy-and-swap, guarded by a PRAGMA check that makes it idempotent. Fresh
databases are created in the target shape and skip it. Recordings still cascade
from their host.
This commit is contained in:
ZacharyZcR
2026-07-28 17:01:33 +08:00
committed by GitHub
parent 64a80f411a
commit 768c64bd6a
5 changed files with 431 additions and 12 deletions
+11 -5
View File
@@ -8,6 +8,7 @@ import { DatabaseFileEncryption } from "../../utils/database-file-encryption.js"
import { SystemCrypto } from "../../utils/system-crypto.js";
import { DatabaseMigration } from "../../utils/database-migration.js";
import { DatabaseSaveTrigger } from "../../utils/database-save-trigger.js";
import { migrateAuditRetention } from "../../utils/audit-retention-migration.js";
import {
assertDataDirIsNotMisconfigured,
DataDirMisconfiguredError,
@@ -482,13 +483,14 @@ async function initializeCompleteDatabase(): Promise<void> {
success INTEGER NOT NULL,
error_message TEXT,
timestamp TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE CASCADE
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE SET NULL
);
CREATE TABLE IF NOT EXISTS session_recordings (
id INTEGER PRIMARY KEY AUTOINCREMENT,
host_id INTEGER NOT NULL,
user_id TEXT NOT NULL,
user_id TEXT,
username TEXT,
access_id INTEGER,
started_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
ended_at TEXT,
@@ -501,7 +503,7 @@ async function initializeCompleteDatabase(): Promise<void> {
terminated_by_owner INTEGER DEFAULT 0,
termination_reason TEXT,
FOREIGN KEY (host_id) REFERENCES ssh_data (id) ON DELETE CASCADE,
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE CASCADE,
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE SET NULL,
FOREIGN KEY (access_id) REFERENCES host_access (id) ON DELETE SET NULL
);
@@ -1647,7 +1649,7 @@ const migrateSchema = () => {
sqlite.exec(`
CREATE TABLE IF NOT EXISTS audit_logs (
id INTEGER PRIMARY KEY AUTOINCREMENT,
user_id TEXT NOT NULL,
user_id TEXT,
username TEXT NOT NULL,
action TEXT NOT NULL,
resource_type TEXT NOT NULL,
@@ -1659,7 +1661,7 @@ const migrateSchema = () => {
success INTEGER NOT NULL,
error_message TEXT,
timestamp TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE CASCADE
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE SET NULL
);
`);
} catch (createError) {
@@ -2507,6 +2509,10 @@ const migrateSchema = () => {
}
// --- sync end ---
// Audit trails and session recordings used to be deleted along with the user
// they referenced, which defeats the point of keeping them.
migrateAuditRetention(sqlite);
databaseLogger.success("Schema migration completed", {
operation: "schema_migration",
});
+7 -6
View File
@@ -641,9 +641,9 @@ export const userRoles = sqliteTable("user_roles", {
export const auditLogs = sqliteTable("audit_logs", {
id: integer("id").primaryKey({ autoIncrement: true }),
userId: text("user_id")
.notNull()
.references(() => users.id, { onDelete: "cascade" }),
// Nullable on purpose: the trail outlives the account, and username keeps the
// entry attributable once the reference is gone.
userId: text("user_id").references(() => users.id, { onDelete: "set null" }),
username: text("username").notNull(),
action: text("action").notNull(),
@@ -669,9 +669,10 @@ export const sessionRecordings = sqliteTable("session_recordings", {
hostId: integer("host_id")
.notNull()
.references(() => hosts.id, { onDelete: "cascade" }),
userId: text("user_id")
.notNull()
.references(() => users.id, { onDelete: "cascade" }),
// Nullable on purpose: a recording is evidence about the host as much as the
// person, so it outlives the account. username keeps it attributable.
userId: text("user_id").references(() => users.id, { onDelete: "set null" }),
username: text("username"),
accessId: integer("access_id").references(() => hostAccess.id, {
onDelete: "set null",
}),
@@ -34,7 +34,8 @@ describe("SessionRecordingRepository", () => {
CREATE TABLE session_recordings (
id INTEGER PRIMARY KEY AUTOINCREMENT,
host_id INTEGER NOT NULL,
user_id TEXT NOT NULL,
user_id TEXT,
username TEXT,
access_id INTEGER,
started_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
ended_at TEXT,
@@ -0,0 +1,190 @@
import { afterEach, describe, expect, it } from "vitest";
import Database from "better-sqlite3";
import {
migrateAuditRetention,
userDeleteIsDestructive,
} from "../../utils/audit-retention-migration.js";
let db: Database.Database | null = null;
afterEach(() => {
db?.close();
db = null;
});
/** The pre-migration shape: both tables cascade from users. */
function legacyDatabase(): Database.Database {
const sqlite = new Database(":memory:");
sqlite.exec(`
PRAGMA foreign_keys = ON;
CREATE TABLE users (
id TEXT PRIMARY KEY,
username TEXT NOT NULL
);
CREATE TABLE ssh_data (
id INTEGER PRIMARY KEY AUTOINCREMENT,
name TEXT NOT NULL
);
CREATE TABLE host_access (
id INTEGER PRIMARY KEY AUTOINCREMENT
);
CREATE TABLE audit_logs (
id INTEGER PRIMARY KEY AUTOINCREMENT,
user_id TEXT NOT NULL,
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,
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE CASCADE
);
CREATE TABLE session_recordings (
id INTEGER PRIMARY KEY AUTOINCREMENT,
host_id INTEGER NOT NULL,
user_id TEXT NOT NULL,
access_id INTEGER,
started_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
ended_at TEXT,
duration INTEGER,
commands TEXT,
dangerous_actions TEXT,
recording_path TEXT,
protocol TEXT NOT NULL DEFAULT 'ssh',
format TEXT NOT NULL DEFAULT 'text',
terminated_by_owner INTEGER DEFAULT 0,
termination_reason TEXT,
FOREIGN KEY (host_id) REFERENCES ssh_data (id) ON DELETE CASCADE,
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE CASCADE,
FOREIGN KEY (access_id) REFERENCES host_access (id) ON DELETE SET NULL
);
INSERT INTO users (id, username) VALUES ('u-1', 'alice'), ('u-2', 'bob');
INSERT INTO ssh_data (id, name) VALUES (1, 'prod-db');
INSERT INTO audit_logs
(user_id, username, action, resource_type, resource_id, success, timestamp)
VALUES
('u-1', 'alice', 'host.delete', 'host', '1', 1, '2026-07-01 10:00:00'),
('u-1', 'alice', 'credential.view', 'credential', '9', 1, '2026-07-02 11:00:00'),
('u-2', 'bob', 'host.create', 'host', '2', 1, '2026-07-03 12:00:00');
INSERT INTO session_recordings
(host_id, user_id, started_at, recording_path, protocol, format)
VALUES
(1, 'u-1', '2026-07-01 10:00:00', '/rec/a.guac', 'ssh', 'text'),
(1, 'u-2', '2026-07-03 12:00:00', '/rec/b.guac', 'ssh', 'text');
`);
return sqlite;
}
describe("audit retention migration", () => {
it("detects the destructive shape and reports it fixed afterwards", () => {
db = legacyDatabase();
expect(userDeleteIsDestructive(db, "audit_logs")).toBe(true);
expect(userDeleteIsDestructive(db, "session_recordings")).toBe(true);
expect(migrateAuditRetention(db)).toEqual([
"audit_logs",
"session_recordings",
]);
expect(userDeleteIsDestructive(db, "audit_logs")).toBe(false);
expect(userDeleteIsDestructive(db, "session_recordings")).toBe(false);
});
it("keeps the audit trail when the user is deleted", () => {
db = legacyDatabase();
migrateAuditRetention(db);
db.exec("DELETE FROM users WHERE id = 'u-1'");
const rows = db
.prepare(
"SELECT user_id, username, action FROM audit_logs ORDER BY timestamp",
)
.all() as { user_id: string | null; username: string; action: string }[];
expect(rows).toHaveLength(3);
// The account is gone, but the record still names who acted.
expect(rows[0]).toEqual({
user_id: null,
username: "alice",
action: "host.delete",
});
expect(rows[2].user_id).toBe("u-2");
});
it("backfills a username onto recordings so they stay attributable", () => {
db = legacyDatabase();
migrateAuditRetention(db);
db.exec("DELETE FROM users WHERE id = 'u-1'");
const rows = db
.prepare(
"SELECT user_id, username, recording_path FROM session_recordings ORDER BY started_at",
)
.all() as { user_id: string | null; username: string | null }[];
expect(rows).toHaveLength(2);
expect(rows[0].user_id).toBeNull();
expect(rows[0].username).toBe("alice");
});
it("loses no data in the copy", () => {
db = legacyDatabase();
const before = db
.prepare("SELECT * FROM audit_logs ORDER BY id")
.all() as Record<string, unknown>[];
migrateAuditRetention(db);
const after = db
.prepare("SELECT * FROM audit_logs ORDER BY id")
.all() as Record<string, unknown>[];
expect(after).toEqual(before);
});
it("still cascades recordings when their host is deleted", () => {
db = legacyDatabase();
migrateAuditRetention(db);
db.exec("PRAGMA foreign_keys = ON");
db.exec("DELETE FROM ssh_data WHERE id = 1");
expect(
db.prepare("SELECT COUNT(*) AS n FROM session_recordings").get(),
).toEqual({ n: 0 });
});
it("is idempotent and leaves an already-migrated database alone", () => {
db = legacyDatabase();
migrateAuditRetention(db);
const rowsAfterFirst = db.prepare("SELECT * FROM audit_logs").all();
expect(migrateAuditRetention(db)).toEqual([]);
expect(db.prepare("SELECT * FROM audit_logs").all()).toEqual(
rowsAfterFirst,
);
});
it("does nothing on a database without the tables", () => {
db = new Database(":memory:");
expect(() => migrateAuditRetention(db)).not.toThrow();
expect(migrateAuditRetention(db)).toEqual([]);
});
});
@@ -0,0 +1,221 @@
import { databaseLogger } from "./logger.js";
export interface MigratableSqlite {
exec(sql: string): unknown;
prepare(sql: string): {
all(...params: unknown[]): unknown[];
get(...params: unknown[]): unknown;
run(...params: unknown[]): unknown;
};
}
interface ForeignKeyRow {
table?: string;
from?: string;
on_delete?: string;
}
interface RetainedTable {
name: string;
/** Column list for the copy, in the order the rebuilt table declares them. */
columns: string[];
createSql: string;
}
/**
* `audit_logs` already denormalises `username`, so nulling `user_id` still
* leaves a record of who acted. `session_recordings` does not, which is why the
* column is added and backfilled before its foreign key is relaxed otherwise
* relaxing it would trade deleted evidence for anonymous evidence.
*/
const AUDIT_LOGS: RetainedTable = {
name: "audit_logs",
columns: [
"id",
"user_id",
"username",
"action",
"resource_type",
"resource_id",
"resource_name",
"details",
"ip_address",
"user_agent",
"success",
"error_message",
"timestamp",
],
createSql: `
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,
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE SET NULL
);
`,
};
const SESSION_RECORDINGS: RetainedTable = {
name: "session_recordings",
columns: [
"id",
"host_id",
"user_id",
"username",
"access_id",
"started_at",
"ended_at",
"duration",
"commands",
"dangerous_actions",
"recording_path",
"protocol",
"format",
"terminated_by_owner",
"termination_reason",
],
createSql: `
CREATE TABLE session_recordings (
id INTEGER PRIMARY KEY AUTOINCREMENT,
host_id INTEGER NOT NULL,
user_id TEXT,
username TEXT,
access_id INTEGER,
started_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP,
ended_at TEXT,
duration INTEGER,
commands TEXT,
dangerous_actions TEXT,
recording_path TEXT,
protocol TEXT NOT NULL DEFAULT 'ssh',
format TEXT NOT NULL DEFAULT 'text',
terminated_by_owner INTEGER DEFAULT 0,
termination_reason TEXT,
FOREIGN KEY (host_id) REFERENCES ssh_data (id) ON DELETE CASCADE,
FOREIGN KEY (user_id) REFERENCES users (id) ON DELETE SET NULL,
FOREIGN KEY (access_id) REFERENCES host_access (id) ON DELETE SET NULL
);
`,
};
const RETAINED_TABLES = [AUDIT_LOGS, SESSION_RECORDINGS];
export function userDeleteIsDestructive(
sqlite: MigratableSqlite,
table: string,
): boolean {
let rows: ForeignKeyRow[];
try {
rows = sqlite
.prepare(`PRAGMA foreign_key_list(${table})`)
.all() as ForeignKeyRow[];
} catch {
// Table absent on a fresh database; it is created in the target shape.
return false;
}
return rows.some(
(row) =>
row.table === "users" &&
row.from === "user_id" &&
(row.on_delete ?? "").toUpperCase() === "CASCADE",
);
}
function hasColumn(
sqlite: MigratableSqlite,
table: string,
column: string,
): boolean {
try {
sqlite.prepare(`SELECT "${column}" FROM ${table} LIMIT 1`).get();
return true;
} catch {
return false;
}
}
/**
* Gives session_recordings a username before its user_id can become null, so
* existing rows stay attributable.
*/
function ensureRecordingUsername(sqlite: MigratableSqlite): void {
if (hasColumn(sqlite, "session_recordings", "username")) return;
sqlite.exec(`ALTER TABLE session_recordings ADD COLUMN username TEXT;`);
sqlite.exec(`
UPDATE session_recordings
SET username = (SELECT username FROM users WHERE users.id = session_recordings.user_id)
WHERE username IS NULL;
`);
}
/**
* SQLite cannot alter a foreign key in place, so the table is copied into a new
* one with the intended constraint and swapped. Foreign keys must be off.
*/
function rebuildTable(sqlite: MigratableSqlite, table: RetainedTable): void {
const columns = table.columns.join(", ");
const temp = `${table.name}_retained`;
sqlite.exec(table.createSql.replace(table.name, temp));
sqlite.exec(
`INSERT INTO ${temp} (${columns}) SELECT ${columns} FROM ${table.name};`,
);
sqlite.exec(`DROP TABLE ${table.name};`);
sqlite.exec(`ALTER TABLE ${temp} RENAME TO ${table.name};`);
}
/**
* Turns ON DELETE CASCADE into ON DELETE SET NULL for the tables that have to
* outlive the account they reference. Idempotent.
*/
export function migrateAuditRetention(sqlite: MigratableSqlite): string[] {
const migrated: string[] = [];
for (const table of RETAINED_TABLES) {
if (!userDeleteIsDestructive(sqlite, table.name)) continue;
try {
if (table.name === "session_recordings") {
ensureRecordingUsername(sqlite);
}
sqlite.exec("PRAGMA foreign_keys = OFF");
sqlite.exec("BEGIN TRANSACTION");
rebuildTable(sqlite, table);
sqlite.exec("COMMIT");
migrated.push(table.name);
databaseLogger.info(`${table.name} now survives user deletion`, {
operation: "audit_retention_migration",
table: table.name,
});
} catch (error) {
try {
sqlite.exec("ROLLBACK");
} catch {
// no transaction open
}
databaseLogger.warn(`Could not migrate ${table.name} retention`, {
operation: "audit_retention_migration_failed",
table: table.name,
error: error instanceof Error ? error.message : String(error),
});
} finally {
sqlite.exec("PRAGMA foreign_keys = ON");
}
}
return migrated;
}