fix: allow opt-in proxy env passthrough in ctx_fetch_and_index (CTX_FETCH_ALLOW_PROXY) - #1041
Open
vishnujayvel wants to merge 2 commits into
Open
fix: allow opt-in proxy env passthrough in ctx_fetch_and_index (CTX_FETCH_ALLOW_PROXY)#1041vishnujayvel wants to merge 2 commits into
vishnujayvel wants to merge 2 commits into
Conversation
…#1039) ctx_fetch_and_index embeds a subprocess via buildFetchCode that strips proxy env vars before fetch() so connect-time DNS rebinding guards stay on-path (mksglu#476 / CWE-918,441). Corporate networks that set HTTP_PROXY etc. therefore saw silent direct egress (reported by @DenisBalan). Do not reverse the mksglu#476 default. Gate preservation behind explicit operator consent: - Default: keep stripping HTTP_PROXY / HTTPS_PROXY / ALL_PROXY (and lowercase / npm_config_* siblings) — mksglu#476 pinning preserved. - When CTX_FETCH_ALLOW_PROXY=1: skip the strip so undici EnvHttpProxyAgent can honor corporate proxies (DenisBalan option 1). Style matches the existing CTX_FETCH_STRICT === "1" gate. Parent-side ssrfGuard still pre-flights every URL. Connect-time dns.lookup patches cover the no-proxy path; a proxy moves resolution out of their sight, which is why preservation must be opt-in. Tests: default strip pin + CTX_FETCH_ALLOW_PROXY=1 preservation. Validation: npx vitest run tests/core/server.test.ts (500 pass); npm run typecheck clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What / Why / How
ctx_fetch_and_indexembeds a subprocess script (buildFetchCode) that, since #476, deletes every proxy env var beforefetch()runs. That is a deliberate CWE-918 / CWE-441 control: a configuredHTTP_PROXY/HTTPS_PROXY/ALL_PROXYwould move DNS resolution to the proxy, so the connect-timedns.lookup/dnsPromises.lookupre-validation never sees a rebound IP. Corporate and audited environments that must route egress through a proxy therefore see silent direct egress — reported in #1039 by @DenisBalan.This PR does not reverse the #476 default. Unconditional preservation would re-open the SSRF control that #476 closed. Instead it implements DenisBalan's proposed option 1: explicit operator opt-in.
HTTP_PROXY/HTTPS_PROXY/ALL_PROXY(and lowercase /npm_config_*siblings). The fix(security): close DNS rebinding gap in ctx_fetch_and_index SSRF guard #476 pinning test remains green.CTX_FETCH_ALLOW_PROXY=1, the strip block is omitted so Node/undiciEnvHttpProxyAgentcan honor the configured proxy. Gate style matches the existingCTX_FETCH_STRICT === "1"precedent inserver.ts.Why parent-side resolve alone is not enough
A single parent-side
ssrfGuardDNS lookup is insufficient against rebinding: an attacker can return a public IP for the pre-flight check and a blocked IP (e.g. IMDS169.254.169.254) for the subprocess connect. The compensating control is the connect-timedns.lookupre-validation inside the generated script. A proxy moves that resolution out of the subprocess's sight, so preserving proxy vars must be operator-consented, not default.Existing carve-out
The original design already allows legit proxy use on
ctx_execute(executor#buildSafeEnvdoes not put proxy keys on the DENIED set). The strip lived only in the fetch-subprocess path. Operators who need corporate egress forctx_fetch_and_indexcan now setCTX_FETCH_ALLOW_PROXY=1without weakening the default sandbox posture for everyone else.Fixes #1039. Credit: @DenisBalan for the report, the exact repro against the embedded delete block, and the opt-in design option this implements.
Limitations
CTX_FETCH_ALLOW_PROXY=1is active there is no runtime signal that the rebinding defense is relaxed; a stderr note is offered as a follow-up if maintainers want operators to see the weaker posture.Affected platforms
(MCP-level fetch path; not adapter-specific.)
Test plan
CTX_FETCH_ALLOW_PROXY=1, generated source preserves proxy vars (nodelete process.env.*PROXY*)npx vitest run tests/core/server.test.ts— 500 passednpm run typecheck— cleanHTTP_PROXYpointed at a local proxy (or closed port) andCTX_FETCH_ALLOW_PROXY=1, confirmctx_fetch_and_indextraffic routes via the proxy (proxy log / connection refused vs direct success). Still unchecked.Checklist
npm testpasses (full suite not run;tests/core/server.test.ts+ typecheck green)npm run typecheckpassesnextbranch (unless hotfix) — staged offmainat base3bad0f4; retarget tonexton ship if preferredCross-platform notes
No path/hook changes. Env var names are the standard cross-platform proxy set (HTTP(S)PROXY, ALL_PROXY, npm_config*). Opt-in flag:
CTX_FETCH_ALLOW_PROXY=1.