From 8b3e3258d343e341e16afd450e470deb890d0707 Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Sun, 18 Oct 2026 23:33:56 -0600 Subject: [PATCH 1/2] feat(scan): rate limit POST /api/scan per IP and per user - Anonymous scans are limited per IP (IPv6 grouped by subnet); signed-in scans are limited per user so shared IPs don't share a budget - Blocked requests get 429 RATE_LIMITED with Retry-After and never launch Chromium - Guard that express-rate-limit keeps a CommonJS entry for Vercel --- backend/README.md | 2 +- backend/app.js | 6 ++- backend/middleware/scanRateLimit.js | 54 ++++++++++++++++++++ backend/package-lock.json | 20 ++++++++ backend/package.json | 1 + backend/routes/index.js | 5 +- backend/routes/scan.js | 7 +-- backend/tests/dependencies.test.js | 10 ++++ backend/tests/scanRateLimit.test.js | 62 +++++++++++++++++++++++ docs/plans/axecore-integration-roadmap.md | 2 +- docs/plans/project-roadmap.md | 2 +- 11 files changed, 162 insertions(+), 9 deletions(-) create mode 100644 backend/middleware/scanRateLimit.js create mode 100644 backend/tests/scanRateLimit.test.js diff --git a/backend/README.md b/backend/README.md index a944854..3580a91 100644 --- a/backend/README.md +++ b/backend/README.md @@ -221,7 +221,7 @@ See also [`docs/guides/auth_storage_guide/githubGoogleAuthStorageImplementation. | GET | `/api/auth/user` | current user profile (no tokens) | | GET | `/api/auth/status` | `{ authenticated, user }` | | POST | `/api/auth/logout` | end session | -| POST | `/api/scan` | run a live Puppeteer + axe-core scan | +| POST | `/api/scan` | run a live Puppeteer + axe-core scan (rate limited: 10 / 15 min per IP, 30 per signed-in user; 429 `RATE_LIMITED`) | | GET | `/api/scan-results?url=` | re-run a scan for a URL (used by deep links) | | GET | `/api/scans` | list saved scans from the user's store | | GET | `/api/scans/:id` | load one saved report from the user's store | diff --git a/backend/app.js b/backend/app.js index b4a1de0..480c7fb 100644 --- a/backend/app.js +++ b/backend/app.js @@ -16,6 +16,7 @@ const mockScanResults = require('./data/mockScanResults'); const ScanRunner = require('./services/scanRunner'); const AuthService = require('./services/authService'); const StorageService = require('./services/storageService'); +const makeScanRateLimit = require('./middleware/scanRateLimit'); const scanRunner = new ScanRunner(); @@ -59,7 +60,10 @@ function buildApp(overrides = {}) { storageService, }); - mountRoutes(app, { scanController, authService, storageService }); + // Built per app so each instance (and each test app) starts with fresh counts. + const scanRateLimit = overrides.scanRateLimit || makeScanRateLimit(); + + mountRoutes(app, { scanController, authService, storageService, scanRateLimit }); return app; } diff --git a/backend/middleware/scanRateLimit.js b/backend/middleware/scanRateLimit.js new file mode 100644 index 0000000..9eeefeb --- /dev/null +++ b/backend/middleware/scanRateLimit.js @@ -0,0 +1,54 @@ +/** + * Rate limiting for POST /api/scan. Every scan launches a headless Chromium, + * so request volume is a cost and abuse surface, not just a fairness concern. + * + * Anonymous requests are limited per IP (IPv6 grouped by subnet, so rotating + * addresses within one allocation doesn't reset the count). Signed-in requests + * are limited per user instead — people sharing an office or campus IP each + * get their own budget. + * + * ponytail: in-memory store, so on Vercel each warm instance counts on its own + * and a cold start resets it. Move to a shared store (e.g. Redis/Upstash via + * express-rate-limit's `store` option) if abuse gets past per-instance limits. + */ +const { rateLimit } = require('express-rate-limit'); + +const WINDOW_MS = 15 * 60 * 1000; +const ANON_LIMIT = 10; +const USER_LIMIT = 30; + +const tooMany = (who) => ({ + error: `Too many scans ${who}. Try again in a few minutes.`, + code: 'RATE_LIMITED', +}); + +/** + * @param {{ windowMs?: number, anonLimit?: number, userLimit?: number }} [options] + * @returns {import('express').RequestHandler[]} + */ +function makeScanRateLimit({ + windowMs = WINDOW_MS, + anonLimit = ANON_LIMIT, + userLimit = USER_LIMIT, +} = {}) { + const common = { windowMs, standardHeaders: 'draft-8', legacyHeaders: false }; + + const perIp = rateLimit({ + ...common, + limit: anonLimit, + skip: (req) => Boolean(req.user), + message: tooMany('from this network'), + }); + + const perUser = rateLimit({ + ...common, + limit: userLimit, + skip: (req) => !req.user, + keyGenerator: (req) => `user:${req.user.id}`, + message: tooMany('from your account'), + }); + + return [perIp, perUser]; +} + +module.exports = makeScanRateLimit; diff --git a/backend/package-lock.json b/backend/package-lock.json index 2b4742b..76aaa5e 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -16,6 +16,7 @@ "cors": "^2.8.6", "dotenv": "^17.4.2", "express": "^5.2.1", + "express-rate-limit": "^8.7.1", "passport": "^0.7.0", "passport-github2": "^0.1.12", "puppeteer-core": "^24.43.1" @@ -1339,6 +1340,25 @@ "url": "https://opencollective.com/express" } }, + "node_modules/express-rate-limit": { + "version": "8.7.1", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-8.7.1.tgz", + "integrity": "sha512-2FXbqkygil3mZiXzEtM0IjE/UBmBNot0J1GEVrJT4JbrvwOSoEtqtO+wHbiMQELWe1eNvHoYRg2dkjOid+6JkA==", + "license": "MIT", + "dependencies": { + "debug": "^4.4.3", + "ip-address": "^10.2.0" + }, + "engines": { + "node": ">= 16" + }, + "funding": { + "url": "https://github.com/sponsors/express-rate-limit" + }, + "peerDependencies": { + "express": ">= 4.11" + } + }, "node_modules/extract-zip": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/extract-zip/-/extract-zip-2.0.1.tgz", diff --git a/backend/package.json b/backend/package.json index dcc0e62..ccc53c8 100644 --- a/backend/package.json +++ b/backend/package.json @@ -22,6 +22,7 @@ "cors": "^2.8.6", "dotenv": "^17.4.2", "express": "^5.2.1", + "express-rate-limit": "^8.7.1", "passport": "^0.7.0", "passport-github2": "^0.1.12", "puppeteer-core": "^24.43.1" diff --git a/backend/routes/index.js b/backend/routes/index.js index 3f28d6e..59883cf 100644 --- a/backend/routes/index.js +++ b/backend/routes/index.js @@ -12,14 +12,15 @@ const makeAuthRouter = require('./auth'); * scanController: import('../controllers/scanController'), * authService: import('../services/authService'), * storageService: import('../services/storageService'), + * scanRateLimit?: import('express').RequestHandler[], * }} deps */ -function mountRoutes(app, { scanController, authService, storageService }) { +function mountRoutes(app, { scanController, authService, storageService, scanRateLimit }) { // Session + passport once for all API routes (scan needs req.user too). app.use(...authService.middleware()); app.use('/api/auth', makeAuthRouter({ authService, storageService })); - app.use('/api', makeScanRouter(scanController)); + app.use('/api', makeScanRouter(scanController, { scanRateLimit })); app.use('/api/problems', makeProblemsRouter(scanController)); } diff --git a/backend/routes/scan.js b/backend/routes/scan.js index dd15992..e6b791e 100644 --- a/backend/routes/scan.js +++ b/backend/routes/scan.js @@ -1,6 +1,6 @@ /** * Scan-related routes: - * - POST /api/scan + * - POST /api/scan (rate limited per IP / per user) * - GET /api/scan-results * - GET /api/scans * - GET /api/scans/:id @@ -14,11 +14,12 @@ const { Router } = require('express'); /** * @param {ScanController} controller + * @param {{ scanRateLimit?: import('express').RequestHandler[] }} [options] * @returns {import('express').Router} */ -function makeScanRouter(controller) { +function makeScanRouter(controller, { scanRateLimit = [] } = {}) { const router = Router(); - router.post('/scan', controller.postScan); + router.post('/scan', ...scanRateLimit, controller.postScan); router.get('/scan-results', controller.getScanResults); router.get('/scans', controller.getSavedScans); router.get('/scans/:id', controller.getSavedScan); diff --git a/backend/tests/dependencies.test.js b/backend/tests/dependencies.test.js index 1e785ef..36084e6 100644 --- a/backend/tests/dependencies.test.js +++ b/backend/tests/dependencies.test.js @@ -28,3 +28,13 @@ test('@octokit/rest ships a CommonJS entry', () => { 'import() before upgrading.', ); }); + +test('express-rate-limit ships a CommonJS entry', () => { + // Same Vercel constraint as @octokit/rest above: an ESM-only release would + // pass locally and 500 POST /api/scan in production. + const pkg = readInstalledPackageJson('express-rate-limit'); + assert.ok( + Boolean(pkg.main) || Boolean(pkg.exports?.['.']?.require), + `express-rate-limit@${pkg.version} is ESM-only, which Vercel cannot require().`, + ); +}); diff --git a/backend/tests/scanRateLimit.test.js b/backend/tests/scanRateLimit.test.js new file mode 100644 index 0000000..86bada2 --- /dev/null +++ b/backend/tests/scanRateLimit.test.js @@ -0,0 +1,62 @@ +/** + * Rate limiting on POST /api/scan — per IP when anonymous, per user when + * signed in. + */ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const express = require('express'); +const request = require('supertest'); + +require('./helpers/testEnv'); +const { buildApp } = require('../app'); +const mockScanResults = require('../data/mockScanResults'); +const makeScanRateLimit = require('../middleware/scanRateLimit'); + +test('POST /api/scan returns 429 once an IP is over the limit, without scanning', async () => { + let scans = 0; + const app = buildApp({ + scanRunner: { run: async () => { scans += 1; return mockScanResults; } }, + scanRateLimit: makeScanRateLimit({ anonLimit: 2 }), + }); + const scan = () => request(app).post('/api/scan').send({ url: 'https://example.com' }); + + assert.equal((await scan()).status, 200); + assert.equal((await scan()).status, 200); + const blocked = await scan(); + + assert.equal(blocked.status, 429); + assert.equal(blocked.body.code, 'RATE_LIMITED'); + assert.match(blocked.body.error, /too many scans/i); + assert.ok(blocked.headers['retry-after']); + assert.equal(scans, 2); +}); + +test('signed-in users are limited per user, not by the shared IP', async () => { + const app = express(); + // Stand-in for passport: the header plays the role of the session user. + app.use((req, _res, next) => { + const id = req.get('x-test-user'); + if (id) req.user = { id }; + next(); + }); + app.post('/scan', ...makeScanRateLimit({ anonLimit: 1, userLimit: 2 }), (_req, res) => { + res.json({ ok: true }); + }); + const scan = (user) => { + const req = request(app).post('/scan'); + return user ? req.set('x-test-user', user) : req; + }; + + assert.equal((await scan('alice')).status, 200); + assert.equal((await scan('alice')).status, 200); + assert.equal((await scan('alice')).status, 429); + + // Same IP, different account: its own budget. + assert.equal((await scan('bob')).status, 200); + + // Signed-in traffic didn't use up the anonymous budget for this IP. + assert.equal((await scan()).status, 200); + const anonBlocked = await scan(); + assert.equal(anonBlocked.status, 429); + assert.match(anonBlocked.body.error, /from this network/); +}); diff --git a/docs/plans/axecore-integration-roadmap.md b/docs/plans/axecore-integration-roadmap.md index a21b0a0..934b2f3 100644 --- a/docs/plans/axecore-integration-roadmap.md +++ b/docs/plans/axecore-integration-roadmap.md @@ -124,7 +124,7 @@ tests. ### Phase 4 — Productionization - [ ] Caching of scan results by URL (with TTL) -- [ ] Rate limiting per IP +- [x] Rate limiting per IP (#128) - [ ] Background job queue (e.g. BullMQ) for slow sites - [ ] Persistence (Postgres) for scan history - [ ] Authentication (JWT) for scan endpoints diff --git a/docs/plans/project-roadmap.md b/docs/plans/project-roadmap.md index 84c259f..1a6fbf3 100644 --- a/docs/plans/project-roadmap.md +++ b/docs/plans/project-roadmap.md @@ -403,7 +403,7 @@ Goal: something we could put a domain in front of. - [ ] Cache scan results by URL (TTL, in-memory first, swap later) so anonymous re-scans of the same URL are cheap. -- [ ] Per-IP rate limiting on `/api/scan`; per-user rate limiting on +- [x] Per-IP rate limiting on `/api/scan`; per-user rate limiting on authenticated scans. - [ ] Background job queue (e.g. BullMQ) for slow sites; revisit sync vs async API decision. From 152c2611712877506a638256c66d8c544facfbc7 Mon Sep 17 00:00:00 2001 From: Samuel Olabode Date: Sun, 18 Oct 2026 23:38:27 -0600 Subject: [PATCH 2/2] fix(scan): rate limit GET /api/scan-results with the same budget GET /api/scan-results also runs a live Chromium scan and is what the results page uses on refresh or a shared link, so it bypassed the limit. Both routes now share one limiter, so a scan costs one unit either way. --- backend/middleware/scanRateLimit.js | 3 ++- backend/routes/scan.js | 5 +++-- backend/tests/scanRateLimit.test.js | 21 +++++++++++++++++++-- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/backend/middleware/scanRateLimit.js b/backend/middleware/scanRateLimit.js index 9eeefeb..3131dd8 100644 --- a/backend/middleware/scanRateLimit.js +++ b/backend/middleware/scanRateLimit.js @@ -1,5 +1,6 @@ /** - * Rate limiting for POST /api/scan. Every scan launches a headless Chromium, + * Rate limiting for the live-scan routes (POST /api/scan and + * GET /api/scan-results). Every scan launches a headless Chromium, * so request volume is a cost and abuse surface, not just a fairness concern. * * Anonymous requests are limited per IP (IPv6 grouped by subnet, so rotating diff --git a/backend/routes/scan.js b/backend/routes/scan.js index e6b791e..cd2b0bc 100644 --- a/backend/routes/scan.js +++ b/backend/routes/scan.js @@ -1,7 +1,7 @@ /** * Scan-related routes: * - POST /api/scan (rate limited per IP / per user) - * - GET /api/scan-results + * - GET /api/scan-results (same limiter — it also runs a live scan) * - GET /api/scans * - GET /api/scans/:id * - DELETE /api/scans/:id @@ -20,7 +20,8 @@ const { Router } = require('express'); function makeScanRouter(controller, { scanRateLimit = [] } = {}) { const router = Router(); router.post('/scan', ...scanRateLimit, controller.postScan); - router.get('/scan-results', controller.getScanResults); + // Same middleware instances, so both routes draw on one shared budget. + router.get('/scan-results', ...scanRateLimit, controller.getScanResults); router.get('/scans', controller.getSavedScans); router.get('/scans/:id', controller.getSavedScan); router.delete('/scans/:id', controller.deleteSavedScan); diff --git a/backend/tests/scanRateLimit.test.js b/backend/tests/scanRateLimit.test.js index 86bada2..424f2e4 100644 --- a/backend/tests/scanRateLimit.test.js +++ b/backend/tests/scanRateLimit.test.js @@ -1,6 +1,6 @@ /** - * Rate limiting on POST /api/scan — per IP when anonymous, per user when - * signed in. + * Rate limiting on the live-scan routes — per IP when anonymous, per user + * when signed in. */ const test = require('node:test'); const assert = require('node:assert/strict'); @@ -31,6 +31,23 @@ test('POST /api/scan returns 429 once an IP is over the limit, without scanning' assert.equal(scans, 2); }); +test('GET /api/scan-results shares the same budget as POST /api/scan', async () => { + let scans = 0; + const run = async () => { scans += 1; return mockScanResults; }; + const app = buildApp({ + scanRunner: { run, getResults: run }, + scanRateLimit: makeScanRateLimit({ anonLimit: 2 }), + }); + + assert.equal((await request(app).post('/api/scan').send({ url: 'https://example.com' })).status, 200); + assert.equal((await request(app).get('/api/scan-results').query({ url: 'https://example.com' })).status, 200); + const blocked = await request(app).get('/api/scan-results').query({ url: 'https://example.com' }); + + assert.equal(blocked.status, 429); + assert.equal(blocked.body.code, 'RATE_LIMITED'); + assert.equal(scans, 2); +}); + test('signed-in users are limited per user, not by the shared IP', async () => { const app = express(); // Stand-in for passport: the header plays the role of the session user.