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.
This commit is contained in:
Sameh Khamis committed 2026-10-21 15:43:29 -07:00
1 parent afe05454d9
commit b75db874c6
5 files changed
+32 -8

No files matched your search

+11 -5
View File
@@ -62,19 +62,25 @@ export const API_HEADERS = Object.freeze({
/** /**
* Whether a request reaches an `/api` mount, by the router's own rule: the * 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 * prefix compared without case and followed by the end, `/` or `.`. The router
* raw path and the parsed one (which also covers an absolute-form target) are * reads an absolute-form target's path without resolving dot segments, so the
* checked, so no spelling the router mounts can miss the headers. * 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) { function isApiPath(url) {
const raw = String(url || '/'); const raw = String(url || '/');
const target = raw
.trim()
.split(/[?#]/, 1)[0]
.replace(/\\/g, '/')
.replace(/^[a-z][a-z0-9+.-]*:(?:\/\/[^/]*)?/i, '');
let parsed = ''; let parsed = '';
try { try {
parsed = new URL(raw, 'http://localhost').pathname; parsed = new URL(raw, 'http://localhost').pathname;
} catch { } 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), /^\/api(?:$|[/.])/i.test(path),
); );
} }
+7 -3
View File
@@ -20,7 +20,7 @@ import {
} from './cctv/constants.js'; } from './cctv/constants.js';
import { sanitizeCctvRangeHeader } from './cctv/range.js'; import { sanitizeCctvRangeHeader } from './cctv/range.js';
import { admitSameSiteKeyless } from './common/same-site.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 { createHlsPuller } from './cctv/stream.js';
import { googleServerApiKey } from './places/google-key.js'; import { googleServerApiKey } from './places/google-key.js';
export { CCTV_FRAME_FETCH_TIMEOUT_MS, fetchCctvImageFromUpstream }; 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 // The Street View fallback spends the Google key, so a forwarded
// request (whose client this server cannot see) gets the placeholder, // request (whose client this server cannot see) gets the placeholder,
// as if no key were configured. // as if no key were configured. Any forwarding header counts, even an
const sv = hasProxySignals(req.headers || {}) // empty one.
const forwarded = PROXY_SIGNALS.some(
(name) => req.headers?.[name] !== undefined,
);
const sv = forwarded
? null ? null
: await streetViewFallback({ : await streetViewFallback({
lat, lat,
+3
View File
@@ -491,6 +491,9 @@ test('a forwarded same-origin request reaches every CCTV route but not the Stree
{ via: '1.1 proxy' }, { via: '1.1 proxy' },
{ forwarded: 'for=203.0.113.9' }, { forwarded: 'for=203.0.113.9' },
{ 'x-real-ip': '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 headers = { host, 'sec-fetch-site': 'same-origin', ...forwarded };
const label = JSON.stringify(forwarded); const label = JSON.stringify(forwarded);
+8
View File
@@ -9,6 +9,7 @@ import { test } from 'node:test';
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import { loadTflSourcesFromOpenData } from '../../server/providers/cctv/sources.js'; import { loadTflSourcesFromOpenData } from '../../server/providers/cctv/sources.js';
import { fetchWithinHost } from '../../server/providers/cctv/media.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/'; 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'); 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,
);
});
+3
View File
@@ -366,6 +366,9 @@ test('every API response carries the API policy and nosniff on real dev and prev
['/api.x', 404], ['/api.x', 404],
[`http://localhost:${port}/api/fixture/ok`, 200], [`http://localhost:${port}/api/fixture/ok`, 200],
[`http://localhost:${port}/API/nope`, 404], [`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' }], ['/api/fixture/ok', 403, policy, { host: 'elsewhere.example' }],
// A route's own policy is added to the API policy, never replaces it. // A route's own policy is added to the API policy, never replaces it.
['/api/fixture/own-policy', 200, `${policy}, frame-ancestors 'none'`], ['/api/fixture/own-policy', 200, `${policy}, frame-ancestors 'none'`],