fix: honor HTTP_PROXY / HTTPS_PROXY / NO_PROXY env vars#50
Closed
XXPermanentXX wants to merge 6 commits into
Closed
fix: honor HTTP_PROXY / HTTPS_PROXY / NO_PROXY env vars#50XXPermanentXX wants to merge 6 commits into
XXPermanentXX wants to merge 6 commits into
Conversation
Node's built-in fetch (undici) ignores proxy environment variables, so bl bypassed VPN / corporate proxies and failed with ECONNRESET. Install an EnvHttpProxyAgent as the global dispatcher at startup, but only when a proxy variable is actually set — behavior is unchanged otherwise. Values are trimmed and passed explicitly to work around undici reading env vars with `??`, where an empty lowercase variable (https_proxy="") masks a configured uppercase one. Invalid proxy URLs fail with a clear usage error instead of a stack trace, and the ECONNRESET hint now suggests exporting HTTPS_PROXY. Tests are fully offline: unit tests cover env parsing, and e2e tests drive a local CONNECT proxy against a .invalid host to verify traffic routes through the proxy, NO_PROXY is honored, and no dispatcher is installed when no proxy is configured. Fixes #35 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
app list resolves a console credential before issuing any request, so in CI (no ~/.bailian/config.json, no credentials) it exited with an AUTH error before fetch ran — the proxy received zero CONNECTs and the "routes through proxy" assertions failed. Locally these passed only because a real config.json supplied a token. Inject a fake DASHSCOPE_ACCESS_TOKEN (highest-priority credential source) so the request is actually issued and intercepted by the local CONNECT proxy. The target stays a .invalid host, so still no real network traffic.
dab4d33 to
712eb96
Compare
Switch the seed-image prompt from a cat sketch to a green leaf for a simpler, more reliably-generated reference frame.
It's a machine-local runtime lock file that shouldn't be in the repo; remove it and add it to .gitignore.
Collaborator
Author
|
Superseded by #51, which rebuilds these changes cleanly on top of |
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.
Summary
Fixes #35 — running
blbehind a VPN / corporate proxy failed withECONNRESET: Client network socket disconnected before secure TLS connection was established.Root cause: Node's built-in
fetch(undici) does not read proxy environment variables, so the CLI always connected directly, bypassing the local proxy and getting blocked.Fix: at startup, install an
EnvHttpProxyAgentas the global undici dispatcher — but only whenHTTP_PROXY/HTTPS_PROXYis actually set. When no proxy variable is configured, the global dispatcher is untouched and behavior is exactly as before.Changes
packages/cli/src/proxy.ts(new):readProxyEnv()+setupProxyFromEnv(). Lowercase variables take precedence over uppercase (curl convention);NO_PROXYis supported.??, where an empty lowercase variable (https_proxy="") masks a configured uppercase one.Invalid proxy configuration: …+ hint) instead of a raw stack trace.packages/cli/src/main.ts: callsetupProxyFromEnv()at module top level, before any fetch is issued (including update-checker).packages/cli/src/error-handler.ts: theECONNRESEThint now suggestsexport HTTPS_PROXY=…for VPN / corporate-proxy users.undiciadded as an explicit dependency (catalog^8.4.1).Tests (fully offline)
tests/proxy.test.ts: env parsing — unset/blank handling, lowercase precedence, empty-string-masking regression,NO_PROXY.tests/e2e/proxy.e2e.test.ts: spins up a local CONNECT proxy and targets a.invalidhost (guaranteed unresolvable, no real network traffic). Verifies:HTTPS_PROXYset, CLI traffic tunnels through the proxy (CONNECT to the target host);https_proxydoes not maskHTTPS_PROXY;NO_PROXYmatch keeps the request direct;All 10 tests pass.
🤖 Generated with Claude Code