| 1 |
# SyncKit Client SDK -- Audit History |
| 2 |
|
| 3 |
See [audit_review.md](./audit_review.md) for current scorecard and grades. |
| 4 |
|
| 5 |
## Changes Since Last Audit |
| 6 |
|
| 7 |
### Seventh audit (2026-03-28, Run 12 cross-project) |
| 8 |
- **Test count:** 297 (197 unit + 99 integration + 1 doctest). 0 clippy warnings. 0 failures. |
| 9 |
- **Grade:** A (maintained). v0.3.0. |
| 10 |
- **No code changes since Run 9.** |
| 11 |
- **New dependency advisory:** rustls-webpki 0.103.9 (RUSTSEC-2026-0049) — upgrade to 0.103.10 via `cargo update -p rustls-webpki`. |
| 12 |
- **Mandatory surprise:** None new. Previous surprise (fresh random Argon2 salt per wrap) still valid and impressive. |
| 13 |
- **No new findings.** All previous items remain resolved. |
| 14 |
|
| 15 |
### Rust Patterns Audit (2026-03-21) |
| 16 |
- `SessionInfo.token` changed from `String` to `Arc<String>` -- `Arc::clone` instead of String clone |
| 17 |
- Auth request structs already use `&'a str` -- confirmed optimal, no change needed |
| 18 |
|
| 19 |
### Sixth audit (2026-03-18, Run 9 cross-project) |
| 20 |
- **Test count:** 298 (197 unit + 99 integration + 1 doctest). 0 clippy warnings. |
| 21 |
- **Grade:** A (maintained). v0.3.0. |
| 22 |
- **No new findings.** All previous items remain resolved. |
| 23 |
- **Crypto audit notes:** XChaCha20-Poly1305, Argon2id with OWASP minimums, ZeroizeOnDrop keys, NFC normalization. 100+ crypto-specific tests. |
| 24 |
- **No sensitive data in logs:** Confirmed — tracing calls log events (e.g., "Master key generated") without leaking key material or passwords. |
| 25 |
- **Mandatory surprise:** Argon2 salt uniqueness — every wrap generates fresh random salt (crypto.rs:130-131). Verified by `two_wraps_use_different_salts` test. Correct design, uncommon rigor. |
| 26 |
|
| 27 |
### Concurrency Upgrade (2026-03-13) |
| 28 |
- **Concurrency:** B+ -> A- |
| 29 |
- Replaced std::sync::RwLock with parking_lot::RwLock. Removed 16 poison-handling .map_err() sites. All 234 tests pass. |
| 30 |
|
| 31 |
### Second audit (2026-03-13, pre-launch skeptical lens) |
| 32 |
- **Grade:** B+ (maintained). S4 fixes resolved 4/6 first-audit issues. New critical finding: blob data not encrypted. |
| 33 |
- **Test count:** 13 -> 109 (+96 tests, mostly from S4 remediation) |
| 34 |
- **S4 fixes:** await_holding_lock, HTTP timeouts, retry with backoff, token expiry detection, client.rs tests (66), keystore.rs tests (18), ChangeOp enum |
| 35 |
- **New findings:** Blob encryption gap (CRITICAL), no key rotation, Mutex unwraps, master key copies not zeroized, public types that should be pub(crate) |
| 36 |
- **Deterministic Argon2 salt:** Persists from first audit (tracked in MNW todo as "consider random salt") |
| 37 |
|
| 38 |
### Post-audit remediation (2026-03-13) |
| 39 |
- **Grade:** B+ -> A-. 5 of 6 new findings from second audit resolved. Only key rotation deferred. |
| 40 |
- **Test count:** 109 -> 118 (+9 tests: 7 blob encrypt/decrypt, 2 salt tests) |
| 41 |
- **Blob encryption:** encrypt_bytes/decrypt_bytes in crypto.rs. blob_upload/blob_download encrypt/decrypt transparently using master key. 40-byte overhead (24 nonce + 16 tag). |
| 42 |
- **Random Argon2 salt:** wrap_master_key generates random 32-byte salt per operation. unwrap_master_key reads salt from envelope. Eliminates deterministic salt precomputation risk. |
| 43 |
- **Previous S4 fixes verified:** Mutex unwraps, ZeroizeOnDrop, pub(crate) restrictions -- all still in place. |
| 44 |
- **Key rotation:** Deferred post-beta. Requires server-side re-encryption of all sync_log entries. |
| 45 |
- Documentation upgraded to A: Device/SyncStatus/ChangeEntry/BlobUploadUrlResponse field docs added. All 12 error variants documented with when-they-occur. Keystore platform behavior documented (macOS/Linux/Windows backends). Client helpers documented (require_token, require_session_ids, etc). SessionInfo field docs. client.rs module doc expanded to 50 lines. architecture.md created (217 lines), README created (78 lines). |
| 46 |
|
| 47 |
### Observability Upgrade (2026-03-13) |
| 48 |
- Added Observability dimension to scorecard (grade A) |
| 49 |
- Added 16 `#[instrument]` annotations to all pub async methods in client.rs with appropriate skip params |
| 50 |
- Sensitive params skipped: password, old_password, new_password, email, code, code_verifier, presigned_url, data |
| 51 |
- `use tracing::instrument;` import added to client.rs |
| 52 |
- `cargo check` passes clean |
| 53 |
|
| 54 |
### Performance Upgrade (2026-03-13) |
| 55 |
- **Performance:** B -> A- |
| 56 |
- Cached JWT `exp` claim in Session struct — `require_token()` and `is_token_expired()` no longer re-parse the JWT on every call |
| 57 |
- Retry request bodies use `bytes::Bytes` instead of `Vec<u8>` — clone in retry closures is O(1) refcount bump, not O(n) copy (10 sites) |
| 58 |
- Batch encrypt/decrypt in `push()`/`pull()` extracts master key once before the loop instead of per-entry lock acquisition |
| 59 |
|
| 60 |
### Resilience Upgrade (2026-03-13) |
| 61 |
- **Resilience:** B- -> A- |
| 62 |
- Added 9 integration tests for encryption setup flows: `setup_encryption_new` (happy path, no-auth, server retry), `setup_encryption_existing` (happy path, wrong password, no-auth, server retry, missing key 404), cross-device encryption roundtrip |
| 63 |
- Cross-device test proves full two-device flow: device 1 creates encryption → pushes data → device 2 recovers key → pulls and decrypts successfully |
| 64 |
- **Test count:** 234 -> 243 (+9 integration tests). 170 unit + 72 integration + 1 doctest. |
| 65 |
|
| 66 |
### Adversarial Test Audit (2026-03-13) |
| 67 |
- **Grade:** A- -> A-. Testing grade upgraded from B- to A. |
| 68 |
- **Test count:** 150 -> 234 (+84 tests: 52 unit, 32 integration). Test density ~94 tests/KLOC. |
| 69 |
- **CRITICAL fix: change_password bypass** -- Old password verification skipped when master key was cached in memory. Attacker with session access (stolen device, malware) could change encryption password without knowing the old one. Fixed: always verify old password against server envelope regardless of cache state. Added 8 tests covering cache hit/miss, wrong old password, concurrent password changes. |
| 70 |
- **HIGH fix: Unicode password normalization** -- NFC vs NFD normalization inconsistency across operating systems could derive different keys from "same" password (e.g., é as single codepoint vs e+combining-acute). Added `unicode-normalization` crate, NFC normalization before all key derivation (wrap_master_key, unwrap_master_key, change_password). 4 tests covering NFC/NFD/mixed inputs. |
| 71 |
- **Empty password rejection** -- wrap_master_key, unwrap_master_key, change_password now return error on empty password. 3 tests. |
| 72 |
- **Password length limits** -- 1024-byte max after UTF-8 encoding. Prevents resource exhaustion on Argon2 (linear memory cost with input length). 2 tests. |
| 73 |
- **Comprehensive crypto tests** -- Tamper detection (flip bits in nonce/ciphertext/tag), envelope validation (version mismatch, truncated fields), key rotation simulation (decrypt with wrong key), concurrent encryption (nonce uniqueness under load), large payload handling (1MB encrypt/decrypt). 28 new crypto unit tests. |
| 74 |
- **Integration tests** -- Error mapping for all 4xx/5xx codes (400/401/403/404/409/413/429/500/502/503), retry behavior (transient vs permanent), auth enforcement (missing token, invalid token, expired token), blob roundtrips (upload -> download, tamper detection, decrypt failure), malformed response handling (invalid JSON, missing fields). 32 new integration tests. |
| 75 |
- **Concurrency tests** -- Parallel encrypt operations (nonce uniqueness), concurrent password changes (last-write-wins, cache invalidation), device registration race (409 conflict), push/pull interleaving (optimistic locking). 9 tests across unit and integration. |
| 76 |
- **Resolved findings:** All 2 critical vulnerabilities from adversarial audit fixed. No new security issues discovered. |
| 77 |
|
| 78 |
### Third audit (2026-03-16, Run 6 cross-project) |
| 79 |
- **Test count:** 297 (unchanged) |
| 80 |
- **Grade:** A (maintained). |
| 81 |
- **Source LOC:** 4,327 src + 2,749 test |
| 82 |
- **New finding (MEDIUM):** Wrapping key in `crypto.rs:99` (`derive_wrapping_key`) is computed on the stack but not wrapped in `ZeroizeOnDrop`. Intermediate key material sits in memory after function returns. Other keys properly use ZeroizeOnDrop. |
| 83 |
- **New finding (LOW):** Unused `sha2` dependency in Cargo.toml. |
| 84 |
- **Mandatory surprise:** Wrapping key not zeroized — Genuine issue (MEDIUM). |
| 85 |
- **Previous items verified:** All previous remediated items confirmed intact. Key rotation still deferred (post-beta). |
| 86 |
|
| 87 |
### Testing Push (2026-03-13) |
| 88 |
- **Grade:** A- -> A. Testing A -> A+. Code Quality, Type Safety, Concurrency, Resilience all upgraded to A. |
| 89 |
- **Test count:** 243 -> 297 (+54 tests). 197 unit + 99 integration + 1 doctest. |
| 90 |
- **types.rs:** 13 unit tests added. Serde roundtrip, Display/serde consistency, from_str_opt edge cases, Copy/Hash trait verification, skip_serializing_if, extra unknown fields tolerance, ISO timestamp deserialization. |
| 91 |
- **error.rs:** 10 unit tests added. Send+Sync compile-time assert, Display for all 8 variants, Debug no-panic, source() chain verification (Json, Base64 have source; leaf variants do not), empty/very-long server messages. |
| 92 |
- **client.rs:** 4 unit tests added. SyncKitClient Send+Sync compile-time assert, with_http_client constructor, unicode table name encrypt/decrypt roundtrip, empty row_id roundtrip. |
| 93 |
- **Integration tests:** 27 new. Retry count verification (exhaustion at 4 requests, 404 not retried, 3rd-attempt success). Malformed responses (HTML body, empty body, missing has_more, wrong cursor type, missing app_id, missing already_exists, 413 error, extra fields ignored). Session edge cases (double authenticate, clear then re-auth, expired token on restore). Encryption setup overwrite. Blob edge cases (confirm retry, download retry, 1MB upload overhead). Device edge cases (empty name, unicode name, empty list). Concurrency stress (50 concurrent session_info reads, 50 has_master_key reads, 20 status checks, 4x100-entry pushes). Timeout tests (slow server timeout, retry after timeout). |
| 94 |
- **New constructor:** `with_http_client(config, client)` enables custom timeout testing without modifying production defaults. |
| 95 |
|
| 96 |
### Performance Upgrade (2026-03-13) |
| 97 |
- **Performance:** A- -> A |
| 98 |
- Pre-built endpoint URLs: new `Endpoints` struct computes all 10 API endpoint URLs once at client construction, eliminating per-request `format!()` string allocations |
| 99 |
- `Arc<String>` session token: `require_token()` returns `Arc<String>` instead of `String`, making per-request token extraction O(1) refcount bump instead of O(n) string clone (~300-500 byte JWT) |
| 100 |
- `key_url_and_token()` returns `(&str, Arc<String>)` instead of `(String, String)`, zero allocations per call |
| 101 |
- All 297 tests pass unchanged (2 test assertions updated for Arc deref) |
| 102 |
|