Skip to main content

max / makenotwork

Fix the blob presign size declaration, bind part checksums Two things about the presigned-PUT contract, found while implementing the deferred checksum item. The bug: presign_upload signs max_bytes as Content-Length, and it really is a SignedHeader (a presigned URL comes back with X-Amz-SignedHeaders=content-length;content-type;host, now pinned by a test). Both consumers declared the plaintext size while blob_upload PUT the sealed ciphertext, which is always longer by the v3 framing — so every one-shot blob upload would fail SigV4 against a real backend. Nothing caught it: the server tests use an in-memory backend that does not sign, and the client tests use wiremock, which does not verify. blob_upload_url now converts to blob_encrypted_len, keeping the wire-format arithmetic in the SDK rather than in three consumer apps, and a test pins the declared size to the actual PUT body length. The checksum: the deferred note claimed binding x-amz-checksum-sha256 would make S3 enforce the content address. It cannot. The stored object is E2E ciphertext with random per-chunk nonces while the address is the SHA-256 of the plaintext, so the server cannot derive the expected digest and any checksum it binds comes from the client — the party whose honesty was in question. What binds bytes to the address is the AEAD, which already seals each chunk under (hash, index, chunk_count). The note is rewritten to say so. Bound anyway, for transport integrity: S3 rehashes each part and rejects a corrupted one before the bytes are durable. Multipart only, and the client now requests one part URL at a time, since a digest exists only once a part is sealed and only one part is ever in memory. The one-shot path keeps no checksum: binding one there would mean sealing before requesting the URL, wasting a full read+seal on every dedup hit. File-backed callers should move to blob_upload_streaming, whose start does dedup before any work.
Co-Authored-By
Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-21 15:31 UTC
Signed with PGP, not checked
Commit: 436a4eb1003bcd29ad43a2941b263ef7d5019414
Parent: 99d418b
11 files changed, +443 insertions, -80 deletions
M server/src/storage.rs +9 -5
@@ -535,7 +535,11 @@
535 535 /// `max_bytes`, when set, is signed as `Content-Length`, the same
536 536 /// defense-in-depth as [`Self::presign_upload`] — the authoritative size
537 537 /// check still happens at confirm time.
538 - async fn presign_upload_part(&self, s3_key: &S3Key, upload_id: &str, part_number: i32, expiry_secs: Option<u64>, max_bytes: Option<i64>) -> Result<String>;
538 + ///
539 + /// `checksum_sha256` (base64 of the raw digest), when set, is signed as
540 + /// `x-amz-checksum-sha256` and IS enforced: S3 rehashes the part and
541 + /// rejects a mismatch before the bytes are durable.
542 + async fn presign_upload_part(&self, s3_key: &S3Key, upload_id: &str, part_number: i32, expiry_secs: Option<u64>, max_bytes: Option<i64>, checksum_sha256: Option<&str>) -> Result<String>;
539 543 /// Complete a multipart upload from the collected `(part_number, etag)`
540 544 /// pairs. Parts may be passed in any order; the backend sorts them.
541 545 async fn complete_multipart_upload(&self, s3_key: &S3Key, upload_id: &str, parts: &[(i32, String)]) -> Result<()>;
@@ -982,9 +986,9 @@
982 986
983 987 /// Presign one `UploadPart` request. See the
984 988 /// [`StorageBackend::presign_upload_part`] trait method.
985 - pub async fn presign_upload_part(&self, s3_key: &S3Key, upload_id: &str, part_number: i32, expiry_secs: Option<u64>, max_bytes: Option<i64>) -> Result<String> {
989 + pub async fn presign_upload_part(&self, s3_key: &S3Key, upload_id: &str, part_number: i32, expiry_secs: Option<u64>, max_bytes: Option<i64>, checksum_sha256: Option<&str>) -> Result<String> {
986 990 self.inner
987 - .presign_upload_part(s3_key.as_str(), upload_id, part_number, expiry_secs.unwrap_or(PRESIGN_EXPIRY_SECS), max_bytes)
991 + .presign_upload_part(s3_key.as_str(), upload_id, part_number, expiry_secs.unwrap_or(PRESIGN_EXPIRY_SECS), max_bytes, checksum_sha256)
988 992 .await
989 993 .map_err(AppError::Storage)
990 994 }
@@ -1269,8 +1273,8 @@
1269 1273 self.create_multipart_upload(s3_key, content_type).await
1270 1274 }
1271 1275
1272 - async fn presign_upload_part(&self, s3_key: &S3Key, upload_id: &str, part_number: i32, expiry_secs: Option<u64>, max_bytes: Option<i64>) -> Result<String> {
1273 - self.presign_upload_part(s3_key, upload_id, part_number, expiry_secs, max_bytes).await
1276 + async fn presign_upload_part(&self, s3_key: &S3Key, upload_id: &str, part_number: i32, expiry_secs: Option<u64>, max_bytes: Option<i64>, checksum_sha256: Option<&str>) -> Result<String> {
1277 + self.presign_upload_part(s3_key, upload_id, part_number, expiry_secs, max_bytes, checksum_sha256).await
1274 1278 }
1275 1279
1276 1280 async fn complete_multipart_upload(&self, s3_key: &S3Key, upload_id: &str, parts: &[(i32, String)]) -> Result<()> {
M server/src/validation/mod.rs +20
@@ -248,6 +248,26 @@
248 248 Ok(())
249 249 }
250 250
251 + /// Validate a base64-encoded SHA-256 digest, the form S3 wants for
252 + /// `x-amz-checksum-sha256`.
253 + ///
254 + /// Bound into a presigned URL, so a malformed value would produce a URL that
255 + /// fails at S3 with an opaque error long after the client committed to it.
256 + /// Reject it here, where the message can say what was wrong.
257 + pub fn validate_sha256_base64(value: &str) -> Result<(), AppError> {
258 + use base64::Engine;
259 + let decoded = base64::engine::general_purpose::STANDARD
260 + .decode(value)
261 + .map_err(|_| AppError::validation("Checksum must be base64".to_string()))?;
262 + if decoded.len() != 32 {
263 + return Err(AppError::validation(format!(
264 + "Checksum must decode to 32 bytes (SHA-256), got {}",
265 + decoded.len()
266 + )));
267 + }
268 + Ok(())
269 + }
270 +
251 271 /// Validate a developer-defined SDK key. Opaque string identifying which
252 272 /// workspace/org/end-user a JWT session belongs to. Rejects empty, oversize,
253 273 /// null bytes, and control characters — same character rules as `validate_sync_row_id`
M server/tests/harness/storage.rs +9 -2
@@ -153,7 +153,7 @@
153 153 Ok(upload_id)
154 154 }
155 155
156 - async fn presign_upload_part(&self, s3_key: &S3Key, upload_id: &str, part_number: i32, _expiry_secs: Option<u64>, _max_bytes: Option<i64>) -> Result<String> {
156 + async fn presign_upload_part(&self, s3_key: &S3Key, upload_id: &str, part_number: i32, _expiry_secs: Option<u64>, _max_bytes: Option<i64>, checksum_sha256: Option<&str>) -> Result<String> {
157 157 // Mirror the production range check so a bad part number fails in tests
158 158 // the same way it would against S3.
159 159 if !(1..=s3_storage::MULTIPART_MAX_PARTS as i32).contains(&part_number) {
@@ -162,7 +162,14 @@
162 162 s3_storage::MULTIPART_MAX_PARTS
163 163 )));
164 164 }
165 - Ok(format!("http://test-storage/{s3_key}?uploadId={upload_id}&partNumber={part_number}"))
165 + // Echo the bound checksum into the URL so tests can assert it reached
166 + // the signer, standing in for the SignedHeaders a real presign carries.
167 + let checksum = checksum_sha256
168 + .map(|c| format!("&checksum={c}"))
169 + .unwrap_or_default();
170 + Ok(format!(
171 + "http://test-storage/{s3_key}?uploadId={upload_id}&partNumber={part_number}{checksum}"
172 + ))
166 173 }
167 174
168 175 async fn complete_multipart_upload(&self, _s3_key: &S3Key, upload_id: &str, parts: &[(i32, String)]) -> Result<()> {
M server/tests/workflows/synckit_blob_multipart.rs +72
@@ -222,6 +222,78 @@
222 222 }
223 223 }
224 224
225 + #[tokio::test]
226 + async fn multipart_parts_binds_the_client_checksum_into_the_signed_url() {
227 + // A checksum bound into the presigned URL is one S3 enforces: it rehashes
228 + // the part and rejects a mismatch before the bytes are durable. The client
229 + // can only supply one for a part it has already sealed, which is why it
230 + // asks for a single part at a time.
231 + use base64::Engine;
232 +
233 + let (mut h, _blobs) = harness_with_blobs().await;
234 + let user_id = h.signup("mp_sum", "mp_sum@example.com", "Password1!").await;
235 + let (app_id, _key) = create_internal_app(&h.db, user_id).await;
236 + seed_subscription(&h.db, user_id, app_id, "active", SYNCKIT_MAX_MULTIPART_BLOB_SIZE_BYTES).await;
237 + auth_as(&mut h, user_id, app_id, "user-key");
238 +
239 + let hash = fake_hash(0x18);
240 + let start: Value = h
241 + .client
242 + .post_json(
243 + "/api/sync/blobs/multipart/start",
244 + &json!({ "hash": hash, "size_bytes": LARGE_BLOB }).to_string(),
245 + )
246 + .await
247 + .json();
248 + let upload_id = start["upload_id"].as_str().unwrap().to_string();
249 +
250 + let checksum = base64::engine::general_purpose::STANDARD.encode([7u8; 32]);
251 + let parts: Value = h
252 + .client
253 + .post_json(
254 + "/api/sync/blobs/multipart/parts",
255 + &json!({
256 + "hash": hash, "upload_id": upload_id, "size_bytes": LARGE_BLOB,
257 + "first_part": 3, "count": 1, "checksums": [checksum],
258 + })
259 + .to_string(),
260 + )
261 + .await
262 + .json();
263 + let url = parts["parts"][0]["url"].as_str().unwrap();
264 + assert!(
265 + url.contains(&format!("checksum={checksum}")),
266 + "the checksum must reach the signer, got: {url}"
267 + );
268 + assert_eq!(parts["parts"][0]["part_number"].as_u64().unwrap(), 3);
269 +
270 + // Malformed or misaligned checksums are refused rather than signed: a
271 + // checksum bound to the wrong part would reject a correct upload.
272 + let bad = [
273 + json!({ "first_part": 1, "count": 1, "checksums": ["not base64!!"] }),
274 + json!({ "first_part": 1, "count": 1, "checksums": [base64::engine::general_purpose::STANDARD.encode([7u8; 16])] }),
275 + json!({ "first_part": 1, "count": 2, "checksums": [checksum] }),
276 + json!({ "first_part": 1, "count": 1, "checksums": [checksum, checksum] }),
277 + ];
278 + for case in bad {
279 + let mut body = json!({
280 + "hash": hash, "upload_id": upload_id, "size_bytes": LARGE_BLOB,
281 + });
282 + for (k, v) in case.as_object().unwrap() {
283 + body[k] = v.clone();
284 + }
285 + let resp = h
286 + .client
287 + .post_json("/api/sync/blobs/multipart/parts", &body.to_string())
288 + .await;
289 + assert!(
290 + resp.status.is_client_error(),
291 + "{case} must be refused: {}",
292 + resp.text
293 + );
294 + }
295 + }
296 +
225 297 #[tokio::test]
226 298 async fn multipart_complete_then_confirm_records_the_blob() {
227 299 // The end-to-end handoff. Confirm is untouched by multipart: it reads the
M shared/s3-storage/src/lib.rs +78 -1
@@ -683,6 +683,14 @@
683 683 /// defense-in-depth (not a hard, server-enforced ceiling) as
684 684 /// [`Self::presign_upload`] — see its docs for why the authoritative size
685 685 /// check still lives at confirm time.
686 + ///
687 + /// `checksum_sha256` (base64 of the raw 32-byte digest) is signed as
688 + /// `x-amz-checksum-sha256`, and unlike the length this one S3 *does*
689 + /// enforce: it hashes the received part and rejects a mismatch with
690 + /// `BadDigest` before the bytes are durable. The caller must therefore send
691 + /// the header — it is in `SignedHeaders`, so omitting it fails the
692 + /// signature. This is transport integrity (the bytes S3 wrote are the bytes
693 + /// the client hashed), not a statement about what those bytes mean.
686 694 pub async fn presign_upload_part(
687 695 &self,
688 696 key: &str,
@@ -690,6 +698,7 @@
690 698 part_number: i32,
691 699 expiry_secs: u64,
692 700 max_bytes: Option<i64>,
701 + checksum_sha256: Option<&str>,
693 702 ) -> Result<String, String> {
694 703 if !(1..=MULTIPART_MAX_PARTS as i32).contains(&part_number) {
695 704 return Err(format!(
@@ -712,6 +721,9 @@
712 721 if let Some(n) = max_bytes {
713 722 req = req.content_length(n);
714 723 }
724 + if let Some(c) = checksum_sha256 {
725 + req = req.checksum_sha256(c);
726 + }
715 727
716 728 let presigned = req
717 729 .presigned(presigning_config)
@@ -1254,6 +1266,44 @@
1254 1266 assert!(err.contains("non-empty"), "unexpected error: {err}");
1255 1267 }
1256 1268
1269 + /// The `X-Amz-SignedHeaders` list from a presigned URL.
1270 + fn signed_headers(url: &str) -> String {
1271 + url.split('&')
1272 + .find_map(|p| p.strip_prefix("X-Amz-SignedHeaders="))
1273 + .map(|v| v.replace("%3B", ";"))
1274 + .expect("presigned URL must carry X-Amz-SignedHeaders")
1275 + }
1276 +
1277 + #[tokio::test]
1278 + async fn presign_upload_signs_content_length_when_bound() {
1279 + // Callers rely on `max_bytes` being enforced, and it is enforced only
1280 + // because it lands in SignedHeaders: a client sending a different
1281 + // Content-Length then fails the signature. That also makes the declared
1282 + // size a hard contract — a caller that declares anything other than the
1283 + // exact body length breaks every upload — so pin it here rather than
1284 + // discovering it against production S3.
1285 + let client = test_client();
1286 +
1287 + let bound = client
1288 + .presign_upload("k", "application/octet-stream", 900, None, Some(12_345))
1289 + .await
1290 + .unwrap();
1291 + let headers = signed_headers(&bound);
1292 + assert!(
1293 + headers.contains("content-length"),
1294 + "max_bytes must be signed, got: {headers}"
1295 + );
1296 +
1297 + let unbound = client
1298 + .presign_upload("k", "application/octet-stream", 900, None, None)
1299 + .await
1300 + .unwrap();
1301 + assert!(
1302 + !signed_headers(&unbound).contains("content-length"),
1303 + "without max_bytes the client is free to send any length"
1304 + );
1305 + }
1306 +
1257 1307 #[tokio::test]
1258 1308 async fn presign_upload_part_rejects_out_of_range_part_number() {
1259 1309 // Pre-flight range check: fires before any network call, so the
@@ -1261,13 +1311,40 @@
1261 1311 let client = test_client();
1262 1312 for bad in [0, MULTIPART_MAX_PARTS as i32 + 1] {
1263 1313 let err = client
1264 - .presign_upload_part("k", "uid", bad, 3600, None)
1314 + .presign_upload_part("k", "uid", bad, 3600, None, None)
1265 1315 .await
1266 1316 .expect_err("out-of-range part number must be rejected");
1267 1317 assert!(err.contains("out of range"), "unexpected error: {err}");
1268 1318 }
1269 1319 }
1270 1320
1321 + #[tokio::test]
1322 + async fn presign_upload_part_signs_the_checksum_when_bound() {
1323 + // S3 enforces a bound checksum by rehashing the part, but only if the
1324 + // client sends the header — which it must, because signing it makes it
1325 + // mandatory. Both halves of that live in SignedHeaders.
1326 + let client = test_client();
1327 +
1328 + let bound = client
1329 + .presign_upload_part("k", "uid", 1, 900, Some(64), Some("Zm9vYmFyYmF6"))
1330 + .await
1331 + .unwrap();
1332 + let headers = signed_headers(&bound);
1333 + assert!(
1334 + headers.contains("x-amz-checksum-sha256"),
1335 + "a bound checksum must be signed, got: {headers}"
1336 + );
1337 +
1338 + let unbound = client
1339 + .presign_upload_part("k", "uid", 1, 900, Some(64), None)
1340 + .await
1341 + .unwrap();
1342 + assert!(
1343 + !signed_headers(&unbound).contains("checksum"),
1344 + "no checksum bound means no checksum header is required"
1345 + );
1346 + }
1347 +
1271 1348 #[tokio::test]
1272 1349 async fn complete_multipart_rejects_empty_parts() {
1273 1350 let client = test_client();
M shared/synckit-client/src/types.rs +5
M shared/synckit-client/tests/integration.rs +143 -17
M server/src/routes/synckit/blobs.rs +43 -11
M server/src/routes/synckit/mod.rs +8
M shared/synckit-client/src/client/blob.rs +51 -44
M server/src/routes/api/internal/uploads.rs +5