test(cli): fix git env-var leak in git-repo integration tests#57
Open
TAJD wants to merge 2 commits into
Open
Conversation
…t helpers advise_diff.rs and orphan_since_report_scope.rs shell out to `git init`/ `git commit` in a disposable temp dir. When these tests run inside a git hook invoked from a linked worktree (e.g. pre-push), git exports GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE pointing at the real repo, and those leak into the child `git` calls -- hijacking them away from the temp dir and onto the real repo's ref. Clearing the three vars before each `git` invocation keeps the tests isolated regardless of caller environment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CpKPA1x3bodpMELHY6ag3U
The prior commit fixed the test harness's own `git init`/`git commit` calls but missed that these tests also spawn the `cofferdam` binary itself, which shells out to git internally for --diff/--since. That inherited the same leaked GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE, making cofferdam analyze the real repo's history instead of the test's temp repo and report zero findings. Route every advise_diff.rs invocation through a new run_cofferdam() helper that clears the same three vars, and do the same for orphan_since_report_scope.rs's single call site. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CpKPA1x3bodpMELHY6ag3U
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
1 participant
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.
Summary
advise_diff.rsandorphan_since_report_scope.rsbuild a disposable git repo in a temp dir and then shell out togitand to thecofferdambinary itself (which internally shells out togitfor--diff/--since).pre-push), git exportsGIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILEpointing at the real repository. Those vars leaked into the childgitcalls, hijackinggit init/git commitaway from the test's temp dir and onto the real repo's branch ref — in one run this actually repointed this worktree's own branch at a bogus 1-file commit (recovered locally viagit reset --mixedback onto the real commit; no data was lost since blobs/commits stayed in the object store).gitinvocation in the test helpers, and add arun_cofferdam()helper inadvise_diff.rsthat does the same for the binary-under-test, since it shells out to git too.Discovered while verifying
crates/cofferdam-engine's CD-6 perf fixes (Arc-wrapped cache values, corpus deep-clone removal, lock-poison recovery) already merged tomain— this PR only contains the test-harness follow-up.Test plan
cargo build --workspacecargo test --workspace(advise_diff + orphan_since_report_scope pass in isolation and via the pre-push hook)cargo clippy --workspace --all-targets -- -D warningscargo fmt --all -- --check🤖 Generated with Claude Code
https://claude.ai/code/session_01CpKPA1x3bodpMELHY6ag3U