fix(db): isolate native SQLite on v0.21.4 - #535
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 5de9a975ed
ℹ️ 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 conn = Connection::open(path, read_only)?; | ||
| // A host application can link another SQLite implementation. Fail before | ||
| // applying a key or writing a schema if the encrypted engine is unavailable. | ||
| conn.execute_batch("PRAGMA cipher = 'chacha20';")?; |
There was a problem hiding this comment.
For upgrades from the released v0.21.4, this explicit cipher selection is format-sensitive, but the added probes only create and reopen databases using the new code, so they would still pass if this sequence made databases produced by f0e3795 unreadable. Add a frozen encrypted database produced by the pre-backport implementation and verify that this path opens and reads it without changing its bytes.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
1 participant
Prevent native WalletKit v0.21.4 from resolving SQLite calls to a host app's unrelated SQLite engine. The original generic
sqlite3_*symbols allow link order to select the wrong engine; v0.21.4 also lacks a cipher-availability check, so successful initialization does not establish that encryption is active.This PR targets
codex/backport-base-v0.21.4, pinned to release commitf0e3795d31985b839d372fbd19a6f92b7bda6a0d. It is independent of the activity backport in #533. PR #532 separately addresses currentmain.Changes
SQLite source/version, compilation settings, encryption parameters, key encoding, vault/envelope formats, dependency lockfile, workspace manifest, and toolchain pins remain those of v0.21.4. This PR contains no credential activity changes or other 0.22.0 functionality.
Validation
cargo test -p walletkit-db --locked: all 20 tests pass, including frozen envelope bytes/content IDs and encrypted storage tests.git diff --checkpass.cargo clippy -p walletkit-db --all-targets --locked -- -D warnings: passes.The local Apple linker emits a nonfatal deployment-target warning (26.5 archive versus 26.0 host). Validation ran in a separate Git worktree using the repository-pinned Rust 1.94.1 toolchain; Nix is unavailable and the Docker daemon is not running.
Review and release
Keep draft for native storage/encryption review. Verify existing encrypted-store upgrade/downgrade behavior and real initialization/proof flows before distribution. The low-level encrypted-open checks preserve and reject plaintext stores; host-level recovery/error handling and any data migration require separate review.
The linking probes remain manual: GitHub rejected workflow edits with the available push credential. Add both commands in
SQLITE_ISOLATION_BACKPORT.mdto macOS CI with a bounded timeout before release. Cross-platform CI and a full Swift build of this revision remain to be checked.Assign a distinct reviewed version before publishing, use a narrow internal rollout, and retain the prior SDK artifact with a tested data-preserving rollback path.
Note
High Risk
Changes native encrypted-storage linkage and fail-closed cipher validation; mis-linking or regressions could break encryption or corrupt host SQLite behavior on iOS/macOS.
Overview
Backports a v0.21.4 fix so WalletKit’s bundled sqlite3mc no longer shares unprefixed
sqlite3_*symbols with a host app’s SQLite. Native builds compile the amalgamation throughnative_sqlite.cwith internal linkage and route Rust FFI through 21walletkit_sqlite3_*wrappers;ffi.rsbinds those symbols explicitly.Encrypted open now selects and verifies ChaCha20 via
PRAGMA cipherbefore applying a key, failing closed if the wrong engine is linked (v0.21.4 previously could “open” without proving encryption).Adds macOS native linking regressions: a staticlib probe (
native_link_probe), a C host that links Apple SQLite in both orders, symbol checks withnm, and synthetic tests for encrypted round-trip, wrong-key rejection, and unchanged bytes on failed opens. Documents validation inSQLITE_ISOLATION_BACKPORT.md. Amalgamation version, cipher settings, and on-disk formats are unchanged.Reviewed by Cursor Bugbot for commit 5de9a97. Bugbot is set up for automated code reviews on this repo. Configure here.