sqlite: Codex round 3 — audit completeness, temp tables, intent, cancel

Five P2 review findings on commit 6118ec3 (the round-2 P1 TOCTOU fix was
accepted). Bounded fixes:

- Committed-audit completeness: a committed `DELETE FROM t` (no WHERE) was
  missing from the committed stream because the truncate optimization skips
  sqlite3_update_hook (sqlite3.h documents this). The authorizer now returns
  SQLITE_IGNORE for a user-table SQLITE_DELETE, which SQLite defines as
  "proceed, but delete row-by-row" — disabling the truncate optimization so
  the hook fires. DDL/internal `sqlite_*` deletes stay OK (DDL is reported as
  a DELETE on sqlite_master). `allowed` is now `decision != SQLITE_DENY` so
  IGNORE still records an allowed attempt. ON CONFLICT REPLACE deletes remain
  hook-invisible — documented as preupdate-hook territory (open-knob #4).

- `.tables` now unions `sqlite_temp_schema`, so TEMP tables/views (which the
  authorizer allows) are listed, matching the real sqlite3 shell.

- Open intent: a non-read-only open of an *existing* DB now reports `.write`
  (was always `.create`), so a Kit `authorize` closure can permit writes to
  an existing file while denying new-file creation.

- Cancellation: `run` now calls `Task.checkCancellation()` before the first
  step. A task cancelled before any sqlite3_step would otherwise run to
  completion (onCancel's interrupt() is a no-op when nothing is in flight).

Documented, not fixed (bounded): unbounded stdin is buffered before the
SQL-length cap applies (open-knob #5; needs an incremental capped read).

Tests: committed DELETE FROM t appears in the committed stream; `.tables`
lists a TEMP table.

Author: claude

PLAN.md

@@ -317,7 +317,8 @@ public extension Shell {
 1. **Journal mode** — WAL (best concurrency; `-wal`/`-shm` siblings) vs rollback (`-journal`; simplest). Both fine on native I/O.
 2. **M7 shim VFS** — ship now vs defer (recommended: defer; §5 confinement is closed/enumerable without it). Partial symlink-race hardening is already in place: the **DB open** canonicalizes before `authorize` and opens with `SQLITE_OPEN_NOFOLLOW`, which is race-free (SQLite rejects the open if any component became a symlink after authorization). Two residual TOCTOU gaps are explicitly **M7-scope**: (a) the **audit-log append** uses leaf-only `O_NOFOLLOW`, so a *parent-directory* swapped to a symlink after authorization is still followed (needs `openat`-style walking from a trusted root fd); and (b) **WAL `-wal`/`-shm` sidecars** aren't separately authorized, so a Kit caller whose `authorize` grants a single file rather than its directory could see siblings created next to it.
 3. **`:memory:`** — the supported answer for in-memory/non-identity-mount callers that can't use a host file URL.
-4. **Value-level audit** — enable `SQLITE_ENABLE_PREUPDATE_HOOK` if old/new row values are needed (vs. table+rowid only).
+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.
 
 ## 14. References (SwiftBash files this design mirrors)
 

Sources/SwiftSQLiteBash/DotCommands.swift

@@ -35,10 +35,18 @@ enum DotCommandRunner {
 
         switch command {
         case ".tables":
+            // Union in `sqlite_temp_schema` so TEMP tables/views (which the
+            // authorizer allows) show up too, matching the real sqlite3 shell.
             return await listSchema(
                 connection,
-                "SELECT name FROM sqlite_schema WHERE type IN ('table','view') "
-                    + "AND name NOT LIKE 'sqlite\\_%' ESCAPE '\\' ORDER BY name;")
+                "SELECT name FROM sqlite_schema "
+                    + "WHERE type IN ('table','view') "
+                    + "AND name NOT LIKE 'sqlite\\_%' ESCAPE '\\' "
+                    + "UNION "
+                    + "SELECT name FROM sqlite_temp_schema "
+                    + "WHERE type IN ('table','view') "
+                    + "AND name NOT LIKE 'sqlite\\_%' ESCAPE '\\' "
+                    + "ORDER BY name;")
 
         case ".indexes", ".indices":
             var sql = "SELECT name FROM sqlite_schema WHERE type='index' "

Sources/SwiftSQLiteKit/EngineContext.swift

@@ -41,7 +41,7 @@ final class EngineContext: @unchecked Sendable {
         events.append(.attempted(
             action: Self.actionName(action),
             table: guardedObjectName(action: action, arg1: arg1, arg2: arg2),
-            allowed: decision == SQLITE_OK))
+            allowed: decision != SQLITE_DENY))
         return decision
     }
 
@@ -78,7 +78,26 @@ final class EngineContext: @unchecked Sendable {
             // them. (`_audit*` was already denied above.)
             return SQLITE_OK
 
-        case SQLITE_INSERT, SQLITE_UPDATE, SQLITE_DELETE,
+        case SQLITE_DELETE:
+            if readOnly { return SQLITE_DENY }
+            // DDL is reported as a DELETE on `sqlite_master` (see the schema
+            // note below) and other internal bookkeeping touches `sqlite_*`
+            // tables — leave all those as OK, unperturbed. For a real
+            // *user-table* delete, return IGNORE so SQLite disables the
+            // truncate optimization and deletes rows individually. Otherwise
+            // `DELETE FROM t` (no WHERE) skips sqlite3_update_hook and the
+            // committed-audit stream would miss those rows (sqlite3.h: the
+            // update hook is "not invoked when rows are deleted using the
+            // truncate optimization"). For SQLITE_DELETE, IGNORE means
+            // "proceed, but row-by-row" (sqlite3.h authorizer docs) — it does
+            // NOT skip the delete. (ON CONFLICT REPLACE deletes are still
+            // missed; capturing those needs the preupdate hook — open-knob #4.)
+            if (object ?? "").lowercased().hasPrefix("sqlite_") {
+                return SQLITE_OK
+            }
+            return SQLITE_IGNORE
+
+        case SQLITE_INSERT, SQLITE_UPDATE,
              SQLITE_CREATE_TABLE, SQLITE_CREATE_TEMP_TABLE,
              SQLITE_CREATE_INDEX, SQLITE_CREATE_TEMP_INDEX,
              SQLITE_CREATE_VIEW, SQLITE_CREATE_TEMP_VIEW,

Sources/SwiftSQLiteKit/SQLiteConnection.swift

@@ -64,7 +64,16 @@ public actor SQLiteConnection {
         let canonicalPath = ConnectionHandle.canonicalize(url.path)
         self.handle = ConnectionHandle(location: canonicalPath, policy: policy, ctx: ctx)
 
-        let intent: AccessIntent = policy.readOnly ? .read : .create
+        // Distinguish opening an existing DB (.write) from creating a new one
+        // (.create) so a caller's authorize closure can permit writes to an
+        // existing file while still denying new-file creation.
+        let intent: AccessIntent
+        if policy.readOnly {
+            intent = .read
+        } else {
+            intent = FileManager.default.fileExists(atPath: canonicalPath)
+                ? .write : .create
+        }
         try await authorize(URL(fileURLWithPath: canonicalPath), intent)
         try handle.open()
         try handle.configure()
@@ -109,7 +118,11 @@ public actor SQLiteConnection {
         let handle = self.handle
         do {
             let result = try await withTaskCancellationHandler {
-                try handle.runScript(sql)
+                // A task cancelled *before* the first sqlite3_step would
+                // otherwise run to completion: onCancel's interrupt() is a
+                // no-op when no statement is in flight. Refuse to start.
+                try Task.checkCancellation()
+                return try handle.runScript(sql)
             } onCancel: {
                 // sqlite3_interrupt is thread-safe (SQLITE_THREADSAFE=1) and
                 // makes the in-flight step return SQLITE_INTERRUPT. No shared

Tests/SwiftSQLiteBashTests/CommandBehaviorTests.swift

@@ -77,4 +77,18 @@ struct CommandBehaviorTests {
         #expect(result.status.isSuccess)
         #expect(result.stdout.contains("42"))
     }
+
+    @Test func dotTablesIncludesTempTables() async throws {
+        let shell = Shell()
+        shell.installShellBuiltin(SqliteCommand.self)
+        let script = """
+            CREATE TABLE perm(x);
+            CREATE TEMP TABLE tmp(y);
+            .tables
+            """
+        let result = try await runCapturing(shell, "sqlite3 :memory:", stdin: script)
+        #expect(result.status.isSuccess, "stderr: \(result.stderr)")
+        #expect(result.stdout.contains("perm"))
+        #expect(result.stdout.contains("tmp"))
+    }
 }

Tests/SwiftSQLiteKitTests/AuditTests.swift

@@ -77,4 +77,22 @@ struct AuditTests {
         #expect(text.contains("\"action\":\"INSERT\""))
         #expect(text.contains("\"action\":\"DROP_TABLE\""))
     }
+
+    /// `DELETE FROM t` (no WHERE) is a truncate-optimization candidate that
+    /// would skip `sqlite3_update_hook`; the authorizer returns IGNORE for
+    /// DELETE so rows are removed individually and the committed stream still
+    /// records them.
+    @Test func committedDeleteAppearsInCommittedStream() async throws {
+        let sink = InMemoryAuditSink()
+        let db = try await SQLiteConnection(inMemory: .default, audit: sink)
+        try await db.execute("CREATE TABLE t(x); INSERT INTO t(x) VALUES (1),(2),(3);")
+        try await db.execute("DELETE FROM t;")
+        await db.close()
+
+        let committed = await sink.committed
+        #expect(committed.contains {
+            if case .committed(_, _, "DELETE") = $0 { return true }
+            return false
+        }, "committed DELETE rows should be audited, got \(committed)")
+    }
 }