From aba47c81e3d95a254de9b751d28a621867112f2a Mon Sep 17 00:00:00 2001
From: Thom Lamb <thomlamb@gmail.com>
Date: Fri, 14 Aug 2026 09:22:45 -0500
Subject: [PATCH] fix: bound admin image uploads and use a CSPRNG for stored
 filenames

SonarQube flagged three hotspots in the admin router: multer was
configured with no content length limits, and stored filenames were
derived from Date.now() plus Math.random().

- Cap the multipart body on every dimension: 6 files, 8 MiB per image,
  8 fields, 64 KiB per field. Without limits a single request could
  fill the uploads volume.
- Generate stored filenames with crypto.randomUUID() so paths are not
  predictable. Image ordering is unaffected; sort_order already drives it.
- Wrap the upload middleware to translate MulterError into 413/400 JSON.
  The app mounts no error handler, so a limit rejection would otherwise
  surface as an HTML 500.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 backend/src/routes/admin.ts | 45 ++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts
index 80d2ba8..df10e9f 100755
--- a/backend/src/routes/admin.ts
+++ b/backend/src/routes/admin.ts
@@ -1,19 +1,54 @@
-import { Router, Request, Response } from 'express';
+import { Router, Request, Response, NextFunction } from 'express';
 import multer from 'multer';
 import path from 'path';
+import { randomUUID } from 'crypto';
 import { pool } from '../db';
 
 const router = Router();
 
 const UPLOADS_DIR = process.env.UPLOADS_DIR || '/app/uploads';
+
+// Multer writes to disk with no size cap unless one is given, so a single
+// request could fill the uploads volume. Bound every dimension of the
+// multipart body: image count, bytes per image, and the small text fields
+// (name/description/price) that accompany them.
+const MAX_IMAGES_PER_REQUEST = 6;
+const MAX_IMAGE_BYTES = 8 * 1024 * 1024;
+const MAX_TEXT_FIELDS = 8;
+const MAX_TEXT_FIELD_BYTES = 64 * 1024;
+
 const storage = multer.diskStorage({
   destination: UPLOADS_DIR,
+  // Stored names come from a CSPRNG rather than a timestamp plus Math.random,
+  // which is predictable enough that a caller could guess (or collide with)
+  // another upload's path.
   filename: (_req, file, cb) => {
     const ext = path.extname(file.originalname);
-    cb(null, `${Date.now()}-${Math.round(Math.random() * 1e6)}${ext}`);
+    cb(null, `${randomUUID()}${ext}`);
   }
 });
-const upload = multer({ storage });
+
+const upload = multer({
+  storage,
+  limits: {
+    fileSize: MAX_IMAGE_BYTES,
+    files: MAX_IMAGES_PER_REQUEST,
+    fields: MAX_TEXT_FIELDS,
+    fieldSize: MAX_TEXT_FIELD_BYTES
+  }
+});
+
+// No error-handling middleware is mounted on the app, so translate multer's
+// limit errors here instead of letting them surface as a generic 500.
+const uploadImages = (req: Request, res: Response, next: NextFunction) => {
+  upload.array('images', MAX_IMAGES_PER_REQUEST)(req, res, (err: unknown) => {
+    if (err instanceof multer.MulterError) {
+      const status = err.code === 'LIMIT_FILE_SIZE' ? 413 : 400;
+      return res.status(status).json({ error: err.message });
+    }
+    return next(err);
+  });
+};
 
 const SELECT_WITH_IMAGES = `
   SELECT i.*,
@@ -31,7 +66,7 @@ router.get('/items', async (_req: Request, res: Response) => {
   res.json(rows);
 });
 
-router.post('/items', upload.array('images', 6), async (req: Request, res: Response) => {
+router.post('/items', uploadImages, async (req: Request, res: Response) => {
   const { name, description, price } = req.body;
   const files = (req.files as Express.Multer.File[]) || [];
   const client = await pool.connect();
@@ -60,7 +95,7 @@ router.post('/items', upload.array('images', 6), async (req: Request, res: Respo
   }
 });
 
-router.put('/items/:id', upload.array('images', 6), async (req: Request, res: Response) => {
+router.put('/items/:id', uploadImages, async (req: Request, res: Response) => {
   const { name, description, price } = req.body;
   const files = (req.files as Express.Multer.File[]) || [];
   const client = await pool.connect();
-- 
2.54.0