Skip to main content

max / makenotwork

server: enqueue old S3 key inside the replace tx (ultra-fuzz Run #1 Storage LOW) On a cover/version replace, the superseded object's deletion was enqueued AFTER tx.commit() (best-effort, swallowed warning), so a crash in that gap orphaned the old object with no durable record - the same window the delete paths closed by enqueuing in-tx (Run #18 B6). Move the old-key enqueue inside the commit tx at all three replace sites (version, project cover, item cover). After commit the row points at the new key, so the old key is non-live; is_s3_key_live guards the worker against any live reference. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-19 21:30 UTC
Commit: fe2aeeae7291b9bde8e0e4027b281455a7cec14e
Parent: 0268eb4
2 files changed, +40 insertions, -29 deletions
M server/src/routes/storage/images.rs +25 -19
@@ -248,6 +248,18 @@ pub(super) async fn project_image_confirm(
248 248 if !ok {
249 249 return Ok(false);
250 250 }
251 + // Enqueue the OLD object for deletion in the SAME tx as the row UPDATE,
252 + // closing the crash-between-commit-and-enqueue orphan window (ultra-fuzz
253 + // Run #1 Storage LOW). is_s3_key_live guards the worker if a row still
254 + // references it.
255 + if let Some(old_key) = old_key_to_delete.as_deref() {
256 + db::pending_s3_deletions::enqueue_deletions(
257 + &mut *tx,
258 + &[(old_key.to_string(), "main".to_string())],
259 + "project_image_replace",
260 + )
261 + .await?;
262 + }
251 263 tx.commit().await?;
252 264 Ok(true)
253 265 }
@@ -271,16 +283,7 @@ pub(super) async fn project_image_confirm(
271 283 // Clear the pending upload record now that the upload is committed
272 284 db::pending_uploads::remove_pending_upload(&state.db, user.id, &req.s3_key, "main").await?;
273 285
274 - // Enqueue old S3 object for durable deletion now that the new URL is committed.
275 - if let Some(old_key) = old_key_to_delete
276 - && let Err(e) = db::pending_s3_deletions::enqueue_deletions(
277 - &state.db,
278 - &[(old_key.clone(), "main".to_string())],
279 - "project_image_replace",
280 - ).await
281 - {
282 - tracing::warn!(key = %old_key, error = ?e, "failed to enqueue old project image for deletion");
283 - }
286 + // (The old S3 object was enqueued for deletion inside the commit tx above.)
284 287
285 288 // Scan enqueue AFTER the DB write commits (Phase 5 chronic fix — the same
286 289 // ordering rule that uploads/versions/media follow via `commit_upload`).
@@ -473,6 +476,17 @@ pub(super) async fn item_image_confirm(
473 476 if !ok {
474 477 return Ok(false);
475 478 }
479 + // Enqueue the OLD cover for deletion in the SAME tx as the row UPDATE
480 + // (ultra-fuzz Run #1 Storage LOW; matches the in-tx ordering the delete
481 + // paths use). is_s3_key_live guards the worker against a live reference.
482 + if let Some(old_key) = old_key_to_delete.as_deref() {
483 + db::pending_s3_deletions::enqueue_deletions(
484 + &mut *tx,
485 + &[(old_key.to_string(), "main".to_string())],
486 + "item_image_replace",
487 + )
488 + .await?;
489 + }
476 490 tx.commit().await?;
477 491 Ok(true)
478 492 }
@@ -494,15 +508,7 @@ pub(super) async fn item_image_confirm(
494 508
495 509 db::pending_uploads::remove_pending_upload(&state.db, user.id, &req.s3_key, "main").await?;
496 510
497 - if let Some(old_key) = old_key_to_delete
498 - && let Err(e) = db::pending_s3_deletions::enqueue_deletions(
499 - &state.db,
500 - &[(old_key.clone(), "main".to_string())],
501 - "item_image_replace",
502 - ).await
503 - {
504 - tracing::warn!(key = %old_key, error = ?e, "failed to enqueue old item cover for deletion");
505 - }
511 + // (The old cover was enqueued for deletion inside the commit tx above.)
506 512
507 513 // Scan enqueue + scan_status flip AFTER the DB write — same ordering rule
508 514 // as uploads/versions/media. The Run #6 audit caught this same bug here.
M server/src/routes/storage/versions.rs +15 -10
@@ -208,6 +208,20 @@ pub(super) async fn version_confirm_upload(
208 208 // Lost race — drop tx to roll back the storage change.
209 209 return Ok(false);
210 210 }
211 + // Enqueue the OLD key for deletion in the SAME tx as the row UPDATE, so
212 + // a crash between commit and a post-commit enqueue can't orphan it with
213 + // no durable record (ultra-fuzz Run #1 Storage LOW; mirrors the in-tx
214 + // ordering delete_version uses). After commit the row points at the new
215 + // key, so the old key is non-live; the worker's is_s3_key_live check is
216 + // the backstop if anything still references it.
217 + if let Some(old_key) = old_s3_key.as_deref() {
218 + db::pending_s3_deletions::enqueue_deletions(
219 + &mut *tx,
220 + &[(old_key.to_string(), "main".to_string())],
221 + "version_replace",
222 + )
223 + .await?;
224 + }
211 225 tx.commit().await?;
212 226 Ok(true)
213 227 }
@@ -250,16 +264,7 @@ pub(super) async fn version_confirm_upload(
250 264 file_size_bytes,
251 265 ).await?;
252 266
253 - // Enqueue old S3 key for deletion now that the DB record points to the new key
254 - if let Some(old_key) = old_s3_key
255 - && let Err(e) = db::pending_s3_deletions::enqueue_deletions(
256 - &state.db,
257 - &[(old_key, "main".to_string())],
258 - "version_replace",
259 - ).await
260 - {
261 - tracing::warn!(error = ?e, "failed to enqueue old version S3 key for deletion");
262 - }
267 + // (The old S3 key was enqueued for deletion inside the commit tx above.)
263 268
264 269 tracing::info!(
265 270 key = %req.s3_key,