Skip to content

fix(tooling): isolate git children from ambient GIT_*, and make a shared core.bare flip loud - #16646

Merged
baozhoutao merged 1 commit into
mainfrom
claude/issue-16624-git-env-isolation-for-gate-self-tests
Sep 7, 2026
Merged

fix(tooling): isolate git children from ambient GIT_*, and make a shared core.bare flip loud#16646
baozhoutao merged 1 commit into
mainfrom
claude/issue-16624-git-env-isolation-for-gate-self-tests

Conversation

@claude

@claude claude Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #16624

A gate self-test that shells out to git inherited the repository location a hook exports — GIT_DIR / GIT_WORK_TREE / GIT_INDEX_FILE, which outrank cwd. Under pre-commit its git add -A staged 8,190 paths as deleted in the real index and its git init wrote core.bare = true into the shared .git/config, breaking the primary checkout for every agent on the box. Every case still printed ok.

Triage graded this p1 for three properties: machine-wide blast radius from one agent's test, a symptom that appeared far from the cause, and an instrument whose entire output said nothing had happened. This PR lands remedies 1 and 3, plus the mechanism half of remedy 2; the migration half is split out, see below.

What changed

scripts/git-env.mjs (new). The class rule with one spelling: gitFreeEnv() for a git child that must stay inside its own cwd, withoutGitEnv() for a call one frame down that passes no environment of its own. The header carries the measured incident and one boundary, below.

scripts/symbol-anchors.mjs — the graded deliverable (remedy 1). trackedFiles resolves through git ls-files and passed no environment of its own, so a sweep of a synthetic root inherited the caller's GIT_DIR and would answer with the real repository's file list. It now passes an explicit stripped environment. Pinned by a self-test case that injects a bogus GIT_DIR, builds a real two-file corpus and requires the sweep to resolve its own tree — before the change that case threw.

The two corpus gates built on that resolver (check-adr-symbol-anchors, check-scripts-symbol-anchors) build their fixture repositories with git init / git add -A, one frame away from the resolver just hardened, so both now spawn stripped too. Bounded in-place fix: same defect class, mechanical, same gate family, no new verification surface.

scripts/check-system-context-census.mjs drops its local copy of the strip and imports the shared one. Its regression pin — the one that injects a bogus GIT_DIR and requires the throwaway tree to come back with its own files staged — is unchanged and still green.

scripts/setup-git-hooks.mjs — remedy 3, the loudness. In the flipped state a commit is impossible, so no hook can be the tripwire; pnpm install is the first thing that runs after the damage and every agent runs it when it creates a worktree. It warns there (this script's "failing is not an option" contract is unchanged — a broken shared config must not also break every install on the box) and refuses under --self-test, whose whole job is to answer "is this clone wired". No existing gate's refusal set moves, and nothing was weakened or slowed to add it.

Two measurements worth keeping

rev-parse --is-inside-work-tree prints false and exits 0 under the flip. An exit-code check reports a healthy clone. git status is what fails, with exit 128, long after the useful moment — so the predicate reads core.bare directly.

The predicate must read the SHARED common dir, not the git dir it happens to be standing in. --absolute-git-dir from a linked worktree answers with that worktree's own private directory, so the layout test read every linked worktree as a legitimately bare repository and the alarm never fired from the very place agents work. This was a real defect in the first spelling, caught by its own self-test case. It matters because under the flip the linked worktrees keep working and only the primary checkout dies: the session that can still ask the question is usually not the one suffering the damage.

⛔ The boundary a blanket strip has, measured

gitFreeEnv() removes every GIT_-prefixed key, which is blunter than a list of the location variables on purpose — a list has to be maintained against git's, and the one it misses is the one that bites. But it is correct only for a child that operates on the repository its cwd and arguments name. It is wrong for a child that talks to a remote: the containers this repo is worked in carry GIT_CONFIG_COUNT with GIT_CONFIG_KEY_* / GIT_CONFIG_VALUE_* pairs that rewrite GitHub remotes and disable interactive credentials, plus GIT_SSL_CAINFO naming the proxy CA bundle. Strip those and fetch / clone / push in the child loses its transport configuration. The rule is stated by what the child does, never by which script it lives in.

Remedy 2 — the enumeration triage asked for

"Several gates build throwaway repos; this is the one that was caught" — measured at 8ae3b8d2cc over every tracked .mjs / .mts / .js / .ts / .sh file that both creates a temp directory and spawns git init against one. 32 files, in three groups:

group count files
blanket strip 5 all of them this PR: scripts/git-env.mjs · symbol-anchors.mjs · check-system-context-census.mjs · check-adr-symbol-anchors.mjs · check-scripts-symbol-anchors.mjs
7-key LEAKED_GIT_ENV allowlist 2 packages/spec/scripts/build-schemas-check-mode.test.ts · packages/spec/scripts/sharded-artifacts.test.ts — a different spelling, landed earlier by #9068 / #9109, both of which are no longer open
no environment at all 25 tiered below

Tier A — invoked BY git, or from a git hook. scripts/git-merge-regen.mjs (3 init sites; it is the merge=os-regen driver, so git runs it and GIT_DIR is exported by construction) · scripts/check-regen-pending.mjs (1; run by pre-commit and pre-push — the exact path this incident travelled).

Tier B — repo gates under scripts/**. ablation-dist-preflight.mjs · check-adr-0087-registration.mjs (6) · check-bash32-floor.mjs · check-changeset-no-major.mjs · check-empty-changeset.mjs (4) · check-engine-split-ratio.mjs · check-nul-bytes.mjs · check-skill-frame-freshness.mjs · check-type-check-coverage.mjs · docs-audit/check-drift-comment.mjs · objectui-changeset-digest.mjs (9) · objectui-range.mjs · collect-release-notes.sh.

Tier C — PM tooling, package tests, agent-hook self-tests. pm/dispatch-gates.mjs (8) · pm/check-governed-merges.mjs · pm/git-history.mjs · pm/release-rehearsal-clone.mjs · pm/os-regen-merge.sh (2) · bump-objectui.selftest.sh (2) · the two .claude/hooks/*.selftest.sh · packages/spec/scripts/check-skill-examples.ts · packages/create-objectstack/src/scaffold-skills-single-copy.test.ts.

Twenty-five is past the "handful" the dispatch set as the split threshold, and one of the tiers touches a governed surface, so migrating them here would make a p1 land slowly or not at all. Enumerated, tiered and filed unassigned as #16644, which also records the second finding: the repo now states one rule three ways, and the hand-maintained key list is the spelling worth retiring. ⛔ A repo-wide gate refusing a new inheriting spawn is deliberately not proposed until the population is at zero — it would land red and get weakened.

Red before green, per remedy

Every ablation ran from the committed state, proved the mutation on disk by occurrence count before taking a reading, restored under a trap with git checkout HEAD --, and proved each restore by blob hash equal to HEAD:path plus an empty git diff HEAD. Final tree: git diff HEAD 0 lines, git status --porcelain 0 lines.

ablation reading
remedy 1 — drop env: gitFreeEnv() from trackedFiles symbol-anchors --self-test exit 1: "⛔ ENV LEAK: sweepCorpus must not inherit a hook's GIT_DIR — with one injected it threw: Command failed: git ls-files". Unmutated baseline in the same run: exit 0.
the shared strip — make the startsWith('GIT_') delete unreachable git-env --self-test exit 1, 2 of 21 cases: the strip itself, and "with gitFreeEnv the corpus gets its OWN repository".
remedy 3 — force the core.bare read to false git-env --self-test exit 1, 3 of 21 cases, including the linked-worktree one.
remedy 3's wiring — force a bare verdict setup-git-hooks --self-test exit 1, printing the alarm and the repair line.

The git-env self-test does not only ablate: it reproduces the incident against real git as a passing red-side case — a victim repository with a commit, a leaky child spawned with GIT_DIR alone (the incident's shape; adding GIT_WORK_TREE makes the case pass while proving nothing, which was measured before the spelling was settled), then the assertion that the victim's own file comes back staged as D and that the leaky git init created no repository where it was pointed. The green side runs the same two commands with the same environment and one difference.

Gates

41 commands derived by node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack against the real change set, every exit code captured before any pipe. All green except:

pnpm install was run in this worktree and its prepare step executed the modified setup-git-hooks.mjs without incident.

Shared-checkout readings

Verified before starting, after every git-spawning experiment, and again at PR time, on /home/user/objectstack: core.bare false · git status --porcelain empty · git ls-files 7947 · git worktree list shows it on claude/pm-dispatch-devx-956ft5, not (bare). Nothing in this branch's work touched the shared checkout or the shared .git/config; every git this branch's tests spawn is itself spawned with a stripped environment.

Changeset

Nothing here is published from any package — root tooling under scripts/ plus one root package.json script line — so skip-changeset is the right disposition. The dispatch reserves repository labels for the PM, so this PR applies none; the standing dev clause says the label is the dev's step. Flagged rather than decided, and recorded in the report's open_questions.


Generated by Claude Code

…ed core.bare flip loud

A gate self-test that shells out to `git` inherited the repository location a
hook exports (`GIT_DIR` / `GIT_WORK_TREE` / `GIT_INDEX_FILE`), which outranks
`cwd`: its `git add -A` staged 8,190 paths as deleted in the real index and its
`git init` wrote `core.bare = true` into the SHARED `.git/config`, breaking the
primary checkout for every agent on the box. Every case still printed `ok`.

- scripts/git-env.mjs: the class rule in one place. `gitFreeEnv()` for a git
  child that must stay inside its own `cwd`, `withoutGitEnv()` for a call one
  frame down that passes no environment of its own, and the boundary stated
  where it can be read: never strip for a child that fetches or pushes, because
  GIT_CONFIG_* and GIT_SSL_* carry the transport configuration. Its self-test
  reproduces the leak against real git (red) before proving the strip (green).

- scripts/symbol-anchors.mjs: `trackedFiles` passes an explicit stripped
  environment, so a sweep of a synthetic root can no longer answer with the real
  repository's file list. Pinned by a self-test case that injects a bogus
  GIT_DIR and requires the sweep to resolve its own tree; before the change that
  case threw.

- The two corpus gates built on that resolver build their fixture repositories
  the same way, so both spawn stripped too.

- scripts/check-system-context-census.mjs drops its local copy of the strip and
  imports the shared one; its regression pin is unchanged.

- scripts/setup-git-hooks.mjs carries the tripwire. In the flipped state no
  commit is possible, so no hook can be the alarm; `pnpm install` is the first
  thing that runs after the damage. It warns there and refuses under
  `--self-test`. Measured: `rev-parse --is-inside-work-tree` prints `false` and
  exits 0 under the flip, so the predicate reads `core.bare` on the shared
  common dir, which is also the only reading a linked worktree can take.

Refs #16624

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file size/l skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants