feat(http): centralize HTTP client with proxy and custom-CA support#1686
Open
fengmk2 wants to merge 14 commits into
Open
feat(http): centralize HTTP client with proxy and custom-CA support#1686fengmk2 wants to merge 14 commits into
fengmk2 wants to merge 14 commits into
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
fengmk2
added
test: sfw
test: e2e
vite-plus
@voidzero-dev/vite-plus-core
@voidzero-dev/vite-plus-prompts
@voidzero-dev/vite-plus-test
@voidzero-dev/vite-plus-cli-darwin-arm64
@voidzero-dev/vite-plus-cli-darwin-x64
@voidzero-dev/vite-plus-cli-linux-arm64-gnu
@voidzero-dev/vite-plus-cli-linux-arm64-musl
@voidzero-dev/vite-plus-cli-linux-x64-gnu
@voidzero-dev/vite-plus-cli-linux-x64-musl
@voidzero-dev/vite-plus-cli-win32-arm64-msvc
@voidzero-dev/vite-plus-cli-win32-x64-msvc
@voidzero-dev/vite-plus-darwin-arm64
@voidzero-dev/vite-plus-darwin-x64
@voidzero-dev/vite-plus-linux-arm64-gnu
@voidzero-dev/vite-plus-linux-arm64-musl
@voidzero-dev/vite-plus-linux-x64-gnu
@voidzero-dev/vite-plus-linux-x64-musl
@voidzero-dev/vite-plus-win32-arm64-msvc
@voidzero-dev/vite-plus-win32-x64-msvc
commit: |
Member
Author
|
@cursor review |
fengmk2
force-pushed
the
feat/sfw-http-client
branch
from
a4c94d8 to
3ef6f6e
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
fengmk2
force-pushed
the
feat/sfw-http-client
branch
from
4de8fa0 to
c8ae23e
Compare
cpojer
approved these changes
May 27, 2026
fengmk2
added a commit
to voidzero-dev/setup-vp
that referenced
this pull request
May 28, 2026
Verifies whether voidzero-dev/vite-plus#1686 (centralized HTTP client with proxy + custom-CA support) fixes the macOS/Windows TLS handshake regression that gated setup-vp#73 to Linux. Installs vp from the pkg-pr-new build (commit c8ae23e), installs sfw via socketdev/action, then runs: - `sfw vp install` on a benign project — should pass on all 3 OSes - `sfw vp install lodahs` — should still block on all 3 OSes (proof sfw is actually inspecting traffic, not just passing it through) Bypasses setup-vp's own bootstrap so the only thing under test is the vp+sfw interaction. Delete once #1686 lands (or another build supersedes it).
fengmk2
force-pushed
the
feat/sfw-http-client
branch
2 times, most recently
from
e63a8ff to
d5d5966
Compare
Member
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ae60979. Configure here.
fengmk2
force-pushed
the
feat/sfw-http-client
branch
from
f4c75e0 to
e430085
Compare
Builds a single shared reqwest::Client in vite_shared that honors HTTPS_PROXY / HTTP_PROXY / NO_PROXY, loads PEM bundles from SSL_CERT_FILE and NODE_EXTRA_CA_CERTS, and exposes a VP_INSECURE_TLS diagnostic opt-in. Routes every existing reqwest::get / Client::new site in vite_install and vite_js_runtime through it so vp can traverse TLS-intercepting tools like Socket Firewall Free (sfw) and corporate MITM proxies. Adds an install-e2e-test-sfw job (Linux/macOS/Windows) that downloads the upstream sfw binary and runs `sfw vp i -g pnpm@9.15.0` plus `sfw vp install` against vitejs/vite. Gated on the `test: sfw` label for PRs, unconditional on push-to-main. Carries VP_INSECURE_TLS=1 until sfw upstream ships the EKU fix (SocketDev/sfw-free#30, #43); flip removed once that lands to also exercise CA injection. Refs voidzero-dev/setup-vp#73
- Replace `.expect()` on Client::build() with `output::error` + exit(1). Pre-PR, build failure (malformed HTTPS_PROXY, TLS init error) returned Err and was propagated; the OnceLock wrapper turned it into a panic that would re-fire on every subsequent call. Now a clean error and exit instead of a stack trace. - Surface CA-bundle read/parse failures via `output::warn` instead of `tracing::warn!`. tracing is silent unless VITE_LOG is set, hiding the misconfiguration from end users. - Parse SSL_CERT_FILE / NODE_EXTRA_CA_CERTS block-by-block via `Certificate::from_pem`. reqwest's `from_pem_bundle` fails the whole bundle on the first non-cert PEM block (e.g. a private key in the same file), dropping every cert silently. Now per-block: bad blocks warn, good blocks are added. - Use `std::env::var_os` so non-UTF-8 cert paths on Unix are honored. - Skip whitespace-only env values. - Enable reqwest's `system-proxy` feature so macOS System Settings and Windows registry proxies are honored, not just HTTPS_PROXY/HTTP_PROXY. - Add `stream` and `json` reqwest features to vite_shared so the API owner declares them (feature unification still keeps consumers working when their crates redeclare). - Add `error_for_status()?` to download_text so 4xx/5xx becomes an error instead of returning the error body as the "text". - Document SSL_CERT_FILE's additive semantics (differs from OpenSSL). - CI: move VP_INSECURE_TLS from job-level env to the single sfw step so unrelated build/setup steps don't run with cert verification off. - CI: add `--remove-on-error` to the sfw curl so a failed download doesn't leave a 0-byte file that the next step tries to exec.
sfw spawns its child process directly via CreateProcess without applying Windows PATHEXT, so a bare `sfw vp ...` invocation fails with "Command 'vp' not found in PATH" — sfw's own error message points at this exact fix. Linux and macOS pass on the bare name. Add a matrix `vp_bin` value (`vp` on POSIX, `vp.cmd` on Windows) and use it on the two sfw invocations.
Verified locally with sfw 0.12.22: the CA cert sfw issues has no Extended Key Usage extension at all (Basic Constraints CA:TRUE + Key Usage critical Certificate Sign), which rustls and native-tls accept per RFC 5280. The original bug (SocketDev/sfw-free#30, #43) — a present-but-empty EKU that rustls rejected — appears to have been fixed upstream. Removing VP_INSECURE_TLS=1 lets this CI job exercise the full CA-injection path (SSL_CERT_FILE -> Certificate::from_pem -> add_root_certificate -> TLS handshake) end-to-end, not just the proxy plumbing. Local reproduction: $ cat /tmp/p/package.json { "name":"sfw-tls-test", "packageManager":"pnpm@9.15.0" } $ rm -rf ~/.vite-plus/package_manager/pnpm/9.15.0* $ unset VP_INSECURE_TLS $ sfw vp install --no-frozen-lockfile Protected by Socket Firewall === Socket Firewall === 1 packages fetched successfully $ ls ~/.vite-plus/package_manager/pnpm/9.15.0/ -> pnpm
Two regressions surfaced in the last CI run: 1. Linux failed with `Failed to download from nodejs.org` — sfw's GitHub release v1.10.0 still issues a CA with a present-but-empty Extended Key Usage, which rustls rejects. My local verification used `sfw` from npm (0.12.x, separate version track) which doesn't carry the bug; the GitHub releases (what CI downloads) do. Restore VP_INSECURE_TLS=1 on this step until SocketDev/sfw-free#30 / #43 are released. 2. Windows failed with `Command 'vp.cmd' not found in PATH`. vp on Windows ships as `vp.exe` (the trampoline), not `vp.cmd`. sfw's earlier "try vp.cmd" suggestion was a generic PATHEXT hint.
`echo "$HOME/.vite-plus/bin" >> $GITHUB_PATH` writes a POSIX path like `/c/Users/runneradmin/.vite-plus/bin`. Git Bash itself can find binaries there, but child processes spawned via CreateProcess (sfw spawns its target this way) cannot — Windows resolves PATH entries with backslash semantics. Match the OS-aware fork used by the `test` job (~L266): write `$USERPROFILE\.vite-plus\bin` on Windows, `$HOME/.vite-plus/bin` elsewhere. Now `sfw vp.exe ...` finds vp on the Windows runner.
sfw v1.11.0 was published 2026-05-27, which may include the EKU fix referenced in SocketDev/sfw-free#30 / #43. Drop the bypass and let CI verify whether rustls (Linux/macOS) and native-tls (Windows) now accept sfw's CA. If any matrix entry fails with an UnknownIssuer-style TLS error, restore VP_INSECURE_TLS=1 with a fresh reference to the upstream issues.
The Linux sfw CI failure showed:
Failed to download from .../SHASUMS256.txt: error sending request
for url (https://.../SHASUMS256.txt)
The user-visible message stopped at reqwest's top-level Display and
dropped the actual cause (e.g. "invalid peer certificate:
UnknownIssuer" inside the rustls error chain). That hides why the
request failed — looks like a network blip when it's actually a TLS
trust problem.
Add `vite_shared::format_error_chain` that walks `Error::source()`
recursively and joins the messages with `: `. Use it in all four
`Error::DownloadFailed { reason }` map_err sites in
vite_js_runtime/src/download.rs.
Now the same failure renders as:
Failed to download from .../SHASUMS256.txt: error sending request
for url (...): client error (Connect): invalid peer certificate:
UnknownIssuer
making the root cause grep-able from the CI log.
The dropped-VP_INSECURE_TLS experiment confirmed via the now-readable error chain that sfw v1.11.0 (releases/latest as of 2026-05-28) still issues a CA cert with a present-but-empty Extended Key Usage: error sending request for url (https://nodejs.org/.../SHASUMS256.txt): client error (Connect): invalid peer certificate: UnknownIssuer (The new error-chain formatter from f105aa9 made the actual rustls reason visible — previously the same failure looked like a generic "error sending request" with no hint.) macOS happened to pass without the flag only because that runner had Node 22.18.0 already cached, so vp didn't have to fetch SHASUMS via sfw — not a real fix. Restore VP_INSECURE_TLS=1 on the sfw step (scoped to that step only to keep build/setup steps unaffected). The plumbing — HTTPS_PROXY + SSL_CERT_FILE + add_root_certificate — is still exercised end-to-end; only certificate *validity* is bypassed until SocketDev/sfw-free#30 and #43 ship.
Match what the `test` job uses (`namespace-profile-mac-default`, restored on main in 5fecd93). Same runner image and arch (aarch64-apple-darwin); just a faster pool.
Findings addressed:
- VP_INSECURE_TLS=0 / false / empty used to enable insecure TLS
because the check was `var_os().is_some()`. Now requires a truthy
value (1/true/yes/on, case-insensitive). Matches user mental model
and avoids a security footgun.
- extract_pem_cert_blocks was greedy: an orphan BEGIN followed by a
valid cert produced one bogus merged block and lost the valid one.
Now detects an intervening BEGIN before the next END and skips past
the orphan, recovering the legitimate cert.
- vite_error::Error::Reqwest and vite_js_runtime::Error::Reqwest were
#[error(transparent)] — Display fell through to reqwest, which only
shows the top-level message. The full source chain (TLS handshake →
UnknownIssuer, hyper IO) was lost for every error propagated via
`?` from `HttpClient::get` (vite_install path) and from the body-
streaming loop in `download_file`. Both now use
vite_shared::format_error_chain via #[error("{}", ...)] — From and
source() semantics intact, so 404 detection via e.status() still
works.
- build_client used std::process::exit(1) inside OnceLock::get_or_init
from a tokio worker — skipped Drop, leaked lockfiles/tempfiles, and
killed an embedding NAPI Node host from native code. Now caches
Result<Client, String> in the OnceLock and panics with the chain
message; subsequent callers see the same cached failure rather than
re-running build_client. The error message uses format_error_chain
rather than reqwest's top-level Display.
- Shared client had no timeout or connect_timeout — one stuck stream
could block every concurrent download through the shared HTTP/2
connection. Added 30s connect / 2 min request timeouts.
- format_error_chain now caps recursion depth at 16 (guards against
cyclic source chains) and skips a source whose Display is already
contained in the accumulated message (de-dupes when a parent
thiserror variant inlines its `#[from]` source via `{0}`).
- CI sfw step: ${{ matrix.vp_bin }} quoted in shell context.
vite_error gains a vite_shared dep (no cycle — vite_shared does not
depend on vite_error). Tests added for is_env_truthy, the orphan-
BEGIN recovery path, and the format_error_chain depth/dedup behavior.
Only Linux actually needs the TLS-bypass against sfw v1.11.0: - Linux (ubuntu-latest): runner doesn't preinstall Node 22.18 into vp's cache, so `sfw vp i -g pnpm@9.15.0` triggers vp's HttpClient to fetch nodejs.org/.../SHASUMS256.txt through sfw. rustls rejects sfw's broken CA (UnknownIssuer) — flag required. - macOS / Windows: runners already have Node 22.18 in vp's cache. vp never traverses sfw with its HttpClient in this test; the only HTTPS through sfw is npm's (lenient Node TLS). No flag needed — and leaving verification enabled there confirms the bypass is scoped, not blanket. Plumbed via a per-matrix-entry `vp_insecure_tls` value. The shared HTTP client treats an empty `VP_INSECURE_TLS` env as unset (the truthy-only parser added in the previous commit), so the empty value on macOS/Windows is a no-op.
The previous rebase resolved Cargo.lock by taking the in-flight version on one conflict, dropping a couple of upstream rolldown entries (rolldown_common's dunce dep, a plugin's rolldown_common dep) that should still be present on main. Re-add them.
…APIs - Replace the hand-rolled extract_pem_cert_blocks (~80 lines + 4 tests) with `reqwest::Certificate::from_pem_bundle`. rustls_pki_types' pem_reader_iter already filters non-CERTIFICATE PEM sections (private keys, comments, etc.) by returning None and `continue`-ing in the iterator — my earlier concern about mixed bundles failing was wrong on the facts. The only case the custom extractor handled better was a single corrupted base64 cert in an otherwise-valid bundle, which is rare enough that custom parsing isn't worth the maintenance cost. - Switch to the non-deprecated reqwest 0.13 builder methods: - add_root_certificate(c) -> tls_certs_merge([c, ...]) - danger_accept_invalid_certs(true) -> tls_danger_accept_invalid_certs(true) Both are the documented successors; add_root_certificate is marked Deprecated in reqwest 0.13.2's docs. - Keep the additive-roots semantics call-out in the module docs (vp treats SSL_CERT_FILE as additive, matching NODE_EXTRA_CA_CERTS — not curl/git's "replace the trust store" behavior).
fengmk2
force-pushed
the
feat/sfw-http-client
branch
from
c5b5103 to
1f443e9
Compare
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
2 participants
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.
Track B of voidzero-dev/setup-vp#73 — makes
vpwork through Socket Firewall Free (sfw) and other TLS-intercepting proxies.What
vite_shared::shared_http_client(): process-widereqwest::Clientthat honorsHTTPS_PROXY/HTTP_PROXY/NO_PROXY, picks up macOS System Settings / Windows registry proxies (system-proxyfeature), loads custom CAs fromSSL_CERT_FILEandNODE_EXTRA_CA_CERTS(additive, Node-style), and has connect + request timeouts.VP_INSECURE_TLS(truthy only —1/true/yes/on) is a diagnostic escape hatch with a loud warning.reqwest::get/reqwest::Client::new()call sites invite_installandvite_js_runtimerouted through the shared client.vite_shared::format_error_chain: walksError::source()so users see... invalid peer certificate: UnknownIssuerinstead of the opaqueerror sending request for url. Wired into bothError::Reqwestvariants and the fourDownloadFailed { reason }sites.install-e2e-test-sfwCI job (Linux / macOS / Windows): downloads upstreamsfw, runssfw vp i -g pnpm@…+sfw vp installagainst a freshvitejs/viteclone. Gated on thetest: sfwlabel for PRs; unconditional on push-to-main.VP_INSECURE_TLS=1is scoped to the Linux entry only (workaround for SocketDev/sfw-free#30 / #43 — present-but-empty EKU rejected by rustls).Out of scope / follow-ups
rustls-native-certsso OS-installed CAs work without env vars — Track B step 5, separate PR.vp_insecure_tlsfrom the Linux matrix once SocketDev/sfw-free ships the EKU fix (one-line change).Refs voidzero-dev/setup-vp#73