From b75db874c695c8e5808eb54f649d6794f8ad48bd Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Wed, 21 Oct 2026 15:43:29 -0700 Subject: [PATCH] Read API paths and forwarded requests as the router does The router reads an absolute-form target's path without resolving dot segments, so a target such as http://host/api/../x still reaches /api middleware. Take that path the same way when deciding whether the API response headers apply. The Street View fallback now treats any forwarding header as forwarded, even an empty one, before it spends the key. --- build/vite.js | 16 +++++++++++----- server/providers/cctv.js | 10 +++++++--- src/data/cctvMediaFeedType.test.mjs | 3 +++ src/data/cctvSourcePins.test.mjs | 8 ++++++++ src/tooling/viteBuild.test.mjs | 3 +++ 5 files changed, 32 insertions(+), 8 deletions(-) diff --git a/build/vite.js b/build/vite.js index 5498a8010..a02f2340f 100644 --- a/build/vite.js +++ b/build/vite.js @@ -62,19 +62,25 @@ export const API_HEADERS = Object.freeze({ /** * Whether a request reaches an `/api` mount, by the router's own rule: the - * prefix compared without case and followed by the end, `/` or `.`. Both the - * raw path and the parsed one (which also covers an absolute-form target) are - * checked, so no spelling the router mounts can miss the headers. + * prefix compared without case and followed by the end, `/` or `.`. The router + * reads an absolute-form target's path without resolving dot segments, so the + * path is taken here the same way (scheme and authority dropped, backslashes as + * slashes), alongside the raw and the fully parsed path; any match counts. */ function isApiPath(url) { const raw = String(url || '/'); + const target = raw + .trim() + .split(/[?#]/, 1)[0] + .replace(/\\/g, '/') + .replace(/^[a-z][a-z0-9+.-]*:(?:\/\/[^/]*)?/i, ''); let parsed = ''; try { parsed = new URL(raw, 'http://localhost').pathname; } catch { - /* the raw path still decides */ + /* the unparsed paths still decide */ } - return [raw.split(/[?#]/, 1)[0], parsed].some((path) => + return [raw.split(/[?#]/, 1)[0], target, parsed].some((path) => /^\/api(?:$|[/.])/i.test(path), ); } diff --git a/server/providers/cctv.js b/server/providers/cctv.js index 3eb1b062f..672b9be42 100644 --- a/server/providers/cctv.js +++ b/server/providers/cctv.js @@ -20,7 +20,7 @@ import { } from './cctv/constants.js'; import { sanitizeCctvRangeHeader } from './cctv/range.js'; import { admitSameSiteKeyless } from './common/same-site.js'; -import { hasProxySignals } from '../../src/localRequestGate.mjs'; +import { PROXY_SIGNALS } from '../../src/localRequestGate.mjs'; import { createHlsPuller } from './cctv/stream.js'; import { googleServerApiKey } from './places/google-key.js'; export { CCTV_FRAME_FETCH_TIMEOUT_MS, fetchCctvImageFromUpstream }; @@ -551,8 +551,12 @@ export function cctvProxy({ sourceRoot = process.cwd() } = {}) { // The Street View fallback spends the Google key, so a forwarded // request (whose client this server cannot see) gets the placeholder, - // as if no key were configured. - const sv = hasProxySignals(req.headers || {}) + // as if no key were configured. Any forwarding header counts, even an + // empty one. + const forwarded = PROXY_SIGNALS.some( + (name) => req.headers?.[name] !== undefined, + ); + const sv = forwarded ? null : await streetViewFallback({ lat, diff --git a/src/data/cctvMediaFeedType.test.mjs b/src/data/cctvMediaFeedType.test.mjs index d5543fdf8..04dd77e28 100644 --- a/src/data/cctvMediaFeedType.test.mjs +++ b/src/data/cctvMediaFeedType.test.mjs @@ -491,6 +491,9 @@ test('a forwarded same-origin request reaches every CCTV route but not the Stree { via: '1.1 proxy' }, { forwarded: 'for=203.0.113.9' }, { 'x-real-ip': '203.0.113.9' }, + { 'x-forwarded-for': '' }, + { 'x-forwarded-for': ' ' }, + { 'cf-connecting-ip': '' }, ]) { const headers = { host, 'sec-fetch-site': 'same-origin', ...forwarded }; const label = JSON.stringify(forwarded); diff --git a/src/data/cctvSourcePins.test.mjs b/src/data/cctvSourcePins.test.mjs index 3ab2536d7..6954623c1 100644 --- a/src/data/cctvSourcePins.test.mjs +++ b/src/data/cctvSourcePins.test.mjs @@ -9,6 +9,7 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { loadTflSourcesFromOpenData } from '../../server/providers/cctv/sources.js'; import { fetchWithinHost } from '../../server/providers/cctv/media.js'; +import { pinnedCameraUrl } from '../../server/providers/cctv/normalize.js'; const BUCKET = 'https://s3-eu-west-1.amazonaws.com/jamcams.tfl.gov.uk/'; @@ -104,3 +105,10 @@ test('a redirect on a pinned camera host stays inside the pinned directory', asy ); assert.equal(await unpinned.text(), 'frame'); }); + +test('a pin prefix without a trailing slash still names a directory', () => { + assert.equal( + pinnedCameraUrl('https://h.example/camsX/a.jpg', 'https://h.example/cams'), + null, + ); +}); diff --git a/src/tooling/viteBuild.test.mjs b/src/tooling/viteBuild.test.mjs index dcb2910a6..7417f7145 100644 --- a/src/tooling/viteBuild.test.mjs +++ b/src/tooling/viteBuild.test.mjs @@ -366,6 +366,9 @@ test('every API response carries the API policy and nosniff on real dev and prev ['/api.x', 404], [`http://localhost:${port}/api/fixture/ok`, 200], [`http://localhost:${port}/API/nope`, 404], + // The router keeps dot segments in an absolute-form target's path. + ['http://elsewhere.example/api/../x', 404], + ['http://elsewhere.example/API/%2e%2e/x', 404], ['/api/fixture/ok', 403, policy, { host: 'elsewhere.example' }], // A route's own policy is added to the API policy, never replaces it. ['/api/fixture/own-policy', 200, `${policy}, frame-ancestors 'none'`],