diff --git a/backend/__tests__/routes/adminPhotoUploadSizeLimit.test.js b/backend/__tests__/routes/adminPhotoUploadSizeLimit.test.js new file mode 100644 index 00000000..f6a96bfd --- /dev/null +++ b/backend/__tests__/routes/adminPhotoUploadSizeLimit.test.js @@ -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'); + }); +}); diff --git a/backend/src/routes/adminPhotos.js b/backend/src/routes/adminPhotos.js index 4c31ef1c..65e1c139 100644 --- a/backend/src/routes/adminPhotos.js +++ b/backend/src/routes/adminPhotos.js @@ -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({ diff --git a/backend/src/routes/v1/__tests__/events.category.test.js b/backend/src/routes/v1/__tests__/events.category.test.js index f3655dc7..bd935526 100644 --- a/backend/src/routes/v1/__tests__/events.category.test.js +++ b/backend/src/routes/v1/__tests__/events.category.test.js @@ -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(() => ({ diff --git a/backend/src/routes/v1/events.js b/backend/src/routes/v1/events.js index bb07a3ab..7f1d3e63 100644 --- a/backend/src/routes/v1/events.js +++ b/backend/src/routes/v1/events.js @@ -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 {