[jayden1711]

Part of #1001 (item 1 on main; item 2 deferred, see below).

What changed

withoutRedirectGitConfig (src/network.ts, run by outboundEnv on every child environment the engine builds) strips TLS, proxy and URL-rewrite keys from git config carried in GIT_CONFIG_COUNT/KEY_n/VALUE_n. It read the count with Number():

• a count git refuses (1 , 0x1, 1e0, 1.0, 1\n) was rewritten as 1, a count git reads, so the caller's GIT_CONFIG_KEY_0 pair was applied though git would have refused it;
• a count over 1000 was returned untouched, with its pairs unstripped. git reads every pair of such a count, so http.sslVerify=false (or a url.*.insteadOf) among 1001 pairs reached git. This bypasses the TLS guard.

Now only an empty count or a plain decimal from 0 to 1000 is read. Any other count (including git-accepted forms +1, 01, 1, \t1) becomes refused, which git refuses ("bogus count"), so that git fails instead of running. This is fail-closed, the same rule #927's gitEnv uses for its own parse (CANONICAL_CONFIG_COUNT, there with a 9999 bound). Counts the engine writes (String(n), in agentEnv, runAsEnv, projectCommand and instanceEnv) are unchanged.

Why this file and not src/os/git.ts

The issue names gitEnv in src/os/git.ts. That module isn't on main: it is in open PR #927 (head 021f6f18), and its head already reads only a canonical count and replaces any other with a refused value (commit 550323cf, #927 rounds 8 and 9). Measured: git show 021f6f18:src/os/git.ts shows CANONICAL_CONFIG_COUNT and REFUSED_CONFIG_COUNT, and git log origin/main -- src/os/git.ts is empty. I don't own #927's branch, so I didn't touch it. The same Number() parse exists on main in withoutRedirectGitConfig, which #927 doesn't change (measured: git diff origin/main 021f6f18 -- src/network.ts is empty), so that is what this PR fixes.

Item 2: GIT_NO_LAZY_FETCH (not done, follow-up)

Not implemented. Reasons:

• It belongs in the git runner's fixed env (GIT_FIXED_ENV in git: one runner for every engine git call, with replace refs and grafts off (#807) [PROTECTED: owner merges] #927's src/os/git.ts), which isn't on main and isn't mine to edit. Putting it in outboundEnv would set it for every child, agents included, and change agents' git in a partial clone. That is a behaviour change for agents, not a hardening of the engine's own git.
• The host's git is 2.43.0 (measured: git --version), which ignores the variable (inferred from the issue and git's 2.44 release notes; not measured here). So I can't show a fail-before/pass-after test for it on this machine or on CI's Ubuntu git without pinning a newer git.
• It is not fail-closed on git < 2.44: setting it there silently does nothing.

Proposed follow-up, once #927 lands: add GIT_NO_LAZY_FETCH: '1' to GIT_FIXED_ENV, and a test that skips with a named reason when git --version < 2.44 (or makes a partial-clone lazy fetch fail on >= 2.44). As the issue says, the decisions index re-checks every object's hash, so this is not a gap in what it accepts (inferred).

Evidence

Fail before (src at origin/main, new tests added), measured:

npx tsc -p . && node --import ./dist/test/isolate.js --test --test-name-pattern=1001 dist/test/network-tls.test.js
not ok 1 - a GIT_CONFIG_COUNT that is not a plain decimal is made one git refuses, never rewritten into one it reads (#1001)
  error: '"1 "'   expected: '1'  actual: '1'   (notEqual: the count was rewritten to 1)
not ok 2 - a GIT_CONFIG_COUNT over the bound is refused, so its TLS key never reaches git unstripped (#1001)
  error: http.sslVerify=false reached git   expected: null  actual: 'false'
# pass 0  # fail 2

Pass after (this head), same command, measured:

ok 1 - a GIT_CONFIG_COUNT that is not a plain decimal ... (#1001)
ok 2 - a GIT_CONFIG_COUNT over the bound is refused ... (#1001)
# pass 2  # fail 0

The tests run the real git (git config --get) on the env outboundEnv returns, so they check what git reads, not only the string. git 2.43 behaviour on the counts, measured with GIT_CONFIG_COUNT=<c> GIT_CONFIG_KEY_0=x.y GIT_CONFIG_VALUE_0=z git config --get x.y: 1, 01, +1, 1, \t1 read the pair; 1 , 0x1, 1e0, abc, 1\n are "bogus count"; -1 is "too many entries"; empty and -0/00 read none.

Tests run (measured)

• npm run typecheck: clean.
• npm run check:denylist: denylist: 1657 files clean.
• Targeted: network-tls, network, runner-golden, child-env-sites, separate-users, project-command-isolation, project-env, push-guard, push-hook-guard: 98 tests, 95 pass, 0 fail, 3 skipped.
• Full suite once, under the shared lock (flock /var/tmp/wl-coord/fulltest.lock nice -n 15 ionice -c3 npm test): 5842 tests, 5716 pass, 17 fail, 69 skipped. The 17 failures are in coordinator.test.js (a sandbox pip cache env), desktop-removed.test.js (node_modules is a symlink to the main checkout's) and machine-sudoers-{closed,narrow,removal}.test.js. With src/network.ts and the test file reset to origin/main, the same five files give the same 17 failures (measured, 113 tests, 17 fail). So they come from this host, not from this change. CI is the authoritative run.

Critical set: yes

outboundEnv is the redirect and TLS-trust guard for every child environment (claude, gh, git, project commands), and this PR changes how it treats git config in that environment. src/network.ts isn't matched by SELF_MODIFICATION_PARTS or .worklane/review.yaml (measured: rg network over both finds nothing). But it is a guard, and the rule is "when unsure, in", so it needs two independent reviews. What a reviewer should check: (1) no count git reads can get past stripping. The only counts kept are '' and 0–1000 in plain decimal, which Number() reads as git does. (2) A refused count leaves the old pairs in place, unstripped. That is safe only because git refuses the whole count; non-git children don't read these variables (inferred). (3) No engine path writes a count over 1000 or a non-canonical one (inferred from the four writers listed above).

Skipped / follow-ups

• GIT_NO_LAZY_FETCH (above).
• Once git: one runner for every engine git call, with replace refs and grafts off (#807) [PROTECTED: owner merges] #927 lands, there are two count parsers (gitEnv's with a 9999 bound, and this one with 1000). They could share one constant. Not done here because git: one runner for every engine git call, with replace refs and grafts off (#807) [PROTECTED: owner merges] #927's module isn't on main.

[jayden1711]

Independent adversarial review (R82b), posted by the coordinator.

VERDICT: PASS @bf038f1d0e607c17f9a138e3eb2264bacd8add06 (adversarial)

Reviewer R82b, adversarial. Head confirmed via gh pr view 1082 --json headRefOid (unchanged at the end).

Findings on what the PR claims

• No count git reads gets a TLS/proxy/rewrite pair past the strip (measured). I ran r82b-probe.mjs against dist/src/network.js with a real git 2.43 and a temp repo, local only, checking git config --list --show-origin. I tried 32 count forms: 01, +1, 1, \t1, \f1, \n1, 0001, -0, 00, +0, 1 , 0x1, 1e0, 1\r, NBSP, fullwidth １, Arabic-Indic ١, Infinity, 1000, 1001, 2^31, 2^32+1, 20 digits and -4294967295. Every one either comes out with the bad key stripped (1, '', 1000) or makes git fail with "bogus count". At 1001 pairs with http.sslVerify at index 999, raw git reads the key; after outboundEnv, git refuses. 23 key shapes are all stripped: case variants, leading/trailing whitespace, a trailing \n, http.<url>.ssl*, proxy*, curloptResolve and url.*.(push)insteadOf. An insteadOf rewrite to a local repo really works on raw git (counts 1 and +1) and is blocked after outboundEnv.
• Gaps in numbering and case variants (measured). A missing KEY_1, a pair past the count, a padded KEY_01, a missing VALUE, lowercase git_config_* and GIT_CONFIG_PARAMETERS in either case all show no leak.
• The engine's own writers are safe (inferred from reading the code). agentEnv, runAsEnv, projectCommand and instanceEnv all set the count before calling outboundEnv, so a refused count's unstripped pairs never get a valid count added after the guard.
• Fail-before and pass-after reproduced (measured). With origin/main's src/network.ts and the new tests: 2 of 2 fail. On the head: 2 of 2 pass. Command: node --import ./dist/test/isolate.js --test --test-name-pattern=1001 dist/test/network-tls.test.js.
• Typecheck and targeted tests (measured). npm run typecheck is clean. The targeted run of network-tls, network, child-env-sites, runner-golden, separate-users, project-env and push-guard gives 54 tests: 51 pass, 0 fail, 3 skipped.
• PR body's git behaviour notes (measured). -1 gives "too many entries". '' and 0 read no pairs.
• CI: 1 of 8 test shards pass so far; the other 7 are pending. The scope, denylist and dco checks pass.

Mutants

I edited the compiled network.js in my own worktree's dist/ only and ran network-tls plus network tests (measured).

Caught:

• accepting leading zeros
• dropping the $ anchor
• raw.trim() before the test
• allowing +
• leaving the raw count in place
• removing the guard

Survived:

• M4/M5: the refused value set to '' or '0'. The test's assert.notEqual(r.status, 0) can't tell "git refused" from "key not found": git config --get exits 1 for a missing key (measured, count0 rc=1). So the claim that the git "fails instead of running" is untested. Security is the same either way, because git reads no pairs, so this is not a FAIL.
• M1/M9: refusing 1000 or ''. These would be false refusals that nothing tests. The engine never writes either value (inferred).
• M2: bound raised to 1999. Same security either way, because the strip still runs.

Follow-ups (adjacent gaps, not claimed by this PR, all exist on main too)

1. include.path / includeIf.*.path through GIT_CONFIG_KEY_n (measured, real effect). The included file's url.*.insteadOf and http.sslVerify=false apply after outboundEnv: the local rewrite succeeds. This is the most serious gap.
2. GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM pass through outboundEnv (measured). A file they point to rewrites URLs. Legacy GIT_CONFIG has no effect on 2.43.
3. remote.<name>.proxy is not stripped (measured). GIT_KEY only covers http.*proxy*.
4. Test strength. Assert the "bogus count" stderr, or use a git command that succeeds without config, so M4/M5 die. Add '' and 1000 cases.
5. A refused count leaves the caller's raw pairs in the env (inferred). That is safe only while nothing re-sets the count downstream. Deleting all GIT_CONFIG_KEY_*/VALUE_* on refusal would make it unconditional.
6. Windows (guess). A case-insensitive env with Git_Config_Count next to GIT_CONFIG_COUNT is untested. Linux CI only.

What I ran

[jayden1711]

Independent final check (R82a), posted by the coordinator.

VERDICT: PASS @bf038f1d0e607c17f9a138e3eb2264bacd8add06 (final check)

PR #1082 (issue #1001). Reviewer r82a, own detached worktree at , TMPDIR= The head was confirmed with gh pr view 1082 --json headRefOid at the start and again after CI. It did not move.

Findings

• The bug is real on main and fixed (measured). I built the head's tests against git show origin/main:src/network.ts and ran node --import ./dist/test/isolate.js --test --test-name-pattern=1001 dist/test/network-tls.test.js. Result: 0 pass, 2 fail. Test 1 fails on "1 " (count rewritten to 1). Test 2 fails with actual: 'false' (http.sslVerify=false reached git). The same command on the head gives 2 pass, 0 fail.
• The new behaviour matches git 2.43 (measured). I ran env -i ... GIT_CONFIG_COUNT=<c> GIT_CONFIG_KEY_0=x.y GIT_CONFIG_VALUE_0=z git config --get x.y in a temp repo. git reads the pair for 1, 01, +1, 1 and \t1. It reports "bogus count" for 1 , 0x1, 1e0, 1.0, 1\n, abc and refused. It reports "too many entries" for -1, 2147483648 and 4294967296. It reads no pairs for empty, 0, -0 and 00. Every value in the PR body matches. The counts kept (empty, and plain decimals 0 to 1000) are read the same way by Number() and by git, so no count git reads gets past the strip. Every other count becomes refused, which git rejects (fail-closed).
• No normal env is broken (measured and inferred). Empty, 0 and 1–1000 behave as on main. The engine's writers (service.ts String(git.length), plus the literal '1' and '2' in runner.ts and os/index.ts) are all canonical (inferred, from rg GIT_CONFIG_COUNT src). Forms like 01 and +1 were stripped on main and now make git fail. The PR states this. I don't count it as an ordinary use.
• Targeted tests pass (measured). network, network-tls, child-env-sites and runner-golden: 32/32 pass. They pass only after unsetting this sandbox's injected proxy variables. With those variables set, one network.test case fails because the sandbox's own SOCKS_PROXY/GIT_HTTP_PROXY leak into its expected env. That comes from the host and has nothing to do with this change.
• Typecheck and denylist (measured). npm run typecheck is clean. npm run check:denylist reports "denylist: 1658 files clean". No brand string in src. The changelog fragment is present and accurate.
• CI (measured). gh pr checks 1082: every required check (agentshield, dco, denylist, new-deps, push-gate, scan, test (ubuntu-latest, 22), test (ubuntu-latest, 24)) and all 8 shards pass on bf038f1. So the builder's 17 local failures don't show up in CI.
• Mutants (measured, perl edits to dist/src/network.js). Caught: no regex gate; a leading-zero digit class; leading \s*; trailing \s*; +?; the m flag. Survived:
  • allowing 4 digits (still safe: every pair is stripped);
  • dropping |1000;
  • dropping the empty alternative;
  • REFUSED set to '0' or ''. Still safe, but the tests' status !== 0 check can't tell "git refused" from "key not found" (exit 1).

None of these is a FAIL under the brief.

Follow-ups (not blocking)

1. Add boundary tests that 1000 and empty are still read (they would catch the |1000 and empty mutants).
2. Make test 1 assert that git reports "bogus count" (stderr or exit 128). That would pin the body's claim that "git fails", not just that the pair is missing.
3. When git: one runner for every engine git call, with replace refs and grafts off (#807) [PROTECTED: owner merges] #927 lands, share one count rule between gitEnv (bound 9999) and this one (bound 1000), as the PR proposes.
4. GIT_NO_LAZY_FETCH (item 2 of git runner: leave a GIT_CONFIG_COUNT git can't parse untouched; consider GIT_NO_LAZY_FETCH #1001) is deferred, as the PR says.