SwiftSQLite: sandboxed sqlite3 command for SwiftBash (M0–M6)#1
SwiftSQLite: sandboxed sqlite3 command for SwiftBash (M0–M6)#1ronaldmannak wants to merge 39 commits into
sqlite3 command for SwiftBash (M0–M6)#1Conversation
Restructure SwiftSQLite into the three-layer design from PLAN.md (rev. 2):
CSQLite (vendored, hardened libsqlite3), SwiftSQLiteKit (engine), and
SwiftSQLiteBash (the command). This commit lands M0 only.
- Package.swift: two library products + CSQLite/Kit/Bash targets + test
targets; SwiftBash dependency; compile-time hardening flags (§6):
OMIT_LOAD_EXTENSION, DQS=0, USE_URI=0, DEFAULT_FOREIGN_KEYS=1, etc.
- CSQLite: committed module map + csqlite_shims.{h,c} (wraps the variadic
sqlite3_db_config the Swift importer can't reach) + a VERSION pin.
- scripts/fetch-sqlite.sh: downloads + SHA3-256-verifies the amalgamation
into Sources/CSQLite, mirroring SwiftBash's fetch-bun-webkit.sh. The big
generated sqlite3.c/.h are gitignored, not committed. Fails closed while
the hash is unpinned.
- PLAN.md (rev. 2) and a README describing build + usage.
https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
The SwiftBash-agnostic engine. Takes a host URL + an injected `authorize` closure (the SwiftBash seam, or a stub in tests) and never resolves paths itself. M1 — SQLiteConnection actor: gated open (authorize → sqlite3_open_v2 with NOFOLLOW, no URI), execute/query/run, value + error mapping, :memory:. M2 — authorizer policy (§7): deny ATTACH/DETACH, all user PRAGMA, schema- table + reserved `_audit*` writes, default-deny on unrecognized actions; runtime hardening (defensive mode, TRUSTED_SCHEMA off, LIMIT_ATTACHED=0, SQL-length cap, busy timeout, temp_store=MEMORY); row cap; progress-handler timeout + sqlite3_interrupt-on-cancel. M3 — two-tier audit: the authorizer records every *attempted* operation; commit + update + rollback hooks record every *committed* row. Events are buffered on the SQL thread and flushed to an AuditSink (in-memory or a JSON-Lines file written outside the DB) after each script / on close. Tests cover CRUD, value/error mapping, row-cap truncation, timeout interrupt, open-gate rejection, every authorizer denial, and the audit semantics (committed appears in both streams; a rolled-back DELETE only in attempted; the file trail survives DROP TABLE). https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
… tests The only target that imports SwiftBash. M0.5 — native-file contract (§4): backingReachesRealDisk() refuses non-real/non-identity backings (in-memory, overlay-over-memory) with a clear error rather than silently opening the wrong file; real-disk and mounted-over-real backings pass, with sandbox.authorize as the actual containment boundary. M4 — SqliteCommand: raw-argv parsing (.captureForPassthrough), sqlite3-style options, output modes (list/csv/json/column/line) + header/separator/ nullvalue, SQL from args or stdin, safe dot-commands (.tables/.schema/ .indexes/.databases/.dump/...) via sqlite_schema queries, disabled dot-commands erroring (not silent), temp pinned in-region, audit wired to a sibling file via the sandbox. M5 — Shell.registerSQLiteCommands(at:) using install(_:at:). M6 — end-to-end tests using only SwiftBash's public API (OutputSink buffer + readAllString capture): the §4 guard matrix (in-memory refused, real round-trips, :memory: works, out-of-sandbox path denied), output modes, dot-commands, and SQL escape hatches (ATTACH / user PRAGMA denied). https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
- Add .github/workflows/swift.yml — macOS (Xcode 26 / Swift 6.2) and Linux (swift:6.2 container) jobs that install SwiftPorts' system C deps (pulled in transitively via BashCommandKit), run scripts/fetch-sqlite.sh, then build + test. The fetch step exports SQLITE_ALLOW_UNPINNED=1 as a documented bootstrap until the official SHA3-256 is pinned in Sources/CSQLite/VERSION (after which verification activates automatically). - Audit.swift: build the JSON object with explicit assignment instead of `table ?? NSNull()` (String? and NSNull don't unify under `??`). https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
The amalgamation SHA3-256 is pinned in Sources/CSQLite/VERSION, so fetch-sqlite.sh verifies every download. Remove the bootstrap override from both jobs so CI strictly enforces the pin (fail-closed on mismatch). https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces SwiftSQLite, a pure-Swift package that adds a sandboxed sqlite3 command to SwiftBash using a compile-time-hardened, vendored SQLite engine. It implements a two-tier audit log, runtime hardening, and a safe subset of dot-commands. The review feedback highlights several important areas for improvement: resolving a CRLF trimming issue that breaks dot-command parsing, fixing tokenization to support empty quoted strings, adding bounds checks to prevent out-of-bounds crashes during database dumps, addressing a TOCTOU race condition in the file audit sink to prevent data corruption, and utilizing overflow-safe arithmetic when calculating nanosecond durations to avoid premature query timeouts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if line.trimmingCharacters(in: .whitespaces).hasPrefix(".") { | ||
| await flush() | ||
| switch await DotCommandRunner.run(line.trimmingCharacters(in: .whitespaces), |
There was a problem hiding this comment.
Using .whitespaces to trim the line only removes spaces and tabs, leaving any carriage return (\\r) intact if the input has CRLF (Windows-style) line endings. This will cause tokenization to append \\r to the last argument, breaking dot-command parsing (e.g., .mode column\\r will fail to match .mode column). Trimming with .whitespacesAndNewlines instead of .whitespaces resolves this issue safely.
| if line.trimmingCharacters(in: .whitespaces).hasPrefix(".") { | |
| await flush() | |
| switch await DotCommandRunner.run(line.trimmingCharacters(in: .whitespaces), | |
| if line.trimmingCharacters(in: .whitespacesAndNewlines).hasPrefix(".") { | |
| await flush() | |
| switch await DotCommandRunner.run(line.trimmingCharacters(in: .whitespacesAndNewlines), |
| private static func tokenize(_ line: String) -> [String] { | ||
| var tokens: [String] = [] | ||
| var current = "" | ||
| var quote: Character? | ||
| for character in line { | ||
| if let active = quote { | ||
| if character == active { quote = nil } else { current.append(character) } | ||
| } else if character == "'" || character == "\"" { | ||
| quote = character | ||
| } else if character == " " || character == "\t" { | ||
| if !current.isEmpty { tokens.append(current); current = "" } | ||
| } else { | ||
| current.append(character) | ||
| } | ||
| } | ||
| if !current.isEmpty { tokens.append(current) } | ||
| return tokens | ||
| } |
There was a problem hiding this comment.
The current tokenize implementation discards empty quoted strings (e.g., "" or ''), because it only appends tokens when !current.isEmpty is true. This makes it impossible to pass empty strings as arguments, which is a common use case (e.g., .separator "" to set an empty separator, or .nullvalue ""). Appending the token and resetting current when the closing quote is encountered fixes this bug.
private static func tokenize(_ line: String) -> [String] {
var tokens: [String] = []
var current = ""
var quote: Character?
for character in line {
if let active = quote {
if character == active {
tokens.append(current)
current = ""
quote = nil
} else {
current.append(character)
}
} else if character == "'" || character == "\"" {
quote = character
} else if character == " " || character == "\t" {
if !current.isEmpty {
tokens.append(current)
current = ""
}
} else {
current.append(character)
}
}
if !current.isEmpty {
tokens.append(current)
}
return tokens
}| guard case let .text(type) = row[0], | ||
| case let .text(name) = row[1], | ||
| case let .text(create) = row[2] else { continue } |
There was a problem hiding this comment.
Accessing row[0], row[1], and row[2] directly without checking row.count is unsafe and can cause a runtime crash if the query results are unexpected or if the schema query is modified in the future. Adding a bounds check ensures safety.
| guard case let .text(type) = row[0], | |
| case let .text(name) = row[1], | |
| case let .text(create) = row[2] else { continue } | |
| guard row.count >= 3, | |
| case let .text(type) = row[0], | |
| case let .text(name) = row[1], | |
| case let .text(create) = row[2] else { continue } |
| if FileManager.default.fileExists(atPath: url.path) { | ||
| guard let handle = try? FileHandle(forWritingTo: url) else { return } | ||
| defer { try? handle.close() } | ||
| _ = try? handle.seekToEnd() | ||
| try? handle.write(contentsOf: blob) | ||
| } else { | ||
| try? FileManager.default.createDirectory( | ||
| at: url.deletingLastPathComponent(), | ||
| withIntermediateDirectories: true) | ||
| try? blob.write(to: url) | ||
| } |
There was a problem hiding this comment.
The current implementation of FileAuditSink.record has a Time-of-Check to Time-of-Use (TOCTOU) race condition and is subject to data corruption under concurrent writes. If the file is created between the fileExists check and the write(to:) call, the concurrent write will be overwritten. Additionally, seekToEnd() followed by write() is not atomic. Using OutputStream(url:append:) is a cleaner, safer, and cross-platform way to atomically append or create the file.
| if FileManager.default.fileExists(atPath: url.path) { | |
| guard let handle = try? FileHandle(forWritingTo: url) else { return } | |
| defer { try? handle.close() } | |
| _ = try? handle.seekToEnd() | |
| try? handle.write(contentsOf: blob) | |
| } else { | |
| try? FileManager.default.createDirectory( | |
| at: url.deletingLastPathComponent(), | |
| withIntermediateDirectories: true) | |
| try? blob.write(to: url) | |
| } | |
| try? FileManager.default.createDirectory( | |
| at: url.deletingLastPathComponent(), | |
| withIntermediateDirectories: true) | |
| guard let stream = OutputStream(url: url, append: true) else { return } | |
| stream.open() | |
| defer { stream.close() } | |
| _ = blob.withUnsafeBytes { buffer in | |
| if let baseAddress = buffer.baseAddress { | |
| stream.write(baseAddress.assumingMemoryBound(to: UInt8.self), maxLength: blob.count) | |
| } | |
| } |
| var nanoseconds: UInt64 { | ||
| let parts = components | ||
| let secs = parts.seconds < 0 ? 0 : UInt64(parts.seconds) | ||
| let attos = parts.attoseconds < 0 ? 0 : UInt64(parts.attoseconds) | ||
| return secs &* 1_000_000_000 &+ (attos / 1_000_000_000) | ||
| } |
There was a problem hiding this comment.
Using overflow-allowed operators (&* and &+) can cause the calculated nanoseconds to wrap around silently if a very large duration is passed (e.g., Duration.max). This would result in a very small or zero deadline, causing queries to time out immediately. Using overflow-safe arithmetic with multipliedReportingOverflow and addingReportingOverflow prevents this and ensures robust behavior.
| var nanoseconds: UInt64 { | |
| let parts = components | |
| let secs = parts.seconds < 0 ? 0 : UInt64(parts.seconds) | |
| let attos = parts.attoseconds < 0 ? 0 : UInt64(parts.attoseconds) | |
| return secs &* 1_000_000_000 &+ (attos / 1_000_000_000) | |
| } | |
| var nanoseconds: UInt64 { | |
| let parts = components | |
| guard parts.seconds >= 0 else { return 0 } | |
| let (secsProd, overflow) = UInt64(parts.seconds).multipliedReportingOverflow(by: 1_000_000_000) | |
| if overflow { return UInt64.max } | |
| let attosNanos = UInt64(max(0, parts.attoseconds)) / 1_000_000_000 | |
| let (result, addOverflow) = secsProd.addingReportingOverflow(attosNanos) | |
| return addOverflow ? UInt64.max : result | |
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42b1cb11bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ctx.beginScript(deadlineNanos: timeoutNanos == 0 | ||
| ? nil | ||
| : DispatchTime.now().uptimeNanoseconds &+ timeoutNanos) |
There was a problem hiding this comment.
If a task awaiting run(_:) is cancelled, the cancellation handler sets ctx.cancelled = true, but the next call only refreshes the deadline. On a reused SQLiteConnection, the progress handler will continue seeing cancelled == true, so subsequent statements are immediately interrupted until the connection is closed.
Useful? React with 👍 / 👎.
| sqlite3_set_authorizer(db, csqliteAuthorizerCallback, appData) | ||
| sqlite3_progress_handler(db, 10_000, csqliteProgressCallback, appData) | ||
| _ = sqlite3_commit_hook(db, csqliteCommitCallback, appData) | ||
| _ = sqlite3_update_hook(db, csqliteUpdateCallback, appData) |
There was a problem hiding this comment.
For writes to SQLite WITHOUT ROWID tables, sqlite3_update_hook is not invoked, so this path never adds pending row records and the commit hook has nothing to flush. That means valid inserts/updates/deletes on those tables appear in the attempted stream but are missing from the committed audit stream, breaking the advertised two-tier audit semantics for that schema shape.
Useful? React with 👍 / 👎.
| .unsafeFlags(["-Wno-unused-but-set-variable", | ||
| "-Wno-implicit-fallthrough"]), |
There was a problem hiding this comment.
Because CSQLite is in the dependency chain of the public SwiftSQLiteKit and SwiftSQLiteBash products, adding .unsafeFlags makes those products ineligible for use as dependencies by downstream SwiftPM packages. This still builds in this repository's CI, but consumers that add SwiftSQLite as a package dependency will be rejected during package resolution for using unsafe build flags.
Useful? React with 👍 / 👎.
| } else { | ||
| // Only write/DDL statements contribute a change count; | ||
| // sqlite3_changes() after a SELECT is stale. | ||
| totalChanges += Int(sqlite3_changes(db)) |
There was a problem hiding this comment.
When a script contains a non-DML statement after a write, such as INSERT ...; CREATE INDEX ...; or BEGIN; INSERT ...; COMMIT;, sqlite3_changes() still reports the most recent INSERT/UPDATE/DELETE count for the later DDL/transaction statement. Adding it for every columnless statement makes RunResult.changes and execute(_:) over-count common scripts that mix writes with transaction control or schema changes.
Useful? React with 👍 / 👎.
| continue | ||
| } | ||
| if let mounted = current as? MountedFileSystem { | ||
| current = mounted.backing |
There was a problem hiding this comment.
For a MountedFileSystem whose virtual paths do not equal host paths, peeling straight to the real backing lets the guard pass even though SqliteCommand later gives SQLite the unresolved virtual path. In that configuration a file database can be opened at the wrong host path, or fail despite the mounted file existing, instead of failing closed as the native-file contract requires.
Useful? React with 👍 / 👎.
| return .real(sqlite3_column_double(statement, index)) | ||
| case SQLITE_TEXT: | ||
| if let text = sqlite3_column_text(statement, index) { | ||
| return .text(String(decodingCString: text, as: UTF8.self)) |
There was a problem hiding this comment.
SQLite TEXT values may contain embedded NUL bytes, but String(decodingCString:) stops at the first NUL. Queries such as SELECT 'a' || char(0) || 'b' will return only "a" through the engine and CLI, silently corrupting result data; use sqlite3_column_bytes() with the returned pointer length instead.
Useful? React with 👍 / 👎.
| func rollback() { | ||
| pending.removeAll(keepingCapacity: true) |
There was a problem hiding this comment.
When SQL uses savepoints, ROLLBACK TO only undoes the changes after that savepoint, but the pending audit buffer has no savepoint boundaries and rollback() can only drop everything. A script like BEGIN; INSERT INTO t VALUES (1); SAVEPOINT s; INSERT INTO t VALUES (2); ROLLBACK TO s; COMMIT; should commit-audit only the first row, but this bookkeeping cannot represent that partial rollback correctly.
Useful? React with 👍 / 👎.
marcprux/swift-archive renamed its default branch master -> swift, but SwiftPorts (a transitive dep via SwiftBash -> BashCommandKit) still pins it by `branch: "master"`. A fresh resolve chases the dead ref and fails: error: could not find a branch named 'master' in .../swift-archive The sibling SwiftGog repo (a working SwiftBash consumer) handles this by committing a Package.resolved that pins swift-archive by *revision* (60f478d…, which is the same commit, now on the `swift` branch). SwiftPM checks out the revision directly and never resolves the branch. Do the same. Also mirror SwiftGog's macOS-only CI: a full SwiftBash consumer builds cleanly on macOS (Xcode 26 / Swift 6.2), whereas the Linux build of the SwiftPorts closure (libgit2 + compression + ripgrep) needs heavier setup. SwiftSQLiteKit is Linux-clean on its own; a Linux engine-only job can follow. https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
Engine (SwiftSQLiteKit):
- SQLiteConnection: count affected rows via sqlite3_total_changes deltas so
DDL / COMMIT after a write no longer re-add the prior DML's count.
- SQLiteConnection: decode TEXT by byte length to preserve embedded NULs.
- EngineContext: reset the cancelled flag per run so a reused connection
isn't interrupted forever after one cancellation.
- Duration.nanoseconds: overflow-safe (clamp to .max) instead of wrapping.
- Audit FileSink: append via an O_APPEND OutputStream (no fileExists→write
race, no non-atomic seek+write).
- Document the inherent two-tier-audit limits: update_hook skips WITHOUT
ROWID tables; rollback tracking can't represent ROLLBACK TO SAVEPOINT.
Command (SwiftSQLiteBash):
- §4 guard: when the real backing is reached through a MountedFileSystem
(private mount table), require a sandbox gate so non-identity mounts fail
closed instead of opening the wrong host path.
- Dot-commands: trim with .whitespacesAndNewlines (CRLF-safe), bounds-check
the .dump schema rows, and keep empty quoted tokens ("" / '').
Packaging:
- Drop CSQLite .unsafeFlags so the products stay usable as a dependency by
downstream SwiftPM packages (e.g. SwiftBash consuming SwiftSQLite).
https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
CI surfaced two real errors once resolution was unblocked: - deinit could not access the actor's non-Sendable `db: OpaquePointer?` - capturing self-isolated `db` in the runScript closure tripped SendingRisksDataRace Move the raw sqlite3 handle and all synchronous C interaction into a dedicated `ConnectionHandle` (@unchecked Sendable class). The actor keeps the async orchestration (gating, audit flush, cancellation) and serializes every call into the handle, so the non-Sendable pointer never crosses an actor-isolation boundary. ConnectionHandle.deinit closes the database as the leak safety net (the actor has no deinit). Public API is unchanged. https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db56c4e15f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _ = csqlite_db_config_onoff(db, SQLITE_DBCONFIG_TRUSTED_SCHEMA, 0) | ||
|
|
||
| sqlite3_limit(db, SQLITE_LIMIT_ATTACHED, 0) | ||
| sqlite3_limit(db, SQLITE_LIMIT_SQL_LENGTH, Int32(clamping: policy.maxSQLLength)) |
There was a problem hiding this comment.
For untrusted queries this only limits the SQL text, not the size of values SQLite may produce; a single-row query such as SELECT zeroblob(500000000) or a huge generated TEXT value still passes the row limit and is then copied into Data/String in columnValue, which can exhaust memory in the sandboxed command. Add a SQLITE_LIMIT_LENGTH/per-cell cap before stepping or reject oversized cells before copying them out.
Useful? React with 👍 / 👎.
| let rows = try await connection.query("SELECT * FROM \"\(escapeIdentifier(table))\";") | ||
| let identifier = "\"\(escapeIdentifier(table))\"" | ||
| for row in rows.rows { | ||
| let values = row.map(sqlLiteral).joined(separator: ",") | ||
| out += "INSERT INTO \(identifier) VALUES(\(values));\n" |
There was a problem hiding this comment.
When .dump is used on a table with generated columns, SELECT * includes the generated values but the emitted INSERT INTO table VALUES(...) has no column list, so replaying the dump fails because generated columns are not insertable. For databases that use SQLite generated columns, the advertised dump output is not restorable; build the dump from the table's non-generated columns instead.
Useful? React with 👍 / 👎.
| if line.trimmingCharacters(in: .whitespacesAndNewlines).hasPrefix(".") { | ||
| await flush() | ||
| switch await DotCommandRunner.run(line.trimmingCharacters(in: .whitespacesAndNewlines), |
There was a problem hiding this comment.
This treats any trimmed line beginning with . as a dot command even when a SQL statement is already being accumulated, so valid multi-line SQL such as INSERT INTO t VALUES('a\n.literal\n'); is split at .literal, the prefix is executed as malformed SQL, and the rest of the statement is lost. Track statement completeness or only recognize dot commands when the SQL buffer is empty.
Useful? React with 👍 / 👎.
| module CSQLite { | ||
| // `sqlite3.h` is fetched + pinned by scripts/fetch-sqlite.sh and is | ||
| // NOT committed (see PLAN.md §9 / §M0). `csqlite_shims.h` IS committed. | ||
| header "sqlite3.h" |
There was a problem hiding this comment.
The package now exports SwiftSQLiteKit/SwiftSQLiteBash, but the C target references sqlite3.h while both that header and sqlite3.c are gitignored and only created by a manual script. When this repository is used as a SwiftPM dependency, SwiftPM checks out the sources and builds directly without running scripts/fetch-sqlite.sh, so downstream consumers fail to build CSQLite from a clean checkout.
Useful? React with 👍 / 👎.
| let location = url.isFileURL ? url.path : url.absoluteString | ||
| self.handle = ConnectionHandle(location: location, policy: policy, ctx: ctx) |
There was a problem hiding this comment.
If a Kit caller passes a non-file URL, the authorized URL no longer matches the thing SQLite opens: absoluteString is handed to sqlite3_open_v2, where values such as foo:bar are treated as local filenames and can create a database outside the caller's file-authorization path. Since this initializer is documented for host file URLs, reject !url.isFileURL instead of translating it into a filename.
Useful? React with 👍 / 👎.
| var object: [String: Any] = [:] | ||
| for (index, column) in rs.columns.enumerated() where index < row.count { | ||
| object[column] = jsonValue(row[index]) |
There was a problem hiding this comment.
In JSON mode, result sets with duplicate column names—common in joins or SELECT 1 AS x, 2 AS x—silently lose data because later assignments overwrite earlier entries in the dictionary. This makes the CLI return an incomplete result even though SQLite returned both columns; preserve column order with unique keys or another representation instead of collapsing by name.
Useful? React with 👍 / 👎.
Two runtime bugs from the first green compile (16/29 tests had passed): - Authorizer denied all DDL. CREATE/DROP/ALTER report their internal sqlite_master row change to the authorizer as INSERT/DELETE on "sqlite_master"; the policy denied writes to sqlite_*, so every CREATE failed with SQLITE_AUTH. Stop denying sqlite_schema writes in the authorizer — SQLITE_DBCONFIG_DEFENSIVE (already on) blocks direct user writes to sqlite_master, so `writingSchemaTableIsDenied` still holds. - Every file open failed with SQLITE_CANTOPEN. SQLITE_OPEN_NOFOLLOW rejects paths containing a symlinked component, and macOS temp/app dirs live under /var -> /private/var. Drop NOFOLLOW; the symlink-escape defense is the caller's sandbox.authorize (symlink-resolved containment, §4/§5). Also folds in two same-area Codex review points: - Cap result value size via SQLITE_LIMIT_LENGTH (EnginePolicy.maxValueBytes, default 50 MB) so `SELECT zeroblob(5e8)` can't exhaust memory (P1). - Reject non-file URLs in init(url:) instead of opening absoluteString. https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
The last failing test, ordinarySQLIsAllowed, ran `CREATE INDEX`. SQLite reports the index build to the authorizer as SQLITE_REINDEX, which the default-deny branch rejected -> SQLITE_AUTH. REINDEX and ANALYZE are safe maintenance operations on the single authorized DB file (and remain readOnly-gated), so allow them. https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6e973144e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| private static func dump(_ connection: SQLiteConnection, only: String?) async -> DotResult { | ||
| do { | ||
| var out = "PRAGMA foreign_keys=OFF;\nBEGIN TRANSACTION;\n" |
There was a problem hiding this comment.
When a user pipes this .dump output back into the same sandboxed sqlite3 command, the first statement is PRAGMA foreign_keys=OFF;, but normal user input runs after the authorizer is installed and EngineContext.decide denies every SQLITE_PRAGMA. Because runSQLSession executes the restore script as one buffer, replay fails before creating any schema or rows; either omit this line or explicitly support this generated dump pragma.
Useful? React with 👍 / 👎.
| case let .text(type) = row[0], | ||
| case let .text(name) = row[1], | ||
| case let .text(create) = row[2] else { continue } | ||
| out += create + ";\n" |
There was a problem hiding this comment.
For databases with triggers, emitting every CREATE statement before any table data means a restored dump creates the trigger before replaying the dumped INSERTs. If an AFTER INSERT trigger populates another dumped table, restore fires it again while also inserting that table's saved rows, which duplicates/corrupts data or trips constraints; table data should be inserted before emitting triggers (and usually indexes/views).
Useful? React with 👍 / 👎.
| if isMemory { | ||
| databasePath = ":memory:" | ||
| connection = try await SQLiteConnection(inMemory: policy, audit: InMemoryAuditSink()) |
There was a problem hiding this comment.
When DBFILE is :memory:, this branch always installs InMemoryAuditSink, so sqlite3 -audit /path/log :memory: ... exits successfully but never creates or appends to the user-requested audit log. Since -audit PATH is documented as writing the persistent audit log, route explicit audit paths through makeAuditSink even for in-memory databases.
Useful? React with 👍 / 👎.
| let data = try? JSONSerialization.data( | ||
| withJSONObject: objects, options: [.sortedKeys]), | ||
| let string = String(data: data, encoding: .utf8) | ||
| else { return "[]\n" } |
There was a problem hiding this comment.
For JSON mode, SQLite can return non-finite REAL values such as SELECT 1e999;; JSONSerialization rejects those numbers, and this fallback returns [] for the whole result set. That silently drops otherwise valid rows whenever one column is infinity, so handle non-finite doubles explicitly instead of replacing the entire output with an empty array.
Useful? React with 👍 / 👎.
| let result = try await withTaskCancellationHandler { | ||
| try handle.runScript(sql) | ||
| } onCancel: { | ||
| ctx.cancelled = true // best-effort; sqlite3_interrupt is authoritative |
There was a problem hiding this comment.
When a task running SQL is cancelled, this handler writes ctx.cancelled from the cancellation thread while the SQLite progress callback can read the same ordinary Bool on the SQL thread. @unchecked Sendable only silences the compiler; it does not make this cross-thread mutable access safe, so cancellation of long-running queries can introduce a Swift data race. Use an atomic/lock, or rely solely on sqlite3_interrupt without sharing a non-atomic flag.
Useful? React with 👍 / 👎.
| var written = 0 | ||
| while written < blob.count { | ||
| let n = stream.write(base + written, maxLength: blob.count - written) | ||
| if n <= 0 { break } |
There was a problem hiding this comment.
If the audit file append fails after opening the stream, for example because the disk is full or the filesystem reports an I/O error, this just breaks out of the write loop and the command continues as if the audit records were persisted. That violates the advertised persistent audit trail exactly in the failure mode where records are most important; surface the failure or fail closed instead of silently dropping the remaining JSON lines.
Useful? React with 👍 / 👎.
Six correctness/robustness fixes on top of the green build: - Cancellation: drop the cross-thread `cancelled` Bool and rely solely on sqlite3_interrupt (thread-safe), removing a data race. The deadline check still distinguishes timeout from cancel. - Audit FileSink: surface write failures (full disk / I/O error) to stderr instead of silently dropping records. - JSON output: emit non-finite REALs (Infinity/NaN) as strings so one bad cell no longer makes the whole result set serialize to "[]". - .dump: omit the un-replayable `PRAGMA foreign_keys=OFF;` (all user PRAGMA is denied on replay), and emit indexes/triggers/views AFTER table data so a restore doesn't fire AFTER-INSERT triggers mid-load. - Command: honor an explicit `-audit PATH` for `:memory:` databases too (previously always in-memory, ignoring the flag). https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd5818b55a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| options: OutputOptions | ||
| ) async -> Bool { | ||
| do { | ||
| let result = try await connection.run(sql) |
There was a problem hiding this comment.
For stdin or multi-statement input this sends the whole accumulated script to connection.run, and SQLiteConnection.runScript throws as soon as one statement fails. In a script such as SELECT no_such_column; SELECT 1;, the second statement is never executed even though this session path is documented as sqlite3-like lenient mode without -bail; execute complete statements incrementally so later input still runs while preserving the failure status.
Useful? React with 👍 / 👎.
| if let only { | ||
| schemaSQL += " AND name='\(escapeSQLString(only))'" | ||
| } |
There was a problem hiding this comment.
When .dump t is requested, filtering sqlite_schema by name = 't' keeps the table itself but drops indexes and triggers whose name differs from the table and whose association is stored in tbl_name. Restoring that filtered dump silently loses table-specific schema such as CREATE INDEX idx_t ON t(...) or triggers on t; include rows where tbl_name matches the selected table for those deferred objects.
Useful? React with 👍 / 👎.
- runSQLSession now executes statements one at a time (via a new sqlite3_complete-based SQLiteStatements.split), so a failed statement is reported but no longer aborts the rest of the buffer — matching the documented sqlite3-like lenient mode (no -bail). BEGIN/COMMIT still work since they run on the same connection. - .dump TABLE now also includes the table's indexes/triggers (matched via tbl_name), so a filtered dump keeps table-owned schema. https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b07abb5678
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var sql = "SELECT sql FROM sqlite_schema WHERE sql IS NOT NULL " | ||
| + "AND name NOT LIKE 'sqlite\\_%' ESCAPE '\\'" | ||
| if let name = args.first { | ||
| sql += " AND name='\(escapeSQLString(name))'" |
There was a problem hiding this comment.
.schema TABLE
When .schema t is used on a table that has indexes or triggers, this filter only keeps rows whose own name is t, so rows like CREATE INDEX idx_t ON t(...) or triggers whose tbl_name is t are omitted. That makes the implemented sqlite3-compatible schema inspection incomplete for common table schemas; include tbl_name matches here as .dump TABLE already does.
Useful? React with 👍 / 👎.
| case SQLITE_ALTER_TABLE: | ||
| return arg2 |
There was a problem hiding this comment.
For ALTER TABLE t RENAME TO _audit_x, SQLite reports the old table name (t) to the authorizer for the ALTER action, so checking only arg2 here allows the rename and the later sqlite_schema updates are treated as legitimate DDL. That bypasses the reserved-prefix guard that correctly rejects CREATE TABLE _audit_x(...), letting user SQL create objects in the hidden audit namespace.
Useful? React with 👍 / 👎.
| // No `PRAGMA foreign_keys=OFF;` here: replaying the dump through | ||
| // this sandboxed command would hit the all-PRAGMA-denied | ||
| // authorizer. The whole restore runs in one transaction instead. | ||
| var out = "BEGIN TRANSACTION;\n" |
There was a problem hiding this comment.
With foreign keys forced on at connection setup and user PRAGMAs denied, this dump is not restorable for valid schemas where data is emitted before its referenced rows, such as tables child then parent ordered by name: replay inserts child rows inside this transaction before parent rows, and immediate FK checks fail unless the constraints were declared deferred. Either emit rows in dependency order or provide a sandbox-approved way for generated dumps to disable/defer FK checks during restore.
Useful? React with 👍 / 👎.
| if rows.count >= policy.rowLimit { | ||
| truncated = true | ||
| break loop |
There was a problem hiding this comment.
When a column-returning statement hits rowLimit, this breaks out before stepping it to SQLITE_DONE, while the statement is only finalized by the deferred finalize at the end of the whole script. In a Kit call like SELECT * FROM t; DROP TABLE t; where the select is truncated, the active cursor keeps the table locked and the later write/DDL in the same run(_:) can fail with SQLITE_LOCKED; drain or finalize the truncated statement before preparing the next one.
Useful? React with 👍 / 👎.
| switch value { | ||
| case .null: return "NULL" | ||
| case .integer(let i): return String(i) | ||
| case .real(let d): return String(d) |
There was a problem hiding this comment.
For databases containing non-finite REAL values such as 1e999, String(Double.infinity) emits inf/-inf, but SQLite parses those as identifiers rather than numeric literals when the dump is replayed. That makes .dump output fail to restore valid rows with infinite REALs; render these values using a SQLite expression that recreates them or otherwise handle them explicitly.
Useful? React with 👍 / 👎.
- .schema TABLE now includes the table's indexes/triggers (tbl_name match), matching the .dump fix. - .dump renders non-finite REALs as restorable literals (9e999 / -9e999; NaN -> NULL) instead of "inf", which SQLite would parse as an identifier. - Document two known limitations: .dump isn't FK-dependency-ordered (and foreign_keys can't be toggled in the sandbox), and ALTER TABLE RENAME into the reserved _audit namespace isn't caught (SQLite doesn't expose the new name to the authorizer; low severity since the audit trail is external). https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Committing the (public-domain, ~9 MB text, infrequently-changing) amalgamation makes SwiftSQLite build from a fresh clone with no prerequisite step and usable as a plain SwiftPM dependency — fixing the consumability gap. fetch-sqlite.sh stays as the SHA3-256-verified *update* tool rather than a build prerequisite. - .gitignore: stop ignoring sqlite3.c / sqlite3.h (keep unused shell.c/ext.h). - Add a "Vendor SQLite amalgamation" workflow that fetches + verifies + commits the amalgamation (manual, or on a VERSION bump) — used to populate it now. - README: update build/update instructions. https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The hash is pinned (no longer a placeholder); clarify that fetch-sqlite.sh verifies against it and the Vendor workflow re-commits sqlite3.c on a bump. https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
sqlite3.c/.h are vendored now, so CI builds from the committed source with no network — mirroring the consumer experience. fetch-sqlite.sh stays as the update tool / the Vendor workflow. https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8d6eaa609
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let sql = "SELECT sql, type, name FROM sqlite_schema \(filter) " | ||
| + "UNION ALL " | ||
| + "SELECT sql, type, name FROM sqlite_temp_schema \(filter) " | ||
| + "ORDER BY (type='table') DESC, name;" |
There was a problem hiding this comment.
.schema UNION ordering parseable
When .schema runs this compound UNION ALL query, SQLite requires each ORDER BY term in a compound SELECT to refer to an output column, but (type='table') is an expression that is not selected. This makes .schema fail with 1st ORDER BY term does not match any column in the result set even for ordinary databases; include the sort key in both SELECT arms or wrap the UNION in a subquery before ordering.
Useful? React with 👍 / 👎.
| var sql = "SELECT name FROM sqlite_schema WHERE type='index' " | ||
| + "AND name NOT LIKE 'sqlite\\_%' ESCAPE '\\'" |
There was a problem hiding this comment.
.indexes
When an allowed session creates indexes on TEMP tables, this dot-command still queries only main.sqlite_schema, so CREATE TEMP TABLE tmp(x); CREATE INDEX tmp_idx ON tmp(x); .indexes tmp reports no indexes even though the index exists in sqlite_temp_schema. Union the temp schema here as .tables does so sqlite3-style introspection remains accurate for permitted TEMP objects.
Useful? React with 👍 / 👎.
| for record in pending { | ||
| events.append(.committed(table: record.table, rowid: record.rowid, op: record.op)) | ||
| } |
There was a problem hiding this comment.
If SQL rolls back to a savepoint inside a still-open transaction, rows written after that savepoint stay in pending because the rollback hook is not invoked for ROLLBACK TO, and this loop promotes every pending row on the later outer COMMIT. For inputs like BEGIN; SAVEPOINT s; INSERT INTO t VALUES (1); ROLLBACK TO s; COMMIT;, the committed audit stream records an insert that never landed, so savepoint-aware pending boundaries or statement-level cleanup are needed before promotion.
Useful? React with 👍 / 👎.
| ctx.beginScript(deadlineNanos: timeoutNanos == 0 | ||
| ? nil | ||
| : DispatchTime.now().uptimeNanoseconds &+ timeoutNanos) |
There was a problem hiding this comment.
When a caller configures a very large statementTimeout, Duration.nanoseconds can legitimately return UInt64.max as an effectively-infinite value, but adding it with &+ wraps the deadline into the past. In that configuration even trivial SQL is interrupted immediately by the progress handler; use a saturating addition or treat the clamped max value as no deadline instead of wrapping.
Useful? React with 👍 / 👎.
…edges
Round-4's `.schema` change made the query a compound UNION ALL but kept
`ORDER BY (type='table') DESC, name` — and a compound SELECT only allows
ORDER BY on output columns, not expressions, so `.schema` failed at runtime
("1st ORDER BY term does not match any column") for every database. That was
the sole CI failure (dotSchemaIncludesTempObjects). Fixed by wrapping the
UNION in a subquery and ordering the outer, non-compound SELECT.
Per the agreed "one more round, security-relevant only" scope, the rest of
Codex's round-5 findings are documented as known follow-ups (PLAN §13):
audit over-reporting across SAVEPOINT rollbacks (needs savepoint-aware
pending boundaries), `.indexes` not unioning temp schema, and a
near-UInt64.max statement-timeout deadline wrapping via `&+`.
Continuing to address Codex findings: - `.dump` now excludes generated columns: it reads pragma_table_xinfo (a SQLITE_READ-authorized TVF, unlike a denied PRAGMA statement) to find generated columns (hidden 2/3) and emits an explicit insertable-column list, so replaying a dump of a table with a generated column no longer fails. Falls back to SELECT * if the introspection is unavailable. - `.indexes` unions sqlite_temp_schema, so indexes on TEMP tables are listed (matching the .tables/.schema temp fixes). - Statement-timeout deadline now uses a saturating add instead of `&+`, so a near-UInt64.max timeout no longer wraps into the past and interrupts every statement immediately. Tests: generated-column dump emits a column list; `.indexes` lists a TEMP index. PLAN §13 updated (these drop off the residual list).
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.
…r-denied) The generated-column dump fix relied on `pragma_table_xinfo`, but accessing it as a TVF trips the SQLITE_PRAGMA authorizer (which we deny), so the `try?` fell back to SELECT * and emitted the same un-replayable positional INSERT — the only failure in the last CI run. Reverted to the simple SELECT * dump and re-documented generated columns as a follow-up (PLAN §13.6): a real fix needs either allowing that one read-only introspection pragma or parsing the schema, neither rushed here. The other round-5 fixes from this batch are CI-validated and kept: savepoint audit boundaries, `.indexes` temp union, and the saturating statement-timeout deadline.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e91c34da8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // it would reject all DDL. Direct user writes to the schema | ||
| // table are instead blocked by SQLITE_DBCONFIG_DEFENSIVE | ||
| // (enabled in configure()). | ||
| return SQLITE_OK |
There was a problem hiding this comment.
When an AUTOINCREMENT table exists, user SQL can still run statements such as UPDATE sqlite_sequence SET seq=0 or DELETE FROM sqlite_sequence, and SQLITE_DBCONFIG_DEFENSIVE does not block those changes. Because this branch returns OK for all writable actions after only relying on defensive mode for schema-table protection, untrusted SQL can tamper with SQLite internal state and change future rowid allocation despite the intended sqlite_* guard; protect internal tables such as sqlite_sequence separately while still allowing SQLite's own DDL/bookkeeping paths.
Useful? React with 👍 / 👎.
| for index in 0..<columnCount { | ||
| row.append(columnValue(statement, Int32(index))) | ||
| } | ||
| rows.append(row) |
There was a problem hiding this comment.
With untrusted SQL that returns many large values, this accumulates the whole capped result set in memory before the CLI can render anything; the default rowLimit (10,000) times maxValueBytes (50 MB) still allows hundreds of GB of copied Data/String values (for example a 10k-row table selecting near-limit blobs). The single-cell SQLite limit therefore does not provide the intended DoS bound, so add a total result-byte cap or stream rows instead of storing every row.
Useful? React with 👍 / 👎.
| } | ||
| do { | ||
| try await shell.sandbox?.authorize(auditURL) | ||
| return FileAuditSink(url: auditURL) |
There was a problem hiding this comment.
When the caller passes an explicit -audit path equal to the database file (or a SQLite sidecar such as data.db-wal), this authorizes it and creates a FileAuditSink that appends JSON audit records into that SQLite file. That corrupts the database/WAL after otherwise successful SQL even though the audit contract says the trail is written outside the database; reject audit paths that resolve to the DB or its sidecars before constructing the sink.
Useful? React with 👍 / 👎.
From the parallel self-review (security/perf/contracts/intent lenses) and Codex's concurrent findings: - Reject an explicit -audit path that resolves to the database file or a SQLite sidecar (-wal/-shm/-journal): a FileAuditSink appending JSON Lines into the live DB would corrupt it (the trail must live outside the DB). - Bound *total* buffered result memory (EnginePolicy.maxResultBytes, 256 MB): rowLimit × maxValueBytes alone permits hundreds of GB, so the single-cell cap was not a real DoS bound. The step loop now accumulates copied-out bytes and sets truncated when the total is exceeded. - Bound the in-memory audit buffers (EnginePolicy.maxAuditRecords, 1M): the pending/events arrays grew one record per written row (amplified by the row-by-row DELETE audit), so untrusted high-volume writes could exhaust memory. Past the cap, records are dropped after one _AUDIT_TRUNCATED marker. The authorizer DECISION is computed before the cap, so bounding the buffer never weakens enforcement. Tests: audit path overlapping the DB is rejected; a small maxResultBytes truncates below rowLimit; audit buffers cap with a marker and a bounded committed stream.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The -json renderer built each row as a [String: Any] dictionary and
serialized with .sortedKeys, which silently dropped duplicate column names
(common with SELECT * over a join, e.g. SELECT a.id, b.id) and reordered
columns alphabetically — silent data loss vs. real sqlite3 -json.
Assemble objects by hand instead, preserving column order and duplicate
keys, with explicit JSON value/number/string encoding (finite reals keep
Foundation's formatting for parity; non-finite reals and blobs stay as
strings/base64 as before).
Test: SELECT 2 AS b, 1 AS a, 3 AS a emits {"b":2,"a":1,"a":3} (ordered, both
`a`s kept).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The contracts/coverage review found several security guarantees with no (or
too-loose) tests. Test-only changes:
- Split the Kit authorizer helper: expectDenied now requires a SQLiteError
(not any error), and a new expectAuthDenied asserts the failure is an
authorizer DENY ("not authorized"). ATTACH/DETACH/PRAGMA/reserved-create/
read-only-write now use the strict form, so removing an authorizer case
can't pass on an unrelated failure. Schema-write (defensive mode) and
load_extension (compile-omitted) stay on the loose form by design.
- New tests: reading a reserved _audit* table is denied (not just creating);
a reserved trigger name is denied; a non-file URL is rejected; and the
headline symlink-escape defense — a DB path symlinked outside the sandbox
resolves to its real target BEFORE the authorize gate, so containment sees
the outside path.
- Bash: .dump escapes double-quotes in table identifiers (a name like a"b
round-trips instead of producing a syntax error / injection).
Record the self-review findings that are real but not fixed here, with the analysis behind each (PLAN §13): - Internal sqlite_* tampering (sqlite_sequence writes, ALTER RENAME into the reserved namespace): infeasible at the authorizer because SQLite's own AUTOINCREMENT DDL fires the same SQLITE_UPDATE/DELETE on the same table as the user statements; bounded impact (rowid perturbation, not escape; the authoritative trail is an external file). - Blanket SQLITE_FUNCTION allow: safe for the pinned build (no file/page/ extension functions compiled in), a latent-widening hardening note. - .dump/large-output buffering and per-byte hex / O(n^2) statement split: performance follow-ups, not correctness. Notes that total result memory and the audit buffers are now bounded.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
readingReservedAuditTableIsDenied failed not because the read wasn't denied, but because a SQLITE_DENY on a column READ surfaces as "access to TABLE.COLUMN is prohibited" rather than "not authorized". The reserved- namespace read-deny works correctly; the strict helper just didn't know the two message forms. Accept either "authoriz" or "prohibit" — still strict enough to distinguish an authorizer DENY from defensive-mode / syntax / missing-function failures.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c7df01d48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| schemaSQL += " ORDER BY (type='table') DESC, name;" | ||
|
|
||
| let schema = try await connection.query(schemaSQL) |
There was a problem hiding this comment.
When a database has more schema rows than EnginePolicy.rowLimit (for example many generated tables/indexes/triggers), this metadata query can return truncated == true, but .dump proceeds to emit COMMIT using only the prefix of schema objects and their rows. The data-query truncation path below now fails closed, but the same check is missing here, so a dump can still be valid-looking while silently omitting whole tables or indexes.
Useful? React with 👍 / 👎.
| } | ||
| do { | ||
| try await shell.sandbox?.authorize(auditURL) | ||
| return FileAuditSink(url: auditURL) |
There was a problem hiding this comment.
When -audit names an authorized but unusable path, such as an existing directory inside the sandbox or a leaf symlink that O_NOFOLLOW later rejects, this still returns a FileAuditSink and lets SQL execute; the eventual record failure only prints to stderr after statements may have committed, and the command status can remain successful. For explicit audit paths, verify the file can actually be opened/appended before constructing the connection, otherwise destructive SQL can run without the persistent audit trail the caller requested.
Useful? React with 👍 / 👎.
| // literal whose data has a line starting with '.' — the dot is data, | ||
| // not a command (matching the real sqlite3 REPL, which checks for | ||
| // dot-commands only when no SQL is pending). | ||
| if trimmed.hasPrefix(".") && SQLiteSQL.isAtStatementBoundary(sqlBuffer) { |
There was a problem hiding this comment.
When a script has only SQL comments before a dot-command, for example -- setup\n.tables, sqlite3_complete returns false for the comment-only buffer, so this branch treats .tables as more SQL instead of a command and the session errors at EOF. Comments do not leave a pending statement in the sqlite3 shell, so the boundary check needs to consider complete/closed comments as flushable no-op SQL before recognizing the dot-command.
Useful? React with 👍 / 👎.
| case SQLITE_DELETE: opName = "DELETE" | ||
| default: opName = "OP(\(op))" | ||
| } | ||
| pending.append((table: table ?? "", rowid: rowid, op: opName)) |
There was a problem hiding this comment.
Inside an explicit transaction, a constraint-aborted statement can fire the update hook for earlier rows in that same statement and then roll those rows back without invoking the rollback hook; for example BEGIN; INSERT INTO t VALUES (1); INSERT INTO t VALUES (2),(1); COMMIT; leaves only the first row but keeps the update-hook record for 2 in pending. If the caller continues and commits, commit() promotes that rolled-back row as committed, so pending updates need statement-level cleanup on sqlite3_step errors that roll back the statement.
Useful? React with 👍 / 👎.
| /// Promote everything in `pending` into the committed stream. | ||
| func commit() { | ||
| for record in pending { | ||
| events.append(.committed(table: record.table, rowid: record.rowid, op: record.op)) |
There was a problem hiding this comment.
This append bypasses maxAuditRecords: recordUpdate caps only the per-transaction pending buffer, but every successful autocommit statement drains pending into events without checking the cap. A single script with many small INSERT statements can therefore grow the committed audit buffer unbounded before flushAudit() runs, defeating the memory DoS guard that the policy comment says applies to attempted and committed records.
Useful? React with 👍 / 👎.
Addressing the actionable findings from Codex's re-review (most of its backlog was already fixed by earlier work; these are genuinely live): - commit() promoted every pending row into `events` without honoring maxAuditRecords, so a script of many small autocommit writes could grow the committed buffer unbounded before the post-script flush — the cap is now enforced there too. This exposed a latch bug: auditTruncated was reset per transaction, but under autocommit that is per statement, so each statement past the cap would append a fresh _AUDIT_TRUNCATED marker. Reset the latch per drain cycle (drainEvents) instead, bounding markers to one per flushed batch. - .dump now fails closed if the schema metadata query itself truncates (more than rowLimit schema objects), matching the per-table data check — otherwise a dump could emit a valid-looking COMMIT while silently omitting whole tables/indexes/triggers. - PLAN §13.4: document that WITHOUT ROWID writes (like ON CONFLICT REPLACE deletes) don't fire the update hook, and the statement-abort-in-open- transaction audit edge — both in the preupdate-hook family. Already-addressed Codex findings verified against current code: Duration overflow (saturating), cancellation (no shared flag), empty quoted tokens (hasToken), .dump TABLE tbl_name match, JSON duplicates, savepoint rollback. Test: committed buffer stays bounded across 100 separate autocommit inserts.
- A dot-command after a comment-only buffer (e.g. `-- note\n.tables`) was swallowed as SQL: sqlite3_complete returns false for a comment-only buffer, so isAtStatementBoundary said "not a boundary". It now treats a buffer of only whitespace + complete SQL comments as a boundary (matching the sqlite3 shell's _all_whitespace), so the dot-command is recognized. - FileAuditSink.preflight() opens (O_NOFOLLOW, like record) the log before any SQL runs; makeAuditSink calls it so an explicit -audit path that is authorized but unusable (a directory, unwritable location, or leaf symlink) fails closed instead of running SQL unaudited and only erroring on the first post-commit flush. Tests: comment-only buffer runs the dot-command; an explicit audit path that is a directory fails closed; a mounted filesystem without a sandbox gate refuses a file DB (the §4 fail-closed branch). Verified already-handled: per-cell SQLITE_LIMIT_LENGTH cap, total_changes-delta counting, per-iteration finalize of truncated statements, FileSystemGuard mount+sandbox gating.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64a8385c58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| while i + 1 < s.count && !(s[i] == "*" && s[i + 1] == "/") { i += 1 } | ||
| i += 2 // skip the closing */ (or run past the end if unterminated) |
There was a problem hiding this comment.
When the pending buffer contains an unterminated /* ... comment before a line that starts with ., this loop runs to the end and then advances as if it had found */, so isAtStatementBoundary returns true. In scripts such as /* note\n.quit\n*/, the .quit line is executed even though SQLite treats it as part of the comment; only classify a block comment as whitespace after confirming the closing delimiter exists.
Useful? React with 👍 / 👎.
| case SQLITE_CREATE_INDEX, SQLITE_CREATE_TEMP_INDEX, | ||
| SQLITE_DROP_INDEX, SQLITE_DROP_TEMP_INDEX, | ||
| SQLITE_CREATE_TRIGGER, SQLITE_CREATE_TEMP_TRIGGER, | ||
| SQLITE_DROP_TRIGGER, SQLITE_DROP_TEMP_TRIGGER: | ||
| return arg2 ?? arg1 |
There was a problem hiding this comment.
For DROP INDEX and DROP TRIGGER, SQLite passes the index/trigger name in arg1 and the owning table in arg2, but this returns the table name and the extra reserved-name check only handles creates. If an existing database has _audit* indexes or triggers on an ordinary table, user SQL can still drop them despite the reserved namespace policy denying writes to those objects.
Useful? React with 👍 / 👎.
| 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 }) |
There was a problem hiding this comment.
SQLite resolves savepoint identifiers case-insensitively, but this exact string lookup misses rollbacks whose casing differs from the original SAVEPOINT. With SAVEPOINT S; INSERT ...; ROLLBACK TO s; COMMIT, the row is rolled back in the database while its pending update-hook record survives and is later emitted as committed, so normalize names the same way SQLite does before matching.
Useful? React with 👍 / 👎.
| if let idx = savepoints.lastIndex(where: { $0.name == name }) { | ||
| savepoints.removeSubrange(idx...) |
There was a problem hiding this comment.
The authorizer runs while preparing RELEASE, before SQLite has actually released or committed the savepoint; if an outermost RELEASE s fails at step time, for example because of a deferred foreign-key violation, SQLite leaves s active but this code has already discarded its marker. A later ROLLBACK TO s; RELEASE s then rolls back the row in SQLite while the stale pending audit row is promoted as committed, so remove markers only after the release succeeds or restore them on step failure.
Useful? React with 👍 / 👎.
| // reserved-prefix guard. Low severity — the audit trail is an | ||
| // external file, not an in-DB `_audit*` table, so the reserved | ||
| // namespace is defense-in-depth only. | ||
| return arg2 |
There was a problem hiding this comment.
For ALTER TABLE t RENAME TO _audit_x, SQLite's authorizer only supplies the old table name here, so the reserved-prefix check sees t and allows the rename. That lets untrusted SQL populate the _audit* namespace despite the advertised reservation, and the object then becomes protected from normal user cleanup; deny or validate table renames that land under the reserved prefix.
Useful? React with 👍 / 👎.
- SQLiteSQL: an unterminated `/* ...` block comment was classified as a complete comment, so isAtStatementBoundary returned true and a following dot-command ran when SQLite would still treat it as comment. Now an unterminated block comment is not a boundary. (Regression from the comment-aware boundary added in the previous batch.) - EngineContext: savepoint names are now matched case-insensitively (SQLite resolves them that way), so `SAVEPOINT S; ... ROLLBACK TO s; COMMIT` no longer over-reports the rolled-back row as committed. - EngineContext: deny DROP of a reserved `_audit*` index/trigger (the name is in arg1 for DROP INDEX/TRIGGER, same as CREATE) so an out-of-band-seeded reserved object can't be dropped despite the namespace policy. PLAN §13.4 documents the remaining prepare-time savepoint edge (a RELEASE that fails at step). ALTER-RENAME-into-reserved stays documented (infeasible at the authorizer — old name only). Tests: boundary classification incl. unterminated block comment; case-mismatched savepoint rollback; DROP of a reserved index denied.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1356cbae11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if current is RealFileSystem { | ||
| return passedThroughMount ? hasSandbox : true |
There was a problem hiding this comment.
When the shell filesystem reaches RealFileSystem through a MountedFileSystem, this returns true solely because a sandbox object exists, but SqliteCommand later opens shell.resolvePath(dbfile) directly as a host URL and the authorize closure only re-checks that same URL. In a non-identity mount, such as virtual /workspace backed by /tmp/ws, the guard allows the command while SQLite opens /workspace/t.db on the host instead of the mounted backing path, so the native engine can read/create the wrong host file rather than failing closed as the comment describes. Require a proven identity mount or a real host-path translation before allowing mounted filesystems.
Useful? React with 👍 / 👎.
| if let explicitPath { | ||
| auditURL = URL(fileURLWithPath: shell.resolvePath(explicitPath)) | ||
| isExplicit = true |
There was a problem hiding this comment.
When DBFILE is :memory:, the native-file guard is skipped, but an explicit -audit path still becomes a host FileAuditSink; with an InMemoryFileSystem and no sandbox, shell.sandbox?.authorize is a no-op and sqlite3 -audit /tmp/audit.log :memory: 'SELECT 1' writes to the host filesystem even though the same shell correctly refuses file databases. Gate explicit audit paths through the same native backing/sandbox checks, or fall back/fail for persistent audit on non-native shells.
Useful? React with 👍 / 👎.
| } | ||
| cursor = tail | ||
| guard let statement else { continue } // whitespace / comment | ||
| defer { sqlite3_finalize(statement) } |
There was a problem hiding this comment.
When a column-returning statement is stopped early by rowLimit or maxResultBytes, this deferred sqlite3_finalize is the call that can surface the final SQLite error, but its return code is discarded. For example, a large INSERT ... RETURNING with deferred foreign-key failure after the returned rows can hit the output cap before stepping to SQLITE_DONE, causing run to report a successful truncated result even though SQLite failed/rolled back the statement; check the finalize/reset result when the step loop exits before SQLITE_DONE or drain DML statements to completion.
Useful? React with 👍 / 👎.
P2 — truncated writing statements (SQLiteConnection): a truncated INSERT … RETURNING previously stopped stepping at the output cap, abandoning the rest of the writes and masking a later error. The step loop now drains a *writing* statement (sqlite3_stmt_readonly == false) to completion so its side effects finish and errors surface; a read-only SELECT still stops at the cap (DoS guard intact). P2 — explicit -audit on a non-native shell (SqliteCommand): the §4 native guard is skipped for :memory:, so an explicit -audit could still write a host file on an InMemory/non-native shell. makeAuditSink now requires native backing for an explicit audit path and fails closed otherwise. P1 — non-identity mount: documented as a known limitation, not code-fixed, because it is not fixable in-repo. SqliteCommand can only see the script's *virtual* path; BashInterpreter deliberately hides host paths (MountedFileSystem.canonicalize returns the virtual path; the mount table and resolve() are not public), so a non-identity mount (virtual != host) can be neither detected nor translated here. The shipping --sandbox uses identity mounts (virtual == host), where the engine is correct. Corrected the over-claiming FileSystemGuard comment and added PLAN §13.10; a true fix needs a trusted host-path-translation API in BashInterpreter + a dependency repoint (PR-coupling), out of scope. Tests: truncated INSERT…RETURNING still inserts all rows; explicit -audit on an in-memory shell fails closed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35541174c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public init() {} | ||
|
|
||
| public func record(_ events: [AuditEvent]) async { | ||
| self.events.append(contentsOf: events) |
There was a problem hiding this comment.
When the CLI uses an in-memory audit sink (for example sqlite3 :memory: without -audit, -no-audit, or a denied default audit path), each connection.run flushes the capped EngineContext events into this actor and then resets the cap, but this append keeps every flushed batch forever. A script with many small mutating statements can therefore grow memory without bound even though EnginePolicy.maxAuditRecords is intended to cap in-memory audit records; use a no-op sink for discarded audit trails or enforce a sink-level cap.
Useful? React with 👍 / 👎.
| // query like `SELECT zeroblob(500000000)` can't exhaust memory when | ||
| // its value is copied out in columnValue (DoS guard). | ||
| sqlite3_limit(db, SQLITE_LIMIT_LENGTH, Int32(clamping: policy.maxValueBytes)) | ||
| sqlite3_busy_timeout(db, policy.busyTimeout.millisecondsInt32) |
There was a problem hiding this comment.
When a caller configures statementTimeout below busyTimeout (for example a 100 ms statement timeout with the default 5 s busy timeout), a write blocked by another connection can sit inside SQLite's busy handler until this timeout expires, and the progress handler deadline is not checked during that lock wait. This violates the per-statement wall-clock budget for contended databases; clamp the busy timeout to the statement deadline or use a busy handler that observes the same deadline.
Useful? React with 👍 / 👎.
| } | ||
| for row in rows.rows { | ||
| let values = row.map(sqlLiteral).joined(separator: ",") | ||
| out += "INSERT INTO \(identifier) VALUES(\(values));\n" |
There was a problem hiding this comment.
.dump instead of buffering it all
When .dump covers many tables or large rows that are each below the per-query caps, every emitted row is appended to one String and nothing is written until the end. The row/result limits reset for each table, so a database with many below-cap tables can still force the CLI to allocate a huge dump buffer; stream chunks to stdout or enforce a cumulative dump-size cap.
Useful? React with 👍 / 👎.
| if policy.readOnly { | ||
| flags = SQLITE_OPEN_READONLY | ||
| } else if allowCreate { | ||
| flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE |
There was a problem hiding this comment.
When the initial existence check says the DB is absent, a caller only authorizes .create, but this READWRITE|CREATE open will also open an existing file if one appears at that path before sqlite3_open_v2 runs. In an attacker-writable directory, a policy that allows creating new DBs but denies writing existing files can therefore be bypassed by racing a file into place; use an exclusive create path or re-authorize as .write if the non-exclusive open finds an existing DB.
Useful? React with 👍 / 👎.
Niceties: - Package.swift: correct the stale comment — the sqlite3.c/sqlite3.h amalgamation IS committed (vendored in-repo), regenerated SHA3-256-pinned by scripts/fetch-sqlite.sh on a version bump. Fixes (genuine, from Codex's latest pass): - Bound discarded audit trails: the CLI's discard cases (-no-audit, :memory: with no -audit, denied default path) used InMemoryAuditSink, which appends every flushed batch forever — defeating EnginePolicy.maxAuditRecords for the common `sqlite3 :memory:` case. Added NoOpAuditSink and use it for all discard paths so a long script can't grow memory through a thrown-away trail. - Clamp busy_timeout to the statement deadline: the progress handler isn't called during a busy-handler lock wait, so a statementTimeout below busyTimeout could be blown on a contended DB. busy_timeout is now min(busyTimeout, statementTimeout) when a statement budget is set. Documented (PLAN §13, narrow / not in-repo-fixable): - §13.11 create-vs-existing open race (needs O_EXCL-style open or re-authorize-as-.write; narrow create-but-not-write policy + writable dir). - .dump whole-database buffering was already documented (§13.9).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03d7025cca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return SQLITE_IGNORE | ||
|
|
||
| case SQLITE_INSERT, SQLITE_UPDATE, | ||
| SQLITE_CREATE_TABLE, SQLITE_CREATE_TEMP_TABLE, |
There was a problem hiding this comment.
When SQL uses CREATE TABLE copy AS SELECT ..., this branch authorizes it as ordinary DDL, but SQLite does not invoke the update hook for the rows populated by CTAS, so recordUpdate never adds committed INSERT events for any copied rows. In an audited untrusted session, a user can therefore materialize arbitrary data into a new table while the committed audit stream records no per-row inserts; either reject CTAS or add a separate audit path for the populated rows.
Useful? React with 👍 / 👎.
| } | ||
| cursor = tail | ||
| guard let statement else { continue } // whitespace / comment | ||
| defer { sqlite3_finalize(statement) } |
There was a problem hiding this comment.
Because this defer is inside the withCString closure rather than a per-statement scope, finalized statements are held until the whole script finishes. When a read-only result is truncated, step(_:) exits before SQLITE_DONE, so a later statement in the same run can see the still-active SELECT and fail with SQLITE_LOCKED (for example, a low rowLimit SELECT * FROM t; DROP TABLE t;). Finalize each prepared statement before preparing/stepping the next one so capped reads do not change subsequent script behavior.
Useful? React with 👍 / 👎.
| SQLITE_CREATE_INDEX, SQLITE_CREATE_TEMP_INDEX, | ||
| SQLITE_CREATE_VIEW, SQLITE_CREATE_TEMP_VIEW, | ||
| SQLITE_CREATE_TRIGGER, SQLITE_CREATE_TEMP_TRIGGER, | ||
| SQLITE_DROP_TABLE, SQLITE_DROP_TEMP_TABLE, |
There was a problem hiding this comment.
When an audited script drops a table that still contains rows, SQLite removes those rows as part of the DDL without invoking sqlite3_update_hook, so recordUpdate never records committed DELETE events for them. Since the committed stream is documented as per-row INSERT/UPDATE/DELETE evidence, allowing DROP TABLE here lets destructive user SQL erase all rows in a table while the committed audit trail contains no corresponding row removals; reject row-bearing drops or emit a committed table-drop audit record.
Useful? React with 👍 / 👎.
Doc-only:
- Comment the per-statement `defer { sqlite3_finalize }` in runScript: it runs
at each loop-iteration end, so every statement is finalized before the next
is prepared (a truncated read releases its cursor before the next statement
— no cross-statement SQLITE_LOCKED). Clarifies a recurring review
false-positive.
- PLAN §13.4: name CTAS (CREATE TABLE … AS SELECT) and DROP TABLE row removal
as members of the update-hook-miss family (same root cause as ON CONFLICT
REPLACE / WITHOUT ROWID). Both are still recorded at DDL granularity in the
attempted stream; only per-row committed enumeration awaits the preupdate hook.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97454f9df2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let dbPath = databaseURL.standardizedFileURL.path | ||
| let auditPath = auditURL.standardizedFileURL.path | ||
| if auditPath == dbPath || ["-wal", "-shm", "-journal"].contains( | ||
| where: { auditPath == dbPath + $0 }) { |
There was a problem hiding this comment.
Fresh evidence in the current diff is that the overlap guard compares only standardizedFileURL.path, which does not resolve symlinks. When DBFILE is a symlink such as link.db -> real.db and the user passes -audit real.db, this check sees different paths, but SQLiteConnection later canonicalizes and opens real.db, so the FileAuditSink appends JSON into the live database file and corrupts it; resolve symlinks/canonicalize both the DB path and audit path before checking the database and sidecar aliases.
Useful? React with 👍 / 👎.
| let identifier = "\"\(escapeIdentifier(table))\"" | ||
| let rows = try await connection.query("SELECT * FROM \(identifier);") |
There was a problem hiding this comment.
When a dumped table has generated columns, SELECT * includes those generated values, but the emitted INSERT INTO "table" VALUES(...) has no column list and SQLite does not allow/generated columns do not count as insertable targets. For example a table like CREATE TABLE t(a, b AS (a+1) STORED) dumps an insert with both a and b, and replaying the dump fails instead of restoring; select only insertable columns and emit an explicit column list for these tables.
Useful? React with 👍 / 👎.
| if sqlArguments.isEmpty { | ||
| text = await Shell.bashCurrent.stdin.readAllString() |
There was a problem hiding this comment.
When SQL is supplied on stdin, this reads the entire stream into memory before SQLITE_LIMIT_SQL_LENGTH or the per-statement splitting can reject it. A caller can send a huge comment or unterminated statement and exhaust the Swift process without SQLite ever seeing a statement, so stdin processing needs to stream or enforce a byte cap before accumulating the full input.
Useful? React with 👍 / 👎.
2 participants
Implements
PLAN.md(rev. 2) — a sandboxedsqlite3command for SwiftBash, backed by a vendored, compile-time-hardenedlibsqlite3. Milestones M0–M6 (M7, the optional shim VFS, deferred per §13).Layers
CSQLite— vendored SQLite amalgamation with §6 hardening flags (OMIT_LOAD_EXTENSION,DQS=0,USE_URI=0, …) + a committed C shim for the variadicsqlite3_db_config. The amalgamation is not committed:scripts/fetch-sqlite.shdownloads + SHA3-256-verifies it (pinned inSources/CSQLite/VERSION), mirroring SwiftBash'sfetch-bun-webkit.sh.SwiftSQLiteKit(SwiftBash-agnostic) — theSQLiteConnectionactor: gated open (NOFOLLOW, no URI), authorizer policy (denyATTACH/all userPRAGMA/schema + reserved_audit*writes, default-deny), runtime hardening, row cap, timeout + interrupt-on-cancel, and a two-tier audit (attempted via the authorizer; committed via the update/commit/rollback hooks) flushed outside the DB.SwiftSQLiteBash—SqliteCommand(argv/options/stdin, 5 output modes, safe dot-commands viasqlite_schema, disabled ones error), the §4 native-file contract guard, andShell.registerSQLiteCommands(at:).Tests (§12)
Engine CRUD/value/error mapping, row-cap truncation, timeout interrupt, open-gate rejection; every authorizer denial; audit semantics (committed in both streams, a rolled-back
DELETEonly in attempted, the file trail survivingDROP TABLE); the contract-guard matrix and SQL escape hatches end-to-end.download.swift.orgwas network-blocked), so it is grounded in SwiftBash's actual public APIs read from source, but not compiler-verified by me. This PR's CI run is the first real compile/test — I'm watching it and will push fixes for whatever it flags.swift:6.2-noble). Toolchain pins may need adjusting to your runners; Swift ≥ 6.2 is required because SwiftBash's manifest declares tools-version 6.2.See
PLAN.mdfor the full design andREADME.mdfor build/usage.https://claude.ai/code/session_01AdvNtQ7wCMVy8etacVQbjg
Generated by Claude Code