fix(security): stop refused uploads accumulating on the volume, and record the hotspot review (#180)

SonarQube reported three security hotspots, all in `routes/admin.ts`. A hotspot is not a defect — it marks code that touches something security-sensitive and needs a human decision — so the work is a recorded review, with a change only where the review finds a real gap. It found one.

The gap: multer writes every file to disk before any route logic runs, and multer's own cleanup only covers errors it raised itself. Everything after that left the bytes behind with nothing referencing them. A request carrying a perfectly valid photograph and a malformed `category_id` is refused with a 400 after the write, and the file stays on the volume permanently — no database row to find it by, and no bound on how many can accumulate. The same held for a malformed `tags` field, for a database error rolling the transaction back, and for `readHead` itself throwing, which returned no message and so cleaned up nothing.

That is the substance of the limits the first hotspot points at. Bounding one request to 8 MB across six files does nothing if every refused request keeps its bytes for ever, and the admin API is the one surface where that is reachable.

The fix is a hook rather than a call at each `return`, registered the moment multer succeeds. A route added later inherits it instead of having to remember it, which matters because the failure being prevented is precisely someone adding a fourth early return. It listens on `close` rather than `finish` so an aborted connection is covered, and checks `writableEnded` so a response that never completed is not mistaken for a success whatever its status code reads.

`verifyUploadedImages` goes back to checking only. Removing the files there as well would unlink twice and log an ENOENT for every refused upload, and the single mechanism covers the case it used to miss.

The other two hotspots are safe, and now say why in the file rather than only in SonarQube's UI — following the precedent of the existing comment that names S5693 by rule number. The upload path is not caller-controlled despite arriving from a request: multer composes it from a server constant and a `randomUUID()` plus an extension looked up from the validated content type, so the caller's `originalname` never reaches the filesystem. That reasoning belongs next to the `fs.open` that depends on it.

Three tests, written first and failing first: a refused sibling field, a refused tags field, and the accepted case, which must not be swept up by the same cleanup. 254 integration and 278 unit tests pass.

Refs #180

backend/tests/integration/uploadValidation.integration.test.ts

@@ -153,3 +153,59 @@ describe('the uploads directory', () => {
     }
   });
 });
+
+/**
+ * Bounding what one request may write is only half of bounding what the volume
+ * accumulates. multer writes to disk before any route logic runs, so a request
+ * refused *after* the write leaves its bytes behind with nothing referencing
+ * them — no database row, no way to find them again, and no upper bound on how
+ * many an attacker with admin access can pile up. Found reviewing the security
+ * hotspots in this file (#180).
+ */
+describe('what it leaves on disk', () => {
+  it('removes the upload when the request is refused for its other fields', async () => {
+    const before = await storedFiles();
+
+    const res = await request(app)
+      .post('/api/admin/items')
+      .field('name', 'Photographed thing')
+      .field('description', '')
+      .field('price', '30')
+      // Valid image, invalid sibling field: the file is already written by the
+      // time the route reads this and returns 400.
+      .field('category_id', 'not-a-number')
+      .attach('images', REAL_PNG, { filename: 'photo.png', contentType: 'image/png' });
+
+    expect(res.status).toBe(400);
+    expect(await storedFiles()).toEqual(before);
+  });
+
+  it('removes the upload when the tags field is refused', async () => {
+    const before = await storedFiles();
+
+    const res = await request(app)
+      .post('/api/admin/items')
+      .field('name', 'Photographed thing')
+      .field('description', '')
+      .field('price', '30')
+      .field('tags', '[not json')
+      .attach('images', REAL_PNG, { filename: 'photo.png', contentType: 'image/png' });
+
+    expect(res.status).toBe(400);
+    expect(await storedFiles()).toEqual(before);
+  });
+
+  // The accepted case must not be swept up by the same cleanup: these files are
+  // the ones the item now points at.
+  it('keeps the upload when the request succeeds', async () => {
+    const before = await storedFiles();
+
+    const res = await createItem().attach('images', REAL_PNG, {
+      filename: 'photo.png',
+      contentType: 'image/png'
+    });
+
+    expect(res.status).toBe(200);
+    expect((await storedFiles()).length).toBe(before.length + 1);
+  });
+});