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

* 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)

* chore(deps): promote p-limit to a direct dependency for the watermark limiter (#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/services/storage/LocalFsStorage.js

@@ -6,6 +6,11 @@ const crypto = require('crypto');
 
 const logger = require('../../utils/logger');
 
+// Staging files older than this are considered orphaned by a crash between
+// copy and rename, and are reclaimed during list() walks. Generous enough
+// that no legitimate in-flight copy (even multi-GB on slow NFS) hits it.
+const STAGING_RECLAIM_AGE_MS = 60 * 60 * 1000;
+
 /**
  * Filesystem-backed implementation of the StorageBackend interface.
  * All keys are relative to `root` (typically process.env.STORAGE_PATH).
@@ -67,8 +72,18 @@ class LocalFsStorage {
   async putFromFile(relPath, localPath, _options = {}) {
     const abs = this._resolve(relPath);
     await fsp.mkdir(path.dirname(abs), { recursive: true });
-    // copyFile is atomic from the destination's perspective on POSIX.
-    await fsp.copyFile(localPath, abs);
+    // copyFile truncates and rewrites the destination in place, so a
+    // concurrent reader (thumbnail/watermark generation, photo serving)
+    // can observe partial or foreign bytes mid-copy (#931). Copy to a
+    // sibling tmp file and rename, like put() above — rename IS atomic.
+    const tmp = `${abs}.tmp.${process.pid}.${crypto.randomBytes(4).toString('hex')}`;
+    try {
+      await fsp.copyFile(localPath, tmp);
+      await fsp.rename(tmp, abs);
+    } catch (err) {
+      await fsp.unlink(tmp).catch(() => {});
+      throw err;
+    }
   }
 
   async get(relPath) {
@@ -126,6 +141,23 @@ class LocalFsStorage {
         throw err;
       }
       for (const ent of dirents) {
+        // Hide in-flight staging files (put/putFromFile write `<key>.tmp.<pid>.<hex>`
+        // siblings before the atomic rename). Without this filter a
+        // concurrent archive/backup listing could stream a partial tmp
+        // entry or fail when the rename wins the race (#931). Stale ones
+        // (a crash between copy and rename orphans them) are reclaimed
+        // here — hiding without reclaiming would let interrupted uploads
+        // accumulate invisible files until the volume fills.
+        if (/\.tmp\.\d+\.[0-9a-f]+$/.test(ent.name)) {
+          const childAbs = path.join(dir, ent.name);
+          try {
+            const st = await fsp.stat(childAbs);
+            if (Date.now() - st.mtimeMs > STAGING_RECLAIM_AGE_MS) {
+              await fsp.unlink(childAbs).catch(() => {});
+            }
+          } catch { /* vanished (rename/cleanup won the race) — fine */ }
+          continue;
+        }
         const childAbs = path.join(dir, ent.name);
         const childRel = relBase ? `${relBase}/${ent.name}` : ent.name;
         if (ent.isDirectory()) {