Merge pull request 'fix: bound admin image uploads and use a CSPRNG for stored filenames' (#4) from fix/sonarqube-upload-limits-and-weak-rng into main

Reviewed-on: #4

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();