Skip to main content

max / makenotwork

server: weave version id into download S3 key (ultra-fuzz Run #1 Storage HIGH) Version download keys were {user}/{item}/download/{filename} with no per-version segment, so two versions of an item sharing a filename (a creator who ships every release as plugin.zip) collided onto one S3 object: the second PUT overwrote the first's bytes and both rows charged storage for one object, baking a double-charge into the weekly recalc. Add generate_version_key embedding the version id (the table PK, so unique by construction) and a partial unique index on versions.s3_key as the backstop. Mirrors the gallery per-entity-uuid fix from migration 147. The migration refuses to apply if a legacy collision exists rather than silently dropping a version row. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-19 20:51 UTC
Commit: b0f29b72600b565b881d26c8f38be64e8b2ff5dd
Parent: 631ef45
3 files changed, +99 insertions, -3 deletions
A server/migrations/148_versions_s3_key_unique.sql +37
@@ -0,0 +1,37 @@
1 + -- Enforce one version row per S3 object (ultra-fuzz Run #1, Storage HIGH).
2 + --
3 + -- Version download keys were `{user}/{item}/download/{filename}` with no
4 + -- version segment, so two versions of the same item that shared a filename
5 + -- (a creator who ships every release as `plugin.zip`) resolved to the SAME S3
6 + -- key. The second upload's PUT overwrote the first's bytes (immutable cache
7 + -- then served the wrong release), and both version rows carried
8 + -- `file_size_bytes` for one physical object, so the weekly
9 + -- `recalculate_all_storage_batch` baked the double-charge into reconciliation
10 + -- (the same non-self-healing drift migration 147 closed for gallery images).
11 + --
12 + -- The key generator now weaves the version id into the path
13 + -- (`generate_version_key` -> `{user}/{item}/download/{version_id}/{filename}`),
14 + -- so every newly-confirmed version is globally unique by construction. This
15 + -- partial unique index is the backstop that makes a future regression a write
16 + -- error instead of a silent overwrite.
17 + --
18 + -- Unlike the gallery fix, we do NOT auto-dedupe: a version row is meaningful
19 + -- (distinct version_number, download history, current flag), so silently
20 + -- dropping one would lose data. Surface any pre-existing legacy collision
21 + -- loudly instead — it needs a human decision, not an automated DELETE.
22 + DO $$
23 + DECLARE
24 + dupes int;
25 + BEGIN
26 + SELECT count(*) INTO dupes FROM (
27 + SELECT s3_key FROM versions
28 + WHERE s3_key IS NOT NULL
29 + GROUP BY s3_key HAVING count(*) > 1
30 + ) d;
31 + IF dupes > 0 THEN
32 + RAISE EXCEPTION 'Cannot enforce unique versions.s3_key: % colliding key(s) exist from the pre-fix format. Resolve manually (re-upload the affected versions) before applying this migration.', dupes;
33 + END IF;
34 + END $$;
35 +
36 + CREATE UNIQUE INDEX IF NOT EXISTS idx_versions_s3_key
37 + ON versions (s3_key) WHERE s3_key IS NOT NULL;
M server/src/routes/storage/versions.rs +4 -2
@@ -77,8 +77,10 @@ pub(super) async fn version_presign_upload(
77 77 // Validate the declared size (if any) before signing it into Content-Length.
78 78 super::validate_declared_upload_size(req.file_size_bytes, file_type, max_file_bytes)?;
79 79
80 - // Generate S3 key using the version's item_id
81 - let s3_key = S3Client::generate_key(user.id, version.item_id, file_type, &req.file_name);
80 + // Generate S3 key woven with the version id so two versions of the same
81 + // item sharing a filename never collide onto one object (ultra-fuzz Run #1
82 + // Storage HIGH). The dedicated generator embeds the download prefix.
83 + let s3_key = S3Client::generate_version_key(user.id, version.item_id, version_id, &req.file_name);
82 84
83 85 // Track the pending upload so the reaper can clean it up if never confirmed
84 86 db::pending_uploads::record_pending_upload(&state.db, user.id, &s3_key, "main").await?;
M server/src/storage.rs +58 -1
@@ -8,7 +8,7 @@
8 8 use std::str::FromStr;
9 9
10 10 use crate::config::StorageConfig;
11 - use crate::db::{ItemId, ProjectId, UserId};
11 + use crate::db::{ItemId, ProjectId, UserId, VersionId};
12 12 use crate::error::{AppError, Result};
13 13
14 14 /// Allowed audio file extensions and their MIME types
@@ -333,6 +333,30 @@ impl S3Client {
333 333 )
334 334 }
335 335
336 + /// Generate an S3 key for a version download file. The version's own id is
337 + /// woven into the path so two versions of the same item that share a
338 + /// filename (e.g. a creator who ships every release as `plugin.zip`) never
339 + /// resolve to the same object — mirrors the per-entity-uuid segment the
340 + /// gallery keys use, except the version id is the table's primary key, so
341 + /// uniqueness is guaranteed by construction rather than by a fresh uuid.
342 + /// Format: {user_id}/{item_id}/download/{version_id}/{filename}
343 + pub fn generate_version_key(
344 + user_id: UserId,
345 + item_id: ItemId,
346 + version_id: VersionId,
347 + filename: &str,
348 + ) -> String {
349 + let safe_filename = sanitize_filename(filename);
350 + format!(
351 + "{}/{}/{}/{}/{}",
352 + user_id,
353 + item_id,
354 + FileType::Download.as_str(),
355 + version_id,
356 + safe_filename
357 + )
358 + }
359 +
336 360 /// Generate an S3 key for a reusable insertion clip (not tied to any item).
337 361 /// Format: {user_id}/insertions/{filename}
338 362 pub fn generate_insertion_key(user_id: UserId, filename: &str) -> String {
@@ -849,6 +873,39 @@ mod tests {
849 873 }
850 874
851 875 #[test]
876 + fn test_generate_version_key_is_unique_per_version() {
877 + let user_id: UserId = "11111111-1111-1111-1111-111111111111".parse().unwrap();
878 + let item_id: ItemId = "22222222-2222-2222-2222-222222222222".parse().unwrap();
879 + let v1: VersionId = "33333333-3333-3333-3333-333333333333".parse().unwrap();
880 + let v2: VersionId = "44444444-4444-4444-4444-444444444444".parse().unwrap();
881 +
882 + // Two versions of the SAME item sharing a filename must not collide.
883 + let k1 = S3Client::generate_version_key(user_id, item_id, v1, "plugin.zip");
884 + let k2 = S3Client::generate_version_key(user_id, item_id, v2, "plugin.zip");
885 + assert_ne!(k1, k2, "same-filename versions must produce distinct keys");
886 +
887 + assert_eq!(
888 + k1,
889 + "11111111-1111-1111-1111-111111111111/22222222-2222-2222-2222-222222222222/download/33333333-3333-3333-3333-333333333333/plugin.zip"
890 + );
891 + // Confirm-handler prefix check is `{user}/{item}/`; the woven key still
892 + // satisfies it.
893 + assert!(k1.starts_with("11111111-1111-1111-1111-111111111111/22222222-2222-2222-2222-222222222222/"));
894 + // Filename is still the last path segment (confirm extracts it via rsplit).
895 + assert_eq!(k1.rsplit('/').next(), Some("plugin.zip"));
896 + }
897 +
898 + #[test]
899 + fn test_generate_version_key_sanitizes_filename() {
900 + let user_id: UserId = "11111111-1111-1111-1111-111111111111".parse().unwrap();
901 + let item_id: ItemId = "22222222-2222-2222-2222-222222222222".parse().unwrap();
902 + let v1: VersionId = "33333333-3333-3333-3333-333333333333".parse().unwrap();
903 +
904 + let key = S3Client::generate_version_key(user_id, item_id, v1, "my release (1).zip");
905 + assert!(key.ends_with("/myrelease1.zip"));
906 + }
907 +
908 + #[test]
852 909 fn test_generate_key_sanitizes_filename() {
853 910 let user_id: UserId = "11111111-1111-1111-1111-111111111111".parse().unwrap();
854 911 let item_id: ItemId = "22222222-2222-2222-2222-222222222222".parse().unwrap();