diff --git a/CHANGELOG.md b/CHANGELOG.md index 7398d8418..92791a08f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,21 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md stale-data fallback. Concurrent identical requests share the same last-good fallback when all mirrors refuse, without duplicating upstream requests. +### Security + +- The GBFS proxy no longer follows upstream redirects. It validated the host + and path of the requested URL and then let `fetch()` follow whatever an + allowlisted operator answered with, so a redirect could carry the proxy to a + destination the allowlist never checked. A 3xx response now returns a 502 to + the client and logs the redirect target server-side. Every registered feed + answers 200 directly today, so nothing changes until an operator moves behind + a redirect, which then fails loudly instead of silently (#30). +- The GBFS proxy enforces its 5 MB response cap while the body streams and + cancels the upstream read the moment the running byte count passes it. It + previously trusted `Content-Length`, then materialized the whole body before + measuring, so a chunked or mislabeled response could grow memory without + bound before the check ever ran (#31). + ## [0.1.1] — 2026-09-01 — Installation and live-data fixes ### Changed diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index a2bb9dad1..445c5bf75 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -2279,7 +2279,7 @@ silently demoting every later lookup for the session. eight-second timeout; the timer is cleared on every success or failure path. - OpenSky response cache stores successful upstream responses only; OAuth token refresh calls are coalesced. - A cold OpenSky failure uses the current camera subpoint only to request a cached adsb.lol point fallback capped at 250 nm. A fresh OpenSky response or last-good cache wins; a nominally successful worldwide snapshot more than two minutes old prefers viewport-scoped adsb.lol when available, otherwise the stale source is reported honestly. The fallback is visibly source-labeled and is never presented as a worldwide snapshot. -- GBFS response size is capped; CCTV health map is bounded. +- GBFS proxy refuses upstream redirects (`redirect: 'manual'`; any 3xx becomes a 502 and the redirect target is logged server-side only) and enforces its 5 MB response cap while the body streams, cancelling the upstream read past the cap; CCTV health map is bounded. - Proxy error payloads are sanitized (no internal error details returned to clients). - `OPENAI_API_KEY` is server-side only; the browser receives ephemeral Realtime client secrets from `/api/realtime/token`. - `AISSTREAM_API_KEY` is server-side only; the browser reads the same-origin `/api/ais-live` cache. diff --git a/src/data/gbfsProxy.test.mjs b/src/data/gbfsProxy.test.mjs new file mode 100644 index 000000000..4f00eb99e --- /dev/null +++ b/src/data/gbfsProxy.test.mjs @@ -0,0 +1,131 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { + GBFS_MAX_BODY_BYTES, + fetchGbfsUpstream, +} from '../../vite.config.js'; + +const STATION_URL = 'https://gbfs.lyft.com/gbfs/2.3/bay/en/station_status.json'; +const STATION_BODY = '{"data":{"stations":[]}}'; + +/** + * A Response whose body streams `chunk` forever until the reader cancels it. + * A zero high-water mark keeps the stream from pulling eagerly at construction, + * so `pulls()` counts only the chunks a reader actually asked for. + */ +function endlessResponse(chunk, onCancel) { + let pulls = 0; + const stream = new ReadableStream({ + pull(controller) { + pulls += 1; + controller.enqueue(chunk); + }, + cancel: onCancel, + }, { highWaterMark: 0 }); + return { response: new Response(stream, { status: 200 }), pulls: () => pulls }; +} + +test('GBFS upstream fetch passes a direct 200 through unchanged', async () => { + let observedOptions = null; + const result = await fetchGbfsUpstream(STATION_URL, { + fetchImpl: async (_url, options) => { + observedOptions = options; + return new Response(STATION_BODY, { + status: 200, + headers: { 'Content-Type': 'application/json; charset=utf-8' }, + }); + }, + }); + + assert.equal(observedOptions.redirect, 'manual', 'fetch must not follow redirects on its own'); + assert.ok(observedOptions.signal instanceof AbortSignal); + assert.deepEqual(result, { + status: 200, + contentType: 'application/json; charset=utf-8', + body: STATION_BODY, + }); +}); + +test('GBFS upstream fetch forwards a non-redirect error status for the middleware to relay', async () => { + const result = await fetchGbfsUpstream(STATION_URL, { + fetchImpl: async () => new Response(null, { status: 404 }), + }); + assert.equal(result.status, 404); + assert.equal(result.contentType, 'application/json', 'a missing upstream content type defaults to JSON'); + assert.equal(result.body, ''); +}); + +test('GBFS upstream fetch rejects every redirect status and never fetches the target', async () => { + for (const status of [301, 302, 303, 307, 308]) { + let calls = 0; + await assert.rejects( + fetchGbfsUpstream(STATION_URL, { + fetchImpl: async () => { + calls += 1; + return new Response('moved', { + status, + headers: { Location: 'https://attacker.example/station_status.json' }, + }); + }, + }), + (error) => error?.code === 'GBFS_REDIRECT' && error.redirectHost === 'attacker.example', + `status ${status} must be refused`, + ); + assert.equal(calls, 1, `status ${status}: the redirect target must not be requested`); + } +}); + +test('GBFS upstream fetch names the host of a relative redirect', async () => { + await assert.rejects( + fetchGbfsUpstream(STATION_URL, { + fetchImpl: async () => new Response(null, { status: 302, headers: { Location: '/v2/station_status.json' } }), + }), + (error) => error?.code === 'GBFS_REDIRECT' && error.redirectHost === 'gbfs.lyft.com', + ); +}); + +test('GBFS upstream fetch rejects an oversized Content-Length before reading the body', async () => { + const { response, pulls } = endlessResponse(new Uint8Array(1024)); + response.headers.set('Content-Length', String(GBFS_MAX_BODY_BYTES + 1)); + await assert.rejects( + fetchGbfsUpstream(STATION_URL, { fetchImpl: async () => response }), + (error) => error?.code === 'RESPONSE_TOO_LARGE', + ); + assert.equal(pulls(), 0, 'the body must not be read when the declared length already exceeds the cap'); +}); + +test('GBFS upstream fetch cancels a length-less body the moment it passes the cap', async () => { + let cancelled = false; + const chunk = new Uint8Array(1024); + const { response, pulls } = endlessResponse(chunk, () => { cancelled = true; }); + await assert.rejects( + fetchGbfsUpstream(STATION_URL, { fetchImpl: async () => response, maxBytes: 4 * chunk.byteLength }), + (error) => error?.code === 'RESPONSE_TOO_LARGE', + ); + assert.equal(cancelled, true, 'the upstream stream must be cancelled, not drained'); + assert.ok(pulls() <= 8, `only a few chunks may be read past the cap, saw ${pulls()}`); +}); + +test('GBFS upstream fetch measures the cap in bytes and accepts a body exactly at it', async () => { + const body = 'é'.repeat(64); + const result = await fetchGbfsUpstream(STATION_URL, { + fetchImpl: async () => new Response(body, { status: 200 }), + maxBytes: Buffer.byteLength(body), + }); + assert.equal(result.body, body); + assert.equal(GBFS_MAX_BODY_BYTES, 5 * 1024 * 1024); +}); + +test('GBFS upstream fetch aborts a stalled connection with the timeout signal', async () => { + const startedAt = Date.now(); + await assert.rejects( + fetchGbfsUpstream(STATION_URL, { + timeoutMs: 20, + fetchImpl: (_url, options) => new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => reject(options.signal.reason), { once: true }); + }), + }), + (error) => error?.name === 'AbortError', + ); + assert.ok(Date.now() - startedAt < 500, 'the timeout must settle promptly'); +}); diff --git a/vite.config.js b/vite.config.js index 5b2a79ad1..f524ba66c 100644 --- a/vite.config.js +++ b/vite.config.js @@ -1320,6 +1320,8 @@ function radioBrowserProxy() { // --------------------------------------------------------------------------- /** Upstream fetch timeout for GBFS requests (ms). */ const GBFS_PROXY_TIMEOUT_MS = 12000; +/** Hard cap on a GBFS upstream body (bytes), enforced while the body streams. */ +export const GBFS_MAX_BODY_BYTES = 5 * 1024 * 1024; // 5 MB /** Allowlisted GBFS hostnames; wildcard *.publicbikesystem.net also accepted. */ const GBFS_ALLOWED_HOSTS = new Set([ 'gbfs.lyft.com', @@ -3335,13 +3337,90 @@ function gbfsCacheControl(pathname) { return 'no-store'; } +/** + * Hostname a redirect Location header points at, resolved against the request + * URL so a relative Location still names a host. Empty when absent or unparsable. + * + * @param {string|null} location + * @param {string} requestUrl + * @returns {string} + */ +function gbfsRedirectHost(location, requestUrl) { + if (!location) return ''; + try { + return new URL(String(location), requestUrl).hostname; + } catch { + return ''; + } +} + +/** + * Fetch one validated GBFS endpoint without following redirects and with the + * body cap enforced while the response streams. + * + * The middleware validates the target before calling this; the network step + * lives apart from it so a unit test can stand in for a hostile upstream — the + * host allowlist keeps the real handler from being pointed at a local server. + * `redirect: 'manual'` makes fetch() return a 3xx response instead of + * following it, so an allowlisted operator that redirects cannot steer the + * proxy to a destination the allowlist never saw; any 3xx is rejected with + * { code:'GBFS_REDIRECT', redirectHost }. An oversized body surfaces as + * { code:'RESPONSE_TOO_LARGE' } from readResponseTextCapped, which cancels the + * upstream read the moment the running byte count passes the cap. + * + * @param {string} url - Validated https GBFS endpoint URL. + * @param {object} [options] + * @param {typeof fetch} [options.fetchImpl=fetch] - Fetch implementation. + * @param {number} [options.timeoutMs=GBFS_PROXY_TIMEOUT_MS] - Upstream abort timeout. + * @param {number} [options.maxBytes=GBFS_MAX_BODY_BYTES] - Body byte cap. + * @returns {Promise<{status:number,contentType:string,body:string}>} + */ +export async function fetchGbfsUpstream(url, { + fetchImpl = fetch, + timeoutMs = GBFS_PROXY_TIMEOUT_MS, + maxBytes = GBFS_MAX_BODY_BYTES, +} = {}) { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), timeoutMs); + let upstream; + try { + upstream = await fetchImpl(url, { + method: 'GET', + headers: { + Accept: 'application/json', + 'User-Agent': 'gods-eye-view-gbfs-proxy/1.0', + }, + redirect: 'manual', + signal: controller.signal, + }); + } finally { + clearTimeout(timeoutId); + } + + if (upstream.status >= 300 && upstream.status < 400) { + try { await upstream.body?.cancel(); } catch { /* no-op */ } + const err = new Error('GBFS upstream redirected; redirects are not followed'); + err.code = 'GBFS_REDIRECT'; + err.redirectHost = gbfsRedirectHost(upstream.headers.get('location'), url); + throw err; + } + + const body = await readResponseTextCapped(upstream, maxBytes); + return { + status: upstream.status, + contentType: upstream.headers.get('content-type') || 'application/json', + body, + }; +} + /** * Vite plugin: GBFS bike-share proxy with host allowlisting and size limits. * * Accepts GET /api/gbfs/ and proxies the request * to the upstream GBFS provider. Validates hostname against an allowlist, * restricts to station_information/station_status paths, enforces HTTPS, - * and caps response body at 5 MB. + * refuses upstream redirects, and caps the response body at 5 MB while it + * streams. * * @returns {import('vite').Plugin} */ @@ -3401,44 +3480,33 @@ function gbfsProxy() { return; } - const controller = new AbortController(); - const timeoutId = setTimeout(() => controller.abort(), GBFS_PROXY_TIMEOUT_MS); let upstream; try { - upstream = await fetch(upstreamUrl.toString(), { - method: 'GET', - headers: { - Accept: 'application/json', - 'User-Agent': 'gods-eye-view-gbfs-proxy/1.0', - }, - signal: controller.signal, - }); - } finally { - clearTimeout(timeoutId); + upstream = await fetchGbfsUpstream(upstreamUrl.toString()); + } catch (error) { + if (error?.code === 'GBFS_REDIRECT') { + // The target host stays server-side; the client only learns that a redirect happened. + console.error( + `[GBFS Proxy] ${upstreamUrl.hostname} redirected to ${error.redirectHost || 'an unparsable Location'}; redirects are not followed`, + ); + res.writeHead(502, { 'Content-Type': 'application/json', 'Cache-Control': 'no-store' }); + res.end(JSON.stringify({ error: 'GBFS upstream redirected; redirects are not followed' })); + return; + } + if (error?.code === 'RESPONSE_TOO_LARGE') { + res.writeHead(502, { 'Content-Type': 'application/json', 'Cache-Control': 'no-store' }); + res.end(JSON.stringify({ error: 'GBFS upstream response too large' })); + return; + } + throw error; } - - // Limit response size to prevent memory exhaustion from malicious upstream - const GBFS_MAX_BODY_BYTES = 5 * 1024 * 1024; // 5 MB - const contentLength = Number(upstream.headers.get('content-length')); - if (Number.isFinite(contentLength) && contentLength > GBFS_MAX_BODY_BYTES) { - res.writeHead(502, { 'Content-Type': 'application/json', 'Cache-Control': 'no-store' }); - res.end(JSON.stringify({ error: 'GBFS upstream response too large' })); - return; - } - const body = await upstream.text(); - if (Buffer.byteLength(body, 'utf8') > GBFS_MAX_BODY_BYTES) { - res.writeHead(502, { 'Content-Type': 'application/json', 'Cache-Control': 'no-store' }); - res.end(JSON.stringify({ error: 'GBFS upstream response too large' })); - return; - } - const contentType = upstream.headers.get('content-type') || 'application/json'; res.writeHead(upstream.status, { - 'Content-Type': contentType, + 'Content-Type': upstream.contentType, 'Cache-Control': gbfsCacheControl(upstreamUrl.pathname), 'X-GBFS-Upstream': upstreamUrl.hostname, 'X-GBFS-Cache': 'MISS', }); - res.end(body); + res.end(upstream.body); } catch (error) { if (error?.name === 'AbortError') { res.writeHead(504, { 'Content-Type': 'application/json', 'Cache-Control': 'no-store' });