Skip to main content

max / makenotwork

Stop cover-image replace from taking published tracks offline A cover upload flipped the shared items.scan_status to Pending, and the download gate (downloads.rs) then NotFounds the audio/video for every non-creator until the cover re-scans — silently delisting a published track. The flip protected nothing: the cover is served via the permanent cover_image_url CDN URL with no per-request gate, like project/gallery images. Treat ItemImage as a no-scan-status-column kind: commit_upload, commit_rescan, and the worker no longer flip items.scan_status for a cover. Quarantine enforcement routes through is_cdn_served_without_gate and NULLs the cover columns in place (cover_s3_key/cover_image_url/ cover_file_size_bytes) rather than deleting the item row, so the track survives and the key still goes dead for the durable-deletion queue. Regression tests: cover_replace_does_not_take_published_track_offline, quarantined_cover_nulls_columns_but_keeps_published_track. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-15 22:19 UTC
Commit: e218483294e57d92a7c174cefd9c1fbfbef4a92d
Parent: 2c4c8a9
6 files changed, +181 insertions, -27 deletions
M server/src/db/scan_jobs.rs +15 -6
@@ -64,13 +64,19 @@ impl ScanTargetKind {
64 64 /// rendering and makes the key non-live for the deletion queue).
65 65 ///
66 66 /// This governs the DB-row step only. The S3-object purge on quarantine is
67 - /// unconditional (see [`quarantine_purges_object`]); these three kinds
68 - /// additionally need their row removed because they have no status column
69 - /// to gate on.
67 + /// unconditional (see [`quarantine_purges_object`]); these kinds
68 + /// additionally need their row removed (or, for `ItemImage`, the cover
69 + /// columns NULLed) because they have no status column to gate on.
70 + ///
71 + /// `ItemImage` is here because the item *cover* (`items.cover_image_url`) is
72 + /// rendered straight from the CDN with no per-request gate — `items.scan_status`
73 + /// gates only the audio/video. On quarantine the purge NULLs the cover columns
74 + /// (keeping the track), it does NOT delete the item row.
70 75 pub fn is_cdn_served_without_gate(&self) -> bool {
71 76 matches!(
72 77 self,
73 - ScanTargetKind::ProjectImage
78 + ScanTargetKind::ItemImage
79 + | ScanTargetKind::ProjectImage
74 80 | ScanTargetKind::GalleryImage
75 81 | ScanTargetKind::ContentInsertion
76 82 )
@@ -405,10 +411,13 @@ mod tests {
405 411 fn only_gateless_image_kinds_delete_their_row() {
406 412 // The DB-row deletion is the gate-less-image-only half of enforcement.
407 413 use ScanTargetKind::*;
408 - for kind in [ProjectImage, GalleryImage, ContentInsertion] {
414 + // ItemImage is gate-less: the item *cover* is CDN-served with no
415 + // per-request gate; `items.scan_status` gates the audio/video, not the
416 + // cover (Run #20 Storage SERIOUS).
417 + for kind in [ItemImage, ProjectImage, GalleryImage, ContentInsertion] {
409 418 assert!(kind.is_cdn_served_without_gate(), "{} is gate-less", kind.as_str());
410 419 }
411 - for kind in [Item, Version, Media, ItemImage] {
420 + for kind in [Item, Version, Media] {
412 421 assert!(!kind.is_cdn_served_without_gate(), "{} has a gate/status", kind.as_str());
413 422 }
414 423 }
M server/src/db/scanning.rs +27 -9
@@ -42,18 +42,25 @@ pub struct HeldVersionRow {
42 42
43 43 /// Insert a scan result record for audit trail.
44 44 #[tracing::instrument(skip_all)]
45 - /// Remove every CDN-served image row referencing `s3_key`, across the three
46 - /// image tables that have no per-row scan gate: `item_images.s3_key`,
47 - /// `project_images.s3_key`, and `content_insertions.storage_key`. Returns the
48 - /// number of rows removed.
45 + /// Remove every CDN-served image reference to `s3_key`, across the image
46 + /// surfaces that have no per-row scan gate: the gallery tables
47 + /// (`item_images.s3_key`, `project_images.s3_key`, `content_insertions.storage_key`)
48 + /// and the item *cover* columns on `items` (`cover_s3_key`/`cover_image_url`).
49 + /// Returns the number of rows affected.
49 50 ///
50 - /// On quarantine of these kinds, deleting the row IS the primary enforcement: it
51 - /// stops the app from ever rendering the (Cloudflare-served) URL again, and —
52 - /// critically — makes the key non-live so the durable S3-deletion queue will
53 - /// actually purge the object instead of parking it behind the `is_s3_key_live`
54 - /// guard. `storage_used` counters self-heal on the weekly
51 + /// On quarantine of these kinds, removing the reference IS the primary
52 + /// enforcement: it stops the app from ever rendering the (Cloudflare-served) URL
53 + /// again, and — critically — makes the key non-live so the durable S3-deletion
54 + /// queue will actually purge the object instead of parking it behind the
55 + /// `is_s3_key_live` guard. `storage_used` counters self-heal on the weekly
55 56 /// `recalculate_all_storage_used` pass; we accept a transient over-count for a
56 57 /// malicious upload rather than join through three ownership paths here.
58 + ///
59 + /// The item cover is a special case: the bad image lives in columns ON the
60 + /// `items` row, so we NULL the three cover columns rather than DELETE the row —
61 + /// deleting it would take the legitimate audio/video track down with the
62 + /// thumbnail. Both cover references (`cover_s3_key` exact + `cover_image_url`
63 + /// suffix) are registered in `S3_KEY_REFS`, so NULLing both makes the key dead.
57 64 #[tracing::instrument(skip_all)]
58 65 pub async fn purge_cdn_image_rows_by_key(db: &PgPool, s3_key: &str) -> Result<u64, sqlx::Error> {
59 66 let mut removed = 0u64;
@@ -64,6 +71,17 @@ pub async fn purge_cdn_image_rows_by_key(db: &PgPool, s3_key: &str) -> Result<u6
64 71 ] {
65 72 removed += sqlx::query(sql).bind(s3_key).execute(db).await?.rows_affected();
66 73 }
74 + // Item cover: NULL the columns in place, keeping the track. Matches on the
75 + // exact key in `cover_s3_key` (the cover_image_url suffix is cleared in the
76 + // same statement so neither reference keeps the object live).
77 + removed += sqlx::query(
78 + "UPDATE items SET cover_s3_key = NULL, cover_image_url = NULL, \
79 + cover_file_size_bytes = NULL, updated_at = NOW() WHERE cover_s3_key = $1",
80 + )
81 + .bind(s3_key)
82 + .execute(db)
83 + .await?
84 + .rows_affected();
67 85 Ok(removed)
68 86 }
69 87
M server/src/routes/storage/mod.rs +20 -7
@@ -180,7 +180,11 @@ pub(crate) enum CommitTarget {
180 180 /// project images do not carry a per-row scan_status column — the worker
181 181 /// only logs and creates a WAM ticket on quarantine).
182 182 ProjectImage(db::ProjectId),
183 - /// An item image (`items.cover_s3_key`); shares the item's scan_status.
183 + /// An item cover image (`items.cover_s3_key`/`cover_image_url`). Like
184 + /// `ProjectImage`/`GalleryImage` it carries NO per-row scan_status: the
185 + /// cover is CDN-served with no per-request gate, and `items.scan_status`
186 + /// gates the *audio/video*, not the cover. The worker logs + tickets on
187 + /// quarantine and NULLs the cover columns.
184 188 ItemImage(db::ItemId),
185 189 /// A gallery image row (`item_images`/`project_images`); like ProjectImage,
186 190 /// no per-row scan_status column — the worker logs + tickets on quarantine.
@@ -257,7 +261,7 @@ pub(crate) async fn commit_upload(
257 261 )
258 262 .await?;
259 263 match target {
260 - CommitTarget::Item(id) | CommitTarget::ItemImage(id) => {
264 + CommitTarget::Item(id) => {
261 265 db::scanning::update_item_scan_status(&state.db, id, scan_status).await?;
262 266 }
263 267 CommitTarget::Version(id) => {
@@ -266,10 +270,17 @@ pub(crate) async fn commit_upload(
266 270 CommitTarget::Media(id) => {
267 271 db::scanning::update_media_file_scan_status(&state.db, id, scan_status).await?;
268 272 }
269 - CommitTarget::ProjectImage(_)
273 + CommitTarget::ItemImage(_)
274 + | CommitTarget::ProjectImage(_)
270 275 | CommitTarget::GalleryImage(_)
271 276 | CommitTarget::ContentInsertion(_) => {
272 - // No per-row scan_status column; worker logs + creates WAM ticket.
277 + // No per-row scan_status column to flip. `ItemImage` writes the item
278 + // *cover* (`items.cover_s3_key`/`cover_image_url`, CDN-served with no
279 + // per-request gate), NOT the audio/video that `items.scan_status`
280 + // gates — flipping that column would take the already-published track
281 + // offline for every fan until the cover re-scan finished (Run #20
282 + // Storage SERIOUS). The worker logs + creates a WAM ticket and, on
283 + // quarantine, NULLs the cover columns; the track stays online.
273 284 }
274 285 }
275 286 tracing::Span::current().record("scan_status", tracing::field::debug(&scan_status));
@@ -306,7 +317,7 @@ pub(crate) async fn commit_rescan(
306 317 .await?;
307 318 let pending = db::FileScanStatus::Pending;
308 319 match target {
309 - CommitTarget::Item(id) | CommitTarget::ItemImage(id) => {
320 + CommitTarget::Item(id) => {
310 321 db::scanning::update_item_scan_status(&state.db, id, pending).await?;
311 322 }
312 323 CommitTarget::Version(id) => {
@@ -315,10 +326,12 @@ pub(crate) async fn commit_rescan(
315 326 CommitTarget::Media(id) => {
316 327 db::scanning::update_media_file_scan_status(&state.db, id, pending).await?;
317 328 }
318 - CommitTarget::ProjectImage(_)
329 + CommitTarget::ItemImage(_)
330 + | CommitTarget::ProjectImage(_)
319 331 | CommitTarget::GalleryImage(_)
320 332 | CommitTarget::ContentInsertion(_) => {
321 - // No per-row scan_status column.
333 + // No per-row scan_status column. An `ItemImage` rescan must not flip
334 + // `items.scan_status` — that gates the audio/video, not the cover.
322 335 }
323 336 }
324 337 Ok(pending)
M server/src/scanning/worker.rs +8 -5
@@ -389,9 +389,11 @@ async fn run_pipeline_and_decide(
389 389 }
390 390
391 391 /// Update the per-entity `scan_status` column for the kinds that have one.
392 - /// `ProjectImage` / `ContentInsertion` don't carry their own column today —
393 - /// the worker still scanned the file and recorded results, but there's no
394 - /// status to flip on those entities.
392 + /// `ItemImage` / `ProjectImage` / `GalleryImage` / `ContentInsertion` don't
393 + /// carry their own column — the worker still scanned the file and recorded
394 + /// results, but there's no status to flip on those entities (the `ItemImage`
395 + /// cover shares the `items` row but must NOT flip `items.scan_status`, which
396 + /// gates the audio/video, not the cover).
395 397 async fn update_entity_status(
396 398 db: &PgPool,
397 399 kind: ScanTargetKind,
@@ -402,7 +404,7 @@ async fn update_entity_status(
402 404 ScanTargetKind::Version => {
403 405 db::scanning::update_version_scan_status(db, VersionId::from(target_id), status).await
404 406 }
405 - ScanTargetKind::Item | ScanTargetKind::ItemImage => {
407 + ScanTargetKind::Item => {
406 408 db::scanning::update_item_scan_status(db, ItemId::from(target_id), status).await
407 409 }
408 410 ScanTargetKind::Media => {
@@ -413,7 +415,8 @@ async fn update_entity_status(
413 415 )
414 416 .await
415 417 }
416 - ScanTargetKind::ProjectImage
418 + ScanTargetKind::ItemImage
419 + | ScanTargetKind::ProjectImage
417 420 | ScanTargetKind::GalleryImage
418 421 | ScanTargetKind::ContentInsertion => Ok(()),
419 422 }
M server/tests/workflows/scanning.rs +54
@@ -118,6 +118,60 @@ async fn confirm_upload_bad_magic_quarantined() {
118 118 assert_eq!(scan_status, "quarantined");
119 119 }
120 120
121 + #[tokio::test]
122 + async fn quarantined_cover_nulls_columns_but_keeps_published_track() {
123 + // Run #20 Storage SERIOUS (flip side): a cover is CDN-served with no
124 + // per-request gate, so enforcing a quarantine verdict NULLs the cover
125 + // columns (stopping the URL from rendering) rather than flipping the
126 + // shared `items.scan_status`. The legitimate audio track and its Clean
127 + // gate status must survive — a malicious thumbnail can't delist a track.
128 + let mut h = TestHarness::with_storage_and_scanner().await;
129 + let (_project_id, item_id) = setup_creator_with_item(&mut h).await;
130 +
131 + // Publish a clean audio track first.
132 + let body = json!({"item_id": item_id, "file_type": "audio", "file_name": "t.mp3", "content_type": "audio/mpeg"});
133 + let resp = h.client.post_json("/api/upload/presign", &body.to_string()).await;
134 + assert!(resp.status.is_success(), "audio presign: {}", resp.text);
135 + let audio_key = resp.json::<Value>()["s3_key"].as_str().unwrap().to_string();
136 + let mut mp3 = b"ID3".to_vec();
137 + mp3.extend_from_slice(&[0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]);
138 + mp3.extend_from_slice(&[0u8; 100]);
139 + h.storage.as_ref().unwrap().put(&audio_key, mp3);
140 + let body = json!({"item_id": item_id, "file_type": "audio", "s3_key": audio_key});
141 + let resp = h.client.post_json("/api/upload/confirm", &body.to_string()).await;
142 + assert!(resp.status.is_success(), "audio confirm: {}", resp.text);
143 +
144 + // Upload a malicious cover (ELF magic disguised as a png) — the worker
145 + // quarantines it the same way it does the bad-magic audio above.
146 + let body = json!({"item_id": item_id, "file_name": "art.png", "content_type": "image/png"});
147 + let resp = h.client.post_json("/api/items/image/presign", &body.to_string()).await;
148 + assert!(resp.status.is_success(), "cover presign: {}", resp.text);
149 + let cover_key = resp.json::<Value>()["s3_key"].as_str().unwrap().to_string();
150 + let mut elf = vec![0x7f, b'E', b'L', b'F'];
151 + elf.extend_from_slice(&[0x02, 0x01, 0x01, 0x00]);
152 + elf.extend_from_slice(&[0u8; 100]);
153 + h.storage.as_ref().unwrap().put(&cover_key, elf);
154 + let body = json!({"item_id": item_id, "s3_key": cover_key});
155 + let resp = h.client.post_json("/api/items/image/confirm", &body.to_string()).await;
156 + assert!(resp.status.is_success(), "cover confirm: {}", resp.text);
157 +
158 + h.drain_scan_jobs().await;
159 +
160 + let (status, audio, ck, cu): (String, Option<String>, Option<String>, Option<String>) =
161 + sqlx::query_as(
162 + "SELECT scan_status, audio_s3_key, cover_s3_key, cover_image_url \
163 + FROM items WHERE id = $1::uuid",
164 + )
165 + .bind(&item_id)
166 + .fetch_one(&h.db)
167 + .await
168 + .expect("the item row must still exist — quarantining a cover must not delete the track");
169 + assert_eq!(status, "clean", "a quarantined cover must not touch the track's gate status");
170 + assert_eq!(audio.as_deref(), Some(audio_key.as_str()), "the audio track must be preserved");
171 + assert_eq!(ck, None, "the quarantined cover key must be NULLed");
172 + assert_eq!(cu, None, "the quarantined cover URL must be NULLed so it stops rendering");
173 + }
174 +
121 175 // ============================================================================
122 176 // Upload Trust Tier Tests
123 177 // ============================================================================
M server/tests/workflows/storage.rs +57
@@ -124,6 +124,63 @@ async fn confirm_item_cover_via_dedicated_route_writes_key_and_url() {
124 124 }
125 125
126 126 #[tokio::test]
127 + async fn cover_replace_does_not_take_published_track_offline() {
128 + // Run #20 Storage SERIOUS: a cover upload used to flip the SHARED
129 + // `items.scan_status` to Pending (cover shares the audio's row), and the
130 + // stream/download gate returns NotFound for non-creators until the cover
131 + // re-scan finishes — silently pulling an already-published track offline
132 + // for every fan. The cover is CDN-served with no per-request gate, so the
133 + // flip protects nothing and only harms the track. Confirming a cover must
134 + // leave `items.scan_status` untouched.
135 + let mut h = TestHarness::with_storage_and_scanner().await;
136 + let (_, _, item_id) = setup_creator_with_item(&mut h, 0).await;
137 +
138 + // Simulate the published state: track already scanned Clean.
139 + sqlx::query("UPDATE items SET scan_status = 'clean' WHERE id = $1::uuid")
140 + .bind(&item_id)
141 + .execute(&h.db)
142 + .await
143 + .unwrap();
144 +
145 + // Upload a new cover. With a scanner configured, the cover scan enqueues as
146 + // Pending; pre-fix that Pending was written straight onto items.scan_status
147 + // by the synchronous confirm.
148 + let body = json!({ "item_id": item_id, "file_name": "art.png", "content_type": "image/png" });
149 + let resp = h.client.post_json("/api/items/image/presign", &body.to_string()).await;
150 + assert!(resp.status.is_success(), "presign failed: {}", resp.text);
151 + let s3_key = resp.json::<Value>()["s3_key"].as_str().unwrap().to_string();
152 + h.storage.as_ref().unwrap().put(&s3_key, b"fake png bytes".to_vec());
153 +
154 + let body = json!({ "item_id": item_id, "s3_key": s3_key });
155 + let resp = h.client.post_json("/api/items/image/confirm", &body.to_string()).await;
156 + assert!(resp.status.is_success(), "cover confirm failed: {}", resp.text);
157 +
158 + // The track's gate status must still be Clean — the cover upload may not
159 + // touch it. (Assert synchronously, before draining the scan worker.)
160 + let scan_status: String = sqlx::query_scalar(
161 + "SELECT scan_status FROM items WHERE id = $1::uuid",
162 + )
163 + .bind(&item_id)
164 + .fetch_one(&h.db)
165 + .await
166 + .unwrap();
167 + assert_eq!(
168 + scan_status, "clean",
169 + "cover upload must not flip the track's scan_status (would take it offline for fans)"
170 + );
171 +
172 + // And the cover itself still landed.
173 + let cover_key: Option<String> = sqlx::query_scalar(
174 + "SELECT cover_s3_key FROM items WHERE id = $1::uuid",
175 + )
176 + .bind(&item_id)
177 + .fetch_one(&h.db)
178 + .await
179 + .unwrap();
180 + assert_eq!(cover_key.as_deref(), Some(s3_key.as_str()));
181 + }
182 +
183 + #[tokio::test]
127 184 async fn confirm_upload_rejects_cover() {
128 185 // The generic confirm route must refuse cover and point at the dedicated
129 186 // route, rather than half-writing the row (no cover_image_url). Run #13.