Prevent git rebase --exec test runs from corrupting the enclosing worktree#2157
Prevent git rebase --exec test runs from corrupting the enclosing worktree#2157
git rebase --exec test runs from corrupting the enclosing worktree#2157Conversation
There was a problem hiding this comment.
Hoping this comment shows up correctly in the PR. ed: it did not! This relates to d893220.
The commit explanation may be misleading with (which is not the case for non-bare repositories). core.bare is present and false (at least in new repos). The test only works because the test is normally run in a git repo. If the working directory/tree is copied to another directory without the .git directory, the test will fail.
|
This relates to d2113d3. It is unfortunate that this and the previous commit are needed. It would be nice if you could just tell git not to search beyond the directory. |
The "finds core.bare" test was a gitConfig() smoke test added in a2ad7db (Start testing the new classes, 2018-05-08) that happened to read core.bare from the enclosing repository. This is exactly the kind of accidental access to the enclosing repo that we want to prevent, and it also only passed by accident: While `git init` _does_ set that config variable, users are at liberty to unset it (because Git considers it to default to "false"). Replace the `core.bare` test with one that reads one of the GIT_CONFIG_* environment variables already set up at the top of the same file, which still exercises gitConfig() without depending on the enclosing repository. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When tests are run from within `git rebase --exec` (e.g. via `exec rm -rf node_modules/ build/ && npm ci && npm run build && npm test`), git sets GIT_DIR in the environment, pointing to the worktree's gitdir. This leaked into child processes spawned by the test infrastructure, causing `git init <target>` to silently reinitialize the *real* repository instead of creating a new one in the target directory. The result was `core.bare` getting flipped in the shared config and test commits, refs and notes landing on the real HEAD. The fix is to delete GIT_DIR and GIT_WORK_TREE from the environment at the very start of testCreateRepo, before any git commands are executed. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When running `npm test` from a Git worktree, test-spawned git processes that happened to operate at or above `.test-dir/` would walk up the directory tree and discover the enclosing real repository. In a worktree setup this is particularly dangerous because the `.git/config` is shared across all worktrees, so any accidental config write (e.g. `core.bare`) affects every checkout. Prevent this by setting `GIT_CEILING_DIRECTORIES` to every ancestor of the `.test-dir/` temporary directory. The temp dir itself is excluded so that git can still discover the test repositories created inside it. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The GIT_CEILING_DIRECTORIES fix in the preceding commit should be sufficient to prevent test-spawned git processes from discovering the enclosing repository. However, when that protection was put to the test by running `rm -rf node_modules/ build/ && npm ci && npm run build && npm test` from a worktree, the test commits still ended up on the real worktree's HEAD for reasons that could not be fully diagnosed. Since the consequences of this happening are severe (the primary worktree was marked as `core.bare = true` and test commits, notes refs, and fake upstream refs all landed on the real repository), an additional layer of defense is warranted. Introduce a `validateWorkDir()` function in `lib/git.ts` that is called from every function that spawns a git process (`git()`, `revParse()`, `revListCount()`, `gitConfig()`, `gitConfigForEach()`, `gitCommandExists()`). When the `GIT_WORK_DIR_PREFIX` environment variable is set, the function verifies that the resolved `workDir` falls inside the expected directory, and throws otherwise. For `git init <path>` and `git --git-dir=<path>` invocations, the target path from the arguments is validated instead of the cwd, since those commands legitimately operate on a path specified as an argument rather than through the `workDir` option. In `testCreateRepo()`, the guard is suspended during the setup phase (which needs to run `git init` and `git config --global` without a test repo `workDir`), then activated by setting `GIT_WORK_DIR_PREFIX` to the `.test-dir/` path before returning. This ensures that all subsequent `git()` calls in the test process must use a `workDir` inside the test directory, or fail loudly. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When a previous test run leaves stale files inside .test-dir/ (e.g. copies of the real worktree after a corrupted run that operated on the wrong repository), Jest would discover and attempt to run .test.ts files found there, producing confusing ENOENT failures. Tell Jest to ignore .test-dir/ entirely. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Indeed, you're right: https://github.com/git/git/blob/v2.53.0/setup.c#L2463-L2466. I must have suffered a side effect when I tried to confirm this behavior. I fixed the commit message! Range-diff
Yes. The |
0d593b3 to
bfa3684
Compare
webstech
left a comment
There was a problem hiding this comment.
Thanks. The git rebase --exec was new to me.
…rupting the enclosing worktree (#2157), 2026-03-04))
…rupting the enclosing worktree (gitgitgadget#2157), 2026-03-04))
Running
git rebase -x 'npm test'from a worktree wreaked havoc on myreal repository:
core.bare = trueand testurl.*.insteadOfsettingsended up in the shared
.git/config, test commits landed on the realHEAD, and a bogus
refs/notes/gitgitgadgetref appeared out of nowhere.For a terrifying moment I thought I had lost actual work.
The root cause turned out to be
git rebase --execsettingGIT_DIRinthe environment. This leaked into the Node.js test processes, causing
git init <target>to silently reinitialize the real repositoryinstead of creating a fresh one in the target directory -- git simply
ignores the target argument when
GIT_DIRis set.Diagnosing this was a bit trickier than I hoped for. Adding
GIT_CEILING_DIRECTORIESalone did not help becauseGIT_DIRtakesprecedence over repository discovery, making ceiling directories
irrelevant. A secondary leak hid in
misc-helper.test.tswhichcaptured
process.envat module load time, beforetestCreateRepohada chance to clear the offending variables.
This series clears
GIT_DIR/GIT_WORK_TREEearly, addsGIT_CEILING_DIRECTORIESas defense-in-depth, introduces avalidateWorkDir()safety net that fails loudly if git would operateoutside the test directory, and fixes a pre-existing test that
accidentally depended on the enclosing repo's config. It also tells Jest
to ignore
.test-dir/so stale files from corrupted runs don't causeconfusing failures on the next run.