From 6eaa737333fd14e58ae026b1ec057559a389b028 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Fri, 21 Aug 2026 14:12:53 +0700 Subject: [PATCH] fix(relay): resolve x-relay-path through the shared guard in the CF worker (#10935) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validado no worktree combinado do lote: typecheck:core, changelog-integrity, complexity, cognitive-complexity, file-size e 163 testes focados (incluindo cloudflare-relay-path-ssrf) todos verdes. Fix de segurança real e bem documentado (SSRF via concatenação pós-validação no Cloudflare relay worker). CI vermelho é o base-red já rastreado em #9985. Obrigado! --- .../10935-cloudflare-relay-path-guard.md | 1 + src/lib/proxyRelay/cloudflareWorkerScript.ts | 16 ++- tests/unit/cloudflare-relay-path-ssrf.test.ts | 105 ++++++++++++++++++ 3 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 changelog.d/fixes/10935-cloudflare-relay-path-guard.md create mode 100644 tests/unit/cloudflare-relay-path-ssrf.test.ts diff --git a/changelog.d/fixes/10935-cloudflare-relay-path-guard.md b/changelog.d/fixes/10935-cloudflare-relay-path-guard.md new file mode 100644 index 0000000000..0799cdc52d --- /dev/null +++ b/changelog.d/fixes/10935-cloudflare-relay-path-guard.md @@ -0,0 +1 @@ +- **fix(relay):** the Cloudflare proxy-relay worker now resolves `x-relay-path` through the shared `resolveRelayTarget()` guard instead of concatenating it onto the validated target. PR #4643 and its follow-up applied that guard to the Deno and Vercel workers; the Cloudflare generator, ported separately from upstream `decolua/9router` PR #1360, kept `fetch(targetBase + relayPath)`. Validating `x-relay-target` and then concatenating is not sufficient — the path re-points the request past the host that was just checked, through userinfo (`/x@evil.com`), a backslash (`\evil.com`), or a protocol-relative path (`//evil.com/x`). The guard is embedded verbatim under a literal `const resolveRelayTarget =` binding so the hardcoded call site still resolves when the SWC-minified standalone build mangles the source function's own name (#6149), and the new regression test pins that property for this worker by renaming the embedded function and re-evaluating the emitted source. The auth check and the private/loopback target guard are unchanged diff --git a/src/lib/proxyRelay/cloudflareWorkerScript.ts b/src/lib/proxyRelay/cloudflareWorkerScript.ts index 73de16b9f2..b01d446b53 100644 --- a/src/lib/proxyRelay/cloudflareWorkerScript.ts +++ b/src/lib/proxyRelay/cloudflareWorkerScript.ts @@ -10,6 +10,12 @@ * - Inlines an SSRF guard rejecting RFC1918 / loopback / link-local / IPv6 ULA * targets — the Edge runtime cannot import Node helpers, the guard lives * here as a string. + * - Resolves x-relay-path through the SAME `resolveRelayTarget()` the Deno and + * Vercel workers use (PR #4643 and its follow-up), instead of concatenating + * it onto the target. Concatenation lets the path re-point the request past + * the validated host. Bound to a LITERAL const name so the hardcoded call + * site still resolves when the SWC-minified standalone build mangles the + * source function's own name in `.toString()` output (#6149). * - Strips Host + relay control headers before forwarding upstream. * * The string template is fed to Cloudflare's PUT /accounts/{id}/workers/scripts/{name} @@ -23,6 +29,7 @@ * - SSRF guard is inlined so a leaked relay URL cannot scan internal IPs. */ import { randomUUID } from "crypto"; +import { resolveRelayTarget } from "@/app/api/settings/proxy/deno-deploy/route"; /** * Build the multipart/form-data request body for Cloudflare's Worker @@ -75,6 +82,8 @@ export function buildCloudflareWorkerScript(relayAuth: string): string { // user-controlled input ever reaches this template, so direct interpolation // into the worker source string is safe. return `// OmniRoute Cloudflare Worker proxy relay — generated at deploy time. +const resolveRelayTarget = ${resolveRelayTarget.toString()}; + function isPrivateHostname(h) { if (!h) return true; const host = h.trim().toLowerCase().replace(/^\\[|\\]$/g, ""); @@ -134,9 +143,12 @@ async function handleRelay(request) { init.body = request.body; init.duplex = "half"; } + const resolved = resolveRelayTarget(target, relayPath); + if (!resolved.ok) { + return new Response(resolved.reason, { status: resolved.status }); + } try { - const targetBase = target.endsWith("/") ? target.slice(0, -1) : target; - const upstream = await fetch(targetBase + relayPath, init); + const upstream = await fetch(resolved.url, init); return new Response(upstream.body, { status: upstream.status, headers: upstream.headers, diff --git a/tests/unit/cloudflare-relay-path-ssrf.test.ts b/tests/unit/cloudflare-relay-path-ssrf.test.ts new file mode 100644 index 0000000000..ada88e1bb0 --- /dev/null +++ b/tests/unit/cloudflare-relay-path-ssrf.test.ts @@ -0,0 +1,105 @@ +// SSRF regression guard for the Cloudflare relay worker — the third worker, +// which missed the PR #4643 relay-path fix that the Deno and Vercel workers +// already carry. +// +// The generated Worker read the attacker-controlled `x-relay-path` header and +// appended it to the (validated) `x-relay-target` origin by string +// concatenation: `fetch(targetBase + relayPath)`. Validating the target and +// then concatenating is not enough — the path can re-point the request past the +// host that was checked, via userinfo (`/x@evil.com`), a backslash +// (`\evil.com`), or a protocol-relative path (`//evil.com/x`). +// +// The fix reuses the SAME pure `resolveRelayTarget()` guard the other two +// workers embed. This mirrors `vercel-deploy-relay-path-ssrf.test.ts`: the pure +// function's own behaviour is covered once in the deno test, so here the focus +// is the Cloudflare worker's wiring — plus the #6149 name-stability property, +// which a bare `function` declaration would silently lose under minification. +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import vm from "node:vm"; +import { resolveRelayTarget } from "../../src/app/api/settings/proxy/deno-deploy/route"; +import { buildCloudflareWorkerScript } from "../../src/lib/proxyRelay/cloudflareWorkerScript"; + +const RELAY_AUTH = "deadbeefcafe"; + +describe("cloudflare relay worker — no string-concat SSRF hole", () => { + it("does not append relayPath to the target by string concatenation", () => { + const worker = buildCloudflareWorkerScript(RELAY_AUTH); + assert.ok( + !worker.includes("+ relayPath"), + "Cloudflare worker must not contain `+ relayPath` string concatenation" + ); + assert.ok(!worker.includes("targetBase"), "the concatenated targetBase form must be gone"); + }); + + it("embeds the shared guard and fetches the resolved url", () => { + const worker = buildCloudflareWorkerScript(RELAY_AUTH); + assert.ok( + worker.includes("resolveRelayTarget"), + "the Cloudflare worker must call the shared resolveRelayTarget guard" + ); + assert.ok( + worker.includes("resolved.url"), + "the Cloudflare worker must fetch the SSRF-validated resolved url" + ); + }); + + it("keeps the auth check and the private-host guard", () => { + const worker = buildCloudflareWorkerScript(RELAY_AUTH); + assert.ok(worker.includes(RELAY_AUTH), "relayAuth secret must still be embedded"); + assert.ok( + worker.includes("isPrivateHostname"), + "the private/loopback target guard must be preserved" + ); + }); + + // #6149: the guard is embedded via Function#toString, so a bare `function` + // declaration would emit the MANGLED name on a minified standalone build + // while the template still calls the literal `resolveRelayTarget`. + it("binds the guard to a literal const name so minification cannot break the call", () => { + const worker = buildCloudflareWorkerScript(RELAY_AUTH); + assert.ok( + /const\s+resolveRelayTarget\s*=/.test(worker), + "the guard must be bound to a literal `const resolveRelayTarget =` name" + ); + + // Simulate the minifier by renaming ONLY the embedded function's own + // declared name, then check the binding still resolves. + const emitted = resolveRelayTarget.toString(); + const mangled = emitted.replace("resolveRelayTarget", "a"); + const minifiedWorker = worker.replace(emitted, mangled); + // Evaluate ONLY the binding, located by its own literal name. Slicing the + // worker at some other declaration would tie this test to unrelated parts + // of the emitted source still being present. + const binding = minifiedWorker.match(/const resolveRelayTarget = [\s\S]*?;\s/); + assert.ok(binding, "the emitted worker must contain the const binding"); + const context: Record = {}; + vm.createContext(context); + vm.runInContext(`${binding[0]} globalThis.__resolve = resolveRelayTarget;`, context); + const resolveFn = (context as { __resolve?: typeof resolveRelayTarget }).__resolve; + assert.equal(typeof resolveFn, "function", "guard must still be reachable after mangling"); + }); +}); + +describe("cloudflare relay worker — host-confusion vectors are rejected", () => { + const TARGET = "https://api.anthropic.com"; + + it("accepts a legitimate absolute path", () => { + const r = resolveRelayTarget(TARGET, "/v1/messages"); + assert.equal(r.ok, true); + if (r.ok) assert.equal(r.url, "https://api.anthropic.com/v1/messages"); + }); + + for (const [label, path] of [ + ["//evil.com/x (host swap)", "//evil.com/x"], + ["/x@evil.com (userinfo)", "/x@evil.com"], + ["\\evil.com (backslash)", "\\evil.com"], + ["evil.com/x (no leading slash)", "evil.com/x"], + ] as const) { + it(`rejects ${label}`, () => { + const r = resolveRelayTarget(TARGET, path); + assert.equal(r.ok, false); + if (!r.ok) assert.equal(r.status, 403); + }); + } +});