fix(uploads): tighten guest max-file-size setting (codex review of #823)
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.
This commit is contained in:
@@ -25,7 +25,7 @@ const { resetSecurityConfigCache } = require('../utils/authSecurity');
|
|||||||
const { errorResponse } = require('../utils/routeHelpers');
|
const { errorResponse } = require('../utils/routeHelpers');
|
||||||
const logger = require('../utils/logger');
|
const logger = require('../utils/logger');
|
||||||
const router = express.Router();
|
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 watermarkService = require('../services/watermarkService');
|
||||||
const watermarkGeneratorService = require('../services/watermarkGeneratorService');
|
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;
|
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 (publicSiteKeysTouched) {
|
||||||
if (Object.prototype.hasOwnProperty.call(settings, 'general_public_site_custom_css')) {
|
if (Object.prototype.hasOwnProperty.call(settings, 'general_public_site_custom_css')) {
|
||||||
settings.general_public_site_custom_css = sanitizeCss(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) {
|
if (uploadLimitTouched) {
|
||||||
clearMaxFilesPerUploadCache();
|
clearMaxFilesPerUploadCache();
|
||||||
|
clearMaxFileSizeCache();
|
||||||
}
|
}
|
||||||
if (Object.prototype.hasOwnProperty.call(settings, 'general_short_gallery_urls')) {
|
if (Object.prototype.hasOwnProperty.call(settings, 'general_short_gallery_urls')) {
|
||||||
clearShareLinkSettingsCache();
|
clearShareLinkSettingsCache();
|
||||||
|
|||||||
@@ -82,6 +82,9 @@ export interface PublicSettings {
|
|||||||
// modal can render the real number in `upload.fileRequirements` and refuse
|
// modal can render the real number in `upload.fileRequirements` and refuse
|
||||||
// oversized batches client-side. Backend enforces the same value too.
|
// oversized batches client-side. Backend enforces the same value too.
|
||||||
general_max_files_per_upload?: number;
|
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 field requirements
|
||||||
event_require_customer_name?: boolean;
|
event_require_customer_name?: boolean;
|
||||||
event_require_customer_email?: boolean;
|
event_require_customer_email?: boolean;
|
||||||
|
|||||||
Reference in New Issue
Block a user