/** * The one validated path from a multipart request to files on the uploads * volume. * * Extracted from routes/admin.ts when a second caller appeared (#222's public * intake endpoint). It is deliberately one module rather than two similar * ones: every property that makes an upload safe here — the type allowlist, * the magic-byte check after the write, names from a CSPRNG rather than from * `originalname`, the re-encode that strips EXIF, and the cleanup of whatever * a refused request left behind — is a property a second implementation would * have to reproduce exactly. A near-copy that drifted would be precisely the * gap #95, #103, #180 and #226 exist to close. * * Nothing below changed in the move. The comments came with it, because they * record why the code is shaped as it is and are the most valuable part of it. */ import { Request, Response, NextFunction } from 'express'; import multer from 'multer'; import { promises as fs } from 'fs'; import { randomUUID } from 'crypto'; import { PoolClient } from 'pg'; import { ALLOWED_IMAGE_TYPES, SIGNATURE_BYTES, extensionFor, isAllowedImageType, signatureMatches } from './uploadTypes'; import { reencodeInPlace } from './imageProcessing'; 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; // 8 MB, not 8 MiB — this is the ceiling S5693 treats as safe, and 8 * 1024 * // 1024 sits just over it. Plenty for a product photo either way. const MAX_IMAGE_BYTES = 8_000_000; const MAX_TEXT_FIELDS = 8; const MAX_TEXT_FIELD_BYTES = 64 * 1024; // Refused before a byte is written. This catches the honest mistake — picking a // PDF by accident — and nothing more, because file.mimetype is whatever the // caller wrote in the multipart headers. The bytes are checked after the write; // see verifyUploadedImages. class UnsupportedImageTypeError extends Error {} 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. // // The extension comes from the validated content type rather than from // path.extname(file.originalname), so the name on disk cannot disagree with // what the file claims to be — a caller cannot get `.html` onto the uploads // volume by naming their file that way. filename: (_req, file, cb) => { const ext = extensionFor(file.mimetype); if (!ext) { // Unreachable while fileFilter runs first, and here so that it stays // unreachable rather than silently writing a file with no extension. cb(new UnsupportedImageTypeError(`unsupported image type ${file.mimetype}`), ''); return; } cb(null, `${randomUUID()}${ext}`); } }); // Reviewed for #180. Bounding one request is only half the problem — see // discardUnlessAccepted below for the other half, which is bounding what the // volume accumulates across requests that were refused. const upload = multer({ storage, limits: { fileSize: MAX_IMAGE_BYTES, files: MAX_IMAGES_PER_REQUEST, fields: MAX_TEXT_FIELDS, fieldSize: MAX_TEXT_FIELD_BYTES }, fileFilter: (_req, file, cb) => { if (!isAllowedImageType(file.mimetype)) { cb(new UnsupportedImageTypeError( `${file.mimetype} is not an accepted image type — allowed: ${ALLOWED_IMAGE_TYPES.join(', ')}` )); return; } cb(null, true); } }); // Reads only the leading bytes — enough to identify a format, not enough to // care how large the file is. The handle is closed before anything is unlinked, // because an open handle makes the unlink fail on Windows. // // Reviewed for #180. The path is not caller-controlled despite arriving from a // request: multer composes it from `destination`, which is a server constant, // and `filename`, which the storage above sets to `randomUUID()` plus an // extension looked up from the validated content type. The caller's // `originalname` is never consulted, so no part of the path traverses anywhere. async function readHead(filePath: string): Promise { const handle = await fs.open(filePath, 'r'); try { const buffer = Buffer.alloc(SIGNATURE_BYTES); const { bytesRead } = await handle.read(buffer, 0, SIGNATURE_BYTES, 0); return buffer.subarray(0, bytesRead); } finally { await handle.close(); } } // Best effort: a file that cannot be removed should not turn a 400 into a 500, // but it must not be left behind quietly either. async function discardUploads(files: Express.Multer.File[]): Promise { await Promise.all( files.map((file) => fs.unlink(file.path).catch((err: unknown) => { console.error(`[upload] could not remove rejected file ${file.path}:`, err); }) ) ); } /** * Removes a request's uploaded files unless the request actually succeeded. * * multer writes to disk before any route logic runs, and its own cleanup only * covers errors it raised itself. Everything after that — a failed signature * check, a malformed `category_id`, a database error, a dropped connection — * previously left the bytes on the volume with nothing referencing them: no row * to find them by, and no bound on how many could accumulate. Bounding the size * of one upload does not help if every refused upload is kept forever (#180). * * Registered as soon as multer succeeds rather than at each `return`, so a * route added later inherits it instead of having to remember it. That is the * whole reason it is a hook and not a call: the failure it prevents is someone * adding a fourth early return. * * `close` rather than `finish`, so an aborted connection is covered too, and * `writableEnded` distinguishes a response that completed from one that never * did — the latter is not a success however its status code reads. */ function discardUnlessAccepted(req: Request, res: Response): void { res.on('close', () => { if (res.writableEnded && res.statusCode < 400) return; void discardUploads((req.files as Express.Multer.File[]) || []); }); } /** * Confirms each stored file actually is what it was declared to be. * * This cannot happen in multer's fileFilter, which runs before the stream has * been read — there are no bytes to look at yet. So the check runs after the * write. * * Checking only: removing the files is discardUnlessAccepted's job, and doing * it here as well would unlink twice and log an ENOENT for every refused * upload. That also covers the case this function used to miss — `readHead` * itself throwing, which returned no message and so cleaned up nothing. * * Returns the message to refuse with, or null when everything checks out. */ async function verifyUploadedImages(req: Request): Promise { const files = (req.files as Express.Multer.File[]) || []; for (const file of files) { const head = await readHead(file.path); if (!signatureMatches(file.mimetype, head)) { return `${file.originalname} does not contain ${file.mimetype} data`; } } return null; } /** * Rebuilds every accepted file so it carries no metadata (#226). * * After verification, deliberately: re-encoding a file whose bytes do not match * its declared type would be doing work on something already refused, and * sharp's own error would replace the clearer message that check produces. * * A failure here refuses the upload rather than storing the original. Storing * it would mean the one case where a photo keeps the coordinates it was taken * at is the case nobody was told about. * * Returns the message to refuse with, or null when every file was rebuilt. */ async function stripUploadedImages(req: Request): Promise { const files = (req.files as Express.Multer.File[]) || []; for (const file of files) { try { await reencodeInPlace(file.path, file.mimetype); } catch (err) { console.error(`[upload] could not re-encode ${file.path}:`, err); return `${file.originalname} could not be processed`; } } return null; } // 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 UnsupportedImageTypeError) { return res.status(400).json({ error: err.message }); } if (err instanceof multer.MulterError) { const status = err.code === 'LIMIT_FILE_SIZE' ? 413 : 400; return res.status(status).json({ error: err.message }); } if (err) { return next(err); } // Every file is on disk by this point and multer will not clean up after // itself again, so the bytes become this request's responsibility before // anything else is allowed to fail. discardUnlessAccepted(req, res); verifyUploadedImages(req) .then((problem) => { if (problem) { res.status(400).json({ error: problem }); return null; } return stripUploadedImages(req); }) .then((problem) => { // The first stage returns null both when it answered and when it found // nothing wrong, so the response itself is what distinguishes them. if (res.headersSent) return; if (problem) { res.status(400).json({ error: problem }); return; } next(); }) .catch(next); }); }; /** * Records uploaded files as an item's images. * * Create and update wrote this loop separately, differing only in where the id * came from and where the sort order started — zero for a new item, one past * the current maximum for an existing one. Both are parameters now. * * It also means the `/uploads/` prefix is written once. That matters more than * it looks: #103 made the stored value the path `uploadUrl` joins an origin * onto, so it is a contract rather than a string, and two places to change it * is one place to forget. */ async function insertItemImages( client: PoolClient, itemId: number, files: Express.Multer.File[], firstSortOrder: number ): Promise { // Iterated by entry rather than by index, so there is no possibly-undefined // element to guard — the create path used to fall back to an empty filename, // which would have stored a path pointing at the uploads directory itself. for (const [offset, file] of files.entries()) { await client.query( `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`, [itemId, `/uploads/${file.filename}`, firstSortOrder + offset] ); } } export { uploadImages, verifyUploadedImages, stripUploadedImages, insertItemImages, MAX_IMAGES_PER_REQUEST, MAX_IMAGE_BYTES };