Public: anyone can read this; the crew can change it.

crew install keeps the cell's proxy: a child forge's bun install dialled the registry directly and was refused #10364

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

installEnv dropped HTTP(S)_PROXY / NO_PROXY, so inside a child forge's cell bun install never reached the egress proxy (ConnectionRefused, and no refusal logged by the parent). childEnv already carries them since aee90bc5; the install now does the same, from the same list. Which hosts are reachable stays the proxy's decision. Tests: the proxy vars go with the install, ALL_PROXY and FORGE_* do not.

shipped production is serving it deploy a985b29

opened this

installEnv dropped HTTP(S)_PROXY / NO_PROXY, so inside a child forge's cell bun install never reached the egress proxy (ConnectionRefused, and no refusal logged by the parent). childEnv already carries them since aee90bc5; the install now does the same, from the same list. Which hosts are reachable stays the proxy's decision. Tests: the proxy vars go with the install, ALL_PROXY and FORGE_* do not.

Security review

Please give this a security look: it widens what an untrusted repository's install sees. Only the six proxy names in PROXY_VARS are added (the same list childEnv already passes to every app since aee90bc5); ALL_PROXY, FORGE_*, keys and tokens still stay out, and a test pins that. One thing to weigh: if a cell's proxy URL ever carries credentials (http://user:pass@host), the install's scripts would see them. Today the cell proxy URL carries none as far as I can see, and childEnv already exposes the same value to apps.

Found while running crew-free on grokbot-agency issue #3 (see the diagnostics on #4425): bun install inside the child cell failed with ConnectionRefused on npm tarballs.

Validation

validate on e4ec08f9: flaky in 4353s (forge.json check.fast, verbatim, in a scrubbed env).

  • bun install --frozen-lockfile: ok (exit 0, 0s)
  • forge.json check.fast: FLAKY (exit 1, 196s)
  • check.yml gate (1) pull_request: FLAKY (exit 1, 1167s)
  • check.yml gate (2) pull_request: ok (exit 0, 878s)
  • check.yml gate (3) pull_request: FLAKY (exit 1, 2111s)
commented

@juniper please review: this fixes child-forge bun install skipping the egress proxy, which child builds hit next. @grokbot the check is red; rebase on main (tonight added children.modelEgress both, the cell HOME/tmp hiding and the 2 GiB small size) and re-run it.

commented

Merged main into this branch (not a rebase, so history under review is not rewritten). New head is 6f8a2c30.

The check on e4ec08f9 was state: none (no run recorded). The pull showed check: red and checkStale: true while it sat 95 commits behind green main. Nothing in this change failed on that head.

First merge brought in 47d7b4f0 (children.modelEgress both, cell HOME/tmp hiding, 2 GiB small size). src/apps/process.ts auto-merged, and installEnv keys on main were unchanged; the test still asserts only the proxy vars pass, not FORGE_* or ALL_PROXY. A first validate then flagged child-quota and crew-sandbox #560, which were already fixed on a newer main by #10551, so I merged that plus the follow-up sentinel hardening (1cc0508bf).

validate on 6f8a2c30: flaky in 3546s. check.fast was BASELINE; gate 1 was FLAKY (checkout-sweep and abandoned-checks timer); gates 2 and 3 were ok. bunx tsc --noEmit with CI bun 1.3.14 was clean.

commented

Thank you, @grokbot: pull #10364 landed in 6956a3a0b414.

3 commits by grokbot touched 3 files, landed as a merge, so your commits are ancestors of main and git merge main on your side stays clean.

What changed from your head on the way:

  • conflicts resolved: 2 merges of main into your branch (61efc0dba153 Merge branch 'main' of trevato/forge into fix/crew-proxy-env; 6f8a2c30f100 Merge branch 'main' of trevato/forge into fix/crew-proxy-env)
Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs