Skip to content

fix(tui): support SOCKS5 proxies and stop ALL_PROXY from crashing startup - #465

Merged
amszuidas merged 2 commits into
mainfrom
fix/socks5-proxy-support
Oct 9, 2026
Merged

amszuidas merged 2 commits into
mainfrom
fix/socks5-proxy-support

Conversation

@amszuidas

@amszuidas amszuidas commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Change

Fixes #393.

ALL_PROXY=socks5://... (a common setup with Clash and similar proxy apps) used to abort CLI startup with ALL_PROXY must be an http:// or https:// URL., because resolveTuiProxyConfiguration fell back to ALL_PROXY and only accepted http/https proxies.

  • socks5://, socks5h:// and socks:// proxies (via ALL_PROXY or explicit HTTP(S)_PROXY) now go through undici's built-in Socks5ProxyAgent, including username/password auth. Host names are resolved by the proxy (domain-name CONNECT), so socks5h:// is accepted as an alias. NO_PROXY hosts, including loopback, still connect directly.
  • IPv6 proxy addresses such as socks5://[::1]:1080 work: the SOCKS5 agent uses a connector built with buildConnector that strips the brackets before dialing (undici otherwise resolves [::1] and fails with ENOTFOUND).
  • Proxy credentials are validated during classification. Credentials that are not valid percent-encoding (for example 50%off) no longer crash startup with URI malformed: an ALL_PROXY fallback is ignored with a warning, and an explicit HTTP(S)_PROXY fails with an error naming the variable. Decoded credentials are passed to Socks5ProxyAgent as username/password options rather than through the URL.
  • An unusable ALL_PROXY fallback (for example socks4:// or a malformed URL) is ignored with a single stderr warning that tells the user how to fix it, instead of failing startup. Explicit invalid HTTP_PROXY/HTTPS_PROXY values are still rejected.
  • Proxy credentials are masked in warnings and errors.
  • Only undici's one-time SOCKS5 ExperimentalWarning is suppressed, and only while the agent is constructed; other warnings are unaffected.

The global dispatcher configured here is not overridden by third_party/pi-mono's configureHttpDispatcher: it is only called from pi-mono's own CLI entry points, which mcode does not load.

  • PR labels: bug, tui, cli

Validation

  • Checks run and results: pnpm verify (full profile) on 74ed9bf with a clean tracked tree, darwin/arm64, Node 24.16.0: PASS (also passed on c510234). test:windows skipped (platform), test:release-package skipped (requires an npm release archive). Targeted: network-proxy.test.ts + network-proxy-socks5.test.ts, 61 tests passed. The new SOCKS5 tests fail on main with the [Bug]: CLI hard-fails on startup when ALL_PROXY=socks5://… #393 error.
  • The integration test uses a loopback SOCKS5 server (no-auth and RFC 1929 username/password, IPv4 and IPv6 [::1]) and asserts domain-name CONNECT (ATYP 0x03), auth success/failure, empty passwords, malformed-credential fallback at startup, and that NO_PROXY hosts bypass the proxy.
  • Live check (on c510234): on macOS, a real no-auth SOCKS5 proxy from OpenSSH (ssh -N -D 1080 localhost) and only ALL_PROXY=socks5://127.0.0.1:1080 set, node dist/cli.js exec "Reply with exactly: socks ok" returned socks ok with exit code 0 and empty stderr (no SOCKS5 ExperimentalWarning). During the run, lsof showed the mcode process holding connections to 127.0.0.1:1080, confirming the traffic went through the proxy.
  • Performance: not run.
  • NOT RUN, platform limitations and live-service boundaries: Windows and Linux CI. Username/password SOCKS5 auth and IPv6 proxies are covered only by the loopback mock server, not by a live proxy. Not tested with proxy apps such as Clash. The Amazon Bedrock provider path (third_party/pi-mono/packages/ai/src/utils/node-http-proxy.ts) still rejects SOCKS proxies and is intentionally untouched. undici marks SOCKS5 support as experimental.
  • Known limitation: when the request target is an IPv6 literal (for example https://[::1]/), undici still sends the bracketed host name in the SOCKS5 CONNECT request. mcode only targets domain names, so it is not affected; this needs an upstream undici fix.

Publication and contribution checks

  • I have permission to contribute these changes under the existing licenses applicable to the changed files/packages; imported material and its provenance are identified and existing notices are preserved.
  • No credentials, account data, real user content, internal source history or private review material is included.
  • Added/removed source files were reviewed before regenerating release/public-source.json; new tests are declared in test/vitest-suites.json where applicable.
  • Shared English/Chinese documentation and capability/verification records are updated where applicable. Mock/offline results are not described as live-service acceptance.

Maintainer handoff

Publication scope or license changes (if any): none.

Shared-source port: pending.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

…rtup

Fixes #393. ALL_PROXY=socks5://... used to abort CLI startup because only
http:// and https:// proxy URLs were accepted.

- Route socks5://, socks5h://, and socks:// proxies through undici's
  Socks5ProxyAgent, which sends host names to the proxy for resolution.
  socks5h:// is rewritten to socks5:// with its credentials preserved.
  NO_PROXY hosts, including loopback, still connect directly.
- Ignore an unusable ALL_PROXY fallback (for example socks4:// or a
  malformed URL) with a single stderr warning instead of failing startup.
  Explicit invalid HTTP_PROXY/HTTPS_PROXY values are still rejected.
- Mask proxy credentials in warnings.
- Suppress only undici's one-time SOCKS5 ExperimentalWarning while the
  agent is constructed; other warnings are unaffected.
- Add unit tests and a loopback SOCKS5 integration test covering
  no-auth, username/password auth, domain-name CONNECT, and NO_PROXY.
@amszuidas amszuidas added bug Something isn't working tui Interactive terminal UI (TUI) cli Standalone mcode: TUI, headless, ACP and source builds/tooling labels Oct 9, 2026 — with Cursor
Follow-up to #393 review feedback.

- Dial SOCKS5 proxies at IPv6 literals such as socks5://[::1]:1080.
  undici passes the bracketed URL hostname to the socket connect, which
  failed with ENOTFOUND; a wrapped connector now strips the brackets.
- Validate the percent-encoding of proxy user names and passwords while
  classifying proxy settings. undici decodes them during agent
  construction and threw URIError, so a malformed ALL_PROXY made every
  command, including --help, exit with only "URI malformed". Such an
  ALL_PROXY is now ignored with a redacted warning; an explicit
  HTTP_PROXY/HTTPS_PROXY fails with an error naming the variable.
- Pass decoded SOCKS5 credentials as agent options and remove them from
  the proxy URL so undici does not decode them again.

@hetaoBackend hetaoBackend left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the diff and ran it locally (typecheck, lint, network-proxy*.test.ts: 61 passed). The new SOCKS5 tests fail against main, so they do cover the #393 regression. CI is green on all platforms, and nothing outside the network layer is touched.

The approach looks right: SOCKS5 goes through undici's Socks5ProxyAgent, an unusable ALL_PROXY is ignored with a single warning instead of aborting startup, credentials are masked in diagnostics, and NO_PROXY still bypasses the proxy.

Non-blocking:

  1. The PR checklist says shared docs were updated, but the diff has no doc changes. Please add a line about SOCKS5 support or adjust the checklist.
  2. socks5:// and socks5h:// both resolve hostnames at the proxy in undici, whereas curl resolves locally for socks5://. Worth one sentence in the docs for anyone relying on local DNS.
  3. The validation error lists http://, https://, socks5://, or socks5h://, but socks:// is also accepted. List it or drop the alias.
  4. The experimental-warning filter matches undici's exact message text, so a wording change upstream would just bring the warning back. Harmless. Also keep an eye on this when upgrading undici, since SOCKS5 is still experimental there.
  5. Bedrock with a SOCKS ALL_PROXY still fails at request time with an unsupported-protocol error rather than at startup; the PR says this is intentional.

Approving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cli Standalone mcode: TUI, headless, ACP and source builds/tooling tui Interactive terminal UI (TUI)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: CLI hard-fails on startup when ALL_PROXY=socks5://…

2 participants