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();