fix(upload): enforce the configured per-file size limit on admin uploads

getMaxFileSizeBytes() (general_max_file_size_mb, default 50MB) was only read
by adminSettings.js to display the value. The admin upload routes streamed
against a hardcoded ceiling instead, so the dropzone's "max. 50MB pro Datei"
was never enforced:

- adminPhotos.js POST /:eventId/upload    -> 10GB hardcoded
- adminPhotos.js POST /:eventId/chunked-upload/init -> 10GB hardcoded
- v1/events.js   POST /events/:id/photos  -> 100MB hardcoded

Resolve the cap per request (it is admin-configurable at runtime) and build
the multer instance from it, mirroring what gallery.js and adminTransfers.js
already do. The 400 names the configured limit and reuses gallery.js's exact
error string so the frontend surfaces it identically. getMaxFileSizeBytes()
clamps to MAX_ALLOWED_FILE_SIZE_MB, so the 10GB hard ceiling still bounds
everything.

gallery.js (guest upload) already enforced this correctly and is unchanged --
the report's claim that it did not is stale.

Interpretation: general_max_file_size_mb is a single per-file cap with no
photo/video split, and gallery.js already applies it blanket to guest video
uploads, so admin video uploads now share it too. On a default install that
means a 200MB video needs the setting raised first -- which is what the UI
has been advertising all along.

Refs testplan REPORT.md #1 (Part 7.06).
This commit is contained in:
Paul Nothaft
2026-09-01 16:23:04 +02:00
parent afaa00f428
commit e18ab0d842
4 changed files with 211 additions and 14 deletions
@@ -0,0 +1,148 @@
/**
* Per-file upload size limit on the admin photo routes.
*
* `general_max_file_size_mb` (Settings → General, default 50MB) is what the
* dropzone advertises ("max. 50MB per file"), but the admin upload route
* hardcoded multer's cap at 10GB and the chunked-upload init route at 10GB
* too — so the advertised limit was never enforced anywhere server-side and a
* 50.74MB JPEG uploaded cleanly.
*
* Pins:
* - a file over the configured cap is rejected with a 400 naming the limit
* - the chunked-upload init route honours the same cap (it would otherwise
* be a trivial bypass of the multipart route's cap)
* - a file under the cap still gets past the size gate
* - the limit is read per request, so an admin raising it takes effect
*/
const path = require('path');
const fs = require('fs');
const os = require('os');
process.env.NODE_ENV = 'test';
process.env.TEST_DATABASE_PATH = path.join(
fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-upload-size-')), 'db.sqlite',
);
process.env.JWT_SECRET = process.env.JWT_SECRET || 'upload-size-test-secret';
process.env.STORAGE_PATH = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-upload-size-storage-'));
const request = require('supertest');
const express = require('express');
const bcrypt = require('bcrypt');
const jwt = require('jsonwebtoken');
const { bootCrmDb, seedMinimal } = require('../integration/helpers/crmDb');
const SLUG = 'upload-size-test-event';
describe('admin upload per-file size limit (general_max_file_size_mb)', () => {
let db;
let cleanup;
let app;
let eventId;
let adminToken;
let uploadSettings;
const setLimitMb = async (mb) => {
await db('app_settings')
.insert({
setting_key: 'general_max_file_size_mb',
setting_value: JSON.stringify(mb),
setting_type: 'general',
updated_at: new Date().toISOString(),
})
.onConflict('setting_key')
.merge({ setting_value: JSON.stringify(mb) });
uploadSettings.clearMaxFileSizeCache();
};
const postUpload = (bytes, filename = 'shot.jpg') => request(app)
.post(`/api/admin/photos/${eventId}/upload`)
.set('Authorization', `Bearer ${adminToken}`)
.attach('photos', Buffer.alloc(bytes, 0x41), { filename, contentType: 'image/jpeg' });
const postChunkedInit = (fileSize) => request(app)
.post(`/api/admin/photos/${eventId}/chunked-upload/init`)
.set('Authorization', `Bearer ${adminToken}`)
.send({ filename: 'clip.mp4', fileSize, mimeType: 'video/mp4', totalChunks: 1 });
beforeAll(async () => {
({ db, cleanup } = await bootCrmDb());
await seedMinimal(db);
const inserted = await db('events').insert({
slug: SLUG,
event_type: 'wedding',
event_name: 'Upload Size Test',
event_date: '2026-09-01',
host_email: 'host@example.com',
admin_email: 'admin@example.com',
password_hash: 'x',
share_link: `/gallery/${SLUG}/share`,
share_token: 'upload-size-share',
expires_at: new Date(Date.now() + 7 * 24 * 3600 * 1000).toISOString(),
is_active: 1,
is_archived: 0,
is_draft: 0,
created_at: new Date().toISOString(),
}).returning('id');
eventId = inserted[0]?.id ?? inserted[0];
const superRole = await db('roles').where({ name: 'super_admin' }).first();
const [rootId] = await db('admin_users').insert({
username: 'upload-size-admin',
email: 'upload-size-admin@example.com',
password_hash: await bcrypt.hash('UploadSize123', 4),
role_id: superRole.id,
is_active: 1,
created_at: new Date().toISOString(),
updated_at: new Date().toISOString(),
}).returning('id').then((r) => [r[0]?.id || r[0]]);
adminToken = jwt.sign(
{ id: rootId, username: 'upload-size-admin', type: 'admin', role: 'super_admin', loginTime: Date.now() },
process.env.JWT_SECRET,
{ expiresIn: '1h', issuer: 'picpeak-auth' }
);
uploadSettings = require('../../src/services/uploadSettings');
app = express();
app.use(express.json());
app.use('/api/admin/photos', require('../../src/routes/adminPhotos'));
}, 120000);
afterAll(async () => { if (cleanup) await cleanup(); });
it('rejects a file over the configured limit with a 400 naming the limit', async () => {
await setLimitMb(1);
const res = await postUpload(2 * 1024 * 1024);
expect(res.status).toBe(400);
expect(res.body.error).toBe('File too large. Maximum size is 1 MB per file.');
});
it('rejects an over-limit chunked upload at init instead of allowing 10GB', async () => {
await setLimitMb(1);
const res = await postChunkedInit(200 * 1024 * 1024);
expect(res.status).toBe(400);
expect(res.body.error).toBe('File too large. Maximum size is 1 MB per file.');
});
it('lets a file under the limit past the size gate', async () => {
await setLimitMb(1);
// Junk bytes, so it still fails downstream on the content check — that is
// the point: the failure is no longer about size.
const res = await postUpload(64 * 1024);
expect(res.status).toBe(400);
expect(res.body.error).toBe('File content does not match declared type: shot.jpg');
});
it('reads the limit per request, so raising it takes effect immediately', async () => {
await setLimitMb(1);
expect((await postUpload(2 * 1024 * 1024)).status).toBe(400);
await setLimitMb(10);
const res = await postUpload(2 * 1024 * 1024);
expect(res.status).toBe(400);
expect(res.body.error).toBe('File content does not match declared type: shot.jpg');
});
});
+30 -10
View File
@@ -17,7 +17,7 @@ const { COLOR_LABELS, dominantColorLabel, SHARED_COLOR_LABEL_IDENTITY } = requir
const feedbackService = require('../services/feedbackService');
const photoAdminMarksService = require('../services/photoAdminMarksService');
const { validateUploadedFiles } = require('../middleware/uploadValidation');
const { getMaxFilesPerUpload, getAllowedMimeTypes } = require('../services/uploadSettings');
const { getMaxFilesPerUpload, getAllowedMimeTypes, getMaxFileSizeBytes, DEFAULT_MAX_FILE_SIZE_MB } = require('../services/uploadSettings');
const { processUploadedPhotos } = require('../services/photoProcessor');
const chunkedUpload = require('../services/chunkedUploadService');
const watermarkGeneratorService = require('../services/watermarkGeneratorService');
@@ -67,10 +67,16 @@ const { validateFileType, createFileUploadValidator } = require('../utils/fileSe
// The allowed types are fetched from the database once per request (before multer
// processes files) and attached to req.allowedMimeTypes so that the fileFilter
// callback can read them synchronously.
const upload = multer({
//
// The per-file size cap is resolved per request too (general_max_file_size_mb),
// so the uploader has to be built per request like the transfer routes do. It
// was hardcoded to 10GB here, which meant the advertised "max. 50MB per file"
// in the dropzone was never enforced anywhere server-side. getMaxFileSizeBytes()
// clamps to MAX_ALLOWED_FILE_SIZE_MB (10GB), so that hard ceiling still applies.
const createUpload = (maxFileSizeBytes) => multer({
storage: storage,
limits: {
fileSize: 10 * 1024 * 1024 * 1024, // 10GB limit per file to support large videos
fileSize: maxFileSizeBytes,
files: 2000, // Hard safety ceiling; actual limit enforced dynamically
fieldSize: 10 * 1024 * 1024, // 10MB for non-file fields
parts: 10000,
@@ -105,7 +111,9 @@ const validateUploadContent = async (req, res, next) => {
const allowedTypes = req.allowedMimeTypes || ['image/jpeg', 'image/png', 'image/webp'];
const validator = createFileUploadValidator({
allowedTypes,
maxFileSize: 10 * 1024 * 1024 * 1024, // 10GB to support large videos
// Same per-request cap multer streamed against, so the two layers can't
// disagree; this one names the offending file in the 400.
maxFileSize: req.maxFileSizeBytes || DEFAULT_MAX_FILE_SIZE_MB * 1024 * 1024,
validateContent: true
});
return validator(req, res, next);
@@ -132,21 +140,25 @@ const uploadTimeout = (timeout = 300000) => { // 5 minutes default
};
// Upload photos for an event
// Max file count is configurable via general settings
// Max file count and max file size are configurable via general settings
router.post('/:eventId/upload', adminAuth, requirePermission('photos.upload'), requireEventOwnership, uploadTimeout(600000), resolveAllowedTypes, async (req, res, next) => { // 10 minute timeout
let maxFilesPerUpload;
let maxFileSizeBytes;
try {
maxFilesPerUpload = await getMaxFilesPerUpload();
maxFileSizeBytes = await getMaxFileSizeBytes();
} catch (error) {
return errorResponse(res, error, 500, 'Unable to determine upload limits');
}
req.maxFileSizeBytes = maxFileSizeBytes;
const maxFileSizeMb = Math.floor(maxFileSizeBytes / (1024 * 1024));
upload.array('photos', maxFilesPerUpload)(req, res, (err) => {
createUpload(maxFileSizeBytes).array('photos', maxFilesPerUpload)(req, res, (err) => {
if (err) {
logger.error('Multer error:', err);
if (err instanceof multer.MulterError) {
if (err.code === 'LIMIT_FILE_SIZE') {
return res.status(400).json({ error: 'File too large. Maximum size is 10GB per file.' });
return res.status(400).json({ error: `File too large. Maximum size is ${maxFileSizeMb} MB per file.` });
}
if (err.code === 'LIMIT_FILE_COUNT' || err.code === 'LIMIT_UNEXPECTED_FILE') {
return res.status(400).json({ error: `Too many files. Maximum ${maxFilesPerUpload} files per upload.` });
@@ -1582,10 +1594,18 @@ router.post('/:eventId/chunked-upload/init', adminAuth, requirePermission('photo
return res.status(400).json({ error: 'Missing required fields: filename, fileSize, mimeType' });
}
// Validate file size (max 10GB)
const maxSize = 10 * 1024 * 1024 * 1024;
// Validate file size against the configured per-file cap. Hardcoding 10GB
// here let the chunked path sidestep general_max_file_size_mb entirely.
let maxSize;
try {
maxSize = await getMaxFileSizeBytes();
} catch {
maxSize = DEFAULT_MAX_FILE_SIZE_MB * 1024 * 1024;
}
if (fileSize > maxSize) {
return res.status(400).json({ error: 'File too large. Maximum size is 10GB.' });
return res.status(400).json({
error: `File too large. Maximum size is ${Math.floor(maxSize / (1024 * 1024))} MB per file.`
});
}
const result = await chunkedUpload.initializeUpload({
@@ -93,6 +93,14 @@ jest.mock('multer', () => {
return factory;
});
// The upload middleware resolves the per-file size cap from app_settings on
// every request (general_max_file_size_mb). That read would consume one of
// this suite's sequenced db chains and shift every later assertion, so stub it.
jest.mock('../../../services/uploadSettings', () => ({
getMaxFileSizeBytes: jest.fn().mockResolvedValue(50 * 1024 * 1024),
DEFAULT_MAX_FILE_SIZE_MB: 50,
}));
// Stub sharp so the happy-path test doesn't actually decode an image
// (the temp file is a 0-byte placeholder — see the beforeAll below).
jest.mock('sharp', () => jest.fn(() => ({
+25 -4
View File
@@ -38,6 +38,7 @@ const { formatBoolean } = require('../../utils/dbCompat');
const { parseBooleanInput } = require('../../utils/parsers');
const { isValidEventType } = require('../../services/eventTypeService');
const { replacePhoto } = require('../../services/photoReplacementService');
const { getMaxFileSizeBytes, DEFAULT_MAX_FILE_SIZE_MB } = require('../../services/uploadSettings');
const downloadZipService = require('../../services/downloadZipService');
const { PhotoFilterBuilder } = require('../../utils/photoFilterBuilder');
const { PhotoExportService } = require('../../services/photoExportService');
@@ -65,14 +66,34 @@ const photoStorage = multer.diskStorage({
cb(null, `v1_${Date.now()}_${crypto.randomBytes(4).toString('hex')}${ext}`);
}
});
const photoUpload = multer({
const buildPhotoUpload = (maxFileSizeBytes) => multer({
storage: photoStorage,
limits: { fileSize: 100 * 1024 * 1024 }, // 100MB per file for v1
limits: { fileSize: maxFileSizeBytes },
fileFilter: (_req, file, cb) => {
if (/^image\//.test(file.mimetype)) cb(null, true);
else cb(new Error('Only image uploads are accepted on this endpoint'));
}
});
}).single('photo');
// The per-file cap was hardcoded to 100MB here, so general_max_file_size_mb
// (Settings → General) didn't apply to the v1 upload either. Resolve it per
// request — the admin can change it at runtime — and turn multer's generic
// "File too large" into a 400 that names the configured limit.
const photoUpload = async (req, res, next) => {
let maxFileSizeBytes;
try {
maxFileSizeBytes = await getMaxFileSizeBytes();
} catch {
maxFileSizeBytes = DEFAULT_MAX_FILE_SIZE_MB * 1024 * 1024;
}
buildPhotoUpload(maxFileSizeBytes)(req, res, (err) => {
if (err && err.code === 'LIMIT_FILE_SIZE') {
const limitMb = Math.floor(maxFileSizeBytes / (1024 * 1024));
return res.status(400).json({ error: `File too large. Maximum size is ${limitMb} MB per file.` });
}
next(err);
});
};
// slugify now imported from ../../utils/slug — shared with adminEvents
// and events.js so the diacritic fix from #502 lands here too (#525).
@@ -616,7 +637,7 @@ router.post(
requireApiScope('write'),
requirePermission('photos.upload'),
requireEventOwnership,
photoUpload.single('photo'),
photoUpload,
async (req, res) => {
let tempPath = null;
try {