diff --git a/src/server.ts b/src/server.ts index 5a884c20b..375b791ad 100644 --- a/src/server.ts +++ b/src/server.ts @@ -2864,6 +2864,20 @@ export function buildFetchCode(url: string, outputPath: string): string { ? `var classifyIp = ${classifyIpInner};` : `var ${classifyIpFnName} = ${classifyIpInner};\nvar classifyIp = ${classifyIpFnName};`; const strictMode = process.env.CTX_FETCH_STRICT === "1"; + // Default strips proxy env so the connect-time rebinding guard runs (#476/#1039). + // Opt-in is exact string "1" only (any other value keeps the strip path). + const allowProxy = process.env.CTX_FETCH_ALLOW_PROXY === "1"; + const proxyEnvBlock = allowProxy + ? `// Proxy env vars preserved under CTX_FETCH_ALLOW_PROXY=1 (issue #1039).` + : `// Strip proxy env by default so DNS rebinding guard sees connect-time IPs (#1039). +delete process.env.HTTP_PROXY; +delete process.env.HTTPS_PROXY; +delete process.env.ALL_PROXY; +delete process.env.http_proxy; +delete process.env.https_proxy; +delete process.env.all_proxy; +delete process.env.npm_config_proxy; +delete process.env.npm_config_https_proxy;`; return ` const TurndownService = require(${turndownPath}); const { gfm } = require(${gfmPath}); @@ -2873,19 +2887,7 @@ const dnsPromises = require('no' + 'de:dns/promises'); const url = ${JSON.stringify(url)}; const outputPath = ${escapedOutputPath}; -// Strip proxy env vars from this subprocess only. A configured outbound -// proxy (HTTP_PROXY / HTTPS_PROXY / ALL_PROXY) would route fetch through -// an arbitrary target — DNS resolution happens at the proxy and the -// in-subprocess DNS rebinding guard never sees the rebound IP. The -// sandbox fetch path has no legitimate need for an upstream proxy. -delete process.env.HTTP_PROXY; -delete process.env.HTTPS_PROXY; -delete process.env.ALL_PROXY; -delete process.env.http_proxy; -delete process.env.https_proxy; -delete process.env.all_proxy; -delete process.env.npm_config_proxy; -delete process.env.npm_config_https_proxy; +${proxyEnvBlock} ${classifyIpSrc} diff --git a/tests/core/server.test.ts b/tests/core/server.test.ts index 661053b72..9fb83e34a 100644 --- a/tests/core/server.test.ts +++ b/tests/core/server.test.ts @@ -3830,17 +3830,69 @@ import { buildFetchCode } from "../../src/server.js"; describe("buildFetchCode — embedded SSRF guard contract", () => { const generated = buildFetchCode("https://example.com/x", "/tmp/x"); - test("strips proxy env vars (HTTP_PROXY / HTTPS_PROXY / ALL_PROXY)", () => { - // A configured outbound proxy would route fetch through an arbitrary - // target; DNS resolution would happen at the proxy and the in-subprocess - // DNS guard would never see the rebound IP. The generated subprocess - // source must delete every proxy env var before any fetch can run. - expect(generated).toMatch(/delete process\.env\.HTTP_PROXY/); - expect(generated).toMatch(/delete process\.env\.HTTPS_PROXY/); - expect(generated).toMatch(/delete process\.env\.ALL_PROXY/); - expect(generated).toMatch(/delete process\.env\.http_proxy/); - expect(generated).toMatch(/delete process\.env\.https_proxy/); - expect(generated).toMatch(/delete process\.env\.all_proxy/); + test("strips proxy env vars (HTTP_PROXY / HTTPS_PROXY / ALL_PROXY) by default (#476 pinning)", () => { + // Default must delete proxy env vars so the in-subprocess DNS guard runs (#476). + const prev = process.env.CTX_FETCH_ALLOW_PROXY; + delete process.env.CTX_FETCH_ALLOW_PROXY; + try { + const src = buildFetchCode("https://example.com/x", "/tmp/x"); + expect(src).toMatch(/delete process\.env\.HTTP_PROXY/); + expect(src).toMatch(/delete process\.env\.HTTPS_PROXY/); + expect(src).toMatch(/delete process\.env\.ALL_PROXY/); + expect(src).toMatch(/delete process\.env\.http_proxy/); + expect(src).toMatch(/delete process\.env\.https_proxy/); + expect(src).toMatch(/delete process\.env\.all_proxy/); + expect(src).toMatch(/delete process\.env\.npm_config_proxy/); + expect(src).toMatch(/delete process\.env\.npm_config_https_proxy/); + } finally { + if (prev === undefined) delete process.env.CTX_FETCH_ALLOW_PROXY; + else process.env.CTX_FETCH_ALLOW_PROXY = prev; + } + }); + + test("preserves proxy env vars when CTX_FETCH_ALLOW_PROXY=1 (#1039 opt-in)", () => { + // Exact "1" opt-in preserves proxy env for corporate egress; parent ssrfGuard remains. + const prev = process.env.CTX_FETCH_ALLOW_PROXY; + process.env.CTX_FETCH_ALLOW_PROXY = "1"; + try { + const src = buildFetchCode("https://example.com/x", "/tmp/x"); + expect(src).not.toMatch(/delete process\.env\.HTTP_PROXY/); + expect(src).not.toMatch(/delete process\.env\.HTTPS_PROXY/); + expect(src).not.toMatch(/delete process\.env\.ALL_PROXY/); + expect(src).not.toMatch(/delete process\.env\.http_proxy/); + expect(src).not.toMatch(/delete process\.env\.https_proxy/); + expect(src).not.toMatch(/delete process\.env\.all_proxy/); + expect(src).not.toMatch(/delete process\.env\.npm_config_proxy/); + expect(src).not.toMatch(/delete process\.env\.npm_config_https_proxy/); + expect(src).not.toMatch(/delete process\.env\[[^\]]*PROXY/i); + expect(src).not.toMatch( + /process\.env\.(HTTP|HTTPS|ALL)_?PROXY\s*=\s*(undefined|null|['"]{2})/i, + ); + expect(src).toMatch(/CTX_FETCH_ALLOW_PROXY=1/); + } finally { + if (prev === undefined) delete process.env.CTX_FETCH_ALLOW_PROXY; + else process.env.CTX_FETCH_ALLOW_PROXY = prev; + } + }); + + test('rejects non-"1" truthy CTX_FETCH_ALLOW_PROXY (fail-secure; still strips)', () => { + // Non-"1" values (e.g. "true") must still strip — fail-secure vs accidental truthy env. + const prev = process.env.CTX_FETCH_ALLOW_PROXY; + process.env.CTX_FETCH_ALLOW_PROXY = "true"; + try { + const src = buildFetchCode("https://example.com/x", "/tmp/x"); + expect(src).toMatch(/delete process\.env\.HTTP_PROXY/); + expect(src).toMatch(/delete process\.env\.HTTPS_PROXY/); + expect(src).toMatch(/delete process\.env\.ALL_PROXY/); + expect(src).toMatch(/delete process\.env\.http_proxy/); + expect(src).toMatch(/delete process\.env\.https_proxy/); + expect(src).toMatch(/delete process\.env\.all_proxy/); + expect(src).toMatch(/delete process\.env\.npm_config_proxy/); + expect(src).toMatch(/delete process\.env\.npm_config_https_proxy/); + } finally { + if (prev === undefined) delete process.env.CTX_FETCH_ALLOW_PROXY; + else process.env.CTX_FETCH_ALLOW_PROXY = prev; + } }); test("embedded SSRF classifier is callable as `classifyIp` even when bundler renames the export (#bug-v1.0.133)", () => {