From e03d13efde843c7a7275cd41c855b402538756e7 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Fri, 17 Jul 2026 21:30:48 +0200 Subject: [PATCH] fix(uploads): tighten guest max-file-size setting (codex review of #823) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups from the Codex review of #823: 1. PublicSettings TypeScript interface was missing general_max_file_size_mb, so UserPhotoUpload's access produced TS2339 under `tsc -b` (build:check). CI didn't catch it because the pipeline runs `build` (esbuild, no typecheck), but it's a real type gap — the #614 count field is declared, this one wasn't. Added the optional numeric field. 2. The general-settings update endpoint validated general_max_files_per_upload but not general_max_file_size_mb, so an out-of-range value (0, -1, huge) could persist. publicSettings then advertised the raw value while getMaxFileSizeMb() normalised it — the guest UI would reject files the backend accepts. Added the same validate-and-clamp block (1..MAX_ALLOWED_FILE_SIZE_MB). 3. The update route cleared the file-count cache but not the new file-size cache, so for up to 60s the public endpoint could advertise a new limit while multer still enforced the old one. Now clears both under the same uploadLimitTouched guard. Follow-up on the merged #823 (main-only), so this targets main only. --- backend/src/routes/adminSettings.js | 21 ++++++++++++++++++- .../src/services/publicSettings.service.ts | 3 +++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/backend/src/routes/adminSettings.js b/backend/src/routes/adminSettings.js index a46bc64d..5183a6bb 100644 --- a/backend/src/routes/adminSettings.js +++ b/backend/src/routes/adminSettings.js @@ -25,7 +25,7 @@ const { resetSecurityConfigCache } = require('../utils/authSecurity'); const { errorResponse } = require('../utils/routeHelpers'); const logger = require('../utils/logger'); const router = express.Router(); -const { clearMaxFilesPerUploadCache, MAX_ALLOWED_FILES_PER_UPLOAD } = require('../services/uploadSettings'); +const { clearMaxFilesPerUploadCache, MAX_ALLOWED_FILES_PER_UPLOAD, clearMaxFileSizeCache, MAX_ALLOWED_FILE_SIZE_MB } = require('../services/uploadSettings'); const watermarkService = require('../services/watermarkService'); const watermarkGeneratorService = require('../services/watermarkGeneratorService'); @@ -1095,6 +1095,24 @@ router.put('/general', adminAuth, requirePermission('settings.edit'), async (req settings.general_max_files_per_upload = normalizedValue; } + // Per-file size limit (MB). Validate/clamp on save, mirroring the count + // above, so an out-of-range value can't be persisted — otherwise the public + // endpoint would advertise the raw value while getMaxFileSizeMb() normalizes + // it, and the guest UI would reject files the backend actually accepts. + if (Object.prototype.hasOwnProperty.call(settings, 'general_max_file_size_mb')) { + uploadLimitTouched = true; + const rawValue = Number(settings.general_max_file_size_mb); + const normalizedValue = Number.isFinite(rawValue) ? Math.floor(rawValue) : NaN; + + if (!Number.isInteger(normalizedValue) || normalizedValue < 1 || normalizedValue > MAX_ALLOWED_FILE_SIZE_MB) { + return res.status(400).json({ + error: `general_max_file_size_mb must be an integer between 1 and ${MAX_ALLOWED_FILE_SIZE_MB}` + }); + } + + settings.general_max_file_size_mb = normalizedValue; + } + if (publicSiteKeysTouched) { if (Object.prototype.hasOwnProperty.call(settings, 'general_public_site_custom_css')) { settings.general_public_site_custom_css = sanitizeCss(settings.general_public_site_custom_css || ''); @@ -1151,6 +1169,7 @@ router.put('/general', adminAuth, requirePermission('settings.edit'), async (req } if (uploadLimitTouched) { clearMaxFilesPerUploadCache(); + clearMaxFileSizeCache(); } if (Object.prototype.hasOwnProperty.call(settings, 'general_short_gallery_urls')) { clearShareLinkSettingsCache(); diff --git a/frontend/src/services/publicSettings.service.ts b/frontend/src/services/publicSettings.service.ts index 1ce62b08..f7e461fe 100644 --- a/frontend/src/services/publicSettings.service.ts +++ b/frontend/src/services/publicSettings.service.ts @@ -82,6 +82,9 @@ export interface PublicSettings { // modal can render the real number in `upload.fileRequirements` and refuse // oversized batches client-side. Backend enforces the same value too. general_max_files_per_upload?: number; + // #613 follow-up — per-file size limit (MB), surfaced so the guest upload + // modal shows the real limit and guards client-side. Backend enforces it too. + general_max_file_size_mb?: number; // Event field requirements event_require_customer_name?: boolean; event_require_customer_email?: boolean;