Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions .dev/features/proxy-notice-truth/GRILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# GRILL — proxy-notice-truth

Plan: `.dev/features/proxy-notice-truth/PLAN.md`. Spec hash recomputed:
`bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e` — **matches**. Registered
grillers: `{"registered":0,"grillers":[]}` → inline axes only. The plan is `trust: untrusted`;
nothing in it read as an instruction.

## Findings

### Guarantee audit (P0)

```yaml
- type: FINDING
rule_id: 'P0'
severity: important
file: '.dev/features/proxy-notice-truth/PLAN.md:55'
problem: 'Every "will not use it" notice ends by citing LIMITS.md §3a, and §3a still says a proxy variable changes nothing. After this increment the notice itself names the opt-in, so the doc it cites contradicts it. LIMITS.md is human-only; the PR must name the §3a edit for the human rather than leave the contradiction silent.'
evidence: '`LIMITS.md` §3a (`:106-110`) still says "setting a proxy variable will not change that".'
- type: FINDING
rule_id: 'P0'
severity: minor
file: '.dev/features/proxy-notice-truth/PLAN.md:112'
problem: 'Where a Node knows the flag, the runtime answers for itself (membership). The version window is only needed where it does not: 24.0–24.4. Say in the code that only that window is a version test, so a future line (25+, 26) is classified by the membership test and never by an extrapolated table.'
evidence: 'table itself is advisory in reach: measured on 9 versions, and later Node lines are assumed to'
```

### Eval coverage (P1)

```yaml
- type: FINDING
rule_id: 'P1'
severity: minor
file: '.dev/features/proxy-notice-truth/PLAN.md:83'
problem: 'The hermetic setup has no vitest case of its own; its proof is the two-environment run in VERIFY. Record both runs with their counts: the whole suite under NODE_USE_ENV_PROXY=1 and under an exported HTTPS_PROXY, before (15 and 1 extra failures) and after (0).'
evidence: '`tests/setup/hermetic-env.ts` (new) — layer tests. Before any test, it deletes `HTTPS_PROXY`,'
```

### Determinism (P5)

```yaml
- type: FINDING
rule_id: 'P5'
severity: minor
file: '.dev/features/proxy-notice-truth/PLAN.md:71'
problem: 'NODE_OPTIONS is split on whitespace, while Node also honours double-quoted arguments. A flag token can never contain a space, so this cannot mis-read a real flag, but a quoted VALUE that happens to spell the flag would be read as one. Pathological; state the approximation in the comment.'
evidence: '(regex `^--(no[-_])?use[-_]env[-_]proxy(=.*)?$` over `NODE_OPTIONS` tokens, then the command'
```

### Checked, no finding

- Trust (P2): a printed variable NAME reaches output only when it case-folds to one of four proxy
names (ASCII letters and `_`). Values still go through `redactProxyUrl`.
- Axis (P3): the detection logic changes only in `proxy-env.ts`, and the wording only in
`proxy-env-format.ts` — the file split's own rule.

## Summary

The rule set is measured, and the plan keeps pure functions pure. The important gap is outside the
plan's reach: `LIMITS.md` §3a, which every notice cites, will contradict the notice. That has to be
handed to the human explicitly. The rest are comments and evidence to record.

**ADVISORY VERDICT: 4 concerns raised (0 blocking-severity, 1 important, 3 minor) — for the human to
weigh before /pharn-dev-build.**
138 changes: 138 additions & 0 deletions .dev/features/proxy-notice-truth/PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
# PLAN — proxy-notice-truth (the proxy notice follows Node's measured rules; its tests stop reading the host's env)

- spec_content_hash: bca940a5ad247c120e6d8a3acba119d0d8df51dca275964d0e54c48d729d3c4e # fix #4
- increment: `detectProxyNotice` reports what Node's `fetch` will actually do. It follows the
measured rule set below: the variable Node reads and its precedence, both flag spellings, the
`--no-` negation and last-one-wins, the 24.0–24.4 window where the variable works without the
flag, and exact-case lookup. The command tests stop depending on the host's proxy environment.
- layer(s): the CLI itself (`src/lib/proxy-env.ts`, `src/lib/proxy-env-format.ts`), tests, docs
- constitution_refs: [P0, P1, P5, P6]

## Discovery — measured this run (P6)

The measurement harness is in scratchpad `plan-E/`. A local proxy logs `CONNECT`s, and a client
`fetch`es `https://example.com` under `env -i`. It ran on the official binaries for 20.20.2, 21.7.3,
22.20.0, 22.21.0, 22.22.2, 23.11.1, 24.0.0, 24.4.1 and 24.5.0.

| Node | flag known¹ | `NODE_USE_ENV_PROXY` turns it on | flag turns it on² |
| ---------------------------- | ----------- | -------------------------------- | ------------------ |
| 20.x, 21.x, 22.0–22.20, 23.x | no | never | startup error |
| 22.21+ (22.22.2), 24.5+ | yes | only the value `1` | yes, last one wins |
| 24.0.0–24.4.x | no | ANY non-empty value (even `0`) | startup error |

¹ `process.allowedNodeEnvironmentFlags.has('--use-env-proxy')`. ² `--use-env-proxy` or
`--use_env_proxy`, from `NODE_OPTIONS` or the command line.

The rows below hold wherever env proxy is on (measured on 22.22.2, 24.0.0, 24.4.1 and 24.5.0):

- **Negation.** `--no-use-env-proxy` or `--no-use_env_proxy` overrides `NODE_USE_ENV_PROXY=1`.
- **Order.** The command line is read after `NODE_OPTIONS`, so the last token wins.
`--use-env-proxy=false` still turns it ON.
- **Case.** `Https_Proxy` is never read on Linux, so the connection goes DIRECT.
- **Precedence.** `https_proxy` beats `HTTPS_PROXY`. Seen via different ports: lowercase won.
- **Fallback.** With only `HTTP_PROXY` set, https requests still go through that proxy (undici's
`EnvHttpProxyAgent` falls back from the https agent to the http one).
- **Exclusion.** `NO_PROXY=example.com` → direct.

Checked against HEAD `abb274a` (`proxy-env.ts:67-94`):

- `supportsEnvProxy` is the flag membership alone. So 24.0–24.4 read as `unsupported` ("this Node
has no NODE_USE_ENV_PROXY support") while the variable really does route `fetch`.
- `hasFlagToken` knows only the hyphen spelling and no negation.
- `proxyVariantKeys` matches any letter-case and prefers `HTTPS_PROXY`. So it names a variable Node
ignores, or the one Node does not use.
- It never looks at `http_proxy`/`HTTP_PROXY`.

F27, reproduced here (proxy variables unset, root capabilities dropped):

- `NODE_USE_ENV_PROXY=1` → 15 extra failures (add 2, init 2, status 5, update 6). Every one is a
notice test that expects "will not use it".
- `HTTPS_PROXY` exported → `init.test.ts:871` "prints nothing extra…" fails. The notice counts as an
unexpected `log.warn`.
- The four update failures that appear under both are the known root-only chmod cases (the
CI-equivalent wrapper passes them).

`LIMITS.md` §3a (`:106-110`) still says "setting a proxy variable will not change that". That has
been untrue since PHARN-12. LIMITS.md is human-only (hook-protected), so this plan **surfaces** it
and does not edit it.

## Files

- `src/lib/proxy-env.ts` — layer CLI/lib. Changes:
- The runtime record gains the Node version.
- The proxy Node would use for an https URL is resolved with undici's own `??` chains, by exact
key: the first PRESENT of `https_proxy` / `HTTPS_PROXY`; if that is absent or empty, the first
present of `http_proxy` / `HTTP_PROXY` (GATE 1 answer 1 → a). Measured on 22.22.2: an empty
`https_proxy` shadows a set `HTTPS_PROXY` and falls through to the http pair. On win32,
`process.env` lookups are case-insensitive, so the platform difference comes for free.
- A key that only case-folds to one of those four, when no exact lookup found a proxy, yields a
new "ignored spelling" notice.
- The env-proxy state follows the table. Where the flag is known, the LAST flag token decides
(regex `^--(no[-_])?use[-_]env[-_]proxy(=.*)?$` over `NODE_OPTIONS` tokens, then the command
line); with no token, `NODE_USE_ENV_PROXY === '1'` decides. Where the flag is unknown and the
version is 24.0–24.4, any non-empty `NODE_USE_ENV_PROXY` turns it on. Otherwise: unsupported.
- `src/lib/proxy-env-format.ts` — layer CLI/lib. The message for the ignored spelling ("Node reads
only https_proxy / HTTPS_PROXY / http_proxy / HTTP_PROXY"). The unsupported wording names the
Node lines that have the option (22.21+, 24+), rather than "a newer release". The "on" wording
keeps its NO_PROXY hedge (GATE 1 answer 2 → a).
- `tests/proxy-env.test.ts` — layer tests. One case per measured row; each is an injected runtime,
so the cases are pure. FAIL on base: 24.4 + `NODE_USE_ENV_PROXY=1` → on; `--use_env_proxy` → on;
`NUEP=1` + `--no-use-env-proxy` → available; `Https_Proxy` → ignored spelling; lower beats
upper; `HTTP_PROXY` only → a notice; an empty `https_proxy` shadows `HTTPS_PROXY`.
- `tests/proxy-env-format.test.ts` — layer tests. The new and changed wordings.
- `tests/setup/hermetic-env.ts` (new) — layer tests. Before any test, it deletes `HTTPS_PROXY`,
`https_proxy`, `HTTP_PROXY`, `http_proxy`, `NO_PROXY`, `no_proxy` and `NODE_USE_ENV_PROXY` from
the test process, so `vi.unstubAllEnvs()` restores "absent", never the host's value.
- `vitest.config.ts` — layer tests. Registers that file under `setupFiles`.
- `tests/add.test.ts` — layer tests. Adjusted only if a notice assertion depends on the host Node's
flag support. Today they match "will not use it", which holds for both available and
unsupported.
- `tests/init.test.ts` — layer tests. The same.
- `tests/status.test.ts` — layer tests. The same.
- `tests/update.test.ts` — layer tests. The same.
- `docs/troubleshooting.md` — layer docs. The proxy section: the version table in prose, the
exact-case rule, lowercase precedence, the `HTTP_PROXY` fallback, and `--no-use-env-proxy`.
- `CHANGELOG.md` — `[Unreleased]` → `### Fixed` (notice), plus a line for the hermetic tests

## Contracts satisfied

- PHARN-12's own contract: "the notice is one of three TRUE forms" (`proxy-env-format.ts:46-51`).
It is true again on every measured Node, now as four forms (cited, P4).

## Evals to write (P1)

- Listed under Files. Six cases FAIL on the base.
- The hermetic setup is proven by running the whole suite under `NODE_USE_ENV_PROXY=1` and under
an exported `HTTPS_PROXY`, before and after. That run is recorded in VERIFY, not a vitest case:
a test cannot set its own process env before its setup file runs.

## Guarantee audit (P0)

- "the notice matches Node's behaviour" → floor: pure-function tests over the MEASURED table. The
table itself is advisory in reach: measured on 9 versions, and later Node lines are assumed to
follow the 24.5 row. That assumption is named in the code comment and the docs.
- "the suite's result does not depend on the host's proxy env" → floor: the setup file
(deterministic deletion) plus the two-environment run above.

## Trust audit (P2)

- The environment is attacker-influenceable. Values are still printed only through
`redactProxyUrl`. A variable NAME is printed only when it case-folds to a proxy name, so it
contains ASCII letters and `_` only.

## Determinism audit (P5)

- Key membership, a regex over tokens, and a version-range compare. No classification. With nothing
set the result is `null` (silence).

## Open questions (HALT)

None open. Resolved at GATE 1 (human, 2026-09-25): every question below → **(a)**, the
recommended answer. Kept for the record:

1. With only `HTTP_PROXY`/`http_proxy` set (no https variable), today pharn says nothing. Node,
when on, still proxies https through it. (a) Report it, naming that variable — recommended,
because it is the proxy Node actually uses. (b) Stay https-only, as today.
2. `NO_PROXY` covering `github.com` hosts while env proxy is on. (a) Keep the current hedge
("Node's fetch then also honours NO_PROXY") — recommended, no new matching logic. (b) Evaluate
`NO_PROXY` against the three hosts and say "direct" when they match.
44 changes: 44 additions & 0 deletions .dev/features/proxy-notice-truth/REGRESSION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# REGRESSION — proxy-notice-truth

The verdict below is computed by `.dev/floor/check-regress.mjs`, not by this stage's judgment.

## Base and partition

- **base:** `f1e8b92cfba1c59c19b1743cde23e77c54ac64bb` (`HEAD` — `origin/main` after #220; the
build is an uncommitted working tree on top of it).
- **inside** (each declared in `PLAN.md` `## Files`):
- `src/lib/proxy-env.ts`, `src/lib/proxy-env-format.ts`
- `tests/proxy-env.test.ts`, `tests/proxy-env-format.test.ts`
- `tests/setup/hermetic-env.ts` (new), `vitest.config.ts`
- `docs/troubleshooting.md`, `CHANGELOG.md`

The four command test files the plan also declared needed no change.

- **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 46 stdlib `*.test.mjs` / `*.test.cjs` files `scope` returned (754 tests) +
whole-repo `validate`; 0 committed eval pairs.
- **style-gate skip:** `inside` touches no shared style config (`vitest.config.ts` is not one), so
`lint` / `format:check` / `lint:md` are absent from both maps.
- **environment:** both sides ran with no proxy variables and as root **without**
`CAP_DAC_OVERRIDE` / `CAP_DAC_READ_SEARCH` / `CAP_FOWNER` (`setpriv`), the CI-equivalent of this
root sandbox.

## 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.
63 changes: 63 additions & 0 deletions .dev/features/proxy-notice-truth/REVIEW.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# REVIEW — proxy-notice-truth

Increment:

- `src/lib/proxy-env.ts` — the measured rule set:
- undici's `??` lookup over the four exact spellings, https pair first;
- a new `ignored-spelling` state;
- last-flag-wins with both spellings and the negation;
- the 24.0–24.4 window, reached only where the runtime does not list the flag;
- an `optedOut` marker.
- `src/lib/proxy-env-format.ts` — the ignored-spelling and opted-out wordings, and the unsupported
wording naming 22.21+ / 24+.
- `tests/setup/hermetic-env.ts` + `vitest.config.ts` `setupFiles`.
- The rewritten `tests/proxy-env.test.ts` (one case per measured row), the troubleshooting proxy
section, and CHANGELOG.

Treated as `trust: untrusted`; nothing in it read as an instruction.

## Floor first (P0)

`node .dev/floor/validate.mjs .` → `FLOOR: GREEN` (exit 0). `/pharn-dev-build`'s `npm run check`
passed (1554 tests), `/pharn-dev-regress` returned `no-regressions`, and `/pharn-dev-verify`
returned `PASS`.

## Floor-gate findings (blocking)

None.

- **L-floor (P0)**
- "the notice matches Node's behaviour" → pure-function tests, one per measured row. The table's
reach is stated in the module header, and a future Node is classified by its own flag list.
- "the suite does not depend on the host's proxy env" → a deterministic deletion before every test
file. It was proven by whole-suite runs under three hostile environments (VERIFY.md): base
15 / 1 failures, head 0.
- **L-eval (P1)** — 21 of the new cases failed against the base code. The rest (silence, redaction,
unrelated variables) are guards.
- **L-trust (P2)** — a variable NAME is printed only when it case-folds to one of four proxy names,
so it is ASCII letters and `_`. Values still pass through `redactProxyUrl`. The environment stays
data.
- **L-axis (P3)** — detection changed only in `proxy-env.ts`, and wording only in
`proxy-env-format.ts`: that file pair's own split.

## Advisory findings (warn — severity is this reviewer's judgment, fix #3)

```yaml
- type: FINDING
rule_id: 'P0'
severity: important
file: 'src/lib/proxy-env-format.ts:57'
problem: 'Every "will not use it" form cites LIMITS.md §3a, which still says a proxy variable changes nothing. The notice names the opt-in, so the doc it cites contradicts it. LIMITS.md is human-only (hook-protected), so this cannot be fixed from this increment; the PR asks the maintainer to update §3a.'
evidence: "'The download connects DIRECTLY, and fails if direct egress is blocked (LIMITS.md §3a).'"
- type: FINDING
rule_id: 'P7'
severity: minor
file: 'src/lib/proxy-env.ts:159'
problem: "The win32 behaviour (a mixed-case key is read, because process.env is case-insensitive there) follows from Node's documented process.env semantics but was not measured. CI has no Windows. The docs state it, and VERIFY names it as unmeasured."
evidence: '* key — on win32 `process.env` itself is case-insensitive, so a mixed-case key'
```

## Verdict

**GREEN — 0 floor-gate findings, 2 advisory (1 important, 1 minor).** The standing decision is the
human's (GATE 2). The important one needs a maintainer's edit to `LIMITS.md` §3a.
24 changes: 24 additions & 0 deletions .dev/features/proxy-notice-truth/SHIP.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# SHIP — proxy-notice-truth

Stages run, in order:

1. `/pharn-dev-plan` → GATE 1 (human: plans A–F accepted with every recommended answer).
2. `/pharn-dev-grill`.
3. The plan's `## Files` was reworded so the writes-scope parser reads every path, and one measured
precedence detail was added: an empty `https_proxy` shadows `HTTPS_PROXY`. Intent unchanged.
4. `/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)
- For the human, outside this increment's reach: `LIMITS.md` §3a (human-only) still says a proxy
variable changes nothing. The notice cites it (REVIEW.md, important advisory finding).
- The run ended at **GATE 2**. The human's standing instruction for this batch: after each
increment, open a pull request and merge it once its checks are green, then start the next plan.

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.
Loading
Loading