From 7c0c5c1cda3921cbd9404dd77d64bdec7a9ae2aa Mon Sep 17 00:00:00 2001 From: Trung-Tin Pham <60747384+AtelyPham@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:42:07 +0700 Subject: [PATCH] fix(upload): let the csrf gate pass application/octet-stream chunks (#1401) The chunked upload route reads the raw request body and its only client sends application/octet-stream, but the CSRF gate on /api rejected every such request with 415 before it reached the route. The endpoint had never accepted a chunk. The gate's origin check is the CSRF defence. Its Content-Type list only has to keep out what a cross-site page can send without a preflight, and octet-stream is not one of those: a form cannot produce it and fetch() with it is not CORS-safelisted. Accept it. express.json leaves an octet-stream body unread, so JSON routes see an empty body as before. Fixes PicPeak/picpeak#1377 --- .../middleware/csrfChunkedUpload.test.js | 75 +++++++++++++++++++ backend/src/middleware/csrf.js | 13 +++- 2 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 backend/__tests__/middleware/csrfChunkedUpload.test.js diff --git a/backend/__tests__/middleware/csrfChunkedUpload.test.js b/backend/__tests__/middleware/csrfChunkedUpload.test.js new file mode 100644 index 00000000..18a56be4 --- /dev/null +++ b/backend/__tests__/middleware/csrfChunkedUpload.test.js @@ -0,0 +1,75 @@ +/** + * The chunked upload client posts each chunk as application/octet-stream and + * the route reads the raw request stream. The CSRF gate answered every such + * request 415 before it reached the route, so the endpoint never accepted a + * chunk (PicPeak/picpeak#1377). + * + * The gate's origin check is the CSRF defence. The Content-Type list only + * has to keep out what a cross-site page can send without a preflight, and + * application/octet-stream is not on that list: an HTML form cannot produce + * it and fetch() with it is not CORS-safelisted. + */ +const express = require('express'); +const request = require('supertest'); + +jest.mock('../../src/utils/logger', () => ({ error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() })); + +function buildApp() { + const app = express(); + // Same order as server.js: scoped JSON parser, then the gate on /api. + app.use(['/api/admin', '/api/v1'], express.json({ limit: '50mb' })); + app.use(express.json({ limit: '2mb' })); + app.use('/api', require('../../src/middleware/csrf')); + // Mirrors the chunk route in adminPhotos.js: consume the raw stream. + app.post('/api/admin/photos/:eventId/chunked-upload/:uploadId/chunk/:chunkIndex', async (req, res) => { + const chunks = []; + for await (const chunk of req) chunks.push(chunk); + res.json({ received: Buffer.concat(chunks).toString('base64') }); + }); + app.post('/api/admin/other', (req, res) => res.json({ body: req.body })); + return app; +} + +const CHUNK_PATH = '/api/admin/photos/1/chunked-upload/abc/chunk/0'; + +describe('CSRF gate and application/octet-stream', () => { + it('lets a same-origin octet-stream chunk reach the route byte for byte', async () => { + // Not valid UTF-8, so a text decode anywhere on the path would show up. + const payload = Buffer.concat([Buffer.from('chunkbytes'), Buffer.from([0xff, 0x00, 0xfe])]); + const res = await request(buildApp()) + .post(CHUNK_PATH) + .set('sec-fetch-site', 'same-origin') + .set('Content-Type', 'application/octet-stream') + .send(payload); + expect(res.status).toBe(200); + expect(Buffer.from(res.body.received, 'base64').equals(payload)).toBe(true); + }); + + it('still rejects a cross-site octet-stream post on origin', async () => { + const res = await request(buildApp()) + .post(CHUNK_PATH) + .set('sec-fetch-site', 'cross-site') + .set('Content-Type', 'application/octet-stream') + .send(Buffer.from('chunkbytes')); + expect(res.status).toBe(403); + }); + + it('still rejects the types a form can send', async () => { + const res = await request(buildApp()) + .post('/api/admin/other') + .set('sec-fetch-site', 'same-origin') + .set('Content-Type', 'text/plain') + .send('x=1'); + expect(res.status).toBe(415); + }); + + it('leaves a JSON route with an empty body on an octet-stream post', async () => { + const res = await request(buildApp()) + .post('/api/admin/other') + .set('sec-fetch-site', 'same-origin') + .set('Content-Type', 'application/octet-stream') + .send(Buffer.from('{"a":1}')); + expect(res.status).toBe(200); + expect(res.body.body).toEqual({}); + }); +}); diff --git a/backend/src/middleware/csrf.js b/backend/src/middleware/csrf.js index 152a6bf7..57ccf780 100644 --- a/backend/src/middleware/csrf.js +++ b/backend/src/middleware/csrf.js @@ -1,5 +1,12 @@ const { mutationOriginAllowed } = require('../utils/requestOrigin'); +// The origin check is the CSRF defence. This list only has to keep out what +// a cross-site page can send without a preflight: a form cannot produce JSON +// or octet-stream, and fetch() with either is not CORS-safelisted. +// octet-stream is how the chunked upload route receives its raw body +// (#1377); express.json leaves it unread for everything else. +const ALLOWED_CONTENT_TYPES = ['application/json', 'multipart/form-data', 'application/octet-stream']; + module.exports = function csrfProtection(req, res, next) { if (!['POST', 'PUT', 'PATCH', 'DELETE'].includes(req.method)) return next(); if (!mutationOriginAllowed(req)) { @@ -7,9 +14,9 @@ module.exports = function csrfProtection(req, res, next) { } const contentType = (req.headers['content-type'] || '').split(';')[0].trim().toLowerCase(); const hasBody = Number(req.headers['content-length']) > 0 || !!req.headers['transfer-encoding']; - const jsonLike = contentType === 'application/json' || contentType.endsWith('+json'); - if (hasBody && !jsonLike && contentType !== 'multipart/form-data') { - return res.status(415).json({ error: 'Unsupported Content-Type. Use application/json or multipart/form-data.' }); + const allowed = ALLOWED_CONTENT_TYPES.includes(contentType) || contentType.endsWith('+json'); + if (hasBody && !allowed) { + return res.status(415).json({ error: `Unsupported Content-Type. Use ${ALLOWED_CONTENT_TYPES.join(', ')}.` }); } next(); };