diff --git a/README.md b/README.md index a4872c7..74ed70d 100644 --- a/README.md +++ b/README.md @@ -64,9 +64,13 @@ DATABASE_URL=postgres://user:password@host:5432/database UPLOAD_PASSWORD=change-me CURRENT_ANALYSIS_VERSION=4 BODY_LIMIT=25mb +PUBLIC_FORM_BODY_LIMIT=100kb PORT=3000 ``` +`BODY_LIMIT` applies to authenticated analyser JSON and text uploads. +`PUBLIC_FORM_BODY_LIMIT` is the smaller limit for the public search form. + Run migrations: ```sh diff --git a/lib/asyncHandler.js b/lib/asyncHandler.js new file mode 100644 index 0000000..a2360a8 --- /dev/null +++ b/lib/asyncHandler.js @@ -0,0 +1,10 @@ +'use strict'; + +// Express 4 does not automatically forward rejected route promises to error +// middleware. Keep async handlers concise while preserving the normal `next` +// error path. +module.exports = function asyncHandler(handler) { + return function wrappedAsyncHandler(req, res, next) { + return Promise.resolve(handler(req, res, next)).catch(next); + }; +}; diff --git a/routes/index.js b/routes/index.js index 340f277..fe632fe 100644 --- a/routes/index.js +++ b/routes/index.js @@ -7,6 +7,7 @@ const jurisdiction = require('../lib/jurisdiction'); const cache = require('../lib/cache'); const { isValidAppId } = require('../lib/appId'); const { classifyAnalysisFailure } = require('../lib/analysisFailure'); +const asyncHandler = require('../lib/asyncHandler'); // Taken from https://reports.exodus-privacy.eu.org/api/trackers const exodusTrackers = JSON.parse(fs.readFileSync('./exodusTrackers.json', 'utf-8')) @@ -150,7 +151,7 @@ async function getSiteData() { } } -router.get('/', async (req, res) => { +router.get('/', asyncHandler(async (req, res) => { try { const data = await getSiteData(); return res.render('form', { @@ -172,10 +173,10 @@ router.get('/', async (req, res) => { jurisdictionMeta: jurisdiction.classificationMeta }); } -}); +})); // Statistics detail page -router.get('/statistics', async (req, res) => { +router.get('/statistics', asyncHandler(async (req, res) => { try { const data = await getSiteData(); return res.render('statistics', { @@ -201,9 +202,9 @@ router.get('/statistics', async (req, res) => { xrayCompanyCount: jurisdiction.xrayCompanyCount }); } -}); +})); -router.get('/healthz', async (req, res) => { +router.get('/healthz', asyncHandler(async (req, res) => { try { await Apps.healthCheck(); res.json({ ok: true }); @@ -211,9 +212,9 @@ router.get('/healthz', async (req, res) => { console.error('Health check failed:', err.message); res.status(503).json({ ok: false }); } -}); +})); -router.get('/healthz/analyser', async (req, res) => { +router.get('/healthz/analyser', (req, res) => { const online = lastPing > Date.now() - 1000*60*60; res.status(online ? 200 : 503).json({ ok: online }); }); @@ -224,7 +225,7 @@ router.post('/search', .isLength({ min: 1 }) .withMessage('Please enter a search term'), ], - async (req, res) => { + asyncHandler(async (req, res) => { const errors = validationResult(req); if (errors.isEmpty()) { @@ -252,9 +253,9 @@ router.post('/search', data: req.body, }); }; -}); +})); -router.get('/analysis/:appId', async (req, res) => { +router.get('/analysis/:appId', asyncHandler(async (req, res) => { if (!isValidAppId(req.params.appId)) return res.status(400).send('Please provide a valid App Store bundle ID.'); let appId = req.params.appId; @@ -325,17 +326,17 @@ router.get('/analysis/:appId', async (req, res) => { trackerNameToExodus: trackerNameToExodus, jurisdictionData: jurisdictionData }); -}); +})); // About page -router.get('/about', async (req, res) => { +router.get('/about', (req, res) => { res.render('about', { title: 'About' }); }); // serve next task to analyser -router.get('/queue', async (req, res) => { +router.get('/queue', asyncHandler(async (req, res) => { let app = await Apps.nextApp(); console.log(app); @@ -343,17 +344,17 @@ router.get('/queue', async (req, res) => { return res.send(); res.send(app.appid); -}); +})); // enable analyser to report online status -router.get('/ping', async (req, res) => { +router.get('/ping', (req, res) => { lastPing = Date.now(); res.send("online"); }); // upload analysis results -router.post('/uploadAnalysis', async (req, res) => { +router.post('/uploadAnalysis', asyncHandler(async (req, res) => { if (!req.query.appId || !req.query.analysisVersion) return res.status(400).send('Please provide appId and analysisVersion'); const appId = req.query.appId; @@ -371,10 +372,10 @@ router.post('/uploadAnalysis', async (req, res) => { const result = await Apps.updateAnalysis(appId, analysis, analysisVersion); cache.invalidate('sitedata'); res.send(result); -}); +})); // avoid a loop: only analyse each app once -router.post('/reportAnalysisFailure', async (req, res) => { +router.post('/reportAnalysisFailure', asyncHandler(async (req, res) => { if (!req.query.appId || !req.query.analysisVersion) return res.status(400).send('Please provide appId and analysisVersion'); @@ -393,7 +394,7 @@ router.post('/reportAnalysisFailure', async (req, res) => { }, req.query.analysisVersion); cache.invalidate('sitedata'); res.send(result); -}); +})); /*router.get('/sitemap.xml', async (req, res) => { try { diff --git a/server.js b/server.js index cd4fd33..10dd58f 100644 --- a/server.js +++ b/server.js @@ -48,7 +48,8 @@ if(os.hostname().indexOf("local") <= -1) { // only on remote host app.use(limiter) } -const bodyLimit = process.env.BODY_LIMIT || '25mb'; +const analyserBodyLimit = process.env.BODY_LIMIT || '25mb'; +const publicFormBodyLimit = process.env.PUBLIC_FORM_BODY_LIMIT || '100kb'; app.use((req, res, next) => { if (isAnalyserPath(req) && !analyserAuthenticated(req)) @@ -61,10 +62,15 @@ app.use((req, res, next) => { app.set('views', path.join(__dirname, 'views')); app.set('view engine', 'pug'); -// set up parsing of form inputs and of application/json -app.use(bodyParser.urlencoded({ extended: true, limit: bodyLimit })); -app.use(bodyParser.json({ limit: bodyLimit })); -app.use(express.text({ limit: bodyLimit })); +// Public requests only need the small search form parser. Large analyser +// payload parsers are mounted on their authenticated endpoints so arbitrary +// public and nonexistent routes cannot consume the analyser body allowance. +app.post('/search', bodyParser.urlencoded({ + extended: true, + limit: publicFormBodyLimit +})); +app.post('/uploadAnalysis', bodyParser.json({ limit: analyserBodyLimit })); +app.post('/reportAnalysisFailure', express.text({ limit: analyserBodyLimit })); // serve static files app.use(express.static('public')); @@ -77,4 +83,22 @@ app.use('/favicon.ico', express.static('favicon.ico')); const routes = require('./routes/index'); app.use('/', routes); +// Express 4 requires rejected async handlers to call next(err). Route handlers +// use asyncHandler for that bridge and all errors terminate here. +app.use((err, req, res, next) => { + if (res.headersSent) + return next(err); + + const errorStatus = err.status || err.statusCode; + const status = Number.isInteger(errorStatus) + && errorStatus >= 400 + && errorStatus <= 599 + ? errorStatus + : 500; + if (status >= 500) + console.error('Request failed:', err.stack || err.message); + const message = err.expose ? err.message : 'Internal server error.'; + return res.status(status).send(message); +}); + module.exports = app; // make accessible to /start.js diff --git a/test/requestHandling.test.js b/test/requestHandling.test.js new file mode 100644 index 0000000..fa2f175 --- /dev/null +++ b/test/requestHandling.test.js @@ -0,0 +1,143 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const test = require('node:test'); +const Apps = require('../models/Apps'); + +process.env.UPLOAD_PASSWORD = 'test-secret'; +process.env.BODY_LIMIT = '2kb'; +process.env.PUBLIC_FORM_BODY_LIMIT = '100b'; + +const app = require('../server'); + +async function withServer(run) { + const server = await new Promise((resolve) => { + const instance = app.listen(0, '127.0.0.1', () => resolve(instance)); + }); + + try { + await run(`http://127.0.0.1:${server.address().port}`); + } finally { + await new Promise((resolve, reject) => { + server.close((err) => err ? reject(err) : resolve()); + }); + } +} + +test('rejected async route promises reach centralized error middleware', async () => { + const originalNextApp = Apps.nextApp; + const originalConsoleError = console.error; + Apps.nextApp = async () => { + throw new Error('database unavailable'); + }; + console.error = () => {}; + + try { + await withServer(async (base) => { + const response = await fetch(`${base}/queue`, { + headers: { authorization: 'Bearer test-secret' } + }); + + assert.equal(response.status, 500); + assert.equal(await response.text(), 'Internal server error.'); + }); + } finally { + Apps.nextApp = originalNextApp; + console.error = originalConsoleError; + } +}); + +test('large body parsers are scoped to authenticated analyser endpoints', async () => { + const originalUpdateAnalysis = Apps.updateAnalysis; + const originalConsoleLog = console.log; + const updates = []; + Apps.updateAnalysis = async (...args) => { + updates.push(args); + return 'updated'; + }; + console.log = () => {}; + + const jsonPayload = JSON.stringify({ report: 'x'.repeat(250) }); + const textPayload = 'failure log '.repeat(30); + const oversizedPayload = JSON.stringify({ report: 'x'.repeat(2100) }); + + try { + await withServer(async (base) => { + const missingResponse = await fetch(`${base}/does-not-exist`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: oversizedPayload + }); + assert.equal(missingResponse.status, 404); + + const unauthenticatedResponse = await fetch( + `${base}/uploadAnalysis?appId=com.example.app&analysisVersion=1`, + { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: oversizedPayload + } + ); + assert.equal(unauthenticatedResponse.status, 400); + + const publicFormResponse = await fetch(`${base}/search`, { + method: 'POST', + headers: { 'content-type': 'application/x-www-form-urlencoded' }, + body: `search=${'x'.repeat(120)}` + }); + assert.equal(publicFormResponse.status, 413); + + const uploadResponse = await fetch( + `${base}/uploadAnalysis?appId=com.example.app&analysisVersion=1`, + { + method: 'POST', + headers: { + authorization: 'Bearer test-secret', + 'content-type': 'application/json' + }, + body: jsonPayload + } + ); + assert.equal(uploadResponse.status, 200); + assert.equal(await uploadResponse.text(), 'updated'); + + const failureResponse = await fetch( + `${base}/reportAnalysisFailure?appId=com.example.app&analysisVersion=1`, + { + method: 'POST', + headers: { + authorization: 'Bearer test-secret', + 'content-type': 'text/plain' + }, + body: textPayload + } + ); + assert.equal(failureResponse.status, 200); + assert.equal(await failureResponse.text(), 'updated'); + + const oversizedUploadResponse = await fetch( + `${base}/uploadAnalysis?appId=com.example.app&analysisVersion=1`, + { + method: 'POST', + headers: { + authorization: 'Bearer test-secret', + 'content-type': 'application/json' + }, + body: oversizedPayload + } + ); + assert.equal(oversizedUploadResponse.status, 413); + + assert.equal(updates.length, 2); + assert.deepEqual(updates[0].slice(0, 2), [ + 'com.example.app', + JSON.parse(jsonPayload) + ]); + assert.equal(updates[1][0], 'com.example.app'); + assert.equal(updates[1][1].logs, textPayload); + }); + } finally { + Apps.updateAnalysis = originalUpdateAnalysis; + console.log = originalConsoleLog; + } +});