Skip to content

test(hooks): isolate nested git fixtures from hook-exported GIT_* state #569

Open

Description

@wz-gsa

Summary

Running the repository's pre-commit suite from Git's pre-commit hook can corrupt the repository that invoked it. Git exports repository-local GIT_* variables to hooks; the Bats suite creates nested fixture repositories with plain git -C <fixture> init / git -C <fixture> commit without clearing those variables. Under the hook, those commands target the parent repository's gitdir instead of the fixture.

This was reproduced twice while resolving the approved #564 / #504 merge.

Observed impact

The global hook (~/.config/git/hooks/pre-commit) invokes the repo's pre-commit suite. During that run:

  1. test/bats/116-clone-option.bats::_mk_repo ran:

    git -C "$1" init -q
    git -C "$1" -c user.email=t@example.com -c user.name=t commit -q -m init
  2. Because hook-local GIT_DIR / related variables were inherited, the test commit landed in the parent quickstart repository, not $STUBDIR/cloneproj. The worktree reflog records the test identity:

    de6b9b3 ... d28b252 t <t@example.com> ... commit (merge): init
    
  3. The shared repository config gained:

    core.worktree=/private/tmp/qs564-merge
    
  4. A later real kit-fetch path (kit_translate_fetch) ran git fetch + git checkout FETCH_HEAD; because the parent gitdir/worktree had been captured, it checked out the patterns bundle's FETCH_HEAD (56d1f5f, patterns release 1.12.0) into the quickstart worktree. The primary quickstart checkout then appeared as mass deletions plus untracked patterns files.

No push occurred. The remote and local worktrees were repaired, but unstaged local edits could have been lost if not independently preserved.

Root cause

The Bats harness isolates HOME, XDG_CONFIG_HOME, state dirs, and backend knobs, but does not clear Git's repository-local environment before creating a different repository:

# test/bats/helper.bash:41-61
acq_setup_stubs() {
  ...
  export HOME="$STUBDIR/home"
  export XDG_CONFIG_HOME="$STUBDIR/xdg"
  unset GIT_CONFIG_GLOBAL GIT_CONFIG_SYSTEM
  ...
}

Git's documented pattern before operating on another repository is:

unset $(git rev-parse --local-env-vars)

(or an equivalent explicit allowlist/clean environment). git -C changes the working directory but does not override an inherited GIT_DIR.

There is also no fail-closed guard in kit_translate_fetch preventing DESTDIR from resolving to the active repository root before it executes:

cd "$destdir"
rm -rf .git
git init -q
git fetch ...
git checkout -q FETCH_HEAD

The test isolation bug is the trigger; the missing destination guard magnifies it into destructive checkout.

Proposed fix

  1. In the Bats setup, clear repository-local Git environment before any nested fixture repo is created, while retaining intentionally-set identity variables per test:

    while IFS= read -r name; do unset "$name"; done < <(git rev-parse --local-env-vars)

    Use a Bash-3.2-compatible form if process substitution is undesirable.

  2. Add a regression test that runs _mk_repo with GIT_DIR/GIT_WORK_TREE pointing at a sentinel parent repo and proves the parent HEAD/config/worktree do not change.

  3. Defense-in-depth in kit_translate_fetch: reject empty DESTDIR, /, and a destination resolving to the current repository root before rm -rf .git / git init. Test each refusal and verify no Git command runs.

  4. Consider running repo tests from pre-commit with Git-local env cleared centrally in the hook wrapper as an additional boundary, but the repository tests must remain safe on their own.

Acceptance

  • Full pre-commit suite can run from a real Git hook without changing parent HEAD, branch, core.worktree, remotes, or tracked files
  • Nested fixture repos still work in direct bats runs and CI
  • Regression test proves inherited GIT_DIR cannot redirect _mk_repo
  • kit_translate_fetch refuses an empty/root/current-repo destination before destructive Git operations
  • fix(msb): route ssh-agent forward through a managed symlink #564 can be rebased onto main and committed with hooks enabled

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions