Skip to main content

max / makenotwork

Make the CLI upload confirm idempotent on replay (ultra-fuzz Run 12 Storage) The internal (ServiceAuth) confirm_upload is non-atomic — storage increment plus item/version writes — with a deterministic S3 key, so a retried confirm double-charged storage and, for downloads, created a duplicate version. Add an early replay guard: before any side effect, detect whether this exact key is already committed (item audio/video field set, or a version already references it) and return success without repeating the increment or write. Regression test proves two identical internal confirms charge storage exactly once. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-01 15:13 UTC
Commit: 03e2a7b6d164e6b85e3fdedd4e2201c2405ced71
Parent: eedac53
2 files changed, +89 insertions, -0 deletions
M server/src/routes/api/internal/uploads.rs +29
@@ -138,6 +138,35 @@ pub(super) async fn confirm_upload(
138 138 ));
139 139 }
140 140
141 + // Idempotent replay: the S3 key is deterministic (`{user}/{item}/...`), and this
142 + // handler is non-atomic (storage increment + item/version writes). A retried
143 + // confirm for a key already committed must not re-charge storage or create a
144 + // duplicate version — detect the committed state and return success without
145 + // repeating the side effects (ultra-fuzz Run 12 Storage: confirm idempotency).
146 + let already_committed = match file_type {
147 + FileType::Audio | FileType::Video => {
148 + match db::items::get_item_by_id(&state.db, req.item_id).await? {
149 + Some(item) if file_type == FileType::Audio => {
150 + item.audio_s3_key.as_deref() == Some(req.s3_key.as_str())
151 + }
152 + Some(item) => item.video_s3_key.as_deref() == Some(req.s3_key.as_str()),
153 + None => false,
154 + }
155 + }
156 + FileType::Download => db::versions::get_versions_by_item(&state.db, req.item_id)
157 + .await?
158 + .iter()
159 + .any(|v| v.s3_key.as_deref() == Some(req.s3_key.as_str())),
160 + _ => false,
161 + };
162 + if already_committed {
163 + tracing::info!(
164 + user = %req.user_id, item = %req.item_id, s3_key = %req.s3_key,
165 + "CLI upload confirm replay — already committed, skipping duplicate side effects"
166 + );
167 + return Ok(Json(InternalConfirmResponse { success: true }));
168 + }
169 +
141 170 // Enforce file size limit
142 171 let file_size_bytes = s3.object_size(&req.s3_key).await?.ok_or_else(|| {
143 172 AppError::BadRequest("Could not determine file size. Please try uploading again.".to_string())
M server/tests/workflows/storage.rs +60
@@ -86,6 +86,66 @@ async fn confirm_upload_audio_updates_db() {
86 86 }
87 87
88 88 #[tokio::test]
89 + async fn internal_confirm_upload_replay_is_idempotent() {
90 + // A retried internal (CLI, ServiceAuth) confirm for the same deterministic key
91 + // must not double-charge storage (ultra-fuzz Run 12 Storage: confirm idempotency).
92 + let mem = std::sync::Arc::new(crate::harness::storage::InMemoryStorage::new());
93 + let mut h = TestHarness::build(crate::harness::BuildOptions {
94 + storage: Some(mem),
95 + cli_service_token: Some("test-cli-token".to_string()),
96 + ..Default::default()
97 + })
98 + .await;
99 +
100 + let setup = h.create_creator_with_item("cliuser", "audio", 0).await;
101 + h.trust_user(setup.user_id).await;
102 + h.grant_tier(setup.user_id, "small_files").await;
103 + let user_id = setup.user_id;
104 + let item_id = setup.item_id.clone();
105 +
106 + // Presign (session auth) then simulate the client PUT to S3.
107 + let presign = json!({
108 + "item_id": item_id, "file_type": "audio",
109 + "file_name": "song.mp3", "content_type": "audio/mpeg",
110 + });
111 + let resp = h.client.post_json("/api/upload/presign", &presign.to_string()).await;
112 + assert!(resp.status.is_success(), "presign failed: {}", resp.text);
113 + let s3_key = resp.json::<Value>()["s3_key"].as_str().unwrap().to_string();
114 + let bytes = b"fake mp3 file bytes".to_vec();
115 + let expected_size = bytes.len() as i64;
116 + h.storage.as_ref().unwrap().put(&s3_key, bytes);
117 +
118 + // Internal confirm (ServiceAuth via bearer) — twice, identical.
119 + let confirm = json!({
120 + "user_id": user_id, "item_id": item_id,
121 + "file_type": "audio", "s3_key": s3_key,
122 + });
123 + h.client.set_bearer_token("test-cli-token");
124 + let r1 = h.client.post_json("/api/internal/upload/confirm", &confirm.to_string()).await;
125 + assert!(r1.status.is_success(), "first internal confirm failed: {}", r1.text);
126 + let r2 = h.client.post_json("/api/internal/upload/confirm", &confirm.to_string()).await;
127 + assert!(r2.status.is_success(), "replayed internal confirm failed: {}", r2.text);
128 + h.client.clear_bearer_token();
129 +
130 + // The key is committed once and storage is charged exactly once, not twice.
131 + let db_key: Option<String> =
132 + sqlx::query_scalar("SELECT audio_s3_key FROM items WHERE id = $1::uuid")
133 + .bind(&item_id)
134 + .fetch_one(&h.db)
135 + .await
136 + .unwrap();
137 + assert_eq!(db_key.as_deref(), Some(s3_key.as_str()));
138 +
139 + let storage_used: i64 =
140 + sqlx::query_scalar("SELECT storage_used_bytes FROM users WHERE id = $1")
141 + .bind(user_id)
142 + .fetch_one(&h.db)
143 + .await
144 + .unwrap();
145 + assert_eq!(storage_used, expected_size, "replay must not double-charge storage");
146 + }
147 +
148 + #[tokio::test]
89 149 async fn confirm_item_cover_via_dedicated_route_writes_key_and_url() {
90 150 // Covers go through /api/items/image/{presign,confirm}, which writes
91 151 // cover_s3_key, cover_file_size_bytes AND cover_image_url together. The