sqlite: audit savepoint boundaries — no over-report on ROLLBACK TO

The committed-audit stream over-reported when SQL rolled back to a
savepoint inside an open transaction (`BEGIN; SAVEPOINT s; INSERT…;
ROLLBACK TO s; COMMIT;`): SQLite fires no rollback hook for `ROLLBACK TO`,
so rows written after the savepoint stayed pending and were promoted to
the committed stream on the outer COMMIT — recording an insert that never
landed.

EngineContext now tracks open savepoints as (name, pending-count) markers,
driven by the SQLITE_SAVEPOINT authorizer events (arg1 = BEGIN/RELEASE/
ROLLBACK, arg2 = name; verified against the amalgamation). Because the
authorizer fires at prepare time and runScript prepares+steps one
statement at a time, by the time `ROLLBACK TO s` is prepared the
intervening rows are already pending, so the marker trims them precisely.
RELEASE merges into the parent; commit()/rollback()/discardPending() clear
the stack.

Tests: a rolled-back-savepoint insert is absent from the committed stream
(and the row doesn't land); a RELEASEd-then-committed insert is present.

Author: claude

PLAN.md

@@ -320,7 +320,6 @@ public extension Shell {
 4. **Value-level audit** — enable `SQLITE_ENABLE_PREUPDATE_HOOK` if old/new row values are needed (vs. table+rowid only). It is also the way to capture the remaining committed-DELETE case: the authorizer returns `SQLITE_IGNORE` for `SQLITE_DELETE` so `DELETE FROM t` is deleted row-by-row (defeating the truncate optimization) and thus seen by `sqlite3_update_hook`, but rows deleted via `ON CONFLICT REPLACE` are still not reported by the update hook — only the preupdate hook sees those.
 5. **Bounded stdin** — SQL piped on stdin is currently read fully into memory before `SQLITE_LIMIT_SQL_LENGTH` (a prepare-time cap) applies. An incremental, capped stdin read (rejecting input past `EnginePolicy.maxSQLLength` as it's consumed) is a follow-up for hardening against a large-pipe memory DoS.
 6. **`.dump` non-UTF-8 TEXT** — SQLite doesn't enforce UTF-8; bytes from e.g. `CAST(x'80' AS TEXT)` are lossily decoded to U+FFFD at read time, so `.dump` can't recover them. Preserving them needs the value model to carry raw bytes for TEXT (vs. a Swift `String`); an advanced edge, follow-up. (Generated columns *are* handled now — the dump emits an explicit insertable-column list.)
-7. **Audit savepoint boundaries** — the committed-audit stream **over-reports** when SQL rolls back to a SAVEPOINT inside an open transaction (`BEGIN; SAVEPOINT s; INSERT…; ROLLBACK TO s; COMMIT;`): the rollback hook isn't invoked for `ROLLBACK TO`, so rows written after the savepoint stay pending and are promoted on the outer `COMMIT`. Correct handling needs savepoint-aware pending boundaries (a marker stack keyed off the `SQLITE_SAVEPOINT` authorizer events). Full COMMIT/ROLLBACK is already accurate; this is a best-effort edge alongside the REPLACE-delete case (knob #4).
 
 ## 14. References (SwiftBash files this design mirrors)
 

Sources/SwiftSQLiteKit/EngineContext.swift

@@ -23,6 +23,10 @@ final class EngineContext: @unchecked Sendable {
     /// Per-row update-hook records awaiting a commit. Promoted to
     /// `committed` events on `commit()`, dropped on `rollback()`.
     private var pending: [(table: String, rowid: Int64, op: String)] = []
+    /// Open savepoints as `(name, pending.count when created)`. Lets a
+    /// `ROLLBACK TO s` discard the pending rows recorded after `s` (SQLite
+    /// fires no rollback hook for `ROLLBACK TO`).
+    private var savepoints: [(name: String, mark: Int)] = []
 
     var deadlineNanos: UInt64?
 
@@ -42,9 +46,41 @@ final class EngineContext: @unchecked Sendable {
             action: Self.actionName(action),
             table: guardedObjectName(action: action, arg1: arg1, arg2: arg2),
             allowed: decision != SQLITE_DENY))
+        // Track savepoint boundaries so `ROLLBACK TO s` drops the pending
+        // committed-audit rows recorded after `s`. arg1 is the operation
+        // ("BEGIN"/"RELEASE"/"ROLLBACK"), arg2 the savepoint name. The
+        // authorizer fires at prepare time and runScript prepares+steps one
+        // statement at a time, so when `ROLLBACK TO s` is prepared the
+        // intervening rows are already in `pending`.
+        if action == SQLITE_SAVEPOINT, decision == SQLITE_OK {
+            applySavepoint(operation: arg1, name: arg2)
+        }
         return decision
     }
 
+    private func applySavepoint(operation: String?, name: String?) {
+        guard let name else { return }
+        switch operation {
+        case "BEGIN":
+            savepoints.append((name: name, mark: pending.count))
+        case "ROLLBACK":
+            // Discard pending rows recorded after `s`, and drop nested
+            // savepoints above it; `s` itself stays active.
+            guard let idx = savepoints.lastIndex(where: { $0.name == name })
+            else { return }
+            let mark = savepoints[idx].mark
+            if mark < pending.count { pending.removeLast(pending.count - mark) }
+            savepoints.removeSubrange((idx + 1)...)
+        case "RELEASE":
+            // `s` (and nested savepoints) merge into the parent; rows remain.
+            if let idx = savepoints.lastIndex(where: { $0.name == name }) {
+                savepoints.removeSubrange(idx...)
+            }
+        default:
+            break
+        }
+    }
+
     private func decide(action: Int32, arg1: String?, arg2: String?) -> Int32 {
         let object = guardedObjectName(action: action, arg1: arg1, arg2: arg2)
 
@@ -189,19 +225,20 @@ final class EngineContext: @unchecked Sendable {
             events.append(.committed(table: record.table, rowid: record.rowid, op: record.op))
         }
         pending.removeAll(keepingCapacity: true)
+        savepoints.removeAll(keepingCapacity: true)
     }
 
     /// Drop pending rows — the transaction rolled back, so they never
     /// committed. They remain in the *attempted* stream (recorded by the
     /// authorizer), never the committed stream.
     ///
-    /// Known limitation: this fires only on a full transaction rollback, not
-    /// on `ROLLBACK TO SAVEPOINT` (SQLite exposes no savepoint-execution
-    /// hook), and the pending buffer has no savepoint boundaries. So rows
-    /// written after a savepoint that is later rolled back to may still be
-    /// promoted to the committed stream. The attempted stream is unaffected.
+    /// `ROLLBACK TO SAVEPOINT` (which fires no rollback hook) is handled
+    /// separately via the savepoint markers in `applySavepoint` — a partial
+    /// rollback trims `pending` back to the savepoint boundary, so this full
+    /// rollback only has to clear what remains.
     func rollback() {
         pending.removeAll(keepingCapacity: true)
+        savepoints.removeAll(keepingCapacity: true)
     }
 
     // MARK: Timeout / cancellation
@@ -229,6 +266,7 @@ final class EngineContext: @unchecked Sendable {
 
     func discardPending() {
         pending.removeAll(keepingCapacity: false)
+        savepoints.removeAll(keepingCapacity: false)
     }
 
     // MARK: Action-code names (for the attempted stream)

Tests/SwiftSQLiteKitTests/AuditTests.swift

@@ -95,4 +95,43 @@ struct AuditTests {
             return false
         }, "committed DELETE rows should be audited, got \(committed)")
     }
+
+    /// A row inserted inside a savepoint that is rolled back before COMMIT
+    /// never lands, so it must not appear in the committed stream (the
+    /// rollback hook doesn't fire for `ROLLBACK TO` — savepoint markers handle
+    /// it).
+    @Test func rolledBackSavepointInsertIsNotCommitted() async throws {
+        let sink = InMemoryAuditSink()
+        let db = try await SQLiteConnection(inMemory: .default, audit: sink)
+        try await db.execute("CREATE TABLE t(x);")
+        try await db.run(
+            "BEGIN; SAVEPOINT s; INSERT INTO t(x) VALUES (1); ROLLBACK TO s; COMMIT;")
+
+        let count = try await db.query("SELECT count(*) FROM t;")
+        #expect(count.rows[0][0] == .integer(0))
+        await db.close()
+
+        let committed = await sink.committed
+        #expect(!committed.contains {
+            if case .committed(_, _, "INSERT") = $0 { return true }
+            return false
+        }, "rolled-back savepoint insert leaked into committed: \(committed)")
+    }
+
+    /// Control: an insert in a savepoint that is RELEASEd (merged into the
+    /// outer transaction) and committed *does* appear in the committed stream.
+    @Test func releasedSavepointInsertIsCommitted() async throws {
+        let sink = InMemoryAuditSink()
+        let db = try await SQLiteConnection(inMemory: .default, audit: sink)
+        try await db.execute("CREATE TABLE t(x);")
+        try await db.run(
+            "BEGIN; SAVEPOINT s; INSERT INTO t(x) VALUES (1); RELEASE s; COMMIT;")
+        await db.close()
+
+        let committed = await sink.committed
+        #expect(committed.contains {
+            if case .committed(_, _, "INSERT") = $0 { return true }
+            return false
+        }, "released savepoint insert should be committed, got \(committed)")
+    }
 }