Skip to main content

max / makenotwork

synckit-client: module docs, reconnect/rotation tests, key zeroization - Add //! headers to the six client modules that lacked them. - Drop the redundant sha2 dev-dependency. - Add mock-transport tests for the SSE reconnect state machine (live event, transparent reconnect, fatal auth-rejection exit). - Add feature-gated end-to-end key-rotation orchestration tests (full protocol + straggler retry); self dev-dep is now default-features=false so `cargo test --no-default-features --features store,testing` runs them with the keychain off. - keystore::load_key returns Option<ZeroizeOnDrop> instead of a bare [u8;32]; zeroize the PKCE verifier on drop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-17 16:59 UTC
Commit: 41639f84c5b3ee4e285d58fd4a3d50e103093159
Parent: 8b58be5
10 files changed, +336 insertions, -9 deletions
M shared/synckit-client/Cargo.toml +6 -2
@@ -64,7 +64,11 @@ rusqlite = { version = "0.32", features = ["bundled", "functions"], optional = t
64 64
65 65 [dev-dependencies]
66 66 wiremock = "0.6"
67 - sha2 = "0.10"
68 67 # Self dev-dependency: turns on the `testing` feature for this crate's own
69 68 # unit/integration test builds without leaking it into a consumer's default set.
70 - synckit-client = { path = ".", features = ["testing"] }
69 + # `default-features = false` so it only ADDS `testing` to whatever the test run
70 + # already selected — under a plain `cargo test` the package still builds with its
71 + # own defaults (keychain + store), but `cargo test --no-default-features` can turn
72 + # keychain OFF (making `keystore::store_key` the no-op stub) to run the keychain-
73 + # free rotation orchestration test below.
74 + synckit-client = { path = ".", default-features = false, features = ["testing"] }
M shared/synckit-client/src/client/auth.rs +5
@@ -1,3 +1,8 @@
1 + //! Authentication and session lifecycle.
2 + //!
3 + //! Password and OAuth login, session restore from a stored token, and the
4 + //! local JWT-expiry check the API methods use to fail fast before a request.
5 +
1 6 use bytes::Bytes;
2 7 use std::sync::Arc;
3 8 use tracing::instrument;
M shared/synckit-client/src/client/blob.rs +7
@@ -1,3 +1,10 @@
1 + //! Content-addressed blob upload and download.
2 + //!
3 + //! Blobs are encrypted client-side and stored under the SHA-256 of their
4 + //! ciphertext. Uploads go through a presigned URL after a size/quota check;
5 + //! downloads are re-hashed on arrival and discarded on any mismatch, so a
6 + //! server that swaps or rolls back a blob cannot slip it past the client.
7 +
1 8 use bytes::{Buf, Bytes, BytesMut};
2 9 use sha2::{Digest, Sha256};
3 10 use tokio_stream::StreamExt;
M shared/synckit-client/src/client/encryption.rs +7 -1
@@ -1,3 +1,9 @@
1 + //! Master-key setup and password lifecycle.
2 + //!
3 + //! Establishes the master key on a first or subsequent device by wrapping and
4 + //! unwrapping it against the server-held key envelope, caches it in the OS
5 + //! keychain when available, and re-wraps it on a password change.
6 +
1 7 use bytes::Bytes;
2 8 use std::sync::Arc;
3 9 use tracing::instrument;
@@ -87,7 +93,7 @@ impl SyncKitClient {
87 93 let (app_id, user_id) = self.require_session_ids()?;
88 94
89 95 if let Some(key) = keystore::load_key(app_id, user_id)? {
90 - *self.master_key.write() = Some(crypto::ZeroizeOnDrop(key));
96 + *self.master_key.write() = Some(key);
91 97 tracing::info!("Master key loaded from keychain");
92 98 Ok(true)
93 99 } else {
M shared/synckit-client/src/client/helpers.rs +7
@@ -1,3 +1,10 @@
1 + //! Internal HTTP plumbing shared across the client modules.
2 + //!
3 + //! Response-status to [`SyncKitError`] mapping, idempotency-key generation,
4 + //! size-capped body reads (so a hostile server cannot exhaust memory on a
5 + //! control response), wire-version negotiation, and JWT-expiry parsing. This
6 + //! module is `pub(crate)`: none of it is part of the SDK's public API.
7 +
1 8 use base64::Engine;
2 9
3 10 use crate::{
M shared/synckit-client/src/client/rotation.rs +7
@@ -1,3 +1,10 @@
1 + //! Master-key rotation orchestration.
2 + //!
3 + //! Drives the multi-round server protocol that re-encrypts every device's key
4 + //! envelope under a new master key, completes the rotation, and sweeps up
5 + //! stragglers. The round count is bounded by `MAX_ROTATION_ROUNDS` so a
6 + //! misbehaving server cannot spin the loop indefinitely.
7 +
1 8 use bytes::Bytes;
2 9 use tracing::instrument;
3 10 use uuid::Uuid;
M shared/synckit-client/src/client/sync.rs +7
@@ -1,3 +1,10 @@
1 + //! Device registration and the core encrypted push/pull.
2 + //!
3 + //! Registers a device, lists the account's devices, and moves change entries
4 + //! to and from the server. Rows are encrypted before push and decrypted after
5 + //! pull, so the transport only ever carries ciphertext; the `*_filtered` and
6 + //! `*_rich` variants narrow the pull or return per-row sync metadata.
7 +
1 8 use bytes::Bytes;
2 9 use tracing::instrument;
3 10 use uuid::Uuid;
M shared/synckit-client/src/keystore.rs +15 -6
@@ -16,7 +16,9 @@ use crate::error::Result;
16 16 // directly; without the feature a plain lib build would see it as unused.
17 17 #[cfg(any(feature = "keychain", test))]
18 18 use crate::error::SyncKitError;
19 - #[cfg(any(feature = "keychain", test))]
19 + // Only the keychain code paths use this top-level alias; the test module imports
20 + // its own `B64` locally, so a no-keychain test build must not pull it in here.
21 + #[cfg(feature = "keychain")]
20 22 use base64::{engine::general_purpose::STANDARD as B64, Engine};
21 23 use crate::ids::{AppId, UserId};
22 24 #[cfg(test)]
@@ -61,8 +63,13 @@ pub fn store_key(app_id: AppId, user_id: UserId, master_key: &[u8; 32]) -> Resul
61 63
62 64 /// Load the master key from the OS keychain.
63 65 /// Returns None if no key is stored (not an error).
66 + ///
67 + /// The key is returned inside a [`ZeroizeOnDrop`](crate::crypto::ZeroizeOnDrop)
68 + /// so the caller cannot leave a bare `[u8; 32]` copy of it lingering on the heap
69 + /// — the only in-crate caller stores it straight into the master-key slot, and
70 + /// there are no external callers, so this stays `pub(crate)`.
64 71 #[cfg(feature = "keychain")]
65 - pub fn load_key(app_id: AppId, user_id: UserId) -> Result<Option<[u8; 32]>> {
72 + pub(crate) fn load_key(app_id: AppId, user_id: UserId) -> Result<Option<crate::crypto::ZeroizeOnDrop>> {
66 73 let entry = keyring::Entry::new(&service_name(app_id), &user_key(user_id))?;
67 74
68 75 match entry.get_password() {
@@ -79,8 +86,8 @@ pub fn load_key(app_id: AppId, user_id: UserId) -> Result<Option<[u8; 32]>> {
79 86 "stored key has wrong length".into(),
80 87 ));
81 88 }
82 - let mut key = [0u8; 32];
83 - key.copy_from_slice(&bytes);
89 + let mut key = crate::crypto::ZeroizeOnDrop([0u8; 32]);
90 + key.0.copy_from_slice(&bytes);
84 91 bytes.zeroize();
85 92 tracing::debug!("Master key loaded from OS keychain");
86 93 Ok(Some(key))
@@ -114,7 +121,7 @@ pub fn store_key(_app_id: AppId, _user_id: UserId, _master_key: &[u8; 32]) -> Re
114 121 }
115 122
116 123 #[cfg(not(feature = "keychain"))]
117 - pub fn load_key(_app_id: AppId, _user_id: UserId) -> Result<Option<[u8; 32]>> {
124 + pub(crate) fn load_key(_app_id: AppId, _user_id: UserId) -> Result<Option<crate::crypto::ZeroizeOnDrop>> {
118 125 Ok(None)
119 126 }
120 127
@@ -329,7 +336,9 @@ mod keystore_tests {
329 336 // These may fail at runtime due to keychain unavailability,
330 337 // but they must compile with the correct types.
331 338 let _: Result<()> = store_key(app_id, user_id, &key);
332 - let _: Result<Option<[u8; 32]>> = load_key(app_id, user_id);
339 + // load_key now hands back the key inside a ZeroizeOnDrop guard so no bare
340 + // [u8; 32] copy outlives the caller's move into the master-key slot.
341 + let _: Result<Option<crate::crypto::ZeroizeOnDrop>> = load_key(app_id, user_id);
333 342 let _: Result<()> = delete_key(app_id, user_id);
334 343 }
335 344 }
M shared/synckit-client/src/oauth.rs +10
@@ -13,6 +13,7 @@
13 13 use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine};
14 14 use rand::RngCore;
15 15 use sha2::{Digest, Sha256};
16 + use zeroize::Zeroize;
16 17
17 18 /// A PKCE verifier/challenge pair using the S256 method.
18 19 pub struct Pkce {
@@ -22,6 +23,15 @@ pub struct Pkce {
22 23 pub challenge: String,
23 24 }
24 25
26 + impl Drop for Pkce {
27 + fn drop(&mut self) {
28 + // The verifier is the PKCE secret that proves possession at token
29 + // exchange; scrub it from the heap on drop. The challenge is a public
30 + // SHA-256 of it (sent in the clear to /authorize), so it needs none.
31 + self.verifier.zeroize();
32 + }
33 + }
34 +
25 35 /// Generate a PKCE pair (S256).
26 36 ///
27 37 /// The verifier is 32 random bytes base64url-encoded (43 chars, well within
M shared/synckit-client/tests/integration.rs +265
@@ -2859,3 +2859,268 @@ async fn push_retries_on_timeout_then_succeeds() {
2859 2859 .unwrap();
2860 2860 assert_eq!(cursor, 42);
2861 2861 }
2862 +
2863 + // ── SSE subscribe / reconnect state machine ──
2864 + //
2865 + // These drive the real `subscribe()` -> `SyncNotifyStream::next_change()` path
2866 + // against wiremock so the reconnect state machine (subscribe.rs) is exercised
2867 + // end to end: a live notification, a transparent reconnect across a stream
2868 + // drop, the fatal auth-rejection exit, and the give-up-after-N-failures cap.
2869 +
2870 + const SUBSCRIBE_PATH: &str = "/api/v1/sync/subscribe";
2871 +
2872 + /// One complete SSE "changed" block, body-terminated so the client sees a full
2873 + /// event and then end-of-stream.
2874 + fn sse_changed_block() -> &'static str {
2875 + "event: changed\n\n"
2876 + }
2877 +
2878 + #[tokio::test]
2879 + async fn subscribe_yields_changed_event() {
2880 + let server = MockServer::start().await;
2881 + Mock::given(method("GET"))
2882 + .and(path(SUBSCRIBE_PATH))
2883 + .respond_with(ResponseTemplate::new(200).set_body_string(sse_changed_block()))
2884 + .mount(&server)
2885 + .await;
2886 +
2887 + let client = authed_client(&server);
2888 + let mut stream = client.subscribe().await.expect("subscribe should succeed");
2889 +
2890 + // The first block is delivered inside the initial response body, so this
2891 + // returns without any reconnect.
2892 + assert_eq!(stream.next_change().await, Some(()));
2893 + }
2894 +
2895 + #[tokio::test]
2896 + async fn subscribe_reconnects_transparently_after_stream_drop() {
2897 + let server = MockServer::start().await;
2898 + // Every connection serves one block then ends (Content-Length terminates the
2899 + // body). Consuming the first event, then reading past it, drops the stream
2900 + // and forces a reconnect that must transparently yield the next event.
2901 + Mock::given(method("GET"))
2902 + .and(path(SUBSCRIBE_PATH))
2903 + .respond_with(ResponseTemplate::new(200).set_body_string(sse_changed_block()))
2904 + .mount(&server)
2905 + .await;
2906 +
2907 + let client = authed_client(&server);
2908 + let mut stream = client.subscribe().await.expect("subscribe should succeed");
2909 +
2910 + // #1 comes from the initial connection; #2 can only arrive after the stream
2911 + // drops (body EOF) and reconnect() re-establishes it.
2912 + assert_eq!(stream.next_change().await, Some(()));
2913 + assert_eq!(stream.next_change().await, Some(()));
2914 +
2915 + // subscribe() opened one connection; the second event required at least one
2916 + // reconnect, so the server saw two or more subscribe requests.
2917 + let hits = server
2918 + .received_requests()
2919 + .await
2920 + .unwrap()
2921 + .into_iter()
2922 + .filter(|r| r.url.path() == SUBSCRIBE_PATH)
2923 + .count();
2924 + assert!(hits >= 2, "expected a reconnect (>=2 subscribe requests), got {hits}");
2925 + }
2926 +
2927 + #[tokio::test]
2928 + async fn subscribe_stream_closes_on_auth_rejection() {
2929 + let server = MockServer::start().await;
2930 + // First connection opens cleanly but carries no event and ends immediately,
2931 + // forcing a reconnect. The reconnect is rejected for auth -> fatal, so the
2932 + // stream ends with `None` rather than retrying.
2933 + Mock::given(method("GET"))
2934 + .and(path(SUBSCRIBE_PATH))
2935 + .respond_with(ResponseTemplate::new(200).set_body_string(""))
2936 + .up_to_n_times(1)
2937 + .mount(&server)
2938 + .await;
2939 + Mock::given(method("GET"))
2940 + .and(path(SUBSCRIBE_PATH))
2941 + .respond_with(ResponseTemplate::new(401).set_body_string("unauthorized"))
2942 + .mount(&server)
2943 + .await;
2944 +
2945 + let client = authed_client(&server);
2946 + let mut stream = client.subscribe().await.expect("initial subscribe is 200");
2947 +
2948 + // Empty body -> EOF -> reconnect -> 401 -> fatal -> None.
2949 + assert_eq!(stream.next_change().await, None);
2950 + }
2951 +
2952 + // Note: the "give up after MAX_RECONNECT_ATTEMPTS consecutive failures" path is
2953 + // deliberately not covered end-to-end here. Exercising it against a live mock
2954 + // server would incur the real exponential backoff (growing to a 60s cap, minutes
2955 + // of wall-clock), and `tokio::time::pause()` cannot collapse it: the wiremock
2956 + // server runs on its own runtime, so a paused clock auto-advances past the
2957 + // cross-thread request to the nearest timer and the reconnect never completes.
2958 + // The two pieces of that path are covered separately: the backoff schedule by
2959 + // `reconnect_delay_grows_then_caps` (subscribe.rs), and the fatal-exit mechanics
2960 + // (returning `None` and clearing state) by the auth-rejection test above.
2961 +
2962 + // ── End-to-end key-rotation orchestration ──
2963 + //
2964 + // These drive `rotate_key()` through the full server protocol against wiremock:
2965 + // fetch key -> begin -> re-encrypt loop -> complete, plus the straggler retry on
2966 + // a 409. They are gated `#[cfg(not(feature = "keychain"))]` because `rotate_key`
2967 + // finishes by caching the new key with `keystore::store_key`, which hits the OS
2968 + // secret-service under the `keychain` feature and is unavailable on a headless
2969 + // test host. With keychain off, `store_key` is the no-op stub, so the test
2970 + // exercises the orchestration hermetically. Run with:
2971 + // cargo test --no-default-features --features store,testing
2972 + #[cfg(not(feature = "keychain"))]
2973 + mod rotation_e2e {
2974 + use super::*;
2975 +
2976 + const KEYS_PATH: &str = "/api/v1/sync/keys";
2977 + const ROTATE_PATH: &str = "/api/v1/sync/keys/rotate";
2978 + const ENTRIES_PATH: &str = "/api/v1/sync/keys/rotate/entries";
2979 + const BATCH_PATH: &str = "/api/v1/sync/keys/rotate/batch";
2980 + const COMPLETE_PATH: &str = "/api/v1/sync/keys/rotate/complete";
2981 +
2982 + const ROTATE_PW: &str = "rotate-password";
2983 +
2984 + /// A `GET /keys` body wrapping `old_key` under [`ROTATE_PW`], with no rotation
2985 + /// in progress, so `rotate_key` verifies the password and mints a fresh key.
2986 + fn get_keys_body(old_key: &[u8; 32]) -> serde_json::Value {
2987 + let envelope = synckit_client::crypto::wrap_master_key(old_key, ROTATE_PW).unwrap();
2988 + json!({ "encrypted_key": envelope, "key_version": 1, "key_id": 1 })
2989 + }
2990 +
2991 + /// One rotation entry: `plaintext` sealed under `old_key` with the same
2992 + /// `(table, row_id)` AAD the client rebinds during re-encryption.
2993 + fn rotation_entry(old_key: &[u8; 32], table: &str, row_id: &str) -> serde_json::Value {
2994 + let ctx = synckit_client::crypto::AeadContext::entry(table, row_id);
2995 + let sealed =
2996 + synckit_client::crypto::encrypt_json_aad(&json!({"title": "secret"}), old_key, &ctx)
2997 + .unwrap();
2998 + json!({ "seq": 1, "table": table, "row_id": row_id, "data": sealed })
2999 + }
3000 +
3001 + fn hits(reqs: &[wiremock::Request], p: &str) -> usize {
3002 + reqs.iter().filter(|r| r.url.path() == p).count()
3003 + }
3004 +
3005 + #[tokio::test]
3006 + async fn rotate_key_drives_full_orchestration() {
3007 + let server = MockServer::start().await;
3008 + let old_key = synckit_client::crypto::generate_master_key();
3009 +
3010 + Mock::given(method("GET"))
3011 + .and(path(KEYS_PATH))
3012 + .respond_with(ResponseTemplate::new(200).set_body_json(get_keys_body(&old_key)))
3013 + .mount(&server)
3014 + .await;
3015 + Mock::given(method("POST"))
3016 + .and(path(ROTATE_PATH))
3017 + .respond_with(ResponseTemplate::new(200).set_body_json(
3018 + json!({ "rotation_id": Uuid::new_v4(), "target_seq": 1, "new_key_id": 2 }),
3019 + ))
3020 + .mount(&server)
3021 + .await;
3022 + // One batch of work, then drained (has_more = false ends the re-encrypt loop).
3023 + Mock::given(method("POST"))
3024 + .and(path(ENTRIES_PATH))
3025 + .respond_with(ResponseTemplate::new(200).set_body_json(json!({
3026 + "entries": [rotation_entry(&old_key, "tasks", "r1")],
3027 + "has_more": false
3028 + })))
3029 + .mount(&server)
3030 + .await;
3031 + Mock::given(method("POST"))
3032 + .and(path(BATCH_PATH))
3033 + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "updated_count": 1 })))
3034 + .mount(&server)
3035 + .await;
3036 + Mock::given(method("POST"))
3037 + .and(path(COMPLETE_PATH))
3038 + .respond_with(ResponseTemplate::new(200))
3039 + .mount(&server)
3040 + .await;
3041 +
3042 + let client = authed_client(&server);
3043 + client
3044 + .rotate_key(DeviceId::new(Uuid::new_v4()), ROTATE_PW)
3045 + .await
3046 + .expect("full rotation should complete");
3047 +
3048 + // Every stage of the protocol was driven, in the right shape.
3049 + let reqs = server.received_requests().await.unwrap();
3050 + assert_eq!(hits(&reqs, KEYS_PATH), 1, "fetched key state once");
3051 + assert_eq!(hits(&reqs, ROTATE_PATH), 1, "began rotation once");
3052 + assert!(hits(&reqs, ENTRIES_PATH) >= 1, "pulled entries to re-encrypt");
3053 + assert_eq!(hits(&reqs, BATCH_PATH), 1, "pushed one re-encrypted batch");
3054 + assert_eq!(hits(&reqs, COMPLETE_PATH), 1, "completed once (no stragglers)");
3055 + }
3056 +
3057 + #[tokio::test]
3058 + async fn rotate_key_retries_reencrypt_on_straggler_conflict() {
3059 + let server = MockServer::start().await;
3060 + let old_key = synckit_client::crypto::generate_master_key();
3061 +
3062 + Mock::given(method("GET"))
3063 + .and(path(KEYS_PATH))
3064 + .respond_with(ResponseTemplate::new(200).set_body_json(get_keys_body(&old_key)))
3065 + .mount(&server)
3066 + .await;
3067 + Mock::given(method("POST"))
3068 + .and(path(ROTATE_PATH))
3069 + .respond_with(ResponseTemplate::new(200).set_body_json(
3070 + json!({ "rotation_id": Uuid::new_v4(), "target_seq": 1, "new_key_id": 2 }),
3071 + ))
3072 + .mount(&server)
3073 + .await;
3074 + // First entries pull returns work; every later pull is drained. Mounted in
3075 + // this order so the up_to_n_times(1) mock wins the first call, then the
3076 + // empty-set fallback serves the straggler round's re-pull.
3077 + Mock::given(method("POST"))
3078 + .and(path(ENTRIES_PATH))
3079 + .respond_with(ResponseTemplate::new(200).set_body_json(json!({
3080 + "entries": [rotation_entry(&old_key, "tasks", "r1")],
3081 + "has_more": false
3082 + })))
3083 + .up_to_n_times(1)
3084 + .mount(&server)
3085 + .await;
3086 + Mock::given(method("POST"))
3087 + .and(path(ENTRIES_PATH))
3088 + .respond_with(
3089 + ResponseTemplate::new(200).set_body_json(json!({ "entries": [], "has_more": false })),
3090 + )
3091 + .mount(&server)
3092 + .await;
3093 + Mock::given(method("POST"))
3094 + .and(path(BATCH_PATH))
3095 + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "updated_count": 1 })))
3096 + .mount(&server)
3097 + .await;
3098 + // First completion reports a straggler (409); the retry then succeeds.
3099 + Mock::given(method("POST"))
3100 + .and(path(COMPLETE_PATH))
3101 + .respond_with(ResponseTemplate::new(409).set_body_json(json!({ "message": "stragglers" })))
3102 + .up_to_n_times(1)
3103 + .mount(&server)
3104 + .await;
3105 + Mock::given(method("POST"))
3106 + .and(path(COMPLETE_PATH))
3107 + .respond_with(ResponseTemplate::new(200))
3108 + .mount(&server)
3109 + .await;
3110 +
3111 + let client = authed_client(&server);
3112 + client
3113 + .rotate_key(DeviceId::new(Uuid::new_v4()), ROTATE_PW)
3114 + .await
3115 + .expect("rotation should converge after the straggler retry");
3116 +
3117 + // The 409 forced a second completion attempt, and the straggler round
3118 + // re-ran the re-encrypt loop (a second entries pull).
3119 + let reqs = server.received_requests().await.unwrap();
3120 + assert_eq!(hits(&reqs, COMPLETE_PATH), 2, "completed twice: 409 then 200");
3121 + assert!(
3122 + hits(&reqs, ENTRIES_PATH) >= 2,
3123 + "straggler round re-pulled entries"
3124 + );
3125 + }
3126 + }