integrity core

[insipx]

Summary

Bottom of the DB-integrity-check stack. Adds read-only SQLite corruption detection to xmtp_db:

• PRAGMA quick_check / integrity_check / cipher_integrity_check behind a structured IntegrityCheckResult (Ok | Corrupt | Unreadable | SaltMissing | Locked | Failed)
• Standalone check_database_integrity(db_path, key?, level): dedicated short-lived read-only connection (file:…?mode=rw, query_only=ON), SQLCipher session pragmas rebuilt from the same builders EncryptedConnection uses (key + plaintext header + validated salt sidecar); never creates or migrates; URI-delimiter-safe paths
• XmtpDb::integrity_check defaulted trait method; NativeDb overrides with the dedicated checker connection; EncryptedMessageStore forwards; wasm gets an async by-path check over OPFS
• In WAL mode the checker is a concurrent reader — proven by a check-while-writing test

Note: quick_check does not reliably detect ciphertext corruption on SQLCipher DBs — only Full's cipher_integrity_check validates per-page HMACs (documented on IntegrityCheckLevel::Quick). Interval sweeps should run Full.

Testing: 13 integrity tests (healthy/corrupt/wrong-key/malformed-salt/missing-salt/missing-db/unencrypted/ephemeral/URI-delimiters/concurrent-writer), full xmtp_db suite 227/227, wasm suite green.

🤖 Generated with Claude Code

https://claude.ai/code/session_01V1AXHNsnQmHW174i5rRUEb

Note

Add integrity_check API to XmtpDb and new integrity module

• Adds a cross-platform integrity module (mod.rs) that runs PRAGMA quick_check/integrity_check and native SQLCipher HMAC checks, returning a structured IntegrityCheckResult.
• Adds integrity_check to the XmtpDb trait with a default that runs on the current connection; NativeDb overrides it to open a dedicated checker connection by path.
• Adds session_pragmas to ConnectionOptions so encrypted backends can supply SQLCipher pragmas for the by-path checker; assemble_session_pragmas centralizes their construction.
• Hardens salt sidecar reading via read_salt_hex, which bounds I/O to 64 bytes and requires exactly 32 hex characters.
• Behavioral Change: EncryptedConnection::new now returns InvalidData for malformed or oversized salt files instead of partially reading them; EncryptedConnection::pragmas now returns String instead of impl Display.

^{Macroscope summarized 3b41bb0.}

[claude]

Claude finished @insipx's task —— View job

---

Code Review — August 27, 2026, 15:47 UTC

I've reviewed this PR focusing on areas not already covered by previous reviewers. The implementation is generally solid and most issues have been addressed through the review process. Here are a few remaining observations:

Architecture & API Design

Positive: The trait-based design with XmtpDb::integrity_check() as a defaulted method is clean. The separation of IntegrityCheckLevel and IntegrityCheckResult provides good flexibility.

Minor concern: The API surfaces check_database_integrity as a public standalone function (in prelude) alongside the trait method. While this serves the by-path use case, it creates two entry points for the same functionality. Consider whether the standalone function should be more clearly documented as "for offline/diagnostic use only" vs the trait method for live databases.

Error Handling

The classify_check_error function (integrity/mod.rs:81) uses string matching on lowercased error messages to classify errors. This is inherently fragile if SQLite/SQLCipher changes error text. However, given that:

1. The error messages being matched are long-standing SQLite constants
2. The classification has reasonable fallthrough to Failed
3. This is already the pattern used elsewhere in the codebase

...this is acceptable, but worth documenting the brittleness.

Test Coverage Gaps

Codecov shows 24 uncovered lines. Looking at the native.rs file, the uncovered lines appear to be in:

1. Error paths in check_database_path (lines around the ro/rw fallback - line 57-64)
2. The missing_db helper (line 24-28)
3. Some branches in escape_uri_path Windows handling

The by_path_missing_db_reports_missing_db_not_salt test covers the missing_db path for encrypted DBs, but there may not be coverage for the mode=ro fallback when mode=rw fails. Consider adding a test that triggers this path (e.g., a read-only filesystem fixture).

Security Observations

Positive findings:

• The bounded read_salt_hex implementation (64 bytes limit) properly prevents memory exhaustion
• PRAGMA query_only = ON is set before any user-data queries
• Session pragmas are assembled from trusted components (the key and validated salt)

Note: The ro/rw semantics have been well-discussed in review comments. The final documented behavior (rw with crash recovery, ro fallback for forensics) is reasonable and well-explained in the docs.

Performance

The dedicated connection approach is correct for WAL concurrency. The test full_check_does_not_block_concurrent_writer validates this well.

Minor: escape_uri_path does multiple string allocations (4 replaces on Windows, 3 on Unix). For a one-time connection open this is negligible, but if this were called in a hot path, consider Cow<str> or a single-pass escape. (Not a blocker for this PR.)

Documentation Quality

The module-level docs and function docs are excellent, especially:

• The note about Quick vs Full on SQLCipher (mod.rs:22-27)
• The crash recovery behavior explanation (native.rs:30-37)

Android Test Failure

The failing test org.xmtp.android.library.ArchiveTest/testNetworkDebugInformation appears unrelated to this PR based on the name. This is a DB integrity feature; network debug information should not be affected. The failure is likely a flaky test or pre-existing issue.

---

Summary

This is a well-designed, thoroughly tested feature. Most concerns raised during review have been addressed. The main remaining items are:

1. Minor: Test coverage gaps in error paths (ro fallback, specific error classifications)
2. Documentation: Consider clarifying the distinction between standalone check_database_integrity and the trait method
3. Android test: Investigate if the failure is related (unlikely) or a flake

The PR is in good shape for merge after addressing the unrelated Android test failure.

---

[codecov]

Codecov Report

❌ Patch coverage is 93.92523% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.94%. Comparing base (a53a97e) to head (3b41bb0).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
...es/xmtp_db/src/encrypted_store/integrity/native.rs	96.30%	11 Missing ⚠️
crates/xmtp_db/src/encrypted_store/mod.rs	0.00%	7 Missing ⚠️
...tes/xmtp_db/src/encrypted_store/database/native.rs	82.35%	3 Missing ⚠️
...pted_store/database/native/sqlcipher_connection.rs	92.50%	3 Missing ⚠️
...rates/xmtp_db/src/encrypted_store/integrity/mod.rs	96.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4029      +/-   ##
==========================================
+ Coverage   85.91%   85.94%   +0.03%     
==========================================
  Files         419      421       +2     
  Lines       68400    68811     +411     
==========================================
+ Hits        58763    59137     +374     
- Misses       9637     9674      +37     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds a substantial cross-platform database integrity subsystem with SQLCipher HMAC checks, filesystem and OPFS access, dedicated connections, and changed salt-file initialization behavior. Its runtime and security-sensitive effects exceed the scope suitable for automatic approval.

^{You can add or adjust custom eligibility rules. Learn more.}