From 5c8994c55e79c872110ab3b2da82a3790b91994f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 09:34:11 +0000 Subject: [PATCH] fix(proxy): say when Node WILL use the proxy, and name the opt-in (PHARN-12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The proxy notice said "pharn will not use it … Node's fetch reads no proxy environment variable on any platform". Measured on Node 22.22.2: with NODE_USE_ENV_PROXY=1 (or --use-env-proxy) fetch does route through HTTPS_PROXY — so the message was false for users who opted in and hid the one workaround from users behind a proxy. Node 20.20.2 has no such option. detectProxyNotice now reports one of three states, using the runtime's own list of accepted flags (process.allowedNodeEnvironmentFlags) rather than a version table: `on` (downloads go through the proxy), `available` (set NODE_USE_ENV_PROXY=1), `unsupported` (this Node cannot). Flags are matched as whole tokens. docs/troubleshooting.md documents the opt-in. LIMITS.md §3a / THREAT-MODEL.md carry the old claim but are human-only. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01TvcuVhk8hTeDskp5pAJhnc --- .dev/features/proxy-env-opt-in/GRILL.md | 23 ++++++ .dev/features/proxy-env-opt-in/PLAN.md | 58 +++++++++++++++ .dev/features/proxy-env-opt-in/REGRESSION.md | 30 ++++++++ .dev/features/proxy-env-opt-in/REVIEW.md | 38 ++++++++++ .dev/features/proxy-env-opt-in/SHIP.md | 17 +++++ .dev/features/proxy-env-opt-in/VERIFY.md | 27 +++++++ .../proxy-env-opt-in/regression-report.json | 23 ++++++ .../proxy-env-opt-in/verify-report.json | 17 +++++ .pharn/writes-scope.json | 4 +- docs/troubleshooting.md | 72 +++++++++++-------- src/lib/proxy-env-format.ts | 22 +++--- src/lib/proxy-env.ts | 53 +++++++++++++- tests/proxy-env-format.test.ts | 63 +++++++++++++--- tests/proxy-env.test.ts | 57 ++++++++++++++- 14 files changed, 452 insertions(+), 52 deletions(-) create mode 100644 .dev/features/proxy-env-opt-in/GRILL.md create mode 100644 .dev/features/proxy-env-opt-in/PLAN.md create mode 100644 .dev/features/proxy-env-opt-in/REGRESSION.md create mode 100644 .dev/features/proxy-env-opt-in/REVIEW.md create mode 100644 .dev/features/proxy-env-opt-in/SHIP.md create mode 100644 .dev/features/proxy-env-opt-in/VERIFY.md create mode 100644 .dev/features/proxy-env-opt-in/regression-report.json create mode 100644 .dev/features/proxy-env-opt-in/verify-report.json diff --git a/.dev/features/proxy-env-opt-in/GRILL.md b/.dev/features/proxy-env-opt-in/GRILL.md new file mode 100644 index 00000000..732e493b --- /dev/null +++ b/.dev/features/proxy-env-opt-in/GRILL.md @@ -0,0 +1,23 @@ +# GRILL — proxy-env-opt-in + +Plan: `.dev/features/proxy-env-opt-in/PLAN.md` · spec-hash `bca940a5…d729d3c4e` matches live +`ARCHITECTURE.md`. Registered grillers: `{"registered":0,"grillers":[]}` → inline axes only. + +## Findings + +```yaml +- type: FINDING + rule_id: 'P5' + severity: important + file: '.dev/features/proxy-env-opt-in/PLAN.md:10' + problem: 'NODE_OPTIONS is a free-form string; match the flag as a whole whitespace-separated token (optionally `=…`), not a substring, or `--use-env-proxy-foo` would count.' + evidence: '`--use-env-proxy` in `process.execArgv` or `NODE_OPTIONS`' +- type: FINDING + rule_id: 'P1' + severity: minor + file: '.dev/features/proxy-env-opt-in/PLAN.md:31' + problem: 'Existing tests that compare the notice with toEqual({ name, value }) will break on the new field; they must be updated deliberately, not loosened.' + evidence: '`ProxyNotice` gains `envProxy`' +``` + +ADVISORY VERDICT: 2 concerns raised (0 blocking-severity, 2 advisory) — for the human to weigh before /pharn-dev-build. diff --git a/.dev/features/proxy-env-opt-in/PLAN.md b/.dev/features/proxy-env-opt-in/PLAN.md new file mode 100644 index 00000000..f1362ad9 --- /dev/null +++ b/.dev/features/proxy-env-opt-in/PLAN.md @@ -0,0 +1,58 @@ +# PLAN — proxy-env-opt-in (PHARN-12: the proxy notice must not say "pharn will not use it" when Node will) + +- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4 +- increment: `detectProxyNotice` also reports whether THIS Node supports env-proxy + (`process.allowedNodeEnvironmentFlags.has('--use-env-proxy')` — a runtime membership test) and whether + it is turned on (`NODE_USE_ENV_PROXY=1`, `--use-env-proxy` in `process.execArgv` or `NODE_OPTIONS`); + the message then says one of three true things: the proxy IS used; it is not used but + `NODE_USE_ENV_PROXY=1` would route pharn through it; or this Node cannot use it at all. +- layer(s): the CLI itself (`src/lib/proxy-env.ts`, `src/lib/proxy-env-format.ts`) +- constitution_refs: [P1, P4, P5] + +## Discovery — verified this run (P6) + +Probed here: Node 22.22.2 with `HTTPS_PROXY=http://127.0.0.1:9` and `NODE_USE_ENV_PROXY=1` (or +`--use-env-proxy`) → `fetch failed` (it dialled the dead proxy); without → direct. Node 20.20.2 ignores the +variable and rejects the flag (`bad option`). `process.allowedNodeEnvironmentFlags.has('--use-env-proxy')` +is `true` on 22.22.2 and `false` on 20.20.2. Current message (`proxy-env-format.ts`): "…but pharn will not +use it: its network calls go through Node's global fetch, which reads no proxy environment variable on +any platform" — false with the opt-in on, and it hides the one workaround from users behind a proxy. +`LIMITS.md` §3a / `THREAT-MODEL.md` carry the same claim but are human-only (hook-protected) — flagged for +a maintainer, not edited here. + +## Files + +- `src/lib/proxy-env.ts` — `ProxyNotice` gains `envProxy: 'on' | 'available' | 'unsupported'`, computed from + injectable inputs (env, execArgv, the allowed-flags set) so it is testable without spawning Node — layer CLI/lib +- `src/lib/proxy-env-format.ts` — one message per state; `on` names that pharn's downloads go through the + proxy (Node also honours `NO_PROXY` then) — layer CLI/lib +- `tests/proxy-env.test.ts` — the detection matrix: env var `1`/other, flag in execArgv, flag in + NODE_OPTIONS, unsupported Node ignores both +- `tests/proxy-env-format.test.ts` — each state's message; redaction unchanged +- `docs/troubleshooting.md` — "Proxy environment variables": the opt-in and which Node supports it (P4) + +## Contracts satisfied + +- P4 (docs cite code): the user-facing claim now matches what Node actually does. + +## Evals to write (P1) + +- listed above; the `on` and `available` messages fail on the base source. + +## Guarantee audit (P0) + +- "the notice says whether Node will use the proxy" → floor: membership of `--use-env-proxy` in + `process.allowedNodeEnvironmentFlags` + exact env/argv string checks. Whether a given proxy actually + WORKS is not claimed (advisory). + +## Trust audit (P2) + +- The proxy value is still rendered only through `redactProxyUrl`. + +## Determinism audit (P5) + +- Membership tests only. + +## Open questions (HALT) + +- none diff --git a/.dev/features/proxy-env-opt-in/REGRESSION.md b/.dev/features/proxy-env-opt-in/REGRESSION.md new file mode 100644 index 00000000..740f098a --- /dev/null +++ b/.dev/features/proxy-env-opt-in/REGRESSION.md @@ -0,0 +1,30 @@ +# REGRESSION — proxy-env-opt-in + +The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment. + +## Base and partition + +- **base:** `8d1190624dae01810592cc1fcb716984bbd429c8` (`origin/main` at build time; the build is an uncommitted working tree on top of it). +- **inside** (each declared in `PLAN.md` `## Files`): `docs/troubleshooting.md`, `src/lib/proxy-env-format.ts`, `src/lib/proxy-env.ts`, `tests/proxy-env-format.test.ts`, `tests/proxy-env.test.ts`. +- **scope partition:** `check-regress.mjs scope` exited **0**, `escaped: []`. `.pharn/` (hook scratch) and this + feature's own stage artifacts are not build output. +- **outside gates:** the stdlib `*.test.mjs` / `*.test.cjs` files + whole-repo `validate`; 0 committed eval pairs. +- **style-gate skip:** `inside` touches no shared style config, so `lint` / `format:check` / `lint:md` are absent from both maps. + +## Per-gate exit codes + +| gate | base | head | flipped? | +| ---------- | ---- | ---- | -------- | +| `tests` | 0 | 0 | no | +| `validate` | 0 | 0 | no | + +- `regressions[]`: **empty** +- `pre_existing[]`: **empty** + +## Verdict + +**REGRESSIONS: none — no deterministically-detectable breakage outside the feature.** +(`regression-report.json` `.verdict` = `no-regressions`.) + +Residual (P0/P7): this catches exactly what its suite catches. The vitest suite exercising `src/**` is +owned by `/pharn-dev-build`'s floor and `/pharn-dev-verify`. This certifies the comparison, never the increment. diff --git a/.dev/features/proxy-env-opt-in/REVIEW.md b/.dev/features/proxy-env-opt-in/REVIEW.md new file mode 100644 index 00000000..bbacecdf --- /dev/null +++ b/.dev/features/proxy-env-opt-in/REVIEW.md @@ -0,0 +1,38 @@ +# REVIEW — proxy-env-opt-in + +Floor first: `node .dev/floor/validate.mjs .` → exit 0 (GREEN). Everything below is **advisory**. + +## Floor-gate findings (blocking) + +None. + +- **L-floor (P0):** the three states reduce to membership of `--use-env-proxy` in + `process.allowedNodeEnvironmentFlags` and exact token/string checks on `NODE_USE_ENV_PROXY`, + `execArgv` and `NODE_OPTIONS` (whole tokens — grill #1). Whether a given proxy works is not claimed. +- **L-eval (P1):** 10 new cases (detection matrix incl. the substring trap; three messages) fail on the + base source; the pre-existing tests were updated deliberately — the "any platform" assertion became + "by default", which is what is now true. Measured on this host: Node 22.22.2 honours the opt-in; + Node 20.20.2 does not and rejects the flag. +- **L-trust (P2):** the proxy value is still rendered only through `redactProxyUrl`. +- **L-axis (P3):** detection in `proxy-env.ts`, wording in `proxy-env-format.ts` — the existing split. + +## Advisory findings + +```yaml +- type: FINDING + rule_id: 'P4' + severity: important + file: 'LIMITS.md' + problem: 'LIMITS.md §3a and THREAT-MODEL.md still state pharn cannot use a proxy at all; they are human-only (write-protected at the floor), so a maintainer must align them with the opt-in.' + evidence: 'The download connects DIRECTLY, and fails if direct egress is blocked (LIMITS.md §3a).' +- type: FINDING + rule_id: 'P4' + severity: minor + file: 'CHANGELOG.md:8' + problem: "No CHANGELOG `[Unreleased]` entry (not in the plan's `## Files`)." + evidence: '## [Unreleased]' +``` + +## Verdict + +**GREEN** — 0 floor-gate findings, 2 advisory findings. No lesson proposed for canon. diff --git a/.dev/features/proxy-env-opt-in/SHIP.md b/.dev/features/proxy-env-opt-in/SHIP.md new file mode 100644 index 00000000..2c90e650 --- /dev/null +++ b/.dev/features/proxy-env-opt-in/SHIP.md @@ -0,0 +1,17 @@ +# SHIP — proxy-env-opt-in + +Stages run, in order: `/pharn-dev-plan` → GATE 1 (human: **Approve as written**) → `/pharn-dev-grill` → +`/pharn-dev-build` → `/pharn-dev-regress` → `/pharn-dev-verify` → `/pharn-dev-review` → GATE 2. + +| stage | structural verdict (verbatim) | +| -------------------- | ------------------------------------------------------ | +| `/pharn-dev-build` | `node .dev/floor/validate.mjs .` exit `0` | +| `/pharn-dev-regress` | `regression-report.json` `.verdict` = `no-regressions` | +| `/pharn-dev-verify` | `verify-report.json` `.verdict` = `PASS` | + +- Review: [`REVIEW.md`](REVIEW.md) · Grill (advisory): [`GRILL.md`](GRILL.md) +- Run ended at **GATE 2**. The human's standing instruction for this batch: open a PR and merge it only if + its CI checks are green. + +chain ran; the named floor verdicts are as shown — this is NOT a judgment that the increment is good or +wise; that is the human's call at the post-review gate. diff --git a/.dev/features/proxy-env-opt-in/VERIFY.md b/.dev/features/proxy-env-opt-in/VERIFY.md new file mode 100644 index 00000000..fac1e681 --- /dev/null +++ b/.dev/features/proxy-env-opt-in/VERIFY.md @@ -0,0 +1,27 @@ +# VERIFY — proxy-env-opt-in + +## FLOOR layer (owns the verdict) + +Gates run over the whole repo with the feature present, as a non-root user on node 22 with the session +proxy variables unset (as root with the proxy set, 5 pre-existing tests in `init.test.ts` / +`update.test.ts` fail for environmental reasons, identically at the baseline). + +| gate | exit | +| -------------- | ---- | +| `format:check` | 0 | +| `lint` | 0 | +| `lint:md` | 0 | +| `test` | 0 | +| `typecheck` | 0 | +| `validate` | 0 | + +No `structural:*` gate — the increment ships no eval-actual pair. + +**VERDICT: PASS** (`.dev/floor/check-verify.mjs`, `failing_gates: []`). + +## ADVISORY layer + +`node .dev/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` — floor gates only. + +Residual (P0/P7): "verified" means the named gates passed — not that the feature is correct in any sense +the suite does not encode. diff --git a/.dev/features/proxy-env-opt-in/regression-report.json b/.dev/features/proxy-env-opt-in/regression-report.json new file mode 100644 index 00000000..821b4394 --- /dev/null +++ b/.dev/features/proxy-env-opt-in/regression-report.json @@ -0,0 +1,23 @@ +{ + "base": "8d1190624dae01810592cc1fcb716984bbd429c8", + "inside": [ + "docs/troubleshooting.md", + "src/lib/proxy-env-format.ts", + "src/lib/proxy-env.ts", + "tests/proxy-env-format.test.ts", + "tests/proxy-env.test.ts" + ], + "outside_gates": { + "tests": { + "base": 0, + "head": 0 + }, + "validate": { + "base": 0, + "head": 0 + } + }, + "regressions": [], + "pre_existing": [], + "verdict": "no-regressions" +} diff --git a/.dev/features/proxy-env-opt-in/verify-report.json b/.dev/features/proxy-env-opt-in/verify-report.json new file mode 100644 index 00000000..d1169e4f --- /dev/null +++ b/.dev/features/proxy-env-opt-in/verify-report.json @@ -0,0 +1,17 @@ +{ + "feature": "proxy-env-opt-in", + "gates": { + "format:check": 0, + "lint": 0, + "lint:md": 0, + "test": 0, + "typecheck": 0, + "validate": 0 + }, + "verdict": "PASS", + "failing_gates": [], + "verifiers": { + "registered": 0, + "findings": [] + } +} diff --git a/.pharn/writes-scope.json b/.pharn/writes-scope.json index 9a69ffe1..3bd882aa 100644 --- a/.pharn/writes-scope.json +++ b/.pharn/writes-scope.json @@ -1,7 +1,7 @@ { "scope": [ - ".dev/features/reinit-preserve-edits/SHIP.md" + ".dev/features/proxy-env-opt-in/SHIP.md" ], "set_by": ".claude/commands/pharn-dev-ship.md", - "set_at": "2026-09-24T09:28:29.312Z" + "set_at": "2026-09-24T09:34:10.397Z" } diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index b1819716..02acb185 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -156,7 +156,7 @@ Symptoms: - Message references `github.com/pharn-dev/pharn-oss` - Exit code 1 -`init` / `add` / `update` / `status` download `pharn-dev/pharn-oss` as a tarball from `codeload.github.com` (after resolving the branch head via `api.github.com`) — default `status` clones too, and reads `SKILLS_VERSION` out of that clone. `update` and `status --no-drift` instead fetch the root `SKILLS_VERSION` from `raw.githubusercontent.com` without cloning. Check network access to all three hosts and that the repo is reachable. Note that `pharn` does not use a proxy — see [Proxy environment variables](#proxy-environment-variables). +`init` / `add` / `update` / `status` download `pharn-dev/pharn-oss` as a tarball from `codeload.github.com` (after resolving the branch head via `api.github.com`) — default `status` clones too, and reads `SKILLS_VERSION` out of that clone. `update` and `status --no-drift` instead fetch the root `SKILLS_VERSION` from `raw.githubusercontent.com` without cloning. Check network access to all three hosts and that the repo is reachable. Note that `pharn` does not use a proxy unless you opt in with `NODE_USE_ENV_PROXY=1` — see [Proxy environment variables](#proxy-environment-variables). A failure to reach the host names it, and includes the underlying diagnosis rather than the runtime's bare `fetch failed`: @@ -197,7 +197,7 @@ change. If you no longer want the capability, `pharn remove `. Symptoms: - `⚠ This pharn is too old for the current github.com/pharn-dev/pharn-oss: it requires - @pharn-dev/pharn vX.Y.Z or newer …` +@pharn-dev/pharn vX.Y.Z or newer …` - Exit code 1, nothing written, temporary clone cleaned up pharn-oss ships an optional root `MIN_CLI` file declaring the minimum CLI version its content needs. @@ -242,21 +242,32 @@ Nothing is written outside your project either: the repo fetch runs first, but i ## Proxy environment variables -**`pharn` does not use an HTTP proxy.** Every network call it makes — the commit-SHA resolve, the repo -tarball, and `SKILLS_VERSION` for `update` / `status --no-drift` — goes through Node's global `fetch`, -which reads **no** proxy environment variable: not `https_proxy`, not `HTTPS_PROXY`, not `no_proxy`, -on any platform. There is no spelling that works and no flag that changes it. +**By default, `pharn` does not use an HTTP proxy.** Every network call it makes — the commit-SHA +resolve, the repo tarball, and `SKILLS_VERSION` for `update` / `status --no-drift` — goes through +Node's global `fetch`, which by default reads **no** proxy environment variable: not `https_proxy`, not +`HTTPS_PROXY`, not `no_proxy`. -If a proxy variable is set, `pharn` says so **before** it fetches, so a network that blocks direct -egress produces an explanation rather than an unexplained timeout: +**Recent Node versions can opt in.** Run `pharn` with `NODE_USE_ENV_PROXY=1` set (or pass +`--use-env-proxy` to Node, e.g. through `NODE_OPTIONS`) and Node's `fetch` routes through `HTTPS_PROXY` +and honours `NO_PROXY`. Older Nodes — Node 20, for example — have no such option. `pharn` checks what +the running Node supports (`process.allowedNodeEnvironmentFlags`) rather than guessing from a version +number. + +If a proxy variable is set, `pharn` says which of the three cases applies **before** it fetches, so a +network that blocks direct egress produces an explanation rather than an unexplained timeout. Without +the opt-in, on a Node that supports it: ```text ⚠ HTTPS_PROXY is set (http://***@proxy.internal:3128), but pharn will not use it: its network calls go through Node's global fetch, which reads no proxy - environment variable on any platform. The download connects DIRECTLY, and - fails if direct egress is blocked (LIMITS.md §3a). + environment variable by default. The download connects DIRECTLY, and fails + if direct egress is blocked (LIMITS.md §3a). This Node can route fetch through + the proxy — re-run with NODE_USE_ENV_PROXY=1 set. ``` +With the opt-in on, the notice instead says the downloads go through that proxy. On a Node without +the option it says so and that a newer Node release has it. + Two details in that message are deliberate: - **It names the variable you actually set**, so a `Https_Proxy` typo shows up as read-and-still-unused @@ -265,13 +276,14 @@ Two details in that message are deliberate: only printed; it is never written to `pharn.config.json`, which lives in your repository and is committed. -If you are behind a mandatory proxy, there is no workaround inside `pharn` today — this is a named -limit (`LIMITS.md` §3a), not a bug to report. +If you are behind a mandatory proxy, set `NODE_USE_ENV_PROXY=1` on a Node that supports it (above). On a +Node without it there is no workaround inside `pharn` — a named limit (`LIMITS.md` §3a), not a bug to +report. ### This changed in the codeload release Earlier versions cloned through `degit`, which read `process.env.https_proxy` **itself**. If you had -set exactly that lowercase spelling, your clone *was* proxied, and it no longer is. Your +set exactly that lowercase spelling, your clone _was_ proxied, and it no longer is. Your `pharn update` and `status --no-drift` were already unproxied — those were always plain `fetch` — so this makes one boundary consistent rather than newly broken, but it does break a setup that worked. @@ -284,7 +296,7 @@ fetch — and **nothing ever reclaimed them**. That is worth stating plainly, because the directory can be much larger than "one leftover download" suggests. `degit` does have a deletion path, but it fires only when an existing **ref's** mapped hash -changes. `pharn` passed the resolved commit SHA *as* the ref, so every fetch wrote a self-mapped +changes. `pharn` passed the resolved commit SHA _as_ the ref, so every fetch wrote a self-mapped `"": ""` entry under a **new key** — the previous hash for that key was always `undefined`, so the delete branch could never run. One tarball accumulated per distinct upstream commit you ever fetched, forever. A CI machine running `pharn status` on each upstream push could reach hundreds of @@ -293,11 +305,11 @@ megabytes on an image nothing purges, and `pharn` printed nothing about it. Check what is there, then delete the whole directory — `pharn` will not do it for you, and nothing reads it any more: -| Platform | Path | -| -------- | ---- | -| macOS | `~/Library/Caches/degit` | -| Windows | `%LOCALAPPDATA%\degit` | -| Other | `$XDG_CACHE_HOME/degit`, else `~/.cache/degit` | +| Platform | Path | +| -------- | ---------------------------------------------- | +| macOS | `~/Library/Caches/degit` | +| Windows | `%LOCALAPPDATA%\degit` | +| Other | `$XDG_CACHE_HOME/degit`, else `~/.cache/degit` | ```bash du -sh ~/Library/Caches/degit # macOS @@ -464,18 +476,18 @@ Usage: ... Every option belongs to exactly one command, and `pharn` refuses the ones a command does not take: -| command | options it accepts | -| ---------------------- | ------------------------------------- | -| `init` | `--archetype` (a deprecated no-op) | -| `add` | none | -| `remove` / `rm` | none | -| `update` | `--force`, `--yes` / `-y` | -| `list` | `--json` | -| `status` | `--strict`, `--no-drift` | -| *any* | `--help` / `-h`, `--version` / `-v` | +| command | options it accepts | +| --------------- | ----------------------------------- | +| `init` | `--archetype` (a deprecated no-op) | +| `add` | none | +| `remove` / `rm` | none | +| `update` | `--force`, `--yes` / `-y` | +| `list` | `--json` | +| `status` | `--strict`, `--no-drift` | +| _any_ | `--help` / `-h`, `--version` / `-v` | The message goes to stderr with the usage text and exits **1**, exactly like `Unknown option` — the -label differs only because the option is one `pharn` *knows*, so "unknown" would send you hunting for +label differs only because the option is one `pharn` _knows_, so "unknown" would send you hunting for a typo that is not there. Before this, such an option was parsed and silently dropped. That was worst in CI: `pharn status @@ -483,7 +495,7 @@ Before this, such an option was parsed and silently dropped. That was worst in C `pharn list --strict` exited 0 no matter what it found. With no command word at all, `pharn --json`, `pharn --force` and `pharn --strict` each ran a **full `init`** while ignoring the option. -Note that only the option *name* is checked. `pharn list --json=false` is still accepted (`--json` is +Note that only the option _name_ is checked. `pharn list --json=false` is still accepted (`--json` is a `list` option) and still prints human-readable output — pass the bare `--json` for JSON. ## Local development issues diff --git a/src/lib/proxy-env-format.ts b/src/lib/proxy-env-format.ts index 91f22427..6647f5db 100644 --- a/src/lib/proxy-env-format.ts +++ b/src/lib/proxy-env-format.ts @@ -42,14 +42,20 @@ export function redactProxyUrl(value: string): string { } /** - * One message, because there is now one situation: a proxy is configured and - * pharn's network calls will not use it. - * - * It states the mechanism (Node's fetch reads no proxy environment variable) - * rather than only the symptom, so a user in a network that blocks direct egress - * can tell this apart from an outage — and it names the variable it found, so - * they can see pharn read the environment correctly and still cannot use it. + * The notice for a configured proxy, in one of three TRUE forms (PHARN-12). + * Node's fetch ignores proxy variables by default — which the old single + * message stated as absolute ("pharn will not use it") — but recent Node + * versions honour them when `NODE_USE_ENV_PROXY=1` / `--use-env-proxy` is set, + * so that message was false for users who had opted in and hid the one + * workaround from users who had not. */ export function proxyNoticeMessage(notice: ProxyNotice): string { - return `${notice.name} is set (${redactProxyUrl(notice.value)}), but pharn will not use it: its network calls go through Node's global fetch, which reads no proxy environment variable on any platform. The download connects DIRECTLY, and fails if direct egress is blocked (LIMITS.md §3a).`; + const shown = `${notice.name} is set (${redactProxyUrl(notice.value)})`; + if (notice.envProxy === 'on') { + return `${shown} and NODE_USE_ENV_PROXY / --use-env-proxy is on, so pharn's downloads go through that proxy (Node's fetch then also honours NO_PROXY).`; + } + const base = `${shown}, but pharn will not use it: its network calls go through Node's global fetch, which reads no proxy environment variable by default. The download connects DIRECTLY, and fails if direct egress is blocked (LIMITS.md §3a).`; + return notice.envProxy === 'available' + ? `${base} This Node can route fetch through the proxy — re-run with NODE_USE_ENV_PROXY=1 set.` + : `${base} This Node (${process.version}) has no NODE_USE_ENV_PROXY support; a newer Node release does.`; } diff --git a/src/lib/proxy-env.ts b/src/lib/proxy-env.ts index e13fb025..9d51081e 100644 --- a/src/lib/proxy-env.ts +++ b/src/lib/proxy-env.ts @@ -43,6 +43,54 @@ const UPPER = 'HTTPS_PROXY'; export interface ProxyNotice { name: string; value: string; + /** + * Whether Node's fetch will actually use the proxy (PHARN-12). Node's fetch + * reads no proxy variable BY DEFAULT, but recent Node versions route it + * through the proxy when `NODE_USE_ENV_PROXY=1` or `--use-env-proxy` is set: + * - `on` — supported and turned on: pharn's downloads use the proxy; + * - `available` — supported but off: setting `NODE_USE_ENV_PROXY=1` would; + * - `unsupported` — this Node has no such option (e.g. Node 20). + */ + envProxy: 'on' | 'available' | 'unsupported'; +} + +/** What the running Node supports and was started with — injectable for tests. */ +export interface ProxyRuntime { + /** Does this Node know `--use-env-proxy`? */ + supportsEnvProxy: boolean; + /** Node's own flags (`process.execArgv`). */ + execArgv: readonly string[]; +} + +const ENV_PROXY_FLAG = '--use-env-proxy'; + +/** The running process, read once per call. */ +export function currentProxyRuntime(): ProxyRuntime { + return { + // A membership test the runtime answers itself: Node lists every option it + // accepts, so there is no version table to keep up to date (P5). + supportsEnvProxy: process.allowedNodeEnvironmentFlags.has(ENV_PROXY_FLAG), + execArgv: process.execArgv, + }; +} + +/** `--use-env-proxy` as a whole token (bare or `=value`), never a substring. */ +function hasFlagToken(tokens: readonly string[]): boolean { + return tokens.some( + (t) => t === ENV_PROXY_FLAG || t.startsWith(`${ENV_PROXY_FLAG}=`), + ); +} + +function envProxyState( + env: Record, + runtime: ProxyRuntime, +): ProxyNotice['envProxy'] { + if (!runtime.supportsEnvProxy) return 'unsupported'; + const on = + env.NODE_USE_ENV_PROXY === '1' || + hasFlagToken(runtime.execArgv) || + hasFlagToken((env.NODE_OPTIONS ?? '').split(/\s+/)); + return on ? 'on' : 'available'; } /** Present AND non-empty. An empty string is not a proxy setting. */ @@ -81,9 +129,12 @@ function proxyVariantKeys(env: Record): string[] { */ export function detectProxyNotice( env: Record, + runtime: ProxyRuntime = currentProxyRuntime(), ): ProxyNotice | null { const name = proxyVariantKeys(env)[0]; if (name === undefined) return null; const value = env[name]; - return isSet(value) ? { name, value } : null; + return isSet(value) + ? { name, value, envProxy: envProxyState(env, runtime) } + : null; } diff --git a/tests/proxy-env-format.test.ts b/tests/proxy-env-format.test.ts index 52a6789e..a6cdbabd 100644 --- a/tests/proxy-env-format.test.ts +++ b/tests/proxy-env-format.test.ts @@ -69,16 +69,24 @@ describe('proxyNoticeMessage', () => { // dependency version to gate the claim on. it('names the variable it found, so the user can see pharn read it', () => { - expect(proxyNoticeMessage({ name: 'HtTpS_PrOxY', value: PROXY })).toContain( - 'HtTpS_PrOxY', - ); + expect( + proxyNoticeMessage({ + name: 'HtTpS_PrOxY', + value: PROXY, + envProxy: 'unsupported', + }), + ).toContain('HtTpS_PrOxY'); }); it('states the mechanism, not just the symptom', () => { // A user in a network that blocks direct egress otherwise sees only a // timeout. Naming fetch's behavior is what separates "known limitation" // from "outage". - const message = proxyNoticeMessage({ name: 'HTTPS_PROXY', value: PROXY }); + const message = proxyNoticeMessage({ + name: 'HTTPS_PROXY', + value: PROXY, + envProxy: 'unsupported', + }); expect(message).toContain('fetch'); expect(message).toContain('no proxy environment variable'); @@ -86,25 +94,62 @@ describe('proxyNoticeMessage', () => { }); it('makes no claim that depends on a platform or a dependency version', () => { - const message = proxyNoticeMessage({ name: 'HTTPS_PROXY', value: PROXY }); + const message = proxyNoticeMessage({ + name: 'HTTPS_PROXY', + value: PROXY, + envProxy: 'unsupported', + }); expect(message).not.toContain('degit'); - expect(message).toContain('any platform'); + // Honest since PHARN-12: fetch ignores proxy variables BY DEFAULT — not on + // "any platform" regardless of options. + expect(message).toContain('by default'); }); it('points at the documented limit rather than implying a bug', () => { - expect(proxyNoticeMessage({ name: 'HTTPS_PROXY', value: PROXY })).toContain( - 'LIMITS.md', - ); + expect( + proxyNoticeMessage({ + name: 'HTTPS_PROXY', + value: PROXY, + envProxy: 'unsupported', + }), + ).toContain('LIMITS.md'); }); it('renders the value redacted', () => { const message = proxyNoticeMessage({ name: 'https_proxy', value: 'http://user:s3cret@proxy.internal:3128', + envProxy: 'unsupported', }); expect(message).toContain('***'); expect(message).not.toContain('s3cret'); }); }); + +describe('proxyNoticeMessage — the three env-proxy states (PHARN-12)', () => { + const notice = (envProxy: 'on' | 'available' | 'unsupported') => ({ + name: 'HTTPS_PROXY', + value: PROXY, + envProxy, + }); + + it('on: says the downloads DO go through the proxy, and never "will not use it"', () => { + const m = proxyNoticeMessage(notice('on')); + expect(m).toContain('go through that proxy'); + expect(m).not.toContain('will not use it'); + }); + + it('available: keeps the warning and names the opt-in', () => { + const m = proxyNoticeMessage(notice('available')); + expect(m).toContain('will not use it'); + expect(m).toContain('NODE_USE_ENV_PROXY=1'); + }); + + it('unsupported: says this Node cannot, and that a newer one can', () => { + const m = proxyNoticeMessage(notice('unsupported')); + expect(m).toContain('will not use it'); + expect(m).toContain('no NODE_USE_ENV_PROXY support'); + }); +}); diff --git a/tests/proxy-env.test.ts b/tests/proxy-env.test.ts index 9dc247ec..f8cfa782 100644 --- a/tests/proxy-env.test.ts +++ b/tests/proxy-env.test.ts @@ -28,8 +28,15 @@ describe('detectProxyNotice', () => { ['Https_Proxy', 'Https_Proxy'], ['HTTPS_proxy', 'HTTPS_proxy'], ])('reports %s — every spelling is equally unread', (name) => { - const notice = detectProxyNotice({ [name]: 'http://proxy:3128' }); - expect(notice).toEqual({ name, value: 'http://proxy:3128' }); + const notice = detectProxyNotice( + { [name]: 'http://proxy:3128' }, + { supportsEnvProxy: false, execArgv: [] }, + ); + expect(notice).toEqual({ + name, + value: 'http://proxy:3128', + envProxy: 'unsupported', + }); }); it('ignores unrelated variables', () => { @@ -65,3 +72,49 @@ describe('detectProxyNotice', () => { expect(notice?.name).toBe('https_proxy'); }); }); + +// PHARN-12: Node's fetch ignores proxy variables BY DEFAULT, but a Node that +// knows --use-env-proxy honours them once NODE_USE_ENV_PROXY=1 / the flag is +// set. The notice must say which of the three is true. +describe('detectProxyNotice — env-proxy opt-in (PHARN-12)', () => { + const env = { HTTPS_PROXY: 'http://proxy:3128' }; + const supported = { supportsEnvProxy: true, execArgv: [] as string[] }; + + it.each([ + ['NODE_USE_ENV_PROXY=1', { ...env, NODE_USE_ENV_PROXY: '1' }, [], 'on'], + ['--use-env-proxy in execArgv', env, ['--use-env-proxy'], 'on'], + [ + '--use-env-proxy in NODE_OPTIONS', + { ...env, NODE_OPTIONS: '--max-old-space-size=512 --use-env-proxy' }, + [], + 'on', + ], + ['nothing set', env, [], 'available'], + [ + 'NODE_USE_ENV_PROXY=0', + { ...env, NODE_USE_ENV_PROXY: '0' }, + [], + 'available', + ], + [ + 'a longer flag that merely starts the same (token, not substring)', + { ...env, NODE_OPTIONS: '--use-env-proxy-foo' }, + [], + 'available', + ], + ])('%s → %s', (_label, e, execArgv, expected) => { + expect( + detectProxyNotice(e as Record, { ...supported, execArgv }) + ?.envProxy, + ).toBe(expected); + }); + + it('a Node without the option is `unsupported` whatever is set', () => { + expect( + detectProxyNotice( + { ...env, NODE_USE_ENV_PROXY: '1', NODE_OPTIONS: '--use-env-proxy' }, + { supportsEnvProxy: false, execArgv: ['--use-env-proxy'] }, + )?.envProxy, + ).toBe('unsupported'); + }); +});