diff --git a/src/backend/database/db/index.ts b/src/backend/database/db/index.ts index f40dc0d1..ee8ca5b2 100644 --- a/src/backend/database/db/index.ts +++ b/src/backend/database/db/index.ts @@ -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 { 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 { 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", }); diff --git a/src/backend/database/db/schema.ts b/src/backend/database/db/schema.ts index 423154b4..8f6942df 100644 --- a/src/backend/database/db/schema.ts +++ b/src/backend/database/db/schema.ts @@ -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", }), diff --git a/src/backend/tests/database/repositories/session-recording-repository.test.ts b/src/backend/tests/database/repositories/session-recording-repository.test.ts index 186128a6..64f3f3ba 100644 --- a/src/backend/tests/database/repositories/session-recording-repository.test.ts +++ b/src/backend/tests/database/repositories/session-recording-repository.test.ts @@ -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, diff --git a/src/backend/tests/utils/audit-retention-migration.test.ts b/src/backend/tests/utils/audit-retention-migration.test.ts new file mode 100644 index 00000000..a4c4c477 --- /dev/null +++ b/src/backend/tests/utils/audit-retention-migration.test.ts @@ -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[]; + + migrateAuditRetention(db); + + const after = db + .prepare("SELECT * FROM audit_logs ORDER BY id") + .all() as Record[]; + + 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([]); + }); +}); diff --git a/src/backend/utils/audit-retention-migration.ts b/src/backend/utils/audit-retention-migration.ts new file mode 100644 index 00000000..ef019165 --- /dev/null +++ b/src/backend/utils/audit-retention-migration.ts @@ -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; +}