fix(uploads): prevent cross-photo contamination from filename collisions and non-atomic writes (#931) (#933)

* fix(uploads): prevent cross-photo contamination from filename collisions and non-atomic writes (#931)

* test: pin the suffixed photo filename format in the NFD pipeline suite (#931)

* test: make the suffix-uniqueness check deterministic-in-practice (#931)

* fix(uploads): widen the anti-collision suffix to 48 bits (#931)

* fix(uploads): hide staging files from list() + share one watermark limiter process-wide (#931)

* fix(uploads): reclaim orphaned staging files + revalidate watermark settings in queued jobs (#931)

---------

Co-authored-by: Paul Nothaft <[email protected]>

backend/src/utils/__tests__/filenameSanitizer.test.js

@@ -89,23 +89,25 @@ describe('sanitizeFilename — edge cases', () => {
 });
 
 describe('generatePhotoFilename — composed name uses the NFD pipeline', () => {
-  it('round-trips Ägypten + individual → Agypten_individual_0050.jpg (#607)', () => {
+  // The trailing _[0-9a-f]{12} is the anti-collision suffix (#931) that
+  // keeps concurrent uploads from assigning the same final storage path.
+  it('round-trips Ägypten + individual → Agypten_individual_0050 (#607)', () => {
     expect(generatePhotoFilename('Ägypten', 'individual', 50, '.jpg'))
-      .toBe('Agypten_individual_0050.jpg');
+      .toMatch(/^Agypten_individual_0050_[0-9a-f]{12}\.jpg$/);
   });
 
   it('handles missing category by defaulting to "uncategorized"', () => {
     expect(generatePhotoFilename('Wedding', null, 1, '.jpg'))
-      .toBe('Wedding_uncategorized_0001.jpg');
+      .toMatch(/^Wedding_uncategorized_0001_[0-9a-f]{12}\.jpg$/);
   });
 
   it('zero-pads the counter to 4 digits', () => {
-    expect(generatePhotoFilename('e', 'c', 7, '.png')).toBe('e_c_0007.png');
-    expect(generatePhotoFilename('e', 'c', 1234, '.png')).toBe('e_c_1234.png');
-    // 5+ digit counters intentionally overflow the pad — pinned because
-    // the unique index in the photos table doesn't care about pad width,
-    // only string uniqueness.
-    expect(generatePhotoFilename('e', 'c', 99999, '.png')).toBe('e_c_99999.png');
+    expect(generatePhotoFilename('e', 'c', 7, '.png')).toMatch(/^e_c_0007_[0-9a-f]{12}\.png$/);
+    expect(generatePhotoFilename('e', 'c', 1234, '.png')).toMatch(/^e_c_1234_[0-9a-f]{12}\.png$/);
+    // 5+ digit counters intentionally overflow the pad — pad width never
+    // mattered for uniqueness (there is no unique index on filenames);
+    // the random suffix is what guarantees it.
+    expect(generatePhotoFilename('e', 'c', 99999, '.png')).toMatch(/^e_c_99999_[0-9a-f]{12}\.png$/);
   });
 });
 

backend/src/utils/filenameSanitizer.js

@@ -1,4 +1,5 @@
 const path = require('path');
+const crypto = require('crypto');
 
 /**
  * Sanitize a string to be used as a filename component
@@ -59,8 +60,18 @@ function generatePhotoFilename(eventName, categoryName, counter, extension) {
   const sanitizedEvent = sanitizeFilename(eventName, 30);
   const sanitizedCategory = sanitizeFilename(categoryName || 'uncategorized', 20);
   const paddedCounter = String(counter).padStart(4, '0');
-  
-  return `${sanitizedEvent}_${sanitizedCategory}_${paddedCounter}${extension}`;
+  // Random suffix (#931): the counter base is `count(*)+1` computed per
+  // upload request, so two concurrent bulk-upload requests can assign the
+  // same counter to different photos. Since files are written to their
+  // final path before any row exists (and photos has no unique index on
+  // filename — one can't be added without a dedupe migration on installs
+  // that already carry historical duplicates), a collision silently
+  // overwrites the first photo's bytes at its recorded path — cross-photo
+  // contamination. 48 bits keep the collision odds negligible even for
+  // pathological concurrency (two simultaneous 2000-photo uploads: ~7e-12).
+  const suffix = crypto.randomBytes(6).toString('hex');
+
+  return `${sanitizedEvent}_${sanitizedCategory}_${paddedCounter}_${suffix}${extension}`;
 }
 
 /**