From 37faa15db00b4d16875cabacd7b5e5a200c9c74c Mon Sep 17 00:00:00 2001 From: Nicholas Velten Date: Tue, 7 Apr 2026 15:39:22 -0300 Subject: [PATCH] feat: SQLite immutable audit triggers (closes #99) Install BEFORE UPDATE and BEFORE DELETE triggers on audit_log during schema init. Any modification or deletion of committed audit rows is aborted by SQLite at the engine level with zero runtime cost. The DELETE trigger is omitted when rotation (max_entries/max_age_days) is active since rotation intentionally prunes old rows. Tamper tests updated to drop triggers before mutating, demonstrating that the hash-chain remains a second layer of defence even if triggers are bypassed. Adds two new trigger unit tests. Co-Authored-By: Claude Sonnet 4.6 --- CHANGELOG.md | 3 ++ src/audit/sqlite.rs | 80 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8c66af..3dcf377 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## [Unreleased] +### Added +- **SQLite immutable audit triggers** (`src/audit/sqlite.rs`): two `BEFORE UPDATE / BEFORE DELETE` triggers are now installed during schema initialisation, enforcing audit record immutability at the database engine level — zero runtime performance cost. Any attempt to modify or delete a committed audit row is aborted by SQLite regardless of which process or connection issues it. The `no_audit_delete` trigger is skipped when rotation (`max_entries` / `max_age_days`) is configured, since rotation intentionally prunes old rows. Closes #99. + ### Fixed - **Federation timeout now returns partial results** (`src/gateway.rs`): when `tools/list` is federated across multiple upstreams and the deadline fires, results from upstreams that already responded are now returned instead of discarding everything with an error. The response includes `"_arbitus_partial": true` when not all upstreams replied in time — clients can proceed with the partial tool catalog rather than seeing an empty list. Closes #97. - **Stack overflow on deeply nested JSON payloads** (`src/middleware/payload_filter.rs`, `src/gateway.rs`): `scan_value` (payload filter) was fully recursive and would overflow the stack on deeply nested JSON arguments. Replaced with an explicit-stack iterative traversal capped at `MAX_DEPTH = 64`. `redact_value` (gateway) was also recursive; refactored to depth-parameterised recursion that returns the value unchanged beyond `REDACT_MAX_DEPTH = 64` instead of panicking. Added two new unit tests covering the boundary. Closes #95. diff --git a/src/audit/sqlite.rs b/src/audit/sqlite.rs index 52642b7..d31e3ed 100644 --- a/src/audit/sqlite.rs +++ b/src/audit/sqlite.rs @@ -192,6 +192,26 @@ impl SqliteAudit { .execute_batch("ALTER TABLE audit_log ADD COLUMN prev_hash TEXT NOT NULL DEFAULT '';"); let _ = conn .execute_batch("ALTER TABLE audit_log ADD COLUMN entry_hash TEXT NOT NULL DEFAULT '';"); + // Immutability triggers — database-level enforcement; zero runtime cost. + // The UPDATE trigger is always installed: no row should ever be modified. + // The DELETE trigger is only installed when rotation is disabled; rotation + // uses DELETE to prune old entries by design, so the two are mutually exclusive. + conn.execute_batch( + "CREATE TRIGGER IF NOT EXISTS no_audit_update + BEFORE UPDATE ON audit_log + BEGIN + SELECT RAISE(ABORT, 'audit records are immutable'); + END;", + )?; + if max_entries.is_none() && max_age_days.is_none() { + conn.execute_batch( + "CREATE TRIGGER IF NOT EXISTS no_audit_delete + BEFORE DELETE ON audit_log + BEGIN + SELECT RAISE(ABORT, 'audit records are immutable'); + END;", + )?; + } let conn = Arc::new(Mutex::new(conn)); let (tx, mut rx) = mpsc::channel::>(CHANNEL_CAPACITY); @@ -655,8 +675,15 @@ mod tests { audit.record(entry(Outcome::Allowed)); audit.flush().await; - // Tamper: overwrite the entry_hash in the database + // Simulate a privileged attacker who bypasses triggers to tamper with the DB. + // Drop triggers first (requires schema-write access), then mutate the row. + // The hash chain must still detect the tampering. let conn = Connection::open(path).unwrap(); + conn.execute_batch( + "DROP TRIGGER IF EXISTS no_audit_update; + DROP TRIGGER IF EXISTS no_audit_delete;", + ) + .unwrap(); conn.execute( "UPDATE audit_log SET entry_hash = 'deadbeef' WHERE id = 1", [], @@ -679,8 +706,13 @@ mod tests { audit.record(entry(Outcome::Allowed)); audit.flush().await; - // Break the chain: change row 2's prev_hash without updating entry_hash + // Simulate a privileged attacker bypassing triggers before mutating. let conn = Connection::open(path).unwrap(); + conn.execute_batch( + "DROP TRIGGER IF EXISTS no_audit_update; + DROP TRIGGER IF EXISTS no_audit_delete;", + ) + .unwrap(); conn.execute( "UPDATE audit_log SET prev_hash = 'badhash' WHERE id = 2", [], @@ -694,6 +726,50 @@ mod tests { ); } + // ── Immutability triggers ───────────────────────────────────────────────── + + #[tokio::test] + async fn trigger_prevents_update_on_audit_log() { + let f = NamedTempFile::new().unwrap(); + let path = f.path().to_str().unwrap(); + let audit = SqliteAudit::new(path, test_metrics()).unwrap(); + audit.record(entry(Outcome::Allowed)); + audit.flush().await; + + let conn = Connection::open(path).unwrap(); + let result = conn.execute("UPDATE audit_log SET outcome = 'blocked' WHERE id = 1", []); + assert!( + result.is_err(), + "UPDATE must be rejected by the immutability trigger" + ); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("immutable"), + "error must mention immutability, got: {err}" + ); + } + + #[tokio::test] + async fn trigger_prevents_delete_on_audit_log() { + let f = NamedTempFile::new().unwrap(); + let path = f.path().to_str().unwrap(); + let audit = SqliteAudit::new(path, test_metrics()).unwrap(); + audit.record(entry(Outcome::Allowed)); + audit.flush().await; + + let conn = Connection::open(path).unwrap(); + let result = conn.execute("DELETE FROM audit_log WHERE id = 1", []); + assert!( + result.is_err(), + "DELETE must be rejected by the immutability trigger" + ); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("immutable"), + "error must mention immutability, got: {err}" + ); + } + #[tokio::test] async fn verify_chain_empty_log_returns_ok() { let f = NamedTempFile::new().unwrap();