Skip to main content

max / makenotwork

synckit: uncap large-body transport, don't hold plaintext during upload (audit Phase 2) Performance remediation from the 2026-07-02 /audit arbiter: - Split the HTTP client in two. `http` keeps the 30s whole-request timeout for small JSON control-plane calls; a new `http_stream` (connect timeout only, no whole-request cap) carries blob up/download, OTA artifacts, and the SSE stream. The prior single 30s cap broke any transfer over ~30s (4 GiB blobs, the paid Big Files/Everything tiers) and tore the notification stream down every 30s. - blob_upload drops the plaintext before the network send, so the sustained peak during a multi-GB transfer is ~1x the blob rather than plaintext+ciphertext. (The retry-loop body clone is a cheap Bytes refcount bump, not a copy.) True 1x streaming that never materializes the full ciphertext lands with the resumable multipart upload in Phase 5. Gate: 405 crate tests pass, cargo clippy --all-targets clean, GoingsOn cargo check clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-03 01:08 UTC
Commit: ff01045c761b3777f00d2bc550aff12d5deacf80
Parent: 898766b
4 files changed, +33 insertions, -7 deletions
@@ -63,10 +63,16 @@ impl SyncKitClient {
63 63 // v3 chunked format: each chunk sealed and bound to (hash, index, count),
64 64 // so the reader verifies and decrypts one chunk at a time.
65 65 let encrypted = Bytes::from(crypto::encrypt_blob_chunked(&data, &master_key, hash)?);
66 + // Release the plaintext before the (potentially long, multi-GB) network
67 + // send so the sustained peak during transfer is ~1x the blob, not ~2x.
68 + // The per-attempt `encrypted.clone()` below is a cheap `Bytes` refcount
69 + // bump, not a copy. True 1x streaming (never materializing the full
70 + // ciphertext) arrives with the resumable multipart upload in a later pass.
71 + drop(data);
66 72
67 73 self.retry_request(Idempotency::Safe, || {
68 74 let req = self
69 - .http
75 + .http_stream
70 76 .put(presigned_url)
71 77 .header("content-type", "application/octet-stream")
72 78 .body(encrypted.clone());
@@ -147,7 +153,7 @@ impl SyncKitClient {
147 153 pub async fn blob_download(&self, expected_hash: &str, presigned_url: &str) -> Result<Vec<u8>> {
148 154 let resp = self
149 155 .retry_request(Idempotency::Safe, || {
150 - let req = self.http.get(presigned_url);
156 + let req = self.http_stream.get(presigned_url);
151 157 async move { check_response(req.send().await?).await }
152 158 })
153 159 .await?;
@@ -168,7 +168,14 @@ pub(crate) struct PendingKeyState {
168 168 /// The SyncKit client. Handles authentication, encryption, and HTTP transport.
169 169 pub struct SyncKitClient {
170 170 config: SyncKitConfig,
171 + /// HTTP client for small JSON control-plane calls. Carries a whole-request
172 + /// timeout — those calls should never run long.
171 173 http: Client,
174 + /// HTTP client for large-body and long-lived transfers (blob up/download,
175 + /// OTA artifacts, the SSE notification stream). Deliberately has NO
176 + /// whole-request timeout — a multi-gigabyte blob or an open push stream must
177 + /// not be torn down by a fixed clock — only a connect timeout.
178 + http_stream: Client,
172 179 endpoints: Endpoints,
173 180 session: RwLock<Option<Session>>,
174 181 master_key: RwLock<Option<crypto::ZeroizeOnDrop>>,
@@ -181,19 +188,31 @@ pub struct SyncKitClient {
181 188 impl SyncKitClient {
182 189 /// Create a new client with the given configuration.
183 190 pub fn new(config: SyncKitConfig) -> Self {
191 + let https_only = requires_https(&config.server_url);
184 192 let http = Client::builder()
185 193 .timeout(Duration::from_secs(30))
186 194 .connect_timeout(Duration::from_secs(10))
187 195 .pool_max_idle_per_host(5)
188 196 .pool_idle_timeout(Duration::from_secs(90))
189 - .https_only(requires_https(&config.server_url))
197 + .https_only(https_only)
190 198 .build()
191 199 .expect("failed to build HTTP client");
192 200
201 + // No whole-request timeout: this client carries multi-gigabyte blobs and
202 + // the long-lived SSE stream, which a fixed 30s cap would break.
203 + let http_stream = Client::builder()
204 + .connect_timeout(Duration::from_secs(10))
205 + .pool_max_idle_per_host(5)
206 + .pool_idle_timeout(Duration::from_secs(90))
207 + .https_only(https_only)
208 + .build()
209 + .expect("failed to build streaming HTTP client");
210 +
193 211 let endpoints = Endpoints::new(&config.server_url);
194 212 Self {
195 213 config,
196 214 http,
215 + http_stream,
197 216 endpoints,
198 217 session: RwLock::new(None),
199 218 master_key: RwLock::new(None),
@@ -212,7 +231,8 @@ impl SyncKitClient {
212 231 let endpoints = Endpoints::new(&config.server_url);
213 232 Self {
214 233 config,
215 - http,
234 + http: http.clone(),
235 + http_stream: http,
216 236 endpoints,
217 237 session: RwLock::new(None),
218 238 master_key: RwLock::new(None),
@@ -185,7 +185,7 @@ impl SyncKitClient {
185 185 let data = Bytes::from(data);
186 186 self.retry_request(Idempotency::Safe, || {
187 187 let req = self
188 - .http
188 + .http_stream
189 189 .put(presigned_url)
190 190 .header("content-type", "application/octet-stream")
191 191 .body(data.clone());
@@ -213,7 +213,7 @@ impl SyncKitClient {
213 213
214 214 let resp = self
215 215 .retry_request(Idempotency::Safe, || {
216 - let req = self.http.get(&url);
216 + let req = self.http_stream.get(&url);
217 217 async move { check_response(req.send().await?).await }
218 218 })
219 219 .await?;
@@ -113,7 +113,7 @@ impl SyncKitClient {
113 113 let url = format!("{}?app_id={}", self.endpoints.subscribe, app_id);
114 114
115 115 let resp = self
116 - .http
116 + .http_stream
117 117 .get(&url)
118 118 .bearer_auth(&*token)
119 119 .send()