diff --git a/CHANGELOG.md b/CHANGELOG.md index 29dbedfaa..f1c6ffb53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +- Bound how often the GBFS proxy will relay. `/api/gbfs` already restricted + where it could go — https, an allowlisted host, a station feed path, no + redirects, a 12-second deadline and a 5 MB body cap — but nothing restricted + how often, so the dev server would make unlimited requests at a third-party + bikeshare operator on any caller's behalf. The route now admits 120 + requests/minute per client address, refusing the rest with 429 and + `Retry-After` before any outbound fetch. The bikeshare layer needs about 15 a + minute with every catalogued system in range, so ordinary use is unchanged. + - CCTV cameras whose bearing is a guess now say so. Packs mark bearings derived from a hash of the camera id as `headingConfidence: 'low'`, but nothing read the flag, so roughly 70% of a default catalog rendered like surveyed facings. The HUD diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index fdb42fc0c..f55945224 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -827,6 +827,15 @@ feed address is read from the path, and a query-string form is refused with a 40 streaming. The 12-second deadline includes reading the body, and rejected or stalled downloads are cancelled. Development and preview use the same handler. +The route also bounds request frequency: 120 requests/minute per client +address, with a global backstop of three times that. A refused request answers +429 with `Retry-After` and `Cache-Control: no-store` and performs no outbound +fetch. The bikeshare layer's own demand is about 15 requests a minute with +every catalogued system in range, so an ordinary session never meets the limit; +the method, host and path guards run unchanged. + +Updated: September 21, 2026 + ## Remaining local service modules Overpass query validation, geometry simplification, disk caching and upstream diff --git a/scripts/format-scope.json b/scripts/format-scope.json index eee546dd5..e7b7a92d0 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -839,5 +839,6 @@ "src/ui/imageryBoxTool.test.mjs", "src/ui/recentImagery.test.mjs", "src/layers/recentImagery/testDoubles.mjs", - "scripts/qa-recent-imagery.mjs" + "scripts/qa-recent-imagery.mjs", + "src/data/gbfsProxyRateLimit.test.mjs" ] diff --git a/scripts/package-boundaries.json b/scripts/package-boundaries.json index 189192f10..1f1db490d 100644 --- a/scripts/package-boundaries.json +++ b/scripts/package-boundaries.json @@ -138,6 +138,8 @@ "modules": [ "server/providers/gbfs.js", "server/providers/common/http.js", + "server/providers/common/rate-limit.js", + "src/sources/rateLimit.js", "src/data/gbfsSource.js", "src/sources/httpBody.js" ], diff --git a/server/providers/gbfs.js b/server/providers/gbfs.js index 9741bb4e6..001e94a7d 100644 --- a/server/providers/gbfs.js +++ b/server/providers/gbfs.js @@ -1,4 +1,5 @@ import { readResponseTextCapped } from './common/http.js'; +import { makeRateLimiter, clientKey } from './common/rate-limit.js'; import { isAllowedGbfsHost, isAllowedGbfsPath, @@ -13,6 +14,19 @@ const GBFS_PROXY_TIMEOUT_MS = 12000; export const GBFS_MAX_BODY_BYTES = 5 * 1024 * 1024; // 5 MB +/** + * Requests/minute/IP admitted by the proxy. + * + * The bikeshare layer's own demand is small and known: `src/layers/bikeshare/ + * registry.js` catalogues 15 systems, station information is cached per city + * after its first fetch, and station status refreshes once per city every + * `STATUS_POLL_MS` (60 s) with in-flight requests deduplicated. Even with every + * catalogued system in camera range at once that is about 15 requests a minute, + * so 120 is eight times a maximal real session while still capping a caller + * using this proxy as an open relay against a third-party GBFS host. + */ +export const GBFS_RATE_PER_MIN = 120; + function gbfsRedirectHost(location, requestUrl) { if (!location) return ''; try { @@ -113,7 +127,12 @@ export async function fetchGbfsUpstream( * * @returns {import('vite').Plugin} */ -export function gbfsProxy() { +export function gbfsProxy({ ratePerMinute = GBFS_RATE_PER_MIN } = {}) { + const allow = makeRateLimiter({ + windowMs: 60_000, + max: ratePerMinute, + globalMax: ratePerMinute * 3, + }); const installMiddleware = (server) => { server.middlewares.use('/api/gbfs', async (req, res) => { try { @@ -126,6 +145,19 @@ export function gbfsProxy() { return; } + // Frequency, checked before the target is even parsed: the allowlist + // already says WHERE this proxy may go, nothing said HOW OFTEN. A + // refused request performs no outbound fetch and starts no timer. + if (!allow(clientKey(req))) { + res.writeHead(429, { + 'Content-Type': 'application/json', + 'Cache-Control': 'no-store', + 'Retry-After': '10', + }); + res.end(JSON.stringify({ error: 'Rate limit exceeded' })); + return; + } + const url = new URL(req.url || '/', 'http://localhost'); const encodedTarget = url.pathname.replace(/^\/+/, ''); if (!encodedTarget) { diff --git a/src/data/gbfsProxyRateLimit.test.mjs b/src/data/gbfsProxyRateLimit.test.mjs new file mode 100644 index 000000000..72ee38152 --- /dev/null +++ b/src/data/gbfsProxyRateLimit.test.mjs @@ -0,0 +1,152 @@ +// GBFS PROXY — the frequency the allowlist never bounded. +// +// `/api/gbfs` already says WHERE it may go: an https allowlisted host, a +// station_information/station_status path, a 12 s timeout, a 5 MB body cap and +// no redirects. Nothing said HOW OFTEN, so the dev server would relay an +// unbounded stream of requests at a third-party bikeshare operator on any +// caller's behalf — one outbound fetch and one timeout timer each. +// +// These cases drive the real mounted middleware. `fetch` is stubbed per test +// and restored afterwards, so a case that reaches the network fails loudly, +// and the stub's call count is what proves a refused request costs nothing. +// +// Run with: npm test +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { gbfsProxy, GBFS_RATE_PER_MIN } from '../../server/providers/gbfs.js'; + +const STATUS_URL = 'https://gbfs.lyft.com/gbfs/2.3/bay/en/station_status.json'; +const STATION_BODY = '{"data":{"stations":[]}}'; + +/** Mount the proxy the way the dev server does; returns its route handler. */ +function mount(options) { + const routes = new Map(); + gbfsProxy(options).configureServer({ + middlewares: { + use(route, handler) { + routes.set(route, handler); + }, + }, + }); + return routes.get('/api/gbfs'); +} + +function request(handler, { target = STATUS_URL, remoteAddress, method } = {}) { + return new Promise((resolve, reject) => { + const req = { + method: method || 'GET', + // connect strips the mount prefix before the handler sees the URL. + url: '/' + encodeURIComponent(target), + headers: {}, + socket: remoteAddress ? { remoteAddress } : undefined, + }; + let status = 200; + let headers = {}; + const res = { + writeHead(code, sent) { + status = code; + headers = sent || {}; + return res; + }, + end(body = '') { + let parsed = null; + try { + parsed = body ? JSON.parse(String(body)) : null; + } catch { + parsed = String(body); + } + resolve({ status, headers, body: parsed }); + }, + }; + Promise.resolve(handler(req, res)).catch(reject); + }); +} + +/** Answer every allowlisted upstream, counting the calls that got through. */ +function stubUpstream(t) { + const asked = []; + const original = globalThis.fetch; + globalThis.fetch = async (url) => { + asked.push(String(url)); + return new Response(STATION_BODY, { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }; + t.after(() => { + globalThis.fetch = original; + }); + return asked; +} + +test('a maximal real session is never throttled', async (t) => { + const asked = stubUpstream(t); + const handler = mount(); + + // The bikeshare layer catalogues 15 systems and refreshes each one's status + // once every 60 s, so a minute of the busiest possible session is ~15 + // requests. Run eight times that and expect every one of them served. + let served = 0; + for (let i = 0; i < 120; i += 1) { + const res = await request(handler); + if (res.status === 200) served += 1; + } + assert.equal(served, 120, 'the shipped ceiling leaves a real session alone'); + assert.equal(asked.length, 120); + assert.equal(GBFS_RATE_PER_MIN, 120, 'the ceiling this case relies on'); +}); + +test('an unbounded relay is refused, and refusal costs no outbound fetch', async (t) => { + const asked = stubUpstream(t); + const handler = mount({ ratePerMinute: 5 }); + + const responses = []; + for (let i = 0; i < 12; i += 1) responses.push(await request(handler)); + + const served = responses.filter((r) => r.status === 200); + const refused = responses.filter((r) => r.status === 429); + assert.equal(served.length, 5, 'exactly the admitted budget is served'); + assert.equal(refused.length, 7); + assert.equal(refused[0].body.error, 'Rate limit exceeded'); + assert.equal(refused[0].headers['Retry-After'], '10'); + assert.equal(refused[0].headers['Cache-Control'], 'no-store'); + assert.equal( + asked.length, + 5, + 'a refused request never reaches the GBFS operator', + ); +}); + +test('a different client keeps its own budget', async (t) => { + stubUpstream(t); + const handler = mount({ ratePerMinute: 3 }); + + for (let i = 0; i < 6; i += 1) + await request(handler, { remoteAddress: '10.0.0.1' }); + const noisy = await request(handler, { remoteAddress: '10.0.0.1' }); + assert.equal(noisy.status, 429, 'the noisy client spent its own budget'); + + const quiet = await request(handler, { remoteAddress: '10.0.0.2' }); + assert.equal(quiet.status, 200, 'a quiet client is not collateral'); +}); + +test('the limiter does not displace the existing guards', async (t) => { + const asked = stubUpstream(t); + const handler = mount({ ratePerMinute: 10 }); + + const wrongMethod = await request(handler, { method: 'POST' }); + assert.equal(wrongMethod.status, 405); + + const offAllowlist = await request(handler, { + target: 'https://example.com/gbfs/station_status.json', + }); + assert.equal(offAllowlist.status, 403); + assert.equal(offAllowlist.body.error, 'GBFS host not allowed'); + + const wrongPath = await request(handler, { + target: 'https://gbfs.lyft.com/gbfs/2.3/bay/en/system_regions.json', + }); + assert.equal(wrongPath.status, 400); + + assert.equal(asked.length, 0, 'none of those reached upstream'); +});