diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b371af53..942a5637d 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. + - Add nine Ontario transit feeds: TTC, MiWay, Hamilton HSR, Durham Region, YRT/Viva, Guelph, Burlington, Barrie and Kingston, each carrying its own licence quote and attribution. A feed can now name its own modes, so Toronto's @@ -160,6 +169,9 @@ - Director timeline seeks cross the dateline along the short longitude arc. +- Keep unknown GBFS availability counts neutral instead of turning null or empty + values into zero bikes. + - Recent Imagery continues past malformed records in full catalog pages, respects the requested record limit, and reports results as truncated when that limit drops valid records. - Regional articles retain unknown publication times when compact source dates do not identify a real calendar instant. diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 6373d7255..116a75bbd 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -1262,6 +1262,15 @@ cancels the body unread. A rejected or timed-out body is an upstream miss and follows each route's existing error and cached-data paths. FIRMS map-key status, the OpenSky token exchange and CCTV media keep their current handling. +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 @@ -1345,6 +1354,9 @@ and stale/error responses remain unchanged. Each has a Node-only package entry under `gods-eye-view/server/providers/`. Portable terrain mechanics, traffic tile math and GBFS source rules are available under `gods-eye-view/sources/`. The browser layers and their rendering remain in their existing modules. +GBFS null or empty availability counts retain an unknown value through station +parsing and rendering; they use the neutral station color rather than reporting +an empty dock. Explicit zero and caller-supplied numeric fallbacks remain valid. ## Terrain height cache bound diff --git a/scripts/format-scope.json b/scripts/format-scope.json index acdd0d043..f299ed4ba 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -1055,5 +1055,6 @@ "src/ui/flyToDataSource.js", "src/ui/flyToDataSource.test.mjs", "scripts/assemble-changelog.mjs", - "src/tooling/changelogFragments.test.mjs" + "src/tooling/changelogFragments.test.mjs", + "src/data/gbfsProxyRateLimit.test.mjs" ] diff --git a/scripts/package-boundaries.json b/scripts/package-boundaries.json index ef3a9fdeb..f6131dcd4 100644 --- a/scripts/package-boundaries.json +++ b/scripts/package-boundaries.json @@ -158,6 +158,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 990642c8b..c6e3ba4ec 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, @@ -17,6 +18,19 @@ export const GBFS_CLIENT_IDENTIFIER = 'gods-eye-view'; 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 { @@ -121,7 +135,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 { @@ -134,6 +153,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'); +}); diff --git a/src/layers/bikeshare/proxyRoute.test.mjs b/src/layers/bikeshare/proxyRoute.test.mjs index 9bb04ced0..e4153a203 100644 --- a/src/layers/bikeshare/proxyRoute.test.mjs +++ b/src/layers/bikeshare/proxyRoute.test.mjs @@ -394,3 +394,55 @@ for (const hook of ['configureServer', 'configurePreviewServer']) { ); }); } + +test('unknown GBFS counts stay unknown through the source, model and station color', async () => { + const { createRendering } = await import('./rendering.js'); + const { COLOR_NEUTRAL, COLOR_RED } = await import('./policy.js'); + for (const missing of [null, '']) { + const source = createBikeshareSource({ + fetchImpl: async () => + Response.json({ + data: { + stations: [ + { + station_id: '2537', + num_bikes_available: missing, + num_docks_available: 4, + is_installed: true, + is_renting: true, + is_returning: true, + }, + ], + }, + }), + }); + const model = createModel({}); + const status = model + .parseStationStatus(await source.getStations(AUSTIN.stationStatusUrl)) + .get('2537'); + const rendering = createRendering({ + state: {}, + services: { render: {} }, + parts: { model }, + }); + assert.equal(status.bikesAvailable, null); + assert.equal( + model.toNonNegativeInteger(status.bikesAvailable), + null, + 'render-map normalization preserves unknown', + ); + assert.equal(rendering.statusToColor(status, 12), COLOR_NEUTRAL); + assert.equal(rendering.resolveCapacity(missing, status), 4); + assert.equal( + rendering.statusToColor({ ...status, bikesAvailable: 0 }, 12), + COLOR_RED, + 'reported zero is still empty', + ); + assert.equal( + model.toNonNegativeInteger(missing, 7), + 7, + 'caller fallback is preserved', + ); + assert.equal(model.toNonNegativeInteger('3'), 3); + } +}); diff --git a/src/sources/gbfsStations.js b/src/sources/gbfsStations.js index efa1e6169..74f2efd23 100644 --- a/src/sources/gbfsStations.js +++ b/src/sources/gbfsStations.js @@ -25,6 +25,7 @@ export function normalizeGbfsBool(value, fallback = true) { * @returns {number|null} */ export function toNonNegativeInteger(value, fallback = null) { + if (value == null || value === '') return fallback; const n = Number(value); if (!Number.isFinite(n) || n < 0) return fallback; return Math.round(n);