Merge pull request #180: harden GBFS upstream requests

Preserve the original GBFS redirect and streaming-cap contribution, adapted to the provider modules with full-body deadlines and response cleanup.

CHANGELOG.md

@@ -13,6 +13,12 @@ of current runtime behavior, see [`docs/CURRENT-STATE.md`](docs/CURRENT-STATE.md
 
 ## [Unreleased]
 
+### Fixed
+
+- GBFS rejects upstream redirects, caps streamed responses at 5 MiB, and keeps
+  its deadline active through body reads. Rejected downloads are cancelled.
+
+
 - Split Overpass/installation search, regional briefing/weather, local voice
   handlers and standalone key setup into focused modules. Preserve routes,
   source behavior, tool schemas and credential restrictions.

docs/CURRENT-STATE.md

@@ -1,5 +1,12 @@
 # God's Eye View Current State
 
+## GBFS upstream bounds
+
+GBFS refuses upstream redirects and enforces its 5 MiB response cap while
+streaming. The 12-second deadline includes reading the body, and rejected or
+stalled downloads are cancelled. Development and preview use the same handler.
+
+
 ## Remaining local service modules
 
 Overpass query validation, geometry simplification, disk caching and upstream
@@ -2585,7 +2592,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.

scripts/package-boundaries.json

@@ -124,7 +124,11 @@
   "gbfs-provider": {
     "runtime": "node",
     "exports": ["./server/providers/gbfs"],
-    "modules": ["server/providers/gbfs.js", "src/data/gbfsSource.js"],
+    "modules": [
+      "server/providers/gbfs.js",
+      "server/providers/common/http.js",
+      "src/data/gbfsSource.js"
+    ],
     "external": []
   },
   "gbfs-source": {

server/providers/common/http.js

@@ -3,44 +3,49 @@
  * oversized Content-Length, then streams with a running cap so a chunked or
  * length-omitted response cannot blow past the limit. Throws { code:'RESPONSE_TOO_LARGE' }.
  */
-export async function readResponseTextCapped(response, maxBytes) {
+export async function readResponseTextCapped(response, maxBytes, signal) {
+  const tooLarge = () =>
+    Object.assign(new Error('Upstream response too large'), {
+      code: 'RESPONSE_TOO_LARGE',
+    });
   const declared = Number(response.headers.get('content-length'));
   if (Number.isFinite(declared) && declared > maxBytes) {
-    const err = new Error('Upstream response too large');
-    err.code = 'RESPONSE_TOO_LARGE';
-    throw err;
+    void response.body?.cancel().catch(() => {});
+    throw tooLarge();
   }
+  signal?.throwIfAborted();
   const reader = response.body?.getReader?.();
   if (!reader) {
     const text = await response.text();
-    if (Buffer.byteLength(text) > maxBytes) {
-      const err = new Error('Upstream response too large');
-      err.code = 'RESPONSE_TOO_LARGE';
-      throw err;
-    }
+    signal?.throwIfAborted();
+    if (Buffer.byteLength(text) > maxBytes) throw tooLarge();
     return text;
   }
+  const cancel = () => {
+    void reader.cancel().catch(() => {});
+  };
+  signal?.addEventListener('abort', cancel, { once: true });
   const decoder = new TextDecoder();
   let out = '';
   let total = 0;
-  for (;;) {
-    const { done, value } = await reader.read();
-    if (done) break;
-    total += value.byteLength;
-    if (total > maxBytes) {
-      try {
-        await reader.cancel();
-      } catch {
-        /* no-op */
-      }
-      const err = new Error('Upstream response too large');
-      err.code = 'RESPONSE_TOO_LARGE';
-      throw err;
+  try {
+    for (;;) {
+      signal?.throwIfAborted();
+      const { done, value } = await reader.read();
+      signal?.throwIfAborted();
+      if (done) break;
+      total += value.byteLength;
+      if (total > maxBytes) throw tooLarge();
+      out += decoder.decode(value, { stream: true });
     }
-    out += decoder.decode(value, { stream: true });
+    return out + decoder.decode();
+  } catch (error) {
+    cancel();
+    throw error;
+  } finally {
+    signal?.removeEventListener('abort', cancel);
+    reader.releaseLock();
   }
-  out += decoder.decode();
-  return out;
 }
 
 /** Parse a fetch() JSON response only after enforcing a hard byte cap. */

server/providers/gbfs.js

@@ -1,3 +1,4 @@
+import { readResponseTextCapped } from './common/http.js';
 import {
   isAllowedGbfsHost,
   isAllowedGbfsPath,
@@ -10,6 +11,98 @@ import {
 /** Upstream fetch timeout for GBFS requests (ms). */
 const GBFS_PROXY_TIMEOUT_MS = 12000;
 
+export const GBFS_MAX_BODY_BYTES = 5 * 1024 * 1024; // 5 MB
+
+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,
+    });
+
+    if (upstream.status >= 300 && upstream.status < 400) {
+      try {
+        void upstream.body?.cancel().catch(() => {});
+      } 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,
+      controller.signal,
+    );
+    return {
+      status: upstream.status,
+      contentType: upstream.headers.get('content-type') || 'application/json',
+      body,
+    };
+  } catch (error) {
+    controller.abort();
+    if (upstream?.body && !upstream.body.locked) {
+      void upstream.body.cancel().catch(() => {});
+    }
+    throw error;
+  } finally {
+    clearTimeout(timeoutId);
+  }
+}
+
 /**
  * Vite plugin: GBFS bike-share proxy with host allowlisting and size limits.
  *
@@ -102,61 +195,14 @@ export 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);
-        }
-
-        // 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';
+        const upstream = await fetchGbfsUpstream(upstreamUrl.toString());
         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, {
@@ -166,6 +212,26 @@ export function gbfsProxy() {
           res.end(JSON.stringify({ error: 'GBFS upstream timeout' }));
           return;
         }
+        if (
+          error?.code === 'GBFS_REDIRECT' ||
+          error?.code === 'RESPONSE_TOO_LARGE'
+        ) {
+          if (error.code === 'GBFS_REDIRECT')
+            console.warn('[GBFS Proxy] Redirect refused', error.redirectHost);
+          res.writeHead(502, {
+            'Content-Type': 'application/json',
+            'Cache-Control': 'no-store',
+          });
+          res.end(
+            JSON.stringify({
+              error:
+                error.code === 'GBFS_REDIRECT'
+                  ? 'GBFS upstream redirect refused'
+                  : 'GBFS upstream response too large',
+            }),
+          );
+          return;
+        }
         console.error('[GBFS Proxy]', error?.message || String(error));
         res.writeHead(502, {
           'Content-Type': 'application/json',

src/data/gbfsProxy.test.mjs

@@ -0,0 +1,270 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import {
+  GBFS_MAX_BODY_BYTES,
+  fetchGbfsUpstream,
+} from '../../server/providers/gbfs.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');
+});
+
+test('GBFS cancels an oversized declared body without pulling it', async () => {
+  let cancelled = false;
+  const { response, pulls } = endlessResponse(new Uint8Array(1), () => {
+    cancelled = true;
+  });
+  response.headers.set('Content-Length', '100');
+  await assert.rejects(
+    fetchGbfsUpstream(STATION_URL, {
+      maxBytes: 10,
+      fetchImpl: async () => response,
+    }),
+    { code: 'RESPONSE_TOO_LARGE' },
+  );
+  assert.equal(cancelled, true);
+  assert.equal(pulls(), 0);
+});
+
+test('GBFS deadline includes a stalled body after successful headers', async () => {
+  let cancelled = false;
+  let signal;
+  const response = new Response(
+    new ReadableStream({
+      pull() {},
+      cancel() {
+        cancelled = true;
+      },
+    }),
+  );
+  await assert.rejects(
+    fetchGbfsUpstream(STATION_URL, {
+      timeoutMs: 20,
+      fetchImpl: async (_url, options) => {
+        signal = options.signal;
+        return response;
+      },
+    }),
+    { name: 'AbortError' },
+  );
+  assert.equal(signal.aborted, true);
+  assert.equal(cancelled, true);
+  assert.equal(response.body.locked, false);
+});
+
+test('GBFS rejects redirects and caps bodies through both server hooks', async () => {
+  const { gbfsProxy } = await import('../../server/providers/gbfs.js');
+  const originalFetch = globalThis.fetch;
+  try {
+    for (const hook of ['configureServer', 'configurePreviewServer']) {
+      let handler;
+      gbfsProxy()[hook]({
+        middlewares: {
+          use(path, callback) {
+            assert.equal(path, '/api/gbfs');
+            handler = callback;
+          },
+        },
+      });
+      for (const response of [
+        new Response(null, { status: 302 }),
+        new Response('x', {
+          headers: { 'Content-Length': String(GBFS_MAX_BODY_BYTES + 1) },
+        }),
+        new Response(STATION_BODY),
+      ]) {
+        const expectedStatus =
+          response.status === 302 || response.headers.has('content-length')
+            ? 502
+            : 200;
+        globalThis.fetch = async () => response;
+        let status, body;
+        await handler(
+          { method: 'GET', url: '/' + encodeURIComponent(STATION_URL) },
+          {
+            writeHead(code) {
+              status = code;
+            },
+            end(value) {
+              body = value;
+            },
+          },
+        );
+        assert.equal(status, expectedStatus);
+        if (status === 200) assert.equal(body, STATION_BODY);
+        else assert.equal(typeof JSON.parse(body).error, 'string');
+      }
+    }
+  } finally {
+    globalThis.fetch = originalFetch;
+  }
+});