mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
fix(release-control): scrub hook git env in remaining scratch-repo git users
The core.bare=true corruption of the shared repository recurred on 2026-07-17: a script that runs scratch git commands while the pre-commit environment from a linked worktree (absolute GIT_DIR) is still exported re-initializes the REAL repository as bare. a0fda6b26 fixed four helper test files but missed two spots in scripts/release_control/internal: - verify_commit_slice_test.py's git() helper only popped GIT_INDEX_FILE, so its scratch 'git init' calls re-init the real repo when GIT_DIR is inherited. It now scrubs via the shared repo_file_io.strip_local_git_env. - verify_commit_slice.py's production git_env() kept the inherited hook env even when unit tests patch REPO_ROOT to a temporary repository, pointing git plumbing (including index writes) at the wrong repo. It now scrubs in the test-patched branch only, matching format_staged_go. Regression teeth: - verify_commit_slice_test.py gains a canary test that exports the real hook env shape (absolute GIT_DIR + GIT_INDEX_FILE, no GIT_WORK_TREE — with GIT_WORK_TREE set the corruption does not reproduce) against a scratch repo + linked worktree and asserts core.bare stays false. - repo_file_io_test.py (runs in the pre-commit battery) gains a static guard failing any release-control *_test.py that runs scratch 'git init' without referencing strip_local_git_env. - The six hand-rolled 4-var pop loops from a0fda6b26 migrate to the shared strip_local_git_env helper so the guard enforces one pattern. Verified: full release-control battery green; every touched test file also green with GIT_DIR/GIT_INDEX_FILE pointed at a canary repo's linked worktree, canary config and status intact afterward.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user