From 3b2e50b15c6afba3ff3b8fbe1bdf0a7a69d29f7b Mon Sep 17 00:00:00 2001 From: Will Pfleger Date: Wed, 23 Sep 2026 12:13:18 -0400 Subject: [PATCH] fix(hooks): strip repo-local git env from pre-push test lanes (#7841) Git exports repository-local variables (`GIT_DIR`, `GIT_INDEX_FILE`, `GIT_WORK_TREE`, ...) into hook processes, and in linked worktrees `GIT_DIR` is absolute. Pre-push lanes inherited that environment unchanged, so any test fixture that spawns `git` in a temp dir (`git init --bare `, `git config user.name ...`, `git commit`, `git push`) actually operated on the checkout being pushed. `current_dir(tmp)` and `git -C tmp` don't escape an inherited `GIT_DIR`. Because `.git/config` is shared by every worktree, one push from any worktree could silently rewrite the repo for all of them. In practice this set `core.bare=true` (breaking the main checkout with `fatal: this operation must be run in a work tree`), replaced the committer identity with a fixture `user.name=Agent`, disabled `commit.gpgSign`, and left fixture aliases behind. The fixtures that did this are the git-identity enforcement tests in #6177, which run in the `rust-tests` lane via `just test-unit`. The pre-push lanes that run test suites or builds now go through `scripts/without-git-env.sh`, which unsets the variables listed by `git rev-parse --local-env-vars` (git's own definition of repo-local state, the same set `git submodule` clears) and then `exec`s the lane, so each subprocess rediscovers the repository from its working directory. Transport, credential, and TLS settings (`GIT_SSH_COMMAND`, `GIT_ASKPASS`, `GIT_TERMINAL_PROMPT`, `GIT_SSL_*`) pass through untouched. Putting this at the hook boundary covers every current and future test without each suite having to defend itself. The `branch-skew` and `push-head-scope` guards run unwrapped: they spawn no fixtures, and `branch-skew`'s `git fetch` needs the caller's transport environment. `pre-commit` isn't wrapped either, since its `stage_fixed: true` lanes need the hook's `GIT_INDEX_FILE` to restage into the temporary index during `git commit `. Related: #6177 --------- Signed-off-by: Will Pfleger --- lefthook.yml | 19 ++++++++++++------- scripts/without-git-env.sh | 10 ++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) create mode 100755 scripts/without-git-env.sh diff --git a/lefthook.yml b/lefthook.yml index 4dc05016ef..fc7e2f9ba9 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -75,6 +75,11 @@ commit-msg: signoff: run: 'git interpret-trailers --if-exists doNothing --trailer "Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed ''s/ [0-9]* [+-][0-9]*$//'')" --in-place {1}' +# Lanes below that run test suites or builds go through +# scripts/without-git-env.sh, which strips git's repository-local variables +# (per `git rev-parse --local-env-vars`) while preserving transport/credential +# settings. The branch-skew and push-head-scope guards run no test fixtures +# and need the caller's transport env, so they run unwrapped. pre-push: parallel: true commands: @@ -90,36 +95,36 @@ pre-push: file-size-check: # The ratchet computes its own merge-base diff, so path filtering here # would only duplicate policy and create another place for coverage drift. - run: just file-size-check + run: ./scripts/without-git-env.sh just file-size-check rust-tests: files: git diff --name-only origin/main...HEAD glob: ["crates/**", "migrations/**", "schema/**", "Cargo.toml", "Cargo.lock", "rust-toolchain.toml", "deny.toml", "scripts/run-tests.sh", "Justfile"] - run: just test-unit + run: ./scripts/without-git-env.sh just test-unit desktop-check: files: git diff --name-only origin/main...HEAD glob: ["desktop/**", "pnpm-lock.yaml"] exclude: ["desktop/src-tauri/**"] - run: just desktop-check + run: ./scripts/without-git-env.sh just desktop-check desktop-typecheck: files: git diff --name-only origin/main...HEAD glob: ["desktop/**", "pnpm-lock.yaml"] exclude: ["desktop/src-tauri/**"] - run: just desktop-typecheck + run: ./scripts/without-git-env.sh just desktop-typecheck desktop-test: files: git diff --name-only origin/main...HEAD glob: ["desktop/**", "pnpm-lock.yaml"] exclude: ["desktop/src-tauri/**"] - run: just desktop-test + run: ./scripts/without-git-env.sh just desktop-test desktop-tauri-checks: # Keep local lint parity with Desktop Core CI for every path that can # affect the Tauri crate or its path dependencies. Run clippy and tests # serially so parallel pre-push hooks do not contend for Cargo's lock. files: git diff --name-only origin/main...HEAD glob: ["desktop/src-tauri/**", "crates/**", "migrations/**", "schema/**", "Cargo.toml", "Cargo.lock", "rust-toolchain.toml", "deny.toml", "scripts/run-tests.sh", "Justfile"] - run: just desktop-tauri-clippy && just desktop-tauri-test + run: ./scripts/without-git-env.sh sh -c 'just desktop-tauri-clippy && just desktop-tauri-test' mobile-checks: # Run analysis and tests serially so parallel pre-push hooks do not # contend for shared Flutter state (.dart_tool, shader bundle). files: git diff --name-only origin/main...HEAD glob: ["mobile/**"] - run: just mobile-check && just mobile-test + run: ./scripts/without-git-env.sh sh -c 'just mobile-check && just mobile-test' diff --git a/scripts/without-git-env.sh b/scripts/without-git-env.sh new file mode 100755 index 0000000000..236b948d8c --- /dev/null +++ b/scripts/without-git-env.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash +# Git exports repository-local variables (GIT_DIR, GIT_INDEX_FILE, ...) to +# hooks. Pre-push lanes run test suites whose fixtures spawn git in temp dirs; +# with those variables inherited, fixtures operate on this checkout instead. +# Transport, credential, and TLS settings (GIT_SSH_COMMAND, GIT_ASKPASS, ...) +# are left intact. +set -euo pipefail +# shellcheck disable=SC2046 +unset $(git rev-parse --local-env-vars) +exec "$@"