From c2f4f8059e9665f05c9597c991653169e7f7ef03 Mon Sep 17 00:00:00 2001 From: rudi193-cmd Date: Fri, 14 Aug 2026 13:54:54 -0600 Subject: [PATCH 1/4] feat(http): honor HTTP_PROXY/HTTPS_PROXY/NO_PROXY on login and submit Node's fetch ignores proxy env vars. Attach undici EnvHttpProxyAgent only when a proxy is set, keep it inside http-client.ts so scan stays offline. Slice 1 of #83. Co-authored-by: Cursor --- CHANGELOG.md | 5 +++ package-lock.json | 16 ++++++-- package.json | 3 +- src/http-client.ts | 35 +++++++++++++++--- src/node-shims.d.ts | 9 +++++ test/http-client.test.ts | 61 +++++++++++++++++++++++++++++++ test/privacy/zero-network.test.ts | 14 +++++++ 7 files changed, 133 insertions(+), 10 deletions(-) create mode 100644 test/http-client.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 82db844..dbda55d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Versioning: strict [semver](https://semver.org/) — bundle schema changes always bump at least minor; breaking schema changes bump major. +## [Unreleased] + +### Added +- Honor `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY` on login and submit via undici's `EnvHttpProxyAgent` (#83). + ## [0.13.0] - 2026-08-14 ### Changed diff --git a/package-lock.json b/package-lock.json index 4d64b59..9d7ff5e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,16 +1,17 @@ { "name": "@redential/cli", - "version": "0.5.0", + "version": "0.12.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@redential/cli", - "version": "0.5.0", + "version": "0.12.0", "license": "Apache-2.0", "dependencies": { "commander": "^12.1.0", - "typescript": "^5.6.0" + "typescript": "^5.6.0", + "undici": "6.28.0" }, "bin": { "redential": "dist/cli.js" @@ -1284,6 +1285,15 @@ "node": ">=14.17" } }, + "node_modules/undici": { + "version": "6.28.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.28.0.tgz", + "integrity": "sha512-LIY910g9TI13YS95lrMFrs8Rm/u/irgHeTWoKCoteeJ04CUJ92eEfj0rVn+7VKMPBpUPiUoBKfhNyLI23EE/KA==", + "license": "MIT", + "engines": { + "node": ">=18.17" + } + }, "node_modules/vite": { "version": "5.4.21", "resolved": "https://registry.npmjs.org/vite/-/vite-5.4.21.tgz", diff --git a/package.json b/package.json index f461e23..a337155 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,8 @@ }, "dependencies": { "commander": "^12.1.0", - "typescript": "^5.6.0" + "typescript": "^5.6.0", + "undici": "6.28.0" }, "devDependencies": { "vitest": "^2.1.0" diff --git a/src/http-client.ts b/src/http-client.ts index fd043d8..1e819ae 100644 --- a/src/http-client.ts +++ b/src/http-client.ts @@ -1,5 +1,28 @@ +import { EnvHttpProxyAgent } from "undici"; import { NetworkError } from "./errors.js"; +/** + * Node's built-in fetch ignores HTTP_PROXY/HTTPS_PROXY (issue #83). Attach + * undici's EnvHttpProxyAgent only when a proxy env var is set, so an unset + * environment stays byte-identical to today's dispatcher-less fetch. + * NO_PROXY/no_proxy are honored by the agent itself. + */ +function proxyEnvSet(): boolean { + return Boolean( + process.env.HTTP_PROXY || + process.env.HTTPS_PROXY || + process.env.http_proxy || + process.env.https_proxy + ); +} + +function cliFetch(url: string, init: RequestInit): Promise { + if (!proxyEnvSet()) { + return fetch(url, init); + } + return fetch(url, { ...init, dispatcher: new EnvHttpProxyAgent() } as RequestInit); +} + /** * The only module allowed to call `fetch` for JSON requests (login.ts and * submit.ts are the other two — see test/privacy/zero-network.test.ts's @@ -15,7 +38,7 @@ export async function postJson( const host = new URL(url).host; let res: Response; try { - res = await fetch(url, { + res = await cliFetch(url, { method: "POST", headers: { "content-type": "application/json", ...headers }, body: JSON.stringify(body), @@ -46,7 +69,7 @@ export async function postRawJson( const host = new URL(url).host; let res: Response; try { - res = await fetch(url, { + res = await cliFetch(url, { method: "POST", headers: { "content-type": "application/json", ...headers }, body: rawBody, @@ -78,7 +101,7 @@ export async function pollJson(url: string, body: unknown): Promise { const host = new URL(url).host; let res: Response; try { - res = await fetch(url, { + res = await cliFetch(url, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(body), @@ -116,7 +139,7 @@ export async function postJsonStatusOnly( ): Promise { const host = new URL(url).host; try { - const res = await fetch(url, { + const res = await cliFetch(url, { method: "POST", headers: { "content-type": "application/json", ...headers }, body: JSON.stringify(body), @@ -135,7 +158,7 @@ export async function postJsonStatusOnly( */ export async function headRequest(url: string, timeoutMs: number): Promise<{ status: number } | null> { try { - const res = await fetch(url, { + const res = await cliFetch(url, { method: "HEAD", redirect: "follow", signal: AbortSignal.timeout(timeoutMs), @@ -161,7 +184,7 @@ export async function getJson( headers: Record = {} ): Promise { try { - const res = await fetch(url, { method: "GET", headers, signal: AbortSignal.timeout(timeoutMs) }); + const res = await cliFetch(url, { method: "GET", headers, signal: AbortSignal.timeout(timeoutMs) }); if (!res.ok) return null; return (await res.json()) as T; } catch { diff --git a/src/node-shims.d.ts b/src/node-shims.d.ts index 0780e6a..397d017 100644 --- a/src/node-shims.d.ts +++ b/src/node-shims.d.ts @@ -154,3 +154,12 @@ declare module "node:readline/promises" { output: unknown; }): Interface; } + +// Exact surface http-client.ts uses from the pinned `undici` package +// (EnvHttpProxyAgent for HTTP_PROXY/HTTPS_PROXY/NO_PROXY — issue #83). +// Hand-written so we still do not depend on @types/node. +declare module "undici" { + export class EnvHttpProxyAgent { + constructor(); + } +} diff --git a/test/http-client.test.ts b/test/http-client.test.ts new file mode 100644 index 0000000..64dd96e --- /dev/null +++ b/test/http-client.test.ts @@ -0,0 +1,61 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { postJson } from "../src/http-client.js"; + +const originalFetch = globalThis.fetch; +const proxyKeys = ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy", "NO_PROXY", "no_proxy"] as const; +const originalProxy: Record = {}; +for (const key of proxyKeys) { + originalProxy[key] = process.env[key]; +} + +afterEach(() => { + globalThis.fetch = originalFetch; + for (const key of proxyKeys) { + const value = originalProxy[key]; + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } +}); + +function jsonOk(): Response { + return new Response("{}", { status: 200, headers: { "content-type": "application/json" } }); +} + +describe("http-client proxy dispatcher (#83 slice 1)", () => { + it("does not pass a dispatcher when no proxy env is set", async () => { + for (const key of proxyKeys) delete process.env[key]; + const fetchMock = vi.fn(async () => jsonOk()); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await postJson("https://example.test/api", {}); + + expect(fetchMock).toHaveBeenCalledOnce(); + const init = fetchMock.mock.calls[0][1] as Record; + expect(init.dispatcher).toBeUndefined(); + }); + + it("passes a dispatcher when HTTP_PROXY is set", async () => { + for (const key of proxyKeys) delete process.env[key]; + process.env.HTTP_PROXY = "http://127.0.0.1:8888"; + const fetchMock = vi.fn(async () => jsonOk()); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await postJson("https://example.test/api", {}); + + expect(fetchMock).toHaveBeenCalledOnce(); + const init = fetchMock.mock.calls[0][1] as Record; + expect(init.dispatcher).toBeDefined(); + }); + + it("passes a dispatcher when only HTTPS_PROXY is set", async () => { + for (const key of proxyKeys) delete process.env[key]; + process.env.HTTPS_PROXY = "http://127.0.0.1:8888"; + const fetchMock = vi.fn(async () => jsonOk()); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await postJson("https://example.test/api", {}); + + const init = fetchMock.mock.calls[0][1] as Record; + expect(init.dispatcher).toBeDefined(); + }); +}); diff --git a/test/privacy/zero-network.test.ts b/test/privacy/zero-network.test.ts index ace6d77..c0faae1 100644 --- a/test/privacy/zero-network.test.ts +++ b/test/privacy/zero-network.test.ts @@ -276,6 +276,20 @@ describe("zero network calls during scan", () => { } }); + // Slice 1 of #83: the proxy agent is a network primitive. It must live in + // http-client.ts with fetch, never in login/submit/scan. + it("imports undici only from http-client.ts", () => { + const srcUrl = new URL("../../src/", import.meta.url); + const files = listSrcTsFiles(srcUrl).filter( + (f) => f !== "http-client.ts" && !f.endsWith(".d.ts") + ); + const undiciPattern = /['"]undici['"]/; + for (const file of files) { + const contents = readFileSync(new URL(file, srcUrl), "utf8"); + expect(contents, `${file} should not import undici`).not.toMatch(undiciPattern); + } + }); + // version-check.ts's checkForUpdate (the post-success "a newer version // exists" notice — see docs/login-submit.md's "Version check" section) // deliberately never references fetch/http/https directly: it goes From 2e2f44bae44fde287474fa5b109b714cc0a2efc0 Mon Sep 17 00:00:00 2001 From: rudi193-cmd Date: Fri, 14 Aug 2026 15:43:43 -0600 Subject: [PATCH 2/4] fix(http): reuse one EnvHttpProxyAgent for the process Device-flow polling was allocating a new agent (and pool) per request and never closing it. Lazy module-level singleton as requested on #84. Co-authored-by: Cursor --- src/http-client.ts | 14 ++++++++++---- test/http-client.test.ts | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/http-client.ts b/src/http-client.ts index 1e819ae..bacc6b8 100644 --- a/src/http-client.ts +++ b/src/http-client.ts @@ -3,9 +3,9 @@ import { NetworkError } from "./errors.js"; /** * Node's built-in fetch ignores HTTP_PROXY/HTTPS_PROXY (issue #83). Attach - * undici's EnvHttpProxyAgent only when a proxy env var is set, so an unset - * environment stays byte-identical to today's dispatcher-less fetch. - * NO_PROXY/no_proxy are honored by the agent itself. + * a single module-level EnvHttpProxyAgent only when a proxy env var is set, + * so an unset environment stays byte-identical to today's dispatcher-less + * fetch. NO_PROXY/no_proxy are honored by the agent itself. */ function proxyEnvSet(): boolean { return Boolean( @@ -16,11 +16,17 @@ function proxyEnvSet(): boolean { ); } +let proxyAgent: InstanceType | undefined; + +function proxyDispatcher(): InstanceType { + return (proxyAgent ??= new EnvHttpProxyAgent()); +} + function cliFetch(url: string, init: RequestInit): Promise { if (!proxyEnvSet()) { return fetch(url, init); } - return fetch(url, { ...init, dispatcher: new EnvHttpProxyAgent() } as RequestInit); + return fetch(url, { ...init, dispatcher: proxyDispatcher() } as RequestInit); } /** diff --git a/test/http-client.test.ts b/test/http-client.test.ts index 64dd96e..1e6e3af 100644 --- a/test/http-client.test.ts +++ b/test/http-client.test.ts @@ -58,4 +58,19 @@ describe("http-client proxy dispatcher (#83 slice 1)", () => { const init = fetchMock.mock.calls[0][1] as Record; expect(init.dispatcher).toBeDefined(); }); + + it("reuses one EnvHttpProxyAgent across calls", async () => { + for (const key of proxyKeys) delete process.env[key]; + process.env.HTTP_PROXY = "http://127.0.0.1:8888"; + const fetchMock = vi.fn(async () => jsonOk()); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await postJson("https://example.test/a", {}); + await postJson("https://example.test/b", {}); + + const first = (fetchMock.mock.calls[0][1] as Record).dispatcher; + const second = (fetchMock.mock.calls[1][1] as Record).dispatcher; + expect(first).toBeDefined(); + expect(second).toBe(first); + }); }); From 5cfef747e8c59da2b3c27db0d1ae5d0deea26002 Mon Sep 17 00:00:00 2001 From: rudi193-cmd Date: Fri, 14 Aug 2026 15:56:33 -0600 Subject: [PATCH 3/4] feat(http): name reach-failure class; document corporate networks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Classify thrown NetworkError from error.code / 407 only — never error.message, headers, or body. Add docs/corporate-networks.md for proxy env, NODE_EXTRA_CA_CERTS, and the captive-proxy visibility probe. Slices 2–3 of #83. Co-authored-by: Cursor --- CHANGELOG.md | 2 + README.md | 1 + docs/corporate-networks.md | 72 ++++++++++++++++++++++++++++++++ docs/exit-codes.md | 2 +- docs/login-submit.md | 14 +++++-- docs/privacy-tests.md | 6 ++- src/errors.ts | 3 +- src/http-client.ts | 84 +++++++++++++++++++++++++++++++------- test/http-client.test.ts | 64 +++++++++++++++++++++++++++++ 9 files changed, 226 insertions(+), 22 deletions(-) create mode 100644 docs/corporate-networks.md diff --git a/CHANGELOG.md b/CHANGELOG.md index dbda55d..0abed8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ always bump at least minor; breaking schema changes bump major. ### Added - Honor `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY` on login and submit via undici's `EnvHttpProxyAgent` (#83). +- Network errors name a closed failure class (`connection refused`, TLS / corporate CA, `proxy required`) without echoing `error.message`, headers, or body (#83). +- Document corporate proxy / CA setup and the submit visibility-probe captive-proxy edge (`docs/corporate-networks.md`, #83). ## [0.13.0] - 2026-08-14 diff --git a/README.md b/README.md index 0171bf9..59301ec 100644 --- a/README.md +++ b/README.md @@ -205,6 +205,7 @@ and what the provenance attestation actually proves. - [docs/scan.md](docs/scan.md) — full `scan` command reference - [docs/exit-codes.md](docs/exit-codes.md) — exit codes for CI and shell scripts - [docs/login-submit.md](docs/login-submit.md) — `login`, `submit`, `logout` +- [docs/corporate-networks.md](docs/corporate-networks.md) — proxy env vars, `NODE_EXTRA_CA_CERTS`, captive-proxy visibility probe - [docs/identity-selection-memory.md](docs/identity-selection-memory.md) — how `scan`/`submit` remember your per-repo identity selection - [docs/private-label.md](docs/private-label.md) — the mandatory private label: what it is, why it travels outside the bundle - [docs/schema.md](docs/schema.md) — every bundle field, explained diff --git a/docs/corporate-networks.md b/docs/corporate-networks.md new file mode 100644 index 0000000..44e159a --- /dev/null +++ b/docs/corporate-networks.md @@ -0,0 +1,72 @@ +# Corporate networks + +`login` and `submit` are the only commands that talk to the network. +`scan` never does, including behind a proxy. + +If device-flow login fails at connect, you are usually missing one of +three things: the proxy env vars, the corporate CA, or both. The CLI +will name the failure class (`connection refused`, `could not verify TLS +certificate`, `proxy required`) without echoing headers, bodies, or +Node's error text — those can contain tokens. + +## Proxy env vars + +Node's built-in `fetch` ignores `HTTP_PROXY` / `HTTPS_PROXY`. This CLI +attaches undici's `EnvHttpProxyAgent` when any of these are set: + +- `HTTP_PROXY` / `http_proxy` +- `HTTPS_PROXY` / `https_proxy` +- `NO_PROXY` / `no_proxy` (honored by the agent: hosts that must bypass + the proxy, typically `localhost` and your internal git host) + +Example: + +```bash +export HTTPS_PROXY=http://proxy.corp.example:8080 +export NO_PROXY=localhost,127.0.0.1,.corp.example +npx redential login +``` + +Leave them unset on a direct network — the client then uses the same +dispatcher-less `fetch` as a machine with no proxy. + +A `407` from the proxy (or undici's `UND_ERR_PROXY`) prints `proxy +required`. That usually means the env var is missing, the URL is wrong, +or the proxy wants authentication the CLI does not prompt for — set the +vars your IT docs specify; do not paste a password into an issue. + +## Corporate CA (`NODE_EXTRA_CA_CERTS`) + +TLS-intercepting proxies re-sign HTTPS with a company CA. Node does not +use the OS trust store the way a browser does. Point it at the PEM your +IT already installed: + +```bash +export NODE_EXTRA_CA_CERTS=/etc/ssl/certs/corp-root.pem +npx redential login +``` + +The path is an example. Use whatever file your IT documents. Without it, +login fails with `could not verify TLS certificate (corporate proxy? see +docs/corporate-networks.md)` — not a Redential outage. + +## `submit`'s visibility probe vs a captive proxy + +Before upload, `submit` may HEAD the git remote — **only** for remotes +that look like github.com / gitlab.com / bitbucket.org, never an +arbitrary self-hosted URL, never with your credentials. A confirmed +`2xx`/`3xx` blocks submit (the repo answered as publicly reachable). +Anything else, including a network error, is fail-open. + +A captive corporate proxy that answers **200 for every host** will make +that probe look like "public." The CLI will refuse to submit and tell +you to connect the GitHub App instead. If the repo is actually private, +the check was wrong — that is the proxy lying, not a leak: the probe is +unauthenticated HEAD, and nothing from your bundle has been sent yet. +Workarounds: add the git host to `NO_PROXY` so the probe reaches the +real origin, or report the false block if you are on a known-public +host that is in fact private. + +`headRequest` stays fail-open (`null` on error). A proxy that **times +out** or **refuses** the probe does not block submit; you get scan's +existing public-host warning and may proceed. diff --git a/docs/exit-codes.md b/docs/exit-codes.md index be67e6d..c18920c 100644 --- a/docs/exit-codes.md +++ b/docs/exit-codes.md @@ -27,7 +27,7 @@ below (it's whatever the underlying error happened to say), but the shape | `ScanError` | Invalid repo or git state, missing `--author` / `--yes` in non-interactive mode, validation failures (private label, `--since`, secrets in bundle), unsupported `explain` skill, malformed internal signature files. | | `AuthError` | `submit` without a stored session, wrong site URL on stored credentials, login denied/expired/timed out. | | `SubmitError` | Upload refused after the remote-visibility gate (confirmed-public repo). | -| `NetworkError` | Login or submit HTTP failures (unreachable host, non-JSON response, unexpected status) after retries where applicable. | +| `NetworkError` | Login or submit HTTP failures (unreachable host, TLS/proxy class, non-JSON response, unexpected status) after retries where applicable. See [corporate-networks.md](corporate-networks.md) for proxy and corporate-CA setup. | Messages are sanitized user-facing strings only — never tokens or bundle payloads. diff --git a/docs/login-submit.md b/docs/login-submit.md index 876cea6..55857f5 100644 --- a/docs/login-submit.md +++ b/docs/login-submit.md @@ -334,6 +334,9 @@ this gate is its real, definitive answer: check must never be flakier than `scan`'s own warn-only heuristic: on an inconclusive result, `submit` falls back to printing `publicHostWarning`'s own (longer) message and proceeds. +- A captive corporate proxy that answers `200` for every host will make + this probe look public and block submit. That is a false block, not a + leak — see [corporate-networks.md](corporate-networks.md). ## Identity corroboration (submit-only) @@ -458,9 +461,14 @@ the network, so the boundary is worth stating precisely: Every command-level error is one of `ScanError` / `AuthError` / `SubmitError` / `NetworkError` (`src/errors.ts`). `NetworkError` messages -are built only from the request's host and HTTP status — never from -response headers or body — so a failed request can never echo a bearer -token or bundle content into a printed error. EOF on any interactive +are built from the request's host, HTTP status, and a closed failure-class +phrase taken from `error.code` (or HTTP 407) — never from response +headers, body, or `error.message` — so a failed request can never echo a +bearer token or bundle content into a printed error. Connect failures +that used to collapse into `Could not reach .` now name +`connection refused`, `could not verify TLS certificate` (see +[corporate-networks.md](corporate-networks.md)), or `proxy required` +when the code is one of those classes. EOF on any interactive prompt (attestation, author selection, or `submit`'s upload confirmation) aborts with a non-zero exit code rather than hanging or silently proceeding, consistent with `scan`'s existing prompts. diff --git a/docs/privacy-tests.md b/docs/privacy-tests.md index 5a825b2..02e40c1 100644 --- a/docs/privacy-tests.md +++ b/docs/privacy-tests.md @@ -99,10 +99,12 @@ is the real, network-backed check described above — an anonymous HTTP `login`/`submit` are the first commands with anything worth leaking through an error message — a bearer token, or the full bundle. `src/http-client.ts` -builds every `NetworkError` from the request's host and HTTP status only, -never from response headers or body. +builds every `NetworkError` from the request's host, HTTP status, and a +closed failure-class phrase from `error.code` — never from response +headers, body, or `error.message`. | Test | Proves | |---|---| | `test/privacy/submit-guardrail.test.ts` → "a failed upload's error message names the host and status, never the token or bundle" | A `500` from the submit endpoint produces a `NetworkError` whose message contains the status code but neither the stored access token nor any bundle field (`schema_version` as a proxy for "the whole bundle got interpolated in"). | +| `test/http-client.test.ts` → reach-error cases | Connect failures interpolate only the host plus a closed class phrase. A planted token in `error.message` or a `407` body never appears in `NetworkError.message`. | | `test/login.test.ts` (all cases, implicit) | `login`'s errors (`AuthError` for denied/expired/timed-out) are static, fixed strings — never built from the device code or any server response field, so there's no path for the code to end up in an error either. | diff --git a/src/errors.ts b/src/errors.ts index 027e79f..e0a9e4b 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -14,6 +14,7 @@ export class SubmitError extends Error {} /** A request to SITE_URL (or a remote host, for the visibility check) * couldn't complete or came back with a non-2xx status. Message is built - * from the request's host and status only — never headers or body — so it + * from the request's host, HTTP status, and a closed failure-class phrase + * from `error.code` — never headers, body, or `error.message` — so it * can never echo a bearer token or bundle content. */ export class NetworkError extends Error {} diff --git a/src/http-client.ts b/src/http-client.ts index bacc6b8..b0eec16 100644 --- a/src/http-client.ts +++ b/src/http-client.ts @@ -29,11 +29,65 @@ function cliFetch(url: string, init: RequestInit): Promise { return fetch(url, { ...init, dispatcher: proxyDispatcher() } as RequestInit); } +const TLS_CODES = new Set([ + "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + "UNABLE_TO_GET_ISSUER_CERT", + "UNABLE_TO_GET_ISSUER_CERT_LOCALLY", + "CERT_UNTRUSTED", + "CERT_HAS_EXPIRED", + "CERT_NOT_YET_VALID", + "CERT_REVOKED", + "DEPTH_ZERO_SELF_SIGNED_CERT", + "SELF_SIGNED_CERT_IN_CHAIN", + "ERR_TLS_CERT_ALTNAME_INVALID", + "ERR_SSL_WRONG_VERSION_NUMBER", +]); + +/** Walk `code` / `cause` / AggregateError `errors` only — never `message`. */ +function collectCodes(err: unknown, seen: Set = new Set()): string[] { + if (!err || typeof err !== "object" || seen.has(err)) return []; + seen.add(err); + const rec = err as { code?: unknown; cause?: unknown; errors?: unknown }; + const codes: string[] = []; + if (typeof rec.code === "string") codes.push(rec.code); + if (rec.cause) codes.push(...collectCodes(rec.cause, seen)); + if (Array.isArray(rec.errors)) { + for (const inner of rec.errors) codes.push(...collectCodes(inner, seen)); + } + return codes; +} + +/** + * Closed failure-class phrases from `error.code` (and 407). Host is the + * only interpolated value — never `error.message`, headers, or body. + */ +function reachErrorMessage(host: string, err: unknown): string { + const codes = collectCodes(err); + if (codes.some((c) => TLS_CODES.has(c))) { + return `Could not reach ${host}: could not verify TLS certificate (corporate proxy? see docs/corporate-networks.md).`; + } + if (codes.some((c) => c === "UND_ERR_PROXY")) { + return `Could not reach ${host}: proxy required.`; + } + if (codes.some((c) => c === "ECONNREFUSED")) { + return `Could not reach ${host}: connection refused.`; + } + return `Could not reach ${host}.`; +} + +function statusErrorMessage(host: string, status: number): string { + if (status === 407) { + return `Could not reach ${host}: proxy required.`; + } + return `Request to ${host} failed with status ${status}.`; +} + /** * The only module allowed to call `fetch` for JSON requests (login.ts and * submit.ts are the other two — see test/privacy/zero-network.test.ts's - * allowlist). Error messages are built from the URL's host and the HTTP - * status only, never from response headers or body, so a failure can never + * allowlist). Error messages are built from the URL's host, HTTP status, + * and a closed failure-class phrase from `error.code` — never from + * response headers, body, or `error.message`, so a failure can never * echo a bearer token or bundle content back into a printed error. */ export async function postJson( @@ -49,11 +103,11 @@ export async function postJson( headers: { "content-type": "application/json", ...headers }, body: JSON.stringify(body), }); - } catch { - throw new NetworkError(`Could not reach ${host}.`); + } catch (err) { + throw new NetworkError(reachErrorMessage(host, err)); } if (!res.ok) { - throw new NetworkError(`Request to ${host} failed with status ${res.status}.`); + throw new NetworkError(statusErrorMessage(host, res.status)); } try { return (await res.json()) as T; @@ -80,11 +134,11 @@ export async function postRawJson( headers: { "content-type": "application/json", ...headers }, body: rawBody, }); - } catch { - throw new NetworkError(`Could not reach ${host}.`); + } catch (err) { + throw new NetworkError(reachErrorMessage(host, err)); } if (!res.ok) { - throw new NetworkError(`Request to ${host} failed with status ${res.status}.`); + throw new NetworkError(statusErrorMessage(host, res.status)); } try { return (await res.json()) as T; @@ -100,8 +154,8 @@ export async function postRawJson( * polling through — as HTTP 400, reserving 200 for `{access_token}` success * (see docs/login-submit.md). Treats 200 and 400 alike as "parse the body"; * any other status is still a real failure. Same error-message discipline - * as postJson: built from the host and status only, never from response - * headers or body, so a failure here can never echo a bearer token. + * as postJson: host, status, and a closed failure-class phrase — never + * response headers, body, or `error.message`. */ export async function pollJson(url: string, body: unknown): Promise { const host = new URL(url).host; @@ -112,11 +166,11 @@ export async function pollJson(url: string, body: unknown): Promise { headers: { "content-type": "application/json" }, body: JSON.stringify(body), }); - } catch { - throw new NetworkError(`Could not reach ${host}.`); + } catch (err) { + throw new NetworkError(reachErrorMessage(host, err)); } if (!res.ok && res.status !== 400) { - throw new NetworkError(`Request to ${host} failed with status ${res.status}.`); + throw new NetworkError(statusErrorMessage(host, res.status)); } try { return (await res.json()) as T; @@ -151,8 +205,8 @@ export async function postJsonStatusOnly( body: JSON.stringify(body), }); return res.status; - } catch { - throw new NetworkError(`Could not reach ${host}.`); + } catch (err) { + throw new NetworkError(reachErrorMessage(host, err)); } } diff --git a/test/http-client.test.ts b/test/http-client.test.ts index 1e6e3af..df4cab8 100644 --- a/test/http-client.test.ts +++ b/test/http-client.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { postJson } from "../src/http-client.js"; +import { NetworkError } from "../src/errors.js"; const originalFetch = globalThis.fetch; const proxyKeys = ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy", "NO_PROXY", "no_proxy"] as const; @@ -74,3 +75,66 @@ describe("http-client proxy dispatcher (#83 slice 1)", () => { expect(second).toBe(first); }); }); + +describe("http-client reach errors (#83 slice 2)", () => { + it("names connection refused from error.code, never error.message", async () => { + const leak = "Bearer extremely-secret-token"; + globalThis.fetch = vi.fn(async () => { + throw Object.assign(new Error(`connect failed ${leak}`), { code: "ECONNREFUSED" }); + }) as unknown as typeof fetch; + + await expect(postJson("https://example.test/api", {})).rejects.toSatisfy((err: unknown) => { + expect(err).toBeInstanceOf(NetworkError); + const message = (err as Error).message; + expect(message).toBe("Could not reach example.test: connection refused."); + expect(message).not.toContain(leak); + return true; + }); + }); + + it("names a TLS failure from cause.code", async () => { + const leak = "https://evil.test/callback?token=abc"; + globalThis.fetch = vi.fn(async () => { + const cause = Object.assign(new Error(`unable to verify ${leak}`), { + code: "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + }); + throw Object.assign(new TypeError("fetch failed"), { cause }); + }) as unknown as typeof fetch; + + await expect(postJson("https://example.test/api", {})).rejects.toSatisfy((err: unknown) => { + expect(err).toBeInstanceOf(NetworkError); + const message = (err as Error).message; + expect(message).toContain("could not verify TLS certificate"); + expect(message).toContain("docs/corporate-networks.md"); + expect(message).not.toContain(leak); + return true; + }); + }); + + it("names proxy required on HTTP 407", async () => { + globalThis.fetch = vi.fn( + async () => new Response("Proxy-Authenticate: Basic", { status: 407 }) + ) as unknown as typeof fetch; + + await expect(postJson("https://example.test/api", {})).rejects.toSatisfy((err: unknown) => { + expect(err).toBeInstanceOf(NetworkError); + const message = (err as Error).message; + expect(message).toBe("Could not reach example.test: proxy required."); + expect(message).not.toContain("Proxy-Authenticate"); + expect(message).not.toContain("Basic"); + return true; + }); + }); + + it("keeps the generic reach message when the code is unknown", async () => { + globalThis.fetch = vi.fn(async () => { + throw Object.assign(new Error("socket hang up with a token=xyz"), { code: "ECONNRESET" }); + }) as unknown as typeof fetch; + + await expect(postJson("https://example.test/api", {})).rejects.toSatisfy((err: unknown) => { + expect((err as Error).message).toBe("Could not reach example.test."); + expect((err as Error).message).not.toContain("token=xyz"); + return true; + }); + }); +}); From ee7986c400962f33f8147122da126e687e5dc31c Mon Sep 17 00:00:00 2001 From: rudi193-cmd Date: Fri, 14 Aug 2026 21:39:39 -0600 Subject: [PATCH 4/4] fix(http): classify undici 6.28.0 proxy codes, not UND_ERR_PROXY CONNECT 407 is UND_ERR_ABORTED; TLS through the proxy is UND_ERR_PRX_TLS. Docs: NO_PROXY does not attach the agent. Co-authored-by: Cursor --- docs/corporate-networks.md | 23 ++++++++++++++--------- src/http-client.ts | 3 ++- test/http-client.test.ts | 29 +++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/docs/corporate-networks.md b/docs/corporate-networks.md index 44e159a..3a4865f 100644 --- a/docs/corporate-networks.md +++ b/docs/corporate-networks.md @@ -12,12 +12,14 @@ Node's error text — those can contain tokens. ## Proxy env vars Node's built-in `fetch` ignores `HTTP_PROXY` / `HTTPS_PROXY`. This CLI -attaches undici's `EnvHttpProxyAgent` when any of these are set: +attaches undici's `EnvHttpProxyAgent` only when one of these is set: - `HTTP_PROXY` / `http_proxy` - `HTTPS_PROXY` / `https_proxy` -- `NO_PROXY` / `no_proxy` (honored by the agent: hosts that must bypass - the proxy, typically `localhost` and your internal git host) + +`NO_PROXY` / `no_proxy` do **not** attach the agent. They are read by +the agent after it exists: hosts that must bypass the proxy, typically +`localhost` and your internal git host. Example: @@ -27,13 +29,16 @@ export NO_PROXY=localhost,127.0.0.1,.corp.example npx redential login ``` -Leave them unset on a direct network — the client then uses the same -dispatcher-less `fetch` as a machine with no proxy. +Leave the HTTP(S)_PROXY vars unset on a direct network — the client +then uses the same dispatcher-less `fetch` as a machine with no proxy. -A `407` from the proxy (or undici's `UND_ERR_PROXY`) prints `proxy -required`. That usually means the env var is missing, the URL is wrong, -or the proxy wants authentication the CLI does not prompt for — set the -vars your IT docs specify; do not paste a password into an issue. +A `407` HTTP response, or a CONNECT tunnel that is not 200 (undici +surfaces that as `UND_ERR_ABORTED`), prints `proxy required`. That +usually means the env var is missing, the URL is wrong, or the proxy +wants authentication the CLI does not prompt for — set the vars your +IT docs specify; do not paste a password into an issue. TLS through +the proxy failing (`UND_ERR_PRX_TLS`) uses the certificate message +below, not this one. ## Corporate CA (`NODE_EXTRA_CA_CERTS`) diff --git a/src/http-client.ts b/src/http-client.ts index b0eec16..1835693 100644 --- a/src/http-client.ts +++ b/src/http-client.ts @@ -41,6 +41,7 @@ const TLS_CODES = new Set([ "SELF_SIGNED_CERT_IN_CHAIN", "ERR_TLS_CERT_ALTNAME_INVALID", "ERR_SSL_WRONG_VERSION_NUMBER", + "UND_ERR_PRX_TLS", ]); /** Walk `code` / `cause` / AggregateError `errors` only — never `message`. */ @@ -66,7 +67,7 @@ function reachErrorMessage(host: string, err: unknown): string { if (codes.some((c) => TLS_CODES.has(c))) { return `Could not reach ${host}: could not verify TLS certificate (corporate proxy? see docs/corporate-networks.md).`; } - if (codes.some((c) => c === "UND_ERR_PROXY")) { + if (codes.some((c) => c === "UND_ERR_ABORTED")) { return `Could not reach ${host}: proxy required.`; } if (codes.some((c) => c === "ECONNREFUSED")) { diff --git a/test/http-client.test.ts b/test/http-client.test.ts index df4cab8..c740ed0 100644 --- a/test/http-client.test.ts +++ b/test/http-client.test.ts @@ -126,6 +126,35 @@ describe("http-client reach errors (#83 slice 2)", () => { }); }); + it("names proxy required when CONNECT is not 200 (UND_ERR_ABORTED)", async () => { + const leak = "Proxy response (407) !== 200 when HTTP Tunneling"; + globalThis.fetch = vi.fn(async () => { + throw Object.assign(new Error(leak), { code: "UND_ERR_ABORTED" }); + }) as unknown as typeof fetch; + + await expect(postJson("https://example.test/api", {})).rejects.toSatisfy((err: unknown) => { + expect(err).toBeInstanceOf(NetworkError); + const message = (err as Error).message; + expect(message).toBe("Could not reach example.test: proxy required."); + expect(message).not.toContain("407"); + expect(message).not.toContain("Tunneling"); + return true; + }); + }); + + it("names a TLS failure for UND_ERR_PRX_TLS", async () => { + globalThis.fetch = vi.fn(async () => { + throw Object.assign(new Error("tls connection to a proxy failed"), { code: "UND_ERR_PRX_TLS" }); + }) as unknown as typeof fetch; + + await expect(postJson("https://example.test/api", {})).rejects.toSatisfy((err: unknown) => { + expect(err).toBeInstanceOf(NetworkError); + expect((err as Error).message).toContain("could not verify TLS certificate"); + expect((err as Error).message).not.toContain("proxy failed"); + return true; + }); + }); + it("keeps the generic reach message when the code is unknown", async () => { globalThis.fetch = vi.fn(async () => { throw Object.assign(new Error("socket hang up with a token=xyz"), { code: "ECONNRESET" });