Repository navigation
fix(wsl): rewrite loopback proxy URLs to the host gateway for guest shells - #21919
Bandersnatch0x wants to merge 6 commits into
Conversation
…hells A Windows loopback proxy configured in Settings > Advanced > Network is unreachable from inside a WSL2 distro (NAT mode): the guest's 127.0.0.1 is the distro itself, so agents in WSL worktrees silently run without the configured proxy. When injecting HTTP_PROXY/HTTPS_PROXY into a WSL PTY, probe the address from inside the distro (mirrored networking keeps loopback working) and otherwise rewrite the hostname to the WSL host gateway, keeping the rewrite only when the guest confirms it is reachable. Probes run through runWslProcess with bounded TTL caching; every failure falls back to the user's original URL.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. ℹ️ Recent review info
Walkthrough
Priority: Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 29.17% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 10 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Description check | The description covers the change, rationale, testing, security, compatibility, and performance impact. However, the required Linked Issue entry is not provided in the required form: it says no existi… | Update the Linked Issue section with the relevant issue reference, such as "Fixes #22024" if that issue tracks this change. Remove or reconcile the statement that no existing issue was found. |
✅ Passed checks (3 passed)
-
Check name Status Explanation Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request. Title check ✅ Passed The title clearly and concisely describes the main change: rewriting loopback proxy URLs to the WSL host gateway for guest shells.
Full details: Description check
-
Explanation
The description covers the change, rationale, testing, security, compatibility, and performance impact. However, the required Linked Issue entry is not provided in the required form: it says no existing issue was found, despite referencing
#22024later.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 89cc9082-2c63-40dd-b0a3-8328c935f16e
📒 Files selected for processing (8)
src/main/ipc/pty/ipc/spawn-env-codex.tssrc/main/ipc/pty/provider/local-configure.tssrc/main/ipc/pty/runtime/spawn-preflight.tssrc/main/rate-limits/claude-pty.test.tssrc/main/rate-limits/claude-pty.tssrc/main/wsl/__fixtures__/wsl-probe-failure-swallow-allowlist.txtsrc/main/wsl/wsl-guest-proxy-gateway.test.tssrc/main/wsl/wsl-guest-proxy-gateway.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The PR's core mechanism does not reach WSL guest shells: the rewritten proxy URL is written into wsl.exe's Windows-side environment, but the proxy keys are never registered in WSLENV, so they are dropped at the boundary. Separately, the new wsl-guest-proxy-gateway.test.ts fails 6 cases on the repo's Linux unit lane because it never stubs process.platform (verified locally on ubuntu-latest: 6 failed | 33 passed).
Reviewed changes
- New
wsl-guest-proxy-gateway.ts— probe-then-rewrite resolver for loopback proxies in WSL, with pure helpers (loopback detection, gateway parsing,/dev/tcpprobe script) and bounded TTL caches; ported from ZCode's remote-WSL backend (Apache-2.0, attributed). - Local-PTY env wiring —
spawn-env-codex.ts,local-configure.ts, andspawn-preflight.tsnow passawait wslProxyForTarget(...)intobuildPtyHostEnv; the hidden Claude usage PTY resolves the settings and exports the proxy inside its guestbash -lccommand. - Tests + allowlist — a 39-case unit suite for the helpers/resolver, one
claude-ptycase, and a probe-failure allowlist entry.
The helper logic itself is sound and well covered, and the hidden Claude-usage path (claude-pty.ts) is the one place the fix is correctly delivered to the guest. The two blocking findings below mean the stated goal — "agents in a WSL pane silently ran without your proxy" — is not yet met for normal panes, and CI is red as-is.
ℹ️ Nitpicks
src/main/wsl/wsl-guest-proxy-gateway.ts:44— the probe defaults a missing port to443forhttps:and80otherwise.normalizeProxyUrlacceptssocks:/socks4:/socks5:with no port, and those schemes have no WHATWG default, so a validsocks5://localhostis probed at port 80 while any client resolving it uses its own default (typically 1080). Consider skipping the rewrite path rather than probing the wrong port.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
- register HTTP(S)_PROXY/ALL_PROXY/NO_PROXY in WSLENV for WSL PTYs: WSLENV is the only channel that carries host variables into a distro, so the injected proxy env — rewritten to the host gateway or not — never reached guest shells without this - treat an errored reachability probe as 'could not ask': keep the user's URL instead of racing into a gateway rewrite (matches the JSDoc contract) - /dev/tcp rejects bracketed IPv6 literals — probe the bare address - stub process.platform in the unit tests so the suite passes on the Linux CI lane - check the abort signal right after proxy resolution in the hidden usage PTY (three sequential 5s probes must not delay cancellation into a spawn)
|
All findings from the bot reviews are addressed in 9f6e34e:
|
There was a problem hiding this comment.
Important
The new WSLENV carry fixes delivery of the resolved proxy into WSL, but it also starts forwarding proxy variables Orca never rewrote — see the inline note.
Reviewed changes
- Registered the proxy env keys in
WSLENVfrombuildPtyHostEnvwhenopts.isWsl, so the resolved/rewritten proxy actually crosses thewsl.exeboundary into the distro. - Exported
PROXY_ENV_KEYS/NO_PROXY_ENV_KEYSfromsrc/shared/network-proxy.tsfor that registration. - Treated an errored loopback probe as "could not ask" (
!== false), so the user's URL survives a probe failure, matching the module's JSDoc. - Probed bare IPv6 addresses over
/dev/tcp(bracketed literals fail name resolution) and updated the assertion. - Stubbed
process.platformin the new suite so it passes on the Linux unit lane. - Re-checked the abort signal after proxy resolution in
fetchViaPty, so a cancellation during the probe window cannot still spawn the hidden PTY.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
- POSIX-quote the hostname in buildWslProxyProbeScript so a crafted proxy host cannot inject a command substitution into the nested bash probe. - Only forward proxy env keys through WSLENV when the configured proxy URL actually crosses the guest boundary (non-loopback), so an env-inherited loopback proxy is not leaked into the distro. - Tighten claude-pty test type-assertions to optional-chained reads.
There was a problem hiding this comment.
Important
The new WSLENV gate closes the inherited-proxy leak, but it also suppresses a loopback proxy the guest has already confirmed reachable (mirrored networking / WSL1), and CI's changed-code cast gate is still red on the reshaped assertions.
Reviewed changes
- Gated
WSLENVproxy registration inbuildPtyHostEnvon a newwslConfiguredProxyCrossesBoundary()helper, so only a non-loopback configured URL crosses thewsl.exeboundary. - Quoted the probe hostname with
quotePosixShellinbuildWslProxyProbeScript, hardening the nestedbash -cagainst command substitution, with updated assertions and a new injection test. - Reshaped the
spawnMock.mock.calls[0]reads inclaude-pty.test.tsinto optional-chained accesses and added a case asserting the gateway-rewritten proxy is exported inside the WSL launch command.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…casts
- resolveWslGuestProxySettings now returns { settings, crossesBoundary }: a
guest-confirmed loopback (mirrored networking / WSL1) crosses into the guest
instead of being suppressed alongside unverified loopbacks, while an errored
probe still stays out. Threaded through wslProxyForTarget and the three spawn
call sites via a wslProxyResolution option on buildPtyHostEnv.
- Drop the type assertions in claude-pty.test that tripped the changed-code
cast gate; the untyped mock's calls are already any.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 45c9a612-d3f9-4a16-8489-3b694d4ee001
📒 Files selected for processing (9)
src/main/ipc/pty/host-env/assembly.tssrc/main/ipc/pty/host-env/types.tssrc/main/ipc/pty/ipc/spawn-env-codex.tssrc/main/ipc/pty/provider/local-configure.tssrc/main/ipc/pty/runtime/spawn-preflight.tssrc/main/rate-limits/claude-pty.test.tssrc/main/rate-limits/claude-pty.tssrc/main/wsl/wsl-guest-proxy-gateway.test.tssrc/main/wsl/wsl-guest-proxy-gateway.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Re-reviewed the delta since the prior pullfrog review (1e7d976 → baf8a54), covering all 11 changed files.
- Resolver now reports whether the proxy crosses —
resolveWslGuestProxySettingsreturns{ settings, crossesBoundary }; a guest-confirmed loopback (mirrored networking / WSL1), a confirmed gateway rewrite, and any non-loopback proxy all setcrossesBoundary: true, while an unverified loopback stays out. WSLENVgate consumes the verdict — the three local-PTY callers passwslProxyResolutionandbuildPtyHostEnvregisters the proxy keys only whencrossesBoundaryis true, which restores the configured proxy for mirrored/WSL1 users without re-opening the inherited-proxy leak.- Hidden Claude usage PTY follows the resolved settings —
claude-pty.tsresolves before spawning and exports the resolved proxy inside the guest command. - CI unblocked — the
consistent-type-assertionscasts inclaude-pty.test.tsare dropped;pnpm run check:code-quality:changedreports 0 findings, and the 58 tests across the two touched suites pass.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…ommand claude-pty exports the resolved proxy inside the guest bash command, so honor crossesBoundary there too: when a WSL probe cannot confirm a loopback proxy, drop it rather than forcing a dead egress path in the guest. Covered by tests for both the crosses and the suppressed cases.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Re-reviewed the delta since the prior pullfrog review (baf8a54 → 469bf0f), covering the two touched files plus the full 11-file PR for context.
claude-pty.tsnow honourscrossesBoundary— for a WSL target the resolved proxy is exported into the guestbash -lccommand only when the resolver confirms it crosses the boundary; an unverified loopback is dropped from both the Windows-side spawn env and the guest command instead of forcing a dead egress path. Non-WSL still getswslProxyResolution.settingsunchanged.- Regression test added —
does not export an unverified loopback proxy into the WSL launch commandfails on the pre-fix source (verified by in-place revert), the existing proxy test is renamed and pinscrossesBoundary: true, and the suite's resolver mock now defaults tocrossesBoundary: false.
This closes the one asymmetry tracked from the previous run. pnpm run check:code-quality:changed reports 0 findings and the two touched suites pass (59 tests).
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
# Conflicts: # src/main/ipc/pty/host-env/assembly.ts # src/main/ipc/pty/ipc/spawn-env-codex.ts # src/main/ipc/pty/provider/local-configure.ts # src/main/ipc/pty/runtime/spawn-preflight.ts # src/main/rate-limits/claude-pty.test.ts # src/main/rate-limits/claude-pty.ts # src/main/wsl/__fixtures__/wsl-probe-failure-swallow-allowlist.txt
|
Closing this in favour of the upstream WSL-via-orcad series (#27792, #27800, #27819, #27834, #27916, #27933, #27934, #27955), which now owns this area of the product. The changes here were kept mergeable against main and still pass their checks, but the design they build on predates the orcad host architecture. Rather than maintain a parallel path, this work waits until the official WSL host implementation lands and can absorb it. No action needed on this PR. |
ELI5
If you run coding agents inside a WSL distro (WSL worktree on Windows) and you set a proxy in Settings > Advanced > Network like
http://127.0.0.1:7890(Clash, v2ray, ...), that proxy is unreachable from inside the distro under WSL2 NAT — the guest's127.0.0.1is the distro itself, not Windows. Every agent in a WSL pane silently ran without your proxy. This PR asks the distro whether the proxy address is reachable; if not, it rewrites the hostname to the WSL host gateway address (e.g.http://172.28.112.193:7890), but only after the guest confirms the rewritten address actually works. Mirrored-networking setups and every failure mode keep the original URL untouched.What Changed
src/main/wsl/wsl-guest-proxy-gateway.ts:ip route show default, falling back to private/link-localresolv.confnameservers), and a/dev/tcpreachability probe script;resolveWslGuestProxySettings(): returns the settings untouched on every fast path (non-WSL, no proxy, non-loopback URL) and on any probe failure, and only produces a rewritten URL when the guest confirms the gateway address is reachable;wslProxyForTarget(): PTY-spawn convenience wrapper keyed off the existingCodexAccountSelectionTarget(same WSL predicate asshouldSkipCodexHomeEnvForWindowsShell);local-configure.ts,spawn-env-codex.ts,spawn-preflight.ts) now pass the resolved settings intobuildPtyHostEnv, which injectsHTTP_PROXY/HTTPS_PROXY/... viabuildConfiguredProxyEnv.rate-limits/claude-pty.ts) gets the same treatment for its WSL launch command.runWslProcess(bash,loginPath: 'none', 5s timeout) — no new directwsl.exespawn; the invocation-boundary and probe-failure-semantics guards stay green (one new allowlist entry for a pure URL-parsecatch, with a note).Why
Before: a loopback proxy configured in Orca settings was injected verbatim into WSL PTYs, where it cannot connect — agents in WSL ran proxy-less (timeouts, or direct egress that the user explicitly tried to route through a proxy). Orca already knows about the WSL NAT loopback hazard for agent hooks (
wslHookRelayManager), but the proxy env path had no equivalent.The probe-then-rewrite shape (and the gateway parsing rules, including rejecting public
resolv.confnameservers that are not the host) is ported from zai-org/ZCode's remote-WSL backend (packages/server/src/remote/wslProxy.ts, Apache-2.0), adapted to Orca'srunWslProcessrunner and proxy settings flow. Alternatives considered: always rewriting to the gateway (breaks mirrored networking), documenting a manual workaround (pushes the problem onto users), and pre-warming a sync cache (first spawn would still get the broken URL).Linked Issue
No existing issue found for this; the problem report is contained in this PR. (Happy to move discussion to an issue if preferred.)
Visual Proof
N/A — no UI change; behavior is inside PTY environment assembly.
Testing
Automated tests added:
src/main/wsl/wsl-guest-proxy-gateway.test.ts(39 cases: pure helpers + resolver behavior — mirrored networking keeps the URL, NAT rewrite applies, unreachable/gateway-failure/probe-error fall back, per-distro gateway cache, verdict cache), plus aclaude-pty.test.tscase asserting the rewritten URL is exported inside the WSL launch command.oxlintclean on all touched files;tsc --noEmitpasses for all four tsconfig projects.Windows 11 (WSL2 Ubuntu): targeted suites pass —
wsl/,rate-limits/claude-pty,pty-ipc-*(36/36), daemon WSL spawn suites identical to the pre-change baseline (4 pre-existing failures are Windows path-separator issues reproducible onmainwithout these changes).I manually tested these changes locally (automated coverage above; end-to-end with a real proxy on a real distro would be a welcome second review)
Automated tests added/updated, or explained why not below
Review
AI review summary: cross-platform — all new code is guarded by
process.platform === 'win32'+isWslflags; non-Windows and non-WSL paths return the settings object identity unchanged, so macOS/Linux/SSH behavior is byte-for-byte identical. SSH/remote — no touch:buildPtyHostEnvis not called for SSH spawns, and the change only affects its local-PTY callers'networkProxySettingsargument. Performance — hot path (PTY spawn) is unaffected unless a loopback proxy is configured AND the shell is WSL; then it is bounded by a 5s probe with 60s/5min caches, and every failure falls back to the original URL. Security — the rewrite only ever replaces the hostname with an address the guest itself reported as reachable; credentials and port are preserved; no new data leaves the machine. Agent compatibility — proxy env reaches all agents via the sharedbuildConfiguredProxyEnv, so the fix is agent-neutral.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.(The boundary targets vercel-labs/skills skill-installer material; this PR ports a small networking helper from zai-org/ZCode (Apache-2.0), attributed in the module header and above.)
Notes
HTTP_PROXYthat reaches the WSL PTY through process-env inheritance) are NOT rewritten — only the Settings-configured proxy, which is the documented path. A follow-up could cover the inherited case.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)Part of the WSL-host series tracked in #22024 (this proxy-gateway fix is independent and can merge on its own).