From 1e2865acbd4ca048e3731bf58e2f60aa5501763e Mon Sep 17 00:00:00 2001 From: kert Date: Tue, 27 Oct 2026 10:54:58 -0400 Subject: [PATCH] fix(hooks): run pre-commit pytest without git's repo-locating env git exports GIT_DIR and GIT_INDEX_FILE to hooks. The pre-commit pytest step inherited them, and tests/dev/test_gen_config_deploy.py's scratch repos (git init --bare / clone / commit / push in tmp dirs) therefore ran against the repository being committed to: on 2026-10-27 a commit that staged infra files left its branch on nine "base / docs only / mc config" commits by author `t`, swapped the index for the fixture's tree, and wrote uploadpack.allowAnySHA1InWant into the .git/config that every worktree shares. It only stopped there because `git push origin` found no `origin` remote. - sem.hooks: the pytest step gets the hook env minus GIT_DIR, GIT_INDEX_FILE, GIT_WORK_TREE, GIT_PREFIX, GIT_COMMON_DIR and the object-dir variables. - test_gen_config_deploy: its git helper and detector runner drop all GIT_* from os.environ, so any wrapper that exports them is safe too. - Regression test: a nested `git init` under a hook-style GIT_DIR lands in its own directory. Refs #1365 --- src/sem/hooks.py | 22 ++++++++++++++++++++++ tests/dev/test_gen_config_deploy.py | 11 +++++++++-- tests/sem/test_hooks.py | 23 +++++++++++++++++++++++ tests/test_final_coverage_gaps.py | 2 +- tests/test_remaining_gaps.py | 6 +++--- 5 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/sem/hooks.py b/src/sem/hooks.py index ae62bda..ab28327 100644 --- a/src/sem/hooks.py +++ b/src/sem/hooks.py @@ -233,6 +233,27 @@ def compute_test_targets( return deduped, ", ".join(reasons) if reasons else "changed files" +# Variables git exports to hooks that pin every nested git call to *this* +# repository. Tests that build scratch repos in tmp dirs inherited them and +# committed into the repo being committed to (and wrote its shared config). +_HOOK_GIT_VARS = ( + "GIT_DIR", + "GIT_INDEX_FILE", + "GIT_WORK_TREE", + "GIT_PREFIX", + "GIT_COMMON_DIR", + "GIT_OBJECT_DIRECTORY", + "GIT_ALTERNATE_OBJECT_DIRECTORIES", +) + + +def _test_env() -> dict[str, str]: + """The hook's environment minus git's repo-locating variables.""" + import os + + return {k: v for k, v in os.environ.items() if k not in _HOOK_GIT_VARS} + + def run_step(label: str, cmd: list[str], **kwargs) -> int: """Run a subprocess step, printing the label.""" print(f"==> pre-commit: {label}") @@ -318,6 +339,7 @@ def main() -> int: ["uv", "run", "python", "-m", "pytest"] + targets + ["--no-cov", "--tb=short", "-q", "-n", "auto"], + env=_test_env(), ) if rc != 0: return rc diff --git a/tests/dev/test_gen_config_deploy.py b/tests/dev/test_gen_config_deploy.py index 99b656e..fed657e 100644 --- a/tests/dev/test_gen_config_deploy.py +++ b/tests/dev/test_gen_config_deploy.py @@ -104,9 +104,16 @@ def test_image_jobs_gate_on_changes_outputs_not_head_commit(): # ── Behavioural: run the detector script against real git history ────── +def _clean_env() -> dict[str, str]: + """os.environ without GIT_* — under a pre-commit hook GIT_DIR and + GIT_INDEX_FILE point at the real repo, and every scratch-repo command + here would commit into it instead.""" + return {k: v for k, v in os.environ.items() if not k.startswith("GIT_")} + + def _git(*args: str, cwd: Path) -> str: env = { - **os.environ, + **_clean_env(), "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@example.invalid", "GIT_COMMITTER_NAME": "t", @@ -170,7 +177,7 @@ def _run_detector(checkout: Path, before: str, sha: str, tmp_path: Path) -> dict ["bash", "-e", "-c", step["run"]], cwd=checkout, env={ - **os.environ, + **_clean_env(), "BEFORE": before, "GITHUB_SHA": sha, "GITHUB_OUTPUT": str(out_file), diff --git a/tests/sem/test_hooks.py b/tests/sem/test_hooks.py index 23cd8aa..d479175 100644 --- a/tests/sem/test_hooks.py +++ b/tests/sem/test_hooks.py @@ -9,6 +9,7 @@ from sem.hooks import ( _changed_test_dirs, _classify, _staged_files, + _test_env, _top_level_tests, check_syntax, compute_test_targets, @@ -195,6 +196,28 @@ class TestRunStep: assert rc == 1 +class TestTestEnv: + def test_strips_repo_locating_git_vars(self, monkeypatch): + monkeypatch.setenv("GIT_DIR", "/repo/.git") + monkeypatch.setenv("GIT_INDEX_FILE", "/repo/.git/index") + monkeypatch.setenv("GIT_AUTHOR_NAME", "kept") + env = _test_env() + assert "GIT_DIR" not in env + assert "GIT_INDEX_FILE" not in env + assert env["GIT_AUTHOR_NAME"] == "kept" + + def test_nested_git_sees_its_own_repo(self, tmp_path, monkeypatch): + """Under a hook, `git init` in a tmp dir must not land in GIT_DIR.""" + import subprocess + + monkeypatch.setenv("GIT_DIR", str(tmp_path / "hooked.git")) + scratch = tmp_path / "scratch" + scratch.mkdir() + subprocess.run(["git", "init", "-q"], cwd=scratch, env=_test_env(), check=True) + assert (scratch / ".git").is_dir() + assert not (tmp_path / "hooked.git").exists() + + class TestMain: @patch("sem.hooks._staged_files", return_value=[]) def test_no_staged_files_returns_zero(self, mock_staged): diff --git a/tests/test_final_coverage_gaps.py b/tests/test_final_coverage_gaps.py index 5f1b6ca..033afcb 100644 --- a/tests/test_final_coverage_gaps.py +++ b/tests/test_final_coverage_gaps.py @@ -1470,7 +1470,7 @@ class TestSemHooksMainEntry: ): call_count = [0] - def step_side_effect(label, cmd): + def step_side_effect(label, cmd, **_kw): call_count[0] += 1 if "notebook run" in label: return 1 # line 348 diff --git a/tests/test_remaining_gaps.py b/tests/test_remaining_gaps.py index 401baad..177fd06 100644 --- a/tests/test_remaining_gaps.py +++ b/tests/test_remaining_gaps.py @@ -100,7 +100,7 @@ class TestSemHooksFormatCheckFails: call_count = [0] - def step_side_effect(label, cmd): + def step_side_effect(label, cmd, **_kw): call_count[0] += 1 if "format" in label: return 1 @@ -144,7 +144,7 @@ class TestSemHooksPytestFails: call_count = [0] - def step_side_effect(label, cmd): + def step_side_effect(label, cmd, **_kw): call_count[0] += 1 if "pytest" in label: return 1 @@ -170,7 +170,7 @@ class TestSemHooksNotebookFail: call_count = [0] - def step_side_effect(label, cmd): + def step_side_effect(label, cmd, **_kw): call_count[0] += 1 if "marimo" in label: return 1