diff --git a/scripts/release_control/contract_audit_test.py b/scripts/release_control/contract_audit_test.py index 63feaf500..5efa767eb 100644 --- a/scripts/release_control/contract_audit_test.py +++ b/scripts/release_control/contract_audit_test.py @@ -5,13 +5,12 @@ import tempfile import unittest from contract_audit import audit_contract_payload, parse_args, repo_roots_for_status +from repo_file_io import strip_local_git_env class ContractAuditTest(unittest.TestCase): def git(self, repo_root: Path, *args: str) -> subprocess.CompletedProcess: - env = os.environ.copy() - for name in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR"): - env.pop(name, None) + env = strip_local_git_env(os.environ.copy()) return subprocess.run( ["git", *args], cwd=repo_root, diff --git a/scripts/release_control/format_staged_go_test.py b/scripts/release_control/format_staged_go_test.py index 0e9273e41..500b96224 100644 --- a/scripts/release_control/format_staged_go_test.py +++ b/scripts/release_control/format_staged_go_test.py @@ -6,16 +6,15 @@ from pathlib import Path from unittest.mock import patch from format_staged_go import format_staged_go_files +from repo_file_io import strip_local_git_env class FormatStagedGoTest(unittest.TestCase): def git(self, repo_root: Path, *args: str) -> subprocess.CompletedProcess: - env = os.environ.copy() # Scrub the full hook environment: with only GIT_INDEX_FILE removed, a # pre-commit run from a linked worktree exports an absolute GIT_DIR and # "git init" here re-initializes the REAL repository as bare. - for name in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR"): - env.pop(name, None) + env = strip_local_git_env(os.environ.copy()) return subprocess.run( ["git", *args], cwd=repo_root, diff --git a/scripts/release_control/governance_stage_guard_test.py b/scripts/release_control/governance_stage_guard_test.py index db95d30ab..338ddd8eb 100644 --- a/scripts/release_control/governance_stage_guard_test.py +++ b/scripts/release_control/governance_stage_guard_test.py @@ -5,6 +5,8 @@ import unittest from pathlib import Path from unittest.mock import patch +from repo_file_io import strip_local_git_env + from governance_stage_guard import ( blocked_unstaged_governance_paths, is_worktree_sensitive_governance_path, @@ -14,12 +16,10 @@ from governance_stage_guard import ( class GovernanceStageGuardTest(unittest.TestCase): def git(self, repo_root: Path, *args: str) -> subprocess.CompletedProcess: - env = os.environ.copy() # Scrub the full hook environment: with only GIT_INDEX_FILE removed, a # pre-commit run from a linked worktree exports an absolute GIT_DIR and # "git init" here re-initializes the REAL repository as bare. - for name in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR"): - env.pop(name, None) + env = strip_local_git_env(os.environ.copy()) return subprocess.run( ["git", *args], cwd=repo_root, diff --git a/scripts/release_control/internal/verify_commit_slice.py b/scripts/release_control/internal/verify_commit_slice.py index 86a4fcf88..edd49be4b 100644 --- a/scripts/release_control/internal/verify_commit_slice.py +++ b/scripts/release_control/internal/verify_commit_slice.py @@ -13,11 +13,19 @@ from typing import Iterable REPO_ROOT = Path(__file__).resolve().parents[3] +DEFAULT_REPO_ROOT = REPO_ROOT HOOK_PATH = REPO_ROOT / ".husky" / "pre-commit" def git_env(index_path: Path) -> dict[str, str]: env = os.environ.copy() + # Unit tests patch REPO_ROOT to a temporary repository. In that case, the + # inherited hook environment (the absolute GIT_DIR a pre-commit run from a + # linked worktree exports) must not point git plumbing at a different + # repository than the patched one. + if REPO_ROOT != DEFAULT_REPO_ROOT: + for name in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR"): + env.pop(name, None) env["GIT_INDEX_FILE"] = str(index_path) return env diff --git a/scripts/release_control/internal/verify_commit_slice_test.py b/scripts/release_control/internal/verify_commit_slice_test.py index 547d1a42b..0c15049ac 100644 --- a/scripts/release_control/internal/verify_commit_slice_test.py +++ b/scripts/release_control/internal/verify_commit_slice_test.py @@ -8,16 +8,21 @@ from unittest.mock import patch INTERNAL_DIR = Path(__file__).resolve().parent -if str(INTERNAL_DIR) not in sys.path: - sys.path.insert(0, str(INTERNAL_DIR)) +RELEASE_CONTROL_DIR = INTERNAL_DIR.parent +for extra_dir in (INTERNAL_DIR, RELEASE_CONTROL_DIR): + if str(extra_dir) not in sys.path: + sys.path.insert(0, str(extra_dir)) +from repo_file_io import strip_local_git_env from verify_commit_slice import main, repo_relative_path class VerifyCommitSliceTest(unittest.TestCase): def git(self, repo_root: Path, *args: str, check: bool = True) -> subprocess.CompletedProcess: - env = os.environ.copy() - env.pop("GIT_INDEX_FILE", None) + # Scrub the full hook environment: with only GIT_INDEX_FILE removed, a + # pre-commit run from a linked worktree exports an absolute GIT_DIR and + # "git init" here re-initializes the REAL repository as bare. + env = strip_local_git_env(os.environ.copy()) return subprocess.run( ["git", *args], cwd=repo_root, @@ -101,6 +106,42 @@ class VerifyCommitSliceTest(unittest.TestCase): ): self.assertEqual(main(["--add-updated"]), 7) + def test_scratch_git_init_under_worktree_hook_env_leaves_hook_repo_intact(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + base = Path(tmpdir) + canary = base / "canary" + linked_worktree = base / "canary-worktree" + scratch = base / "scratch" + canary.mkdir() + scratch.mkdir() + + self.init_repo(canary) + (canary / "tracked.txt").write_text("tracked\n", encoding="utf-8") + self.git(canary, "add", "tracked.txt") + self.git(canary, "commit", "--no-verify", "-m", "initial") + self.git(canary, "worktree", "add", str(linked_worktree), "-b", "hook-branch") + + git_dir = self.git( + linked_worktree, "rev-parse", "--path-format=absolute", "--git-dir" + ).stdout.strip() + # A pre-commit run from a linked worktree exports exactly this + # shape: absolute GIT_DIR plus GIT_INDEX_FILE, no GIT_WORK_TREE. + # (With GIT_WORK_TREE also set, "git init" keeps bare=false and + # the corruption does not reproduce.) + hook_env = { + "GIT_DIR": git_dir, + "GIT_INDEX_FILE": str(Path(git_dir) / "index"), + } + + with patch.dict(os.environ, hook_env, clear=False): + self.git(scratch, "init") + + self.assertTrue((scratch / ".git").is_dir()) + config_text = (canary / ".git" / "config").read_text(encoding="utf-8") + self.assertNotIn("bare = true", config_text) + self.git(canary, "status") + self.git(linked_worktree, "status") + if __name__ == "__main__": unittest.main() diff --git a/scripts/release_control/readiness_assertion_guard_test.py b/scripts/release_control/readiness_assertion_guard_test.py index 2c16b5bf8..54b92e594 100644 --- a/scripts/release_control/readiness_assertion_guard_test.py +++ b/scripts/release_control/readiness_assertion_guard_test.py @@ -7,6 +7,7 @@ from pathlib import Path from unittest import mock import readiness_assertion_guard +from repo_file_io import strip_local_git_env def write_status(repo_root: Path, payload: dict) -> None: @@ -35,12 +36,10 @@ def base_payload() -> dict: class ReadinessAssertionGuardTest(unittest.TestCase): def git(self, repo_root: Path, *args: str) -> subprocess.CompletedProcess: - env = os.environ.copy() # Scrub the full hook environment: with only GIT_INDEX_FILE removed, a # pre-commit run from a linked worktree exports an absolute GIT_DIR and # "git init" here re-initializes the REAL repository as bare. - for name in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR"): - env.pop(name, None) + env = strip_local_git_env(os.environ.copy()) return subprocess.run( ["git", *args], cwd=repo_root, diff --git a/scripts/release_control/repo_file_io_test.py b/scripts/release_control/repo_file_io_test.py index f416d53ea..abf102756 100644 --- a/scripts/release_control/repo_file_io_test.py +++ b/scripts/release_control/repo_file_io_test.py @@ -1,5 +1,6 @@ import os import json +import re import subprocess import tempfile import unittest @@ -249,6 +250,29 @@ class RepoFileIoTest(unittest.TestCase): self.assertEqual(canonical_repo_id(linked_worktree), "pulse") self.assertEqual(canonical_workspace_repos_root(linked_worktree), (workspace / "repos").resolve()) + def test_scratch_git_init_tests_scrub_env_through_shared_helper(self) -> None: + # Running "git init" in a scratch directory while the pre-commit hook + # environment from a linked worktree (absolute GIT_DIR et al.) is still + # exported re-initializes the REAL repository with core.bare=true, + # breaking git for every checkout. Two rounds of per-file fixes each + # missed a straggler, so every release-control test that creates + # scratch repos must route its git env through strip_local_git_env. + release_control_dir = Path(__file__).resolve().parent + scratch_init = re.compile(r"\.git\([^)]*\"init\"|\[\s*\"git\",\s*\"init\"") + offenders = [] + for test_file in sorted(release_control_dir.rglob("*_test.py")): + source = test_file.read_text(encoding="utf-8") + if not scratch_init.search(source): + continue + if "strip_local_git_env" not in source: + offenders.append(test_file.relative_to(release_control_dir).as_posix()) + self.assertEqual( + offenders, + [], + "these test files run scratch 'git init' without scrubbing the " + "inherited hook git env via repo_file_io.strip_local_git_env", + ) + if __name__ == "__main__": unittest.main() diff --git a/scripts/release_control/status_audit_test.py b/scripts/release_control/status_audit_test.py index a946d3ac2..d50578c8e 100644 --- a/scripts/release_control/status_audit_test.py +++ b/scripts/release_control/status_audit_test.py @@ -10,6 +10,7 @@ from pathlib import Path from unittest import mock import status_audit +from repo_file_io import strip_local_git_env from status_audit import ( RC_READY_ASSERTIONS_BLOCKER, RC_RELEASE_GATES_BLOCKER, @@ -323,9 +324,7 @@ def base_payload( class StatusAuditTest(unittest.TestCase): def git(self, repo_root: Path, *args: str) -> subprocess.CompletedProcess: - env = os.environ.copy() - for name in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR"): - env.pop(name, None) + env = strip_local_git_env(os.environ.copy()) return subprocess.run( ["git", *args], cwd=repo_root, diff --git a/scripts/release_control/subsystem_contracts_test.py b/scripts/release_control/subsystem_contracts_test.py index cea26fef1..9d621268f 100644 --- a/scripts/release_control/subsystem_contracts_test.py +++ b/scripts/release_control/subsystem_contracts_test.py @@ -5,6 +5,8 @@ from pathlib import Path import subprocess import tempfile +from repo_file_io import strip_local_git_env + from subsystem_contracts import ( contract_reference_matches_path, load_contract_index, @@ -17,12 +19,10 @@ from subsystem_contracts import ( class SubsystemContractsTest(unittest.TestCase): def git(self, repo_root: Path, *args: str) -> subprocess.CompletedProcess: - env = os.environ.copy() # Scrub the full hook environment: with only GIT_INDEX_FILE removed, a # pre-commit run from a linked worktree exports an absolute GIT_DIR and # "git init" here re-initializes the REAL repository as bare. - for name in ("GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR"): - env.pop(name, None) + env = strip_local_git_env(os.environ.copy()) return subprocess.run( ["git", *args], cwd=repo_root,