fix: bound admin image uploads and use a CSPRNG for stored filenames
SonarQube Analysis / sonarqube (pull_request) Successful in 3m49s
Tests / backend-unit (pull_request) Successful in 33s
Tests / backend-integration (pull_request) Successful in 1m6s
Tests / frontend-e2e (pull_request) Failing after 1m7s

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>
This commit is contained in:
2026-08-14 09:22:45 -05:00
co-authored by Claude Opus 5
parent da240621db
commit aba47c81e3
+40 -5
View File
@@ -1,19 +1,54 @@
import { Router, Request, Response } from 'express'; import { Router, Request, Response, NextFunction } from 'express';
import multer from 'multer'; import multer from 'multer';
import path from 'path'; import path from 'path';
import { randomUUID } from 'crypto';
import { pool } from '../db'; import { pool } from '../db';
const router = Router(); const router = Router();
const UPLOADS_DIR = process.env.UPLOADS_DIR || '/app/uploads'; 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({ const storage = multer.diskStorage({
destination: UPLOADS_DIR, 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) => { filename: (_req, file, cb) => {
const ext = path.extname(file.originalname); 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 = ` const SELECT_WITH_IMAGES = `
SELECT i.*, SELECT i.*,
@@ -31,7 +66,7 @@ router.get('/items', async (_req: Request, res: Response) => {
res.json(rows); 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 { name, description, price } = req.body;
const files = (req.files as Express.Multer.File[]) || []; const files = (req.files as Express.Multer.File[]) || [];
const client = await pool.connect(); 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 { name, description, price } = req.body;
const files = (req.files as Express.Multer.File[]) || []; const files = (req.files as Express.Multer.File[]) || [];
const client = await pool.connect(); const client = await pool.connect();