Skip to main content

max / makenotwork

Scan-then-promote uploads to immutable content keys (C1) Uploads presigned a client PUT to a deterministic, served, mutable key, so the owner could re-PUT malware to it after the scanner marked it Clean — the bytes served to a buyer were not provably the bytes that were scanned (across all 8 scanned entity kinds). Now every presign hands out an unserved staging/{uuid}/{filename} key. On a Clean verdict the scan worker copies the object to a content-addressed {owner}/c/{sha256}.{ext} key, repoints the entity (rebuilding image URLs), marks it clean, and enqueues the staging object for deletion — copy first, then an atomic repoint+delete-enqueue, so any failure leaves the job retryable on the unserved staging key. A shared promote_staging_to_content helper backs both the worker Clean path and the admin approve-held path (per-row and bulk) so they can't diverge. Confirm handlers prove ownership via pending_uploads (a staging key carries no owner in its path) instead of a key-prefix check, placed after each handler's idempotent re-confirm. A grep guard test fails the build if any route mints a served key — only the worker promote may. This also closes S1: a content key is not derivable from {owner_id, item_id}. Also fixes filename preservation (staging key keeps the sanitized name), media logical-name handling (folder/filename from the request again, guarded by the unique index), and leaves build-runner OTA server-side uploads on their final keys.
Co-Authored-By
Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-07 04:02 UTC
Signed with PGP, not checked
Commit: ed08ae5d31983f770487712456f1604ca134bbcf
Parent: dae1711
21 files changed, +758 insertions, -142 deletions
M server/src/storage.rs +94 -8
@@ -530,11 +530,16 @@
530 530 /// its content-addressed [`content_key`](Self::content_key) and deletes the
531 531 /// staging object. The upload's extension is carried in the staging key so
532 532 /// the worker can build the content key without re-reading the entity row.
533 - /// Format: `staging/{uuid}.{ext}`. The random uuid means a replayed presigned
534 - /// PUT can only re-write the (unserved, post-scan-deleted) staging object,
535 - /// never the served content key (ultra-fuzz Run #24 Storage HIGH).
533 + /// Format: `staging/{uuid}/{sanitized_filename}`. The random uuid segment
534 + /// means a replayed presigned PUT can only re-write the (unserved,
535 + /// post-scan-deleted) staging object, never the served content key (ultra-fuzz
536 + /// Run #24 Storage HIGH), and two uploads of the same filename never collide.
537 + /// The original filename is preserved after the uuid so a confirm can recover
538 + /// it (e.g. a version download's suggested name) — `sanitize_filename` strips
539 + /// any `/`, so the name can't add path segments or escape the `staging/`
540 + /// prefix. The extension still rides along for the content key.
536 541 pub fn generate_staging_key(filename: &str) -> S3Key {
537 - S3Key(format!("staging/{}.{}", uuid::Uuid::new_v4(), extension_for(filename)))
542 + S3Key(format!("staging/{}/{}", uuid::Uuid::new_v4(), sanitize_filename(filename)))
538 543 }
539 544
540 545 /// Content-addressed served key: `{user_id}/c/{sha256}.{ext}`. The object's
@@ -895,6 +900,10 @@
895 900 /// filename can't bloat the key. Content keys carry an extension purely so
896 901 /// CDN-served objects keep a sensible suffix (content-type sniffing, browser
897 902 /// "save as"); the hash is the identity, the extension is cosmetic.
903 + // Retained as a tested key-extension utility; `generate_staging_key` now embeds
904 + // the full sanitized filename (which carries the extension) instead of calling
905 + // this, so it has no production caller today.
906 + #[allow(dead_code)]
898 907 pub(crate) fn extension_for(filename: &str) -> String {
899 908 let ext: String = std::path::Path::new(filename)
900 909 .extension()
@@ -909,11 +918,8 @@
909 918 }
910 919
911 920 /// The extension segment of a key's basename (the text after the last `.`), or
912 - /// `"bin"`. Lets the scan worker carry a `staging/{uuid}.{ext}` object's
921 + /// `"bin"`. Lets the scan worker's promote step carry a staging object's
913 922 /// extension onto its content key without re-reading the entity row.
914 - // Consumed by the scan worker's copy-on-clean step (Phase 2 of the
915 - // content-addressed scan-integrity change); tested now as a Phase 1 foundation.
916 - #[allow(dead_code)]
917 923 pub(crate) fn key_extension(key: &str) -> &str {
918 924 key.rsplit('/')
919 925 .next()
@@ -1689,3 +1695,83 @@
1689 1695 }
1690 1696 }
1691 1697 }
1698 +
1699 + /// C1 scan-then-promote seal: route handlers must never mint a *served* S3 key.
1700 + ///
1701 + /// A presigned client upload can only ever land at a `staging/{uuid}` key
1702 + /// ([`S3Client::generate_staging_key`]); the served, content-addressed key
1703 + /// ([`S3Client::content_key`]) is created in exactly one place — the scan
1704 + /// worker's promote step, after a Clean verdict — so the bytes a buyer is served
1705 + /// are provably the bytes that were scanned. The mutable-served-key class (a
1706 + /// presign minting `{user}/{item}/type/filename`, then the owner re-PUTting to it
1707 + /// after it goes Clean) is what this closes.
1708 + ///
1709 + /// This guard fails the build if any file under `src/routes/` names a served-key
1710 + /// generator or `content_key`. It is stronger than `pub(crate)` visibility —
1711 + /// route code lives in the same crate, so `pub(crate)` would not stop it from
1712 + /// calling these — and it is the same grep-proof discipline as the delete seal
1713 + /// above. (The build runner uploads OTA artifacts server-side to a deterministic
1714 + /// key via `generate_ota_artifact_key`; it lives outside `src/routes/`, so it is
1715 + /// legitimately unaffected.)
1716 + #[cfg(test)]
1717 + mod served_key_seal_guard {
1718 + use std::path::Path;
1719 +
1720 + #[test]
1721 + fn routes_never_mint_served_keys() {
1722 + let routes_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("src/routes");
1723 + // Every served-key generator, plus the content-key minter. `staging`
1724 + // keys are the ONLY key a route may mint, so `generate_staging_key` is
1725 + // deliberately absent from this list.
1726 + // Anchored to the `S3Client::` call prefix so an unrelated `generate_key`
1727 + // (e.g. `license_keys::generate_key`, `helpers::generate_key_code`) is not
1728 + // a false positive — only the storage generators are S3Client methods.
1729 + const FORBIDDEN: &[&str] = &[
1730 + "S3Client::generate_key(",
1731 + "S3Client::generate_version_key(",
1732 + "S3Client::generate_insertion_key(",
1733 + "S3Client::generate_media_key(",
1734 + "S3Client::generate_project_image_key(",
1735 + "S3Client::generate_ota_artifact_key(",
1736 + "S3Client::generate_item_gallery_key(",
1737 + "S3Client::generate_project_gallery_key(",
1738 + "S3Client::content_key(",
1739 + ];
1740 + let mut offenders = Vec::new();
1741 + walk(&routes_dir, &mut |path, contents| {
1742 + for (i, line) in contents.lines().enumerate() {
1743 + if line.trim_start().starts_with("//") {
1744 + continue;
1745 + }
1746 + for needle in FORBIDDEN {
1747 + if line.contains(needle) {
1748 + offenders.push(format!("{}:{}: {}", path.display(), i + 1, line.trim()));
1749 + }
1750 + }
1751 + }
1752 + });
1753 + assert!(
1754 + offenders.is_empty(),
1755 + "C1 seal violated — route handlers must presign only `generate_staging_key`; \
1756 + the served/content key is minted solely by the scan worker's promote step. \
1757 + Offending lines:\n{}",
1758 + offenders.join("\n")
1759 + );
1760 + }
1761 +
1762 + fn walk(dir: &Path, f: &mut impl FnMut(&Path, &str)) {
1763 + let Ok(entries) = std::fs::read_dir(dir) else {
1764 + return;
1765 + };
1766 + for entry in entries.flatten() {
1767 + let path = entry.path();
1768 + if path.is_dir() {
1769 + walk(&path, f);
1770 + } else if path.extension().is_some_and(|e| e == "rs")
1771 + && let Ok(contents) = std::fs::read_to_string(&path)
1772 + {
1773 + f(&path, &contents);
1774 + }
1775 + }
1776 + }
1777 + }
M server/src/db/pending_uploads.rs +26
@@ -60,6 +60,32 @@
60 60 Ok(())
61 61 }
62 62
63 + /// Whether `s3_key` (in `bucket`) is a pending upload this user presigned.
64 + ///
65 + /// The confirm handlers used to prove ownership by checking the client-supplied
66 + /// key started with the user's `{user_id}/{item_id}/…` prefix. With scan-then-
67 + /// promote the presigned key is an owner-less `staging/{uuid}.{ext}`, so that
68 + /// structural check no longer binds the key to a user. Every presign records the
69 + /// key here against its owner (`record_pending_upload`); confirm now proves
70 + /// ownership by looking it up — a caller cannot confirm a staging key it did not
71 + /// presign (and cannot guess another user's random staging uuid).
72 + pub async fn is_owned(
73 + pool: &PgPool,
74 + user_id: UserId,
75 + s3_key: &str,
76 + bucket: &str,
77 + ) -> Result<bool> {
78 + let owned = sqlx::query_scalar::<_, bool>(
79 + "SELECT EXISTS (SELECT 1 FROM pending_uploads WHERE s3_key = $1 AND user_id = $2 AND bucket = $3)",
80 + )
81 + .bind(s3_key)
82 + .bind(user_id)
83 + .bind(bucket)
84 + .fetch_one(pool)
85 + .await?;
86 + Ok(owned)
87 + }
88 +
63 89 /// Per-tick cap on the orphan-upload reaper. The reaper runs every scheduler
64 90 /// tick under the tick-wide advisory lock and deletes serially (one S3 round-
65 91 /// trip per row), so an unbounded result set lets a backlog wedge the tick
M server/src/db/scanning.rs +118
@@ -240,6 +240,124 @@
240 240 Ok(())
241 241 }
242 242
243 + /// Most recent content hash recorded for a key, from the scan-result audit
244 + /// trail. The scan worker computes and stores the sha256 in `file_scan_results`
245 + /// at scan time; the admin-approve promote path (which flips a held file to
246 + /// Clean and must copy it to its content-addressed key) has only the staging
247 + /// key in scope, so it reads the hash back here. Empty hashes — recorded by a
248 + /// degraded/held scan that never fully hashed the object — are skipped, so a
249 + /// content key is never derived from a blank digest.
250 + #[tracing::instrument(skip_all)]
251 + pub async fn latest_sha256_by_key(
252 + db: &PgPool,
253 + s3_key: &str,
254 + ) -> Result<Option<String>, sqlx::Error> {
255 + sqlx::query_scalar::<_, String>(
256 + "SELECT sha256 FROM file_scan_results \
257 + WHERE s3_key = $1 AND sha256 <> '' \
258 + ORDER BY scanned_at DESC LIMIT 1",
259 + )
260 + .bind(s3_key)
261 + .fetch_optional(db)
262 + .await
263 + }
264 +
265 + /// Repoint a GATED entity (Item audio/video, Version, Media, OTA artifact) from
266 + /// its staging key to the immutable content key and mark it Clean in one write.
267 + ///
268 + /// This is the scan-then-promote closing move for the kinds that carry their own
269 + /// `scan_status` gate. The `(table, key column)` pair is a compile-time constant
270 + /// selected by `(kind, file_type)` — never user input — so formatting it into the
271 + /// statement is safe. Uses the runtime `query` (not the `!` macro) so extending
272 + /// the promote set needs no offline-cache regeneration. Accepts any executor so
273 + /// the caller can run it inside the same transaction as the staging-key delete
274 + /// enqueue.
275 + pub async fn promote_gated<'e>(
276 + executor: impl sqlx::PgExecutor<'e>,
277 + kind: crate::db::scan_jobs::ScanTargetKind,
278 + file_type: crate::storage::FileType,
279 + target_id: Uuid,
280 + content_key: &str,
281 + ) -> Result<(), sqlx::Error> {
282 + use crate::db::scan_jobs::ScanTargetKind as K;
283 + use crate::storage::FileType as F;
284 + let sql: &'static str = match (kind, file_type) {
285 + (K::Item, F::Audio) => {
286 + "UPDATE items SET audio_s3_key = $1, scan_status = 'clean', updated_at = NOW() WHERE id = $2"
287 + }
288 + (K::Item, F::Video) => {
289 + "UPDATE items SET video_s3_key = $1, scan_status = 'clean', updated_at = NOW() WHERE id = $2"
290 + }
291 + (K::Version, _) => "UPDATE versions SET s3_key = $1, scan_status = 'clean' WHERE id = $2",
292 + (K::Media, _) => "UPDATE media_files SET s3_key = $1, scan_status = 'clean' WHERE id = $2",
293 + (K::OtaArtifact, _) => {
294 + "UPDATE ota_artifacts SET s3_key = $1, scan_status = 'clean' WHERE id = $2"
295 + }
296 + (other_kind, other_ft) => {
297 + // A CDN-image kind (or an Item file role with no key column) must go
298 + // through `promote_cdn_image_by_key` instead; reaching here is a wiring
299 + // bug, not a data condition.
300 + return Err(sqlx::Error::Protocol(format!(
301 + "promote_gated: {other_kind:?}/{other_ft:?} is not a gated promote target"
302 + )));
303 + }
304 + };
305 + sqlx::query(sql)
306 + .bind(content_key)
307 + .bind(target_id)
308 + .execute(executor)
309 + .await?;
310 + Ok(())
311 + }
312 +
313 + /// Repoint every CDN-served image surface that currently references `staging_key`
314 + /// to the immutable `content_key` (and its rebuilt public `content_url`), marking
315 + /// the row Clean. Mirrors [`set_cdn_image_scan_status_by_key`]'s surface list so
316 + /// the promote and the status-stamp can't drift apart. Only the one surface whose
317 + /// key matches is touched; the others no-op. `content_insertions` carries no URL
318 + /// column (it is served presigned, not straight from the CDN) so only its key and
319 + /// status move. Returns the number of rows repointed (expected: exactly 1).
320 + pub async fn promote_cdn_image_by_key(
321 + conn: &mut sqlx::PgConnection,
322 + staging_key: &str,
323 + content_key: &str,
324 + content_url: &str,
325 + ) -> Result<u64, sqlx::Error> {
326 + let mut affected = 0u64;
327 + // (SQL, binds_content_url) — the surfaces with a materialized public URL take
328 + // three binds (key, url, where-key); content_insertions takes two.
329 + for (sql, has_url) in [
330 + (
331 + "UPDATE item_images SET s3_key = $1, image_url = $2, scan_status = 'clean' WHERE s3_key = $3",
332 + true,
333 + ),
334 + (
335 + "UPDATE project_images SET s3_key = $1, image_url = $2, scan_status = 'clean' WHERE s3_key = $3",
336 + true,
337 + ),
338 + (
339 + "UPDATE items SET cover_s3_key = $1, cover_image_url = $2, cover_scan_status = 'clean' WHERE cover_s3_key = $3",
340 + true,
341 + ),
342 + (
343 + "UPDATE projects SET cover_s3_key = $1, cover_image_url = $2, cover_scan_status = 'clean' WHERE cover_s3_key = $3",
344 + true,
345 + ),
346 + (
347 + "UPDATE content_insertions SET storage_key = $1, scan_status = 'clean' WHERE storage_key = $2",
348 + false,
349 + ),
350 + ] {
351 + let q = if has_url {
352 + sqlx::query(sql).bind(content_key).bind(content_url).bind(staging_key)
353 + } else {
354 + sqlx::query(sql).bind(content_key).bind(staging_key)
355 + };
356 + affected += q.execute(&mut *conn).await?.rows_affected();
357 + }
358 + Ok(affected)
359 + }
360 +
243 361 /// Get items held for review, joined with creator info + latest scan layers.
244 362 /// Oldest first.
245 363 #[tracing::instrument(skip_all)]
M server/src/routes/ota.rs +10 -6
@@ -284,7 +284,9 @@
284 284 Path((app_id, release_id)): Path<(SyncAppId, OtaReleaseId)>,
285 285 Json(req): Json<UploadArtifactRequest>,
286 286 ) -> Result<impl IntoResponse> {
287 - let app = verify_app_owner(&state, &sync_user, app_id).await?;
287 + // Ownership check (side effect); the app object itself is no longer needed to
288 + // build the key now that artifacts land at a random staging key.
289 + verify_app_owner(&state, &sync_user, app_id).await?;
288 290 validate_target(&req.target)?;
289 291 validate_arch(&req.arch)?;
290 292
@@ -292,14 +294,16 @@
292 294 return Err(AppError::BadRequest("file_size must be positive".to_string()));
293 295 }
294 296
295 - // Verify the release belongs to this app (scoped lookup, not a full list scan)
296 - let release = db::ota::get_release(&state.db, app_id, release_id)
297 + // Verify the release belongs to this app (scoped lookup, not a full list scan).
298 + db::ota::get_release(&state.db, app_id, release_id)
297 299 .await?
298 300 .ok_or(AppError::NotFound)?;
299 301
300 - let s3_key = crate::storage::S3Client::generate_ota_artifact_key(
301 - app.id, &release.version, &req.target, &req.arch,
302 - );
302 + // Staging key (unserved); the scan worker promotes it to the content key on a
303 + // Clean verdict (C1). The artifact ROW stays singleton per
304 + // (release, target, arch) — `create_artifact` below overwrites its `s3_key`
305 + // pointer on re-upload — so the object no longer needs a deterministic name.
306 + let s3_key = crate::storage::S3Client::generate_staging_key("artifact.bin");
303 307
304 308 let synckit_s3 = state.require_synckit_s3()?;
305 309
M server/src/scanning/mod.rs +76
@@ -256,6 +256,82 @@
256 256 }
257 257 }
258 258
259 + /// Scan-then-promote closing move (C1): copy a Clean object from its unserved
260 + /// `staging/{uuid}` key to the immutable, content-addressed
261 + /// `{owner}/c/{sha256}.{ext}` key, repoint the entity (and, for CDN images, its
262 + /// materialized public URL) at the content key, mark it Clean, and enqueue the
263 + /// staging object for durable deletion. The bytes a buyer is served are then
264 + /// provably the bytes that were scanned: the content key is named by the scanned
265 + /// hash, the owner holds no presign to it, and the (owner-less, random) staging
266 + /// key they *can* re-PUT to is unserved and deleted.
267 + ///
268 + /// Shared by the scan worker's Clean path and the admin approve-held path so the
269 + /// two promote sites can't diverge. Ordering is fail-safe: the S3 copy runs
270 + /// first, then a single transaction repoints the row and enqueues the staging
271 + /// delete. If the copy or the DB write fails, the row keeps pointing at the
272 + /// still-present (unserved) staging key and the caller leaves the work un-done,
273 + /// so a retry re-promotes — the copy is idempotent (hash-named destination), and
274 + /// an already-promoted `{owner}/c/…` key short-circuits.
275 + #[allow(clippy::too_many_arguments)]
276 + pub async fn promote_staging_to_content(
277 + db: &sqlx::PgPool,
278 + backend: &dyn crate::storage::StorageBackend,
279 + cdn_base_url: Option<&str>,
280 + kind: crate::db::scan_jobs::ScanTargetKind,
281 + file_type: FileType,
282 + target_id: uuid::Uuid,
283 + owner: crate::db::UserId,
284 + staging_key: &str,
285 + sha256: &str,
286 + bucket: crate::storage::S3Bucket,
287 + ) -> crate::error::Result<()> {
288 + use crate::db::scan_jobs::ScanTargetKind;
289 + use crate::storage::{S3Client, S3Key};
290 +
291 + if sha256.is_empty() {
292 + return Err(crate::error::AppError::Storage(format!(
293 + "cannot promote {staging_key}: scan recorded no content hash"
294 + )));
295 + }
296 + // Already promoted (worker/admin retry, admin bulk over a mixed set): a
297 + // content key is `{owner}/c/…`, never `staging/…`. Nothing to copy or repoint.
298 + if !staging_key.starts_with("staging/") {
299 + return Ok(());
300 + }
301 +
302 + let ext = crate::storage::key_extension(staging_key);
303 + let content = S3Client::content_key(owner, sha256, ext);
304 + let content_str = content.as_str().to_string();
305 +
306 + // 1. Promote the object in storage (copy staging -> content). Idempotent.
307 + backend
308 + .copy_object(&S3Key::from_stored(staging_key), &content)
309 + .await?;
310 +
311 + // 2. Repoint the row + enqueue the staging delete atomically.
312 + let mut tx = db.begin().await?;
313 + if kind.is_cdn_served_without_gate() {
314 + let content_url = if matches!(kind, ScanTargetKind::ContentInsertion) {
315 + // No materialized URL column (insertions are served presigned).
316 + String::new()
317 + } else {
318 + crate::storage::build_project_image_url(backend, cdn_base_url, &content_str).await?
319 + };
320 + crate::db::scanning::promote_cdn_image_by_key(&mut tx, staging_key, &content_str, &content_url)
321 + .await?;
322 + } else {
323 + crate::db::scanning::promote_gated(&mut *tx, kind, file_type, target_id, &content_str).await?;
324 + }
325 + crate::db::pending_s3_deletions::enqueue_deletions(
326 + &mut *tx,
327 + &[(staging_key.to_string(), bucket.as_str().to_string())],
328 + "scan_promote_staging",
329 + )
330 + .await?;
331 + tx.commit().await?;
332 + Ok(())
333 + }
334 +
259 335 /// Aggregate scan result across all layers
260 336 #[derive(Debug, Clone)]
261 337 pub struct ScanResult {
M server/src/scanning/worker.rs +38 -1
@@ -268,6 +268,14 @@
268 268 return Err(e);
269 269 }
270 270 };
271 + // Stamp the entity's terminal scan_status. For a Clean staging upload the
272 + // promote inside `run_pipeline_and_decide` already set the key column AND
273 + // `scan_status = 'clean'` in one transaction; this re-stamp is then a
274 + // harmless idempotent write. It is load-bearing, though, for a Clean file
275 + // that was uploaded server-side to a NON-staging key (the build runner's OTA
276 + // artifacts) — there is nothing to promote, so this is the only place its
277 + // status is cleared. (For the gate-less image kinds this is a no-op; their
278 + // row was stamped by the promote or the held-image branch above.)
271 279 update_entity_status(&ctx.db, kind, target_id, entity_status).await?;
272 280
273 281 // Verdict + duration metrics (Run 20 Observability): quarantine/hold/error
@@ -564,7 +572,36 @@
564 572 // upload's image stays hidden until an admin clears it — the same
565 573 // fail-closed posture the gated Item/Version/Media kinds already get.
566 574 // Quarantine never reaches here (it returned above with the row purged).
567 - if kind.is_cdn_served_without_gate() {
575 + // C1 scan-then-promote: a Clean verdict copies the object from its unserved
576 + // staging key to the immutable content key and repoints the entity (gated
577 + // kinds by id, CDN-image kinds by staging key, incl. rebuilding the public
578 + // URL) in one shared step. This is the ONLY place a served key comes into
579 + // existence — the presign handlers can only mint staging keys (the sealed
580 + // generators are `pub(crate)`, callable only from here via `content_key`).
581 + // Fail-safe: a copy/DB error bubbles up so `process_job` leaves the job
582 + // un-done and the entity keeps pointing at the (unserved) staging key; the
583 + // retry re-promotes.
584 + if status == FileScanStatus::Clean && job.s3_key.starts_with("staging/") {
585 + // Client-presigned upload: copy staging -> content key and repoint the
586 + // entity. A Clean file at a non-staging key was uploaded server-side to
587 + // its final key (build-runner OTA) and needs no promote — its status is
588 + // stamped by `update_entity_status` in `process_job`.
589 + super::promote_staging_to_content(
590 + &ctx.db,
591 + backend.as_ref(),
592 + ctx.cdn_base_url.as_deref(),
593 + kind,
594 + file_type,
595 + job.target_id,
596 + job.user_id,
597 + &job.s3_key,
598 + &result.sha256,
599 + bucket,
600 + )
601 + .await?;
602 + } else if status != FileScanStatus::Clean && kind.is_cdn_served_without_gate() {
603 + // Held/pending image (never promoted): stamp the row keyed on its staging
604 + // key so the fail-closed render gate hides it until an admin clears it.
568 605 match db::scanning::set_cdn_image_scan_status_by_key(&ctx.db, &job.s3_key, status).await {
569 606 Ok(n) => tracing::info!(
570 607 s3_key = %job.s3_key, target_kind = %kind.as_str(), scan_status = %status, rows = n,
M server/tests/workflows/content_insertions.rs +17 -1
@@ -12,6 +12,19 @@
12 12 user_id.to_string()
13 13 }
14 14
15 + /// Record a pending-upload row for a test-forged key. Under scan-then-promote the
16 + /// confirm proves ownership via `pending_uploads` (a `staging/{uuid}` key has no
17 + /// user in its path), so a test that fabricates a key + object must also register
18 + /// it as if presign had — otherwise the ownership gate correctly rejects it.
19 + async fn record_pending(h: &TestHarness, user_id: &str, s3_key: &str) {
20 + sqlx::query("INSERT INTO pending_uploads (user_id, s3_key, bucket) VALUES ($1::uuid, $2, 'main') ON CONFLICT DO NOTHING")
21 + .bind(user_id)
22 + .bind(s3_key)
23 + .execute(&h.db)
24 + .await
25 + .expect("record pending upload");
26 + }
27 +
15 28 #[tokio::test]
16 29 async fn presign_requires_auth() {
17 30 let mut h = TestHarness::with_storage().await;
@@ -95,9 +108,11 @@
95 108 let mut h = TestHarness::with_storage().await;
96 109 let user_id = setup_creator(&mut h).await;
97 110
98 - // Pre-populate storage with a fake object (key must match user_id prefix)
111 + // Pre-populate storage with a fake object and record it as a pending upload
112 + // (the confirm now proves ownership via pending_uploads, not a key prefix).
99 113 let s3_key = format!("{}/insertions/intro.mp3", user_id);
100 114 h.storage.as_ref().unwrap().put(&s3_key, vec![0u8; 1024]);
115 + record_pending(&h, &user_id, &s3_key).await;
101 116
102 117 let resp = h
103 118 .client
@@ -207,6 +222,7 @@
207 222 async fn confirm_clip(h: &mut TestHarness, user_id: &str, file: &str, mime: &str) -> Value {
208 223 let s3_key = format!("{}/insertions/{}", user_id, file);
209 224 h.storage.as_ref().unwrap().put(&s3_key, vec![0u8; 1024]);
225 + record_pending(h, user_id, &s3_key).await;
210 226 let resp = h
211 227 .client
212 228 .post_json(
M server/tests/workflows/gallery.rs +4 -1
@@ -260,7 +260,10 @@
260 260 .unwrap();
261 261 assert_eq!(count, 1);
262 262 assert_eq!(db_key, s3_key);
263 - assert!(s3_key.starts_with(&format!("projects/{}/gallery/", project_id)));
263 + // No scanner in this harness, so the row still holds the unserved staging key
264 + // (a Clean scan would later promote it to a content key). Presign hands out
265 + // `staging/{uuid}/{filename}`, never the old `projects/{id}/gallery/…` key.
266 + assert!(s3_key.starts_with("staging/"), "gallery presign must return a staging key: {s3_key}");
264 267 assert_eq!(storage_used(&h, &user_id).await, 500);
265 268 }
266 269
M server/tests/workflows/media_library.rs +13 -4
@@ -541,15 +541,24 @@
541 541 .bind(&user_id).fetch_one(&h.db).await.unwrap();
542 542 assert_eq!(used_after_first, TINY_PNG.len() as i64);
543 543
544 - // Re-confirm the SAME key. Rejected as a duplicate ...
545 - let resp = h.client.post_json("/api/media/confirm", &confirm_body.to_string()).await;
544 + // A genuine duplicate under scan-then-promote: presign a SECOND upload of the
545 + // same folder+filename (a fresh staging key, so it passes the ownership gate),
546 + // then confirm. It collides on the (user, folder, filename) unique index and
547 + // is rejected as "already exists".
548 + let presign2 = json!({"file_name": "pic.png", "content_type": "image/png", "folder": ""});
549 + let resp = h.client.post_json("/api/media/presign", &presign2.to_string()).await;
550 + assert!(resp.status.is_success(), "second presign failed: {}", resp.text);
551 + let s3_key2 = resp.json::<Value>()["s3_key"].as_str().unwrap().to_string();
552 + h.storage.as_ref().unwrap().put(&s3_key2, TINY_PNG.to_vec());
553 + let confirm2 = json!({"s3_key": s3_key2, "file_name": "pic.png", "content_type": "image/png", "folder": ""});
554 + let resp = h.client.post_json("/api/media/confirm", &confirm2.to_string()).await;
546 555 assert!(resp.status.is_client_error(), "duplicate confirm should be rejected: {} {}", resp.status, resp.text);
547 556 assert!(resp.text.contains("already exists"), "rejection should name the collision: {}", resp.text);
548 557
549 - // ... but the live object the committed row points at must SURVIVE (the HIGH).
558 + // ... but the FIRST upload's live object must SURVIVE (the HIGH).
550 559 assert!(
551 560 h.storage.as_ref().unwrap().object_exists(&s3_key).await.unwrap(),
552 - "duplicate confirm must NOT delete the live object"
561 + "duplicate confirm must NOT delete the first upload's live object"
553 562 );
554 563 // ... and the rolled-back tx must not have double-charged storage.
555 564 let used_after_second: i64 = sqlx::query_scalar("SELECT storage_used_bytes FROM users WHERE id = $1::uuid")
M server/tests/workflows/scanning.rs +73 -3
@@ -4,6 +4,7 @@
4 4 use serde_json::{json, Value};
5 5
6 6 use makenotwork::db::UserId;
7 + use makenotwork::storage::StorageBackend;
7 8
8 9 /// Helper: set up a trusted creator with a project and audio item.
9 10 async fn setup_creator_with_item(h: &mut TestHarness) -> (String, String) {
@@ -49,7 +50,9 @@
49 50 // completion before asserting final state.
50 51 h.drain_scan_jobs().await;
51 52
52 - // Verify audio_s3_key was set in DB
53 + // C1 scan-then-promote: a Clean scan copies the object from its staging key
54 + // to a content-addressed served key and repoints the row. audio_s3_key must
55 + // NO LONGER be the staging key — it is now `{user}/c/{sha256}.mp3`.
53 56 let db_key: Option<String> = sqlx::query_scalar(
54 57 "SELECT audio_s3_key FROM items WHERE id = $1::uuid",
55 58 )
@@ -57,7 +60,16 @@
57 60 .fetch_one(&h.db)
58 61 .await
59 62 .unwrap();
60 - assert_eq!(db_key.as_deref(), Some(s3_key.as_str()));
63 + let db_key = db_key.expect("audio_s3_key set");
64 + assert_ne!(db_key, s3_key, "clean scan must promote off the staging key");
65 + assert!(!db_key.starts_with("staging/"), "promoted key must not be a staging key: {db_key}");
66 + assert!(db_key.contains("/c/"), "promoted key must be content-addressed: {db_key}");
67 + assert!(db_key.ends_with(".mp3"), "content key keeps the extension: {db_key}");
68 +
69 + // The served object exists at the content key; the staging object is gone
70 + // (enqueued for durable deletion after the promote copy).
71 + let store = h.storage.as_ref().unwrap();
72 + assert!(store.object_exists(&db_key).await.unwrap(), "content object must exist after promote");
61 73
62 74 // Verify scan_status is clean
63 75 let scan_status: String = sqlx::query_scalar(
@@ -70,6 +82,59 @@
70 82 assert_eq!(scan_status, "clean");
71 83 }
72 84
85 + /// The C1 invariant end-to-end: after a Clean scan promotes an upload to its
86 + /// content key, a creator re-PUT to the (still-known) staging URL cannot change
87 + /// the bytes a buyer is served. The served key is the content key; the staging
88 + /// object is a dead end.
89 + #[tokio::test]
90 + async fn repost_to_staging_after_clean_cannot_change_served_bytes() {
91 + let mut h = TestHarness::with_storage_and_scanner().await;
92 + let (_project_id, item_id) = setup_creator_with_item(&mut h).await;
93 +
94 + // Presign → the client only ever holds a presign to the staging key.
95 + let body = json!({"item_id": item_id, "file_type": "audio", "file_name": "song.mp3", "content_type": "audio/mpeg"});
96 + let resp = h.client.post_json("/api/upload/presign", &body.to_string()).await;
97 + assert!(resp.status.is_success(), "presign: {}", resp.text);
98 + let staging_key = resp.json::<Value>()["s3_key"].as_str().unwrap().to_string();
99 + assert!(staging_key.starts_with("staging/"));
100 +
101 + // Upload clean bytes and confirm.
102 + let mut clean = b"ID3".to_vec();
103 + clean.extend_from_slice(&[0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]);
104 + clean.extend_from_slice(&[0xAAu8; 200]);
105 + h.storage.as_ref().unwrap().put(&staging_key, clean.clone());
106 + let body = json!({"item_id": item_id, "file_type": "audio", "s3_key": staging_key});
107 + let resp = h.client.post_json("/api/upload/confirm", &body.to_string()).await;
108 + assert!(resp.status.is_success(), "confirm: {}", resp.text);
109 + h.drain_scan_jobs().await;
110 +
111 + // The row now serves a content key, NOT the staging key.
112 + let served_key: String = sqlx::query_scalar::<_, Option<String>>("SELECT audio_s3_key FROM items WHERE id = $1::uuid")
113 + .bind(&item_id)
114 + .fetch_one(&h.db)
115 + .await
116 + .unwrap()
117 + .expect("audio promoted");
118 + assert!(served_key.contains("/c/"), "served key must be content-addressed: {served_key}");
119 + assert_ne!(served_key, staging_key);
120 +
121 + let store = h.storage.as_ref().unwrap();
122 + let served_before = store.download_object(&served_key).await.unwrap();
123 + assert_eq!(served_before, clean, "the content object holds the scanned bytes");
124 +
125 + // The attack: re-PUT malware to the staging key the creator still holds a
126 + // presign for. This is the exact move the old mutable-served-key design let
127 + // a creator use to swap post-scan bytes.
128 + let malware = vec![0x7f, b'E', b'L', b'F', 0x02, 0x01, 0x01, 0x00];
129 + store.put(&staging_key, malware.clone());
130 +
131 + // The served key is untouched: a buyer still gets the scanned bytes. The
132 + // staging object is irrelevant — it is not what the row serves.
133 + let served_after = store.download_object(&served_key).await.unwrap();
134 + assert_eq!(served_after, clean, "re-PUT to the staging key must NOT change the served bytes");
135 + assert_ne!(served_after, malware);
136 + }
137 +
73 138 #[tokio::test]
74 139 async fn confirm_upload_bad_magic_quarantined() {
75 140 let mut h = TestHarness::with_storage_and_scanner().await;
@@ -167,7 +232,12 @@
167 232 .await
168 233 .expect("the item row must still exist — quarantining a cover must not delete the track");
169 234 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");
235 + // The audio track was clean-scanned, so it was promoted off its staging key to
236 + // a content-addressed key — the point is it survives the cover quarantine
237 + // (non-null, promoted, still gated Clean), not that it keeps the staging name.
238 + let audio = audio.expect("the audio track must be preserved");
239 + assert_ne!(audio, audio_key, "the audio track must have been promoted, not delisted");
240 + assert!(audio.contains("/c/"), "the surviving track must be content-addressed: {audio}");
171 241 assert_eq!(ck, None, "the quarantined cover key must be NULLed");
172 242 assert_eq!(cu, None, "the quarantined cover URL must be NULLed so it stops rendering");
173 243 }
M server/tests/workflows/storage.rs +6 -1
M server/tests/workflows/video.rs +5 -4
M server/src/routes/admin/uploads.rs +15 -8
M server/src/routes/api/content_insertions.rs +17 -9
M server/src/routes/storage/gallery.rs +16 -18
M server/src/routes/storage/images.rs +19 -15
M server/src/routes/storage/media.rs +38 -22
M server/src/routes/storage/mod.rs +109
M server/src/routes/storage/uploads.rs +24 -9
M server/src/routes/storage/versions.rs +25 -26
M server/src/routes/api/internal/uploads.rs +15 -6