fix(scripts): give every throwaway-repo git child an environment of its own (#16644 Tier B) - #19168
Merged
os-try-charles merged 4 commits intoSep 19, 2026
Merged
Conversation
…ts own Git exports GIT_DIR / GIT_WORK_TREE / GIT_INDEX_FILE into every child it runs and those outrank `cwd`, so a `git init` / `add -A` / `ls-files` aimed at a temp directory lands on the repository those variables name. Measured on #16624: 8,190 paths staged as deleted in a shared index and `core.bare = true` written into the `.git/config` every linked worktree reads, from a self-test that printed ticks throughout. Applies the landed `gitFreeEnv()` strip (scripts/git-env.mjs, #16624, applied in Tier A by #16753) to the Tier B spawn points, per spawn point rather than per file, and converges the two `packages/spec` fixtures onto the same spelling by retiring their hand-maintained GIT_* allowlist. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
…set-digest too
The `execFileSync('bash', [bump-objectui.sh, ...])` end-to-end leg still spread
`process.env`, so under a leaked GIT_DIR the driver's own `git rev-parse
<sha>^{commit}` resolved against the leaked repository and the self-test died.
Measured: with this spawn left ambient the `--self-test` exited 1 under the leak
probe while every other file exited 0.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
Contributor
📓 Docs Drift Check
What this run could not see
Coarse fallback — 136 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
Converging its fixture environment onto `gitFreeEnv()` makes the file import `scripts/git-env.mjs`, so it now reads outside its package. `check:cross-package -test-inputs` said so (exit 1, naming the file): an escaping test left in the `local` project runs under a task whose hash never moves with what it reads, so it can go red on main while every PR reports green. Verified after: the gate prints OK, and `vitest list` puts the file in `repo` and no longer in `local`. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
CI red on 6b398c3: `packages/spec` `pnpm run typecheck` exited 2 with TS7016 at both converted fixtures -- "Could not find a declaration file for module '../../../scripts/git-env.mjs'". Reproduced locally against `check:scripts-typecheck`, the lane that owns it: `tsconfig.scripts.json` includes `scripts/**/*`, inherits `strict` from the root config and sets no `allowJs`, so an untyped `.mjs` import from `packages/spec/scripts/` is TS7016. The mechanical difference from the live precedent: `scripts/js-comment-mask.mjs` and `scripts/check-regen-pending.mjs` each ship a hand-written `.d.mts` sibling (#5475, #10398) -- nine of them existed before this commit -- and `scripts/git-env.mjs` did not. That is also why the pre-existing `check-regen-pending.mjs` import in the very same test file type-checks. So this copies that shape and nothing else: one sibling declaration, PARTIAL on purpose per `invoked-as.d.mts`, declaring only the export TypeScript consumes. `check:declaration-mirrors` discovers the corpus rather than listing it and now reports "OK: 10 hand-written declaration(s) agree with their modules on name, kind and required arity" -- `gitFreeEnv`'s `Function.length` is 0 and the declaration's required arity is 0.⚠️ This is one file beyond the declared surface, and it is declared as such: it is the mandatory accompaniment of the import triage ordered, and the two remedies that avoid a new file are the two the dispatch refuses. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
os-try-charles
marked this pull request as ready for review
September 19, 2026 04:53
This was referenced Sep 19, 2026
os-try-charles
deleted the
claude/issue-16644-git-env-isolation-tier-b
branch
September 19, 2026 05:24
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #16644
Clause-②: no
What this changes
Every
gitchild these files spawn against a throwaway repository now carries anenvironment of its own —
gitFreeEnv()fromscripts/git-env.mjs, the blanketGIT_*strip #16624 landed and #16753 applied in Tier A. The judgement is made per spawn
point, not per file: a child that operates on the repository its
cwdand argumentsname gets the strip; a child that talks to a remote keeps the ambient environment and is
marked as doing so.
these files — the card is observation-class and says so. The mechanism was measured on
#16624: 8,190 paths staged as deleted in a shared index, and
core.bare = truewritteninto the
.git/configevery linked worktree of that clone reads, from a self-test thatprinted ticks throughout. What follows is that shape, reproduced against a disposable
stand-in and then closed.
Scope, and why it is wider than the card's Tier B
scripts/**— the card's enumeration, re-measured onorigin/main@5d0ee8f60as still bare.packages/spec/scripts/build-schemas-check-mode.test.tsandpackages/spec/scripts/sharded-artifacts.test.ts— added on the triage ruling(comment 5578727562, acceptance point 4): 「
packages/spec两个文件收敛到gitFreeEnv(),LEAKED_GIT_ENV一并删除。⛔ 不要保留"两种都行"的过渡态」. Theirhand-maintained ten-name allowlist of
GIT_*location variables is gone; the reason itgoes rather than gains an entry is the card's own: an allowlist has to be kept level
with git's list, and the key it misses is the key that bites.
scripts/git-merge-regen.mjs,scripts/check-regen-pending.mjs) — already done by Tier A of the #16624 class: themerge=os-regendriver and the pre-commit/pre-push gate build throwaway git repos with no environment of their own — the two files git itself invokes #16753, which triage split outof this card as
priority:p1; and Tier C, which containsscripts/pm/**— thedomain:skillslane.weakened. It belongs after Tier C, on its own card.
Per spawn point: local-only or network-touching
gitFreeEnv()is applied ONLY to the local-only column. The boundary is triage's,fixed verbatim from the card: this container really does carry
GIT_CONFIG_COUNTwithGIT_CONFIG_KEY_0=credential.interactive,GIT_CONFIG_KEY_1/2=url.https://github.com/.insteadOfand
GIT_SSL_CAINFO=/root/.ccr/ca-bundle.crt(read off the box while writing this), so anetwork child stripped of them loses its transport.
scripts/ablation-dist-preflight.mjsstatus --porcelain -z;show HEAD:PATH; the self-test'sgit()over a mkdtemp corpusscripts/check-adr-0087-registration.mjsgit(args, cwd)helper (every argv form);cat-file --batchfetch --unshallowtexts are remedy prose, not spawns)scripts/check-bash32-floor.mjslistPopulation'sls-files -z;fixtureRepo'sinit -qandadd -A; the index-vs-disk control'sls-filesscripts/check-changeset-no-major.mjsgit()helper:init/config/add/commit/diff/show/rev-parse/merge-base, and the #4690 leg'sfetchwhose remote is a local mkdtemp PATHscripts/check-empty-changeset.mjsgit()helper, same census, two local-pathfetchlegsscripts/check-engine-split-ratio.mjsgithelper; the self-testghelper (init/config/add/log+ twoclonefrom afile://URL under its own mkdtemp); the datedcommit; the--cwdre-entry childscripts/check-nul-bytes.mjslsFiles;repoRoot; the self-test'sinit,config,add -A -f,ls-files,add -Ascripts/check-skill-frame-freshness.mjsrev-parse/init/config/remote add/update-ref/commit, through agit()helper that strips by defaultfetch --quiet --no-tags origin main— NETWORK-TOUCHING. Marked⛔ AMBIENT ENVIRONMENT ON PURPOSE, reached through an explicitnetwork: true. Stripped, this gate would degrade to its offline rung on every run: a warning where a verdict belongsscripts/check-type-check-coverage.mjscheck-ignore --stdin -z;readIgnoredPaths'ls-files --others --ignored; the self-test'sgtscchildren are notgit)scripts/docs-audit/check-drift-comment.mjsGIT_ENV's base, so the fixturegit()helper and the mapper child both inherit the stripGIT_CONFIG_GLOBAL/SYSTEMpins are DELIBERATE and re-applied on top of itscripts/objectui-changeset-digest.mjsgit(cwd, args)helper; theclonefrom afile://mkdtemp source;rev-parse HEAD;merge-base --is-ancestor; and the sixbash bump-objectui.shfixture drivers, whose env BASE is now stripped because that script spawnsgitone frame downbashcapability probes carryingBASH_ENV— they spawn nogitat all, so they are not spawn points for this cardscripts/objectui-range.mjsgit(cwd, args)helper; the end-to-endnodere-entry childscripts/collect-release-notes.shunsetof every ambientGIT_*name, before any child: all four report sections read checkouts by path, and the self-test'sinit/add/commit/threeclones/onefetchare all under its ownmktemproot with afile://remoteGIT_AUTHOR_DATE=… git commitprefixes in the fixture loop are applied per invocation, after that line, and are deliberately unaffectedpackages/spec/scripts/build-schemas-check-mode.test.ts,…/sharded-artifacts.test.tsHERMETIC_ENVis now built fromgitFreeEnv(); every fixture git and every generator run inside one operates on anfs.mkdtempdirectoryGIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM/GIT_CONFIG_NOSYSTEMstay set on top: they are the file's own "read no config" pins, not inherited leakage验收备注
Triage's six acceptance points (comment 5578727562), and what was measured against each.
1. 先红后绿, every batch. A one-time instrument built a disposable stand-in
repository (
git init+ a tracked file + a commit), pointed the hook-exported locationvariables at it, ran each file's temp-repo path, and compared a fingerprint of the
stand-in's
config, index (ls-files), refs (for-each-ref) andHEADbefore andafter. ⭐ The stand-in is disposable on purpose: pointing those variables at this checkout
would not test the incident, it would be the incident, for every agent on the box — the
same discipline
scripts/git-merge-regen.mjsstates for its own probe.Two leak shapes were run, because they are not interchangeable and
scripts/git-env.mjsrecords why: with
GIT_WORK_TREEalso set, git resolves the work tree to the stand-in andadd -Are-adds its own files, so the index comes back unchanged and a case can passproving nothing.
GIT_DIRalone is the incident's shape.git rev-parse --absolute-git-dirin an unrelated directory had to answer with the stand-in's git dir.It did, in all 52 probes. A control that does not fire measures nothing.
5d0ee8f60,each proven on disk by comparing
git hash-objectagainst that blob, then restored fromHEADand proven restored by an emptygit diff HEAD): 12 of 13 wrote thestand-in under
GIT_DIRalone — config in 10, index in 11, refs in 10,HEADin 4.⭐ Three of them (
ablation-dist-preflight,check-adr-0087-registration,docs-audit/check-drift-comment) exited 0 while doing it — [finding] a gate self-test'sgit init/git add -Ainherited ambient GIT_* under pre-commit — staged 8,190 deletions in the real index and wrotecore.bare = trueinto the SHARED .git/config, breaking the primary checkout for every agent on the box #16624's signatureexactly: a green self-test writing another repository. The 13th,
check-nul-bytes.mjs, did not write the stand-in; it exited 1, because the leakedinitcreated no repository in the directory it was handed and the fixture could not bebuilt. Red in a different key, and stated as such rather than counted as a write.
shapes (26 probes), and all 26 child runs exit 0 — the isolation is complete enough
that the self-tests still pass with a leaked git environment in the process.
status --porcelain+ the sharedconfig) before and after. It never moved, in any of the 52 probes, and the whole-treegit status --porcelainafter the red leg was empty.2. Negative control. With those variables absent — ordinary CI and hand runs — every
changed file behaves as today: 19 invocations (each gate's bare call and its
--self-test, both batteries, never--self-testalone), all exit 0, real batteriesobserved in the logs (
check-nul-bytes75 assertions,check-bash32-floor177 cases,check-type-check-coverage55+97+56+28+19+18 cases,objectui-rangeall checks,collect-release-notesall cases). The twopackages/specfiles: 2 files / 114 testspassed, 487s.
3. Per spawn point, local-only vs network-touching. The table above. One
network-touching child in the whole surface, and it keeps the ambient environment.
4.
packages/specconverged, allowlist deleted. Both files now buildHERMETIC_ENVfrom
gitFreeEnv(); the allowlist constant is gone from both and is not re-spelled ineither file, so a census of the retired shape does not match the paragraph that explains
it. ⛔ No fourth spelling was introduced. Equivalence is structural rather than argued: the
ten names the allowlist removed are a strict subset of "every
GIT_-prefixed key", so thenew fixture environment is a subset of the old one, and the 114 tests above pass on it.
5. No repo-wide gate. None added.
6. Closing count, with the card's own classifier — and it is NOT zero. Reported in
the report and below: the classifier's spawn probe was run in the argv-array form
(
'git'plus a separateinit/add/ls-filesprobe), never as a command string,because triage's own first measurement was a false negative for exactly that reason and
「对照不发火的零什么都没测到」.
Acceptance notes
packages/create-objectstack/src/template-consistency.test.tscarries a THIRD copy of theretired
GIT_*allowlist (same ten names, samefor … delete env[key]shape). Triage'scensus of that shape on 2026-09-08 found two carriers and named both; this one is not on
its list. It is left out of this PR deliberately — converging it would add a package, and
therefore a verification surface, that neither triage nor the dispatch declared — and it
is reported for the Tier C lap so the "one spelling, not two" ruling can actually close.
Until it lands, the allowlist shape survives in one file.
Noted, not filed: nothing else.
Local verification, and what is declared to CI
dispatch-gates --commandsre-derived in this worktree against the real changed set(15 paths, merge base
5d0ee8f60, three-dot) → 82 commands; run, with results in thereport.
pnpm check:pm-dispatch-gateswas detached and waited on withtail --pid, perits recorded 430–450s prescription.
wide-population families, the 5 workflow-valued families and the 5 path-scheduled CI
jobs
dispatch-gatesnames as outside its runnable list, and the repo-widepnpm lint.skip-changeset: nothing published moves.packages/spec'sfiles[]does not listscripts/, and repo-rootscripts/**is inside no package.Generated by Claude Code
PM addendum — the declared surface, corrected (
domain:devx执行席, 2026-09-19T04:48Z)The two files beyond the originally dispatched 13
scripts/**+ 2packages/specfixtures, both forced rather than chosen:packages/spec/vitest.repo-tests.json(+1 line) — verified by the PM's own ablation at 2026-09-19T04:15:38Z: with the line removedcheck-cross-package-test-inputsexits 1 namingscripts/sharded-artifacts.test.ts; with it, 0. The blob was restored to its pristine hash andgit diff HEADwas clean.scripts/git-env.d.mts(43 lines) — the sanctioned shape, not a new invention. PM control, read at 2026-09-19T04:47:44Z:packages/spec/scripts/build-schemas-check-mode.test.tsimportsgit-env.mjsat:57andcheck-regen-pending.mjsat:58— adjacent lines, same file, same TS program, same lane — and only the second had a sibling.d.mts. Nine such mirrors existed before this PR; there are now ten, andcheck-declaration-mirrorsdiscovers its corpus (both batteries exit 0 on this head).⛔ The PM has kept the mirror rather than reverting it: the two remedies that avoid a new file are a
@ts-expect-errorsuppression and duplicating the retired allowlist back into the fixtures, and both are refused — the first trades a red for a suppression, the second reverses triage's 「收敛到gitFreeEnv(),退役 allowlist」 ruling.Generated by Claude Code