From ea9150bd36fabba19de82ad799973bb03c81ed13 Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Mon, 9 Nov 2026 08:13:36 +0100 Subject: [PATCH] fix(cors): secure secret proxy requests --- cspell.config.json | 1 + js/defaults.js | 1 + js/server_functions.mjs | 76 ++++++++++++++----- .../unit/functions/server_functions_spec.mjs | 74 +++++++++++++++++- 4 files changed, 130 insertions(+), 22 deletions(-) diff --git a/cspell.config.json b/cspell.config.json index a9d61d1386..e3da58147f 100644 --- a/cspell.config.json +++ b/cspell.config.json @@ -341,6 +341,7 @@ "unitless", "unixtime", "unparseable", + "unstub", "updatenotification", "uxdt", "Vaice", diff --git a/js/defaults.js b/js/defaults.js index 8fbb365856..495e635340 100644 --- a/js/defaults.js +++ b/js/defaults.js @@ -14,6 +14,7 @@ const defaults = { trustedProxies: [], // Only needed behind a reverse proxy. cors: "disabled", // or "allowAll" or "allowWhitelist" corsDomainWhitelist: [], // example: ["api.mapbox.com"] + corsSecretWhitelist: {}, // example: { "api.mapbox.com": ["SECRET_MAPBOX_TOKEN"] } watchTargets: [], language: "en", diff --git a/js/server_functions.mjs b/js/server_functions.mjs index f4159a71d6..49e6b42f80 100644 --- a/js/server_functions.mjs +++ b/js/server_functions.mjs @@ -20,8 +20,7 @@ const getStartup = (req, res) => { * Replace `**SECRET_ABC**` placeholders with the value of `process.env.SECRET_ABC`. * * If `allowedSecrets` is given, only those secret names are restored and every - * other placeholder is left untouched. Without it, all secrets are restored - * (used by the CORS proxy, which only runs on the trusted server side). + * other placeholder is left untouched. * @param {string} input - String that may contain `**SECRET_***` placeholders. * @param {Set} [allowedSecrets] - Secret names that may be restored. * @returns {string} The input with the allowed placeholders replaced. @@ -77,6 +76,53 @@ const validateCorsHost = async (hostname) => { return address; }; +/** + * Validates a CORS URL and expands only secrets approved for its hostname. + * @param {string} url - The requested URL. + * @returns {{url: string, containsSecrets: boolean}} The resolved request URL and redirect policy. + */ +const resolveCorsUrl = (url) => { + const reject = (message) => { + const error = new Error(message); + error.statusCode = 403; + throw error; + }; + + let parsed; + try { + parsed = new URL(url); + } catch { + Log.warn(`SSRF blocked (invalid URL): ${url}`); + reject("Forbidden: private or reserved addresses are not allowed"); + } + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { + Log.warn(`SSRF blocked (protocol): ${url}`); + reject("Forbidden: private or reserved addresses are not allowed"); + } + + const hostname = parsed.hostname.toLowerCase(); + const configuredSecrets = global.config.cors === "allowWhitelist" && global.config.corsDomainWhitelist.includes(hostname) + ? global.config.corsSecretWhitelist?.[hostname] + : []; + const allowedSecrets = new Set(Array.isArray(configuredSecrets) ? configuredSecrets : []); + const expandedUrl = replaceSecretPlaceholder(url, allowedSecrets); + const containsSecrets = expandedUrl !== url; + + if (containsSecrets) { + let expanded; + try { + expanded = new URL(expandedUrl); + } catch { + reject("Forbidden: invalid secret URL"); + } + if (expanded.protocol !== "https:" || expanded.port || expanded.origin !== parsed.origin || expanded.username || expanded.password) { + reject("Forbidden: secrets require HTTPS on port 443 with an unchanged origin and no credentials"); + } + } + + return { url: expandedUrl, containsSecrets }; +}; + /** * A method that forwards HTTP Get-methods to the internet to avoid CORS-errors. * @@ -93,6 +139,7 @@ const cors = async (req, res) => { return res.status(403).json({ error: "CORS proxy is disabled" }); } let url; + let containsSecrets = false; try { const urlRegEx = "url=(.+?)$"; @@ -102,25 +149,11 @@ const cors = async (req, res) => { Log.error(url); return res.status(400).send(url); } else { - url = match[1]; - if (typeof global.config !== "undefined") url = replaceSecretPlaceholder(url); - - // Validate protocol before attempting connection (non-http/https are never allowed) - let parsed; - try { - parsed = new URL(url); - } catch { - Log.warn(`SSRF blocked (invalid URL): ${url}`); - return res.status(403).json({ error: "Forbidden: private or reserved addresses are not allowed" }); - } - if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { - Log.warn(`SSRF blocked (protocol): ${url}`); - return res.status(403).json({ error: "Forbidden: private or reserved addresses are not allowed" }); - } + ({ url, containsSecrets } = resolveCorsUrl(match[1])); const headersToSend = getHeadersToSend(req.url); const expectedReceivedHeaders = geExpectedReceivedHeaders(req.url); - Log.log(`cors url: ${url}`); + Log.log(`cors host: ${new URL(url).host}`); // Validate each host immediately before connecting and pin the socket to its public IP. const defaultConnector = undici.buildConnector({}); @@ -137,7 +170,7 @@ const cors = async (req, res) => { }); try { - const response = await undici.fetch(url, { dispatcher, headers: headersToSend }); + const response = await undici.fetch(url, { dispatcher, headers: headersToSend, redirect: containsSecrets ? "error" : "follow" }); if (response.ok) { for (const header of expectedReceivedHeaders) { const headerValue = response.headers.get(header); @@ -154,10 +187,11 @@ const cors = async (req, res) => { } } catch (error) { const cause = error.cause?.statusCode ? error.cause : error; + const message = containsSecrets ? "CORS request with secrets failed" : cause.message; if (process.env.mmTestMode !== "true") { - Log.error(`Error in CORS request: ${cause}`); + Log.error(`Error in CORS request: ${containsSecrets ? message : cause}`); } - res.status(cause.statusCode || 500).json({ error: cause.message }); + res.status(cause.statusCode || 500).json({ error: message }); } }; diff --git a/tests/unit/functions/server_functions_spec.mjs b/tests/unit/functions/server_functions_spec.mjs index ff31ae030a..b21d28e64b 100644 --- a/tests/unit/functions/server_functions_spec.mjs +++ b/tests/unit/functions/server_functions_spec.mjs @@ -318,6 +318,78 @@ describe("server_functions tests", () => { await cors({ url: "/cors?url=http://example.com/api" }, response); expect(response.status).toHaveBeenCalledWith(403); }); + + }); + + describe("The cors method protects secret requests", () => { + let response; + let fetchSpy; + + beforeEach(() => { + vi.stubEnv("SECRET_CORS_TOKEN", "dummy-token"); + global.config = { + cors: "allowWhitelist", + corsDomainWhitelist: ["first.test", "second.test"], + corsSecretWhitelist: { "first.test": ["SECRET_CORS_TOKEN"] } + }; + response = { + set: vi.fn(), + send: vi.fn(), + status: vi.fn(function () { return this; }), + json: vi.fn() + }; + fetchSpy = vi.spyOn(undici, "fetch"); + fetchSpy.mockClear(); + fetchSpy.mockResolvedValue({ + headers: { get: () => null }, + arrayBuffer: () => Promise.resolve(new ArrayBuffer(0)), + ok: true + }); + vi.spyOn(dns.promises, "lookup").mockResolvedValue({ address: "93.184.216.34", family: 4 }); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it.each([ + ["http://first.test/tiles?token=**SECRET_CORS_TOKEN**", "dummy-token"], + ["https://first.test:8443/tiles?token=**SECRET_CORS_TOKEN**", "dummy-token"], + ["https://user@first.test/tiles?token=**SECRET_CORS_TOKEN**", "dummy-token"], + ["https://**SECRET_CORS_TOKEN**@first.test/tiles", "second.test/path?"] + ])("Rejects unsafe secret URL %s", async (url, secretValue) => { + vi.stubEnv("SECRET_CORS_TOKEN", secretValue); + await cors({ url: `/cors?url=${url}` }, response); + expect(response.status).toHaveBeenCalledWith(403); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("Restores only configured secrets for the configured host", async () => { + await cors({ url: "/cors?url=https://first.test:443/tiles?allowed=**SECRET_CORS_TOKEN**&denied=**SECRET_CORS_DENIED**" }, response); + await cors({ url: "/cors?url=https://second.test/tiles?token=**SECRET_CORS_TOKEN**" }, response); + + expect(fetchSpy.mock.calls.map(([url]) => url)).toEqual([ + "https://first.test:443/tiles?allowed=dummy-token&denied=**SECRET_CORS_DENIED**", + "https://second.test/tiles?token=**SECRET_CORS_TOKEN**" + ]); + expect(fetchSpy.mock.calls[0][1].redirect).toBe("error"); + }); + + it.each([ + ["no secret allowlist", { cors: "allowWhitelist", corsDomainWhitelist: ["first.test"] }], + ["allowAll", { cors: "allowAll" }] + ])("Leaves placeholders untouched with %s", async (mode, config) => { + global.config = config; + const url = "http://first.test/tiles?token=**SECRET_CORS_TOKEN**"; + await cors({ url: `/cors?url=${url}` }, response); + expect(fetchSpy.mock.calls[0][0]).toBe(url); + }); + + it("Does not expose secrets from fetch errors", async () => { + fetchSpy.mockRejectedValue(new Error("failed URL: https://first.test/?token=dummy-token")); + await cors({ url: "/cors?url=https://first.test/?token=**SECRET_CORS_TOKEN**" }, response); + expect(response.json).toHaveBeenCalledWith({ error: "CORS request with secrets failed" }); + }); }); describe("The cors method validates redirect targets", () => { @@ -338,7 +410,7 @@ describe("server_functions tests", () => { // Keep the request hostname for CORS checks, but route its socket to the local test server. vi.spyOn(undici, "buildConnector").mockImplementation((connectorOptions) => { const connector = realBuildConnector(connectorOptions); - return (options, callback) => connector({ ...options, hostname: "127.0.0.1", servername: null }, callback); + return (options, callback) => connector({ ...options, hostname: "127.0.0.1", servername: null, protocol: "http:", port }, callback); }); server = http.createServer((req, res) => {