Share One Git Location Variable List Across Self-Tests

scripts/local_review.py

@@ -119,6 +119,9 @@
 from datetime import UTC, datetime
 from pathlib import Path
 
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "spec"))
+from git_env import GIT_LOCATION_VARS
+
 RECEIPT_VERSION = 1
 RECEIPT_NAME = "local-review-receipt.json"
 # Kept in the git directory rather than the working tree, so staging the tree does not stage these.
@@ -216,7 +219,7 @@ class CannotRun(Exception):
 # A git hook runs with GIT_INDEX_FILE pointing at the commit it is gating, measured on a real pre-commit hook.
 # A partial commit points it at a pending index rather than the real one.
 # Inheriting that would silently read a different index than the one this branch would push.
-INHERITED_REDIRECTS = ("GIT_INDEX_FILE", "GIT_OBJECT_DIRECTORY", "GIT_ALTERNATE_OBJECT_DIRECTORIES")
+INHERITED_REDIRECTS = GIT_LOCATION_VARS
 
 
 def git(*args: str, root: Path | None = None, env: dict[str, str] | None = None) -> str:

spec/audit.py

@@ -54,6 +54,7 @@
 from typing import Any
 
 import validate  # sibling, import-safe (its main is guarded)
+from git_env import git_location_cleared
 
 ROOT = pathlib.Path(__file__).resolve().parent.parent
 
@@ -3764,6 +3765,15 @@ def read_text(path):
 
 
 def _selftest():
+    """Run the self-test with no variable naming a repository in the environment.
+
+    A git hook sets `GIT_DIR`. A case building a temporary repository would otherwise build it inside the hook's own repository.
+    """
+    with git_location_cleared():
+        return _selftest_cases()
+
+
+def _selftest_cases():
     """Exercise the interface engine on synthetic fixtures, no network - verify the mechanism, not the fleet."""
     pr_check = "  check-workflow-status:\n    name: Check pull request workflow status job\n    needs: [changes]\n    runs-on: ubuntu-latest\n"
     pr_head = "name: Test\non: pull_request\njobs:\n  changes:\n    runs-on: ubuntu-latest\n"

spec/git_env.py

@@ -0,0 +1,50 @@
+"""The environment variables that name a git repository, and the ways to run without them.
+
+A git hook sets some of them (`GIT_DIR`, `GIT_INDEX_FILE`, and the like). A script or test that
+builds a temporary repository with `git -C <tmp>` or `cwd=<tmp>` and inherits them builds it
+inside the hook's own repository instead, and reads its answers back from there.
+
+The list is what `git rev-parse --local-env-vars` prints.
+"""
+
+from __future__ import annotations
+
+import contextlib
+import os
+from collections.abc import Iterator, Mapping
+
+GIT_LOCATION_VARS = (
+    "GIT_ALTERNATE_OBJECT_DIRECTORIES",
+    "GIT_CONFIG",
+    "GIT_CONFIG_PARAMETERS",
+    "GIT_CONFIG_COUNT",
+    "GIT_OBJECT_DIRECTORY",
+    "GIT_DIR",
+    "GIT_WORK_TREE",
+    "GIT_IMPLICIT_WORK_TREE",
+    "GIT_GRAFT_FILE",
+    "GIT_INDEX_FILE",
+    "GIT_NO_REPLACE_OBJECTS",
+    "GIT_REPLACE_REF_BASE",
+    "GIT_PREFIX",
+    "GIT_SHALLOW_FILE",
+    "GIT_COMMON_DIR",
+)
+
+
+def without_git_location(env: Mapping[str, str] | None = None) -> dict[str, str]:
+    """Return a copy of `env` (default: the process environment) with every location variable removed."""
+    source = os.environ if env is None else env
+    return {k: v for k, v in source.items() if k not in GIT_LOCATION_VARS}
+
+
+@contextlib.contextmanager
+def git_location_cleared() -> Iterator[None]:
+    """Remove every location variable from the process environment, restoring them on exit."""
+    cleared = {name: os.environ.pop(name) for name in GIT_LOCATION_VARS if name in os.environ}
+    try:
+        yield
+    finally:
+        for name in GIT_LOCATION_VARS:
+            os.environ.pop(name, None)
+        os.environ.update(cleared)

tests/test_git_env.py

@@ -0,0 +1,100 @@
+#!/usr/bin/env python3
+"""The repository-location variables a self-test must not inherit from a git hook."""
+
+from __future__ import annotations
+
+import os
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+from unittest import mock
+
+REPO = Path(__file__).resolve().parent.parent
+sys.path.insert(0, str(REPO / "spec"))
+import git_env
+
+
+def _git(cwd: Path, *args: str, env: dict[str, str] | None = None) -> str:
+    """A checked git call that reads UTF-8, run with no ambient location variable."""
+    proc = subprocess.run(
+        ["git", *args],
+        cwd=cwd,
+        env=git_env.without_git_location(env),
+        capture_output=True,
+        text=True,
+        encoding="utf-8",
+        check=True,
+    )
+    return proc.stdout
+
+
+class TestLocationList(unittest.TestCase):
+    def test_the_list_covers_every_name_git_calls_local(self) -> None:
+        printed = _git(REPO, "rev-parse", "--local-env-vars").split()
+
+        self.assertEqual(sorted(set(printed) - set(git_env.GIT_LOCATION_VARS)), [])
+
+    def test_without_git_location_drops_only_those_names(self) -> None:
+        env = {"GIT_DIR": "x", "GIT_INDEX_FILE": "y", "GIT_AUTHOR_NAME": "n", "PATH": "p"}
+
+        self.assertEqual(git_env.without_git_location(env), {"GIT_AUTHOR_NAME": "n", "PATH": "p"})
+
+    def test_the_context_manager_restores_the_environment(self) -> None:
+        with mock.patch.dict(os.environ, {"GIT_DIR": "sentinel"}):
+            with git_env.git_location_cleared():
+                self.assertNotIn("GIT_DIR", os.environ)
+            self.assertEqual(os.environ["GIT_DIR"], "sentinel")
+
+
+class TestSetInsideTheContext(unittest.TestCase):
+    def test_a_variable_set_inside_the_context_does_not_outlive_it(self) -> None:
+        with mock.patch.dict(os.environ):
+            os.environ.pop("GIT_INDEX_FILE", None)
+            with git_env.git_location_cleared():
+                os.environ["GIT_INDEX_FILE"] = "inside"
+            self.assertNotIn("GIT_INDEX_FILE", os.environ)
+
+
+@unittest.skipIf(
+    sys.platform == "win32",
+    "audit.py --selftest fails on Windows at its invalid-byte filename probe",
+)
+class TestAuditSelftestIgnoresAmbientGitDir(unittest.TestCase):
+    def test_the_selftest_leaves_the_ambient_repository_unchanged(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            scratch = Path(tmp)
+            _git(scratch, "init", "-q", "--initial-branch=develop", ".")
+            _git(scratch, "config", "user.email", "scratch@example.invalid")
+            _git(scratch, "config", "user.name", "scratch")
+            _git(scratch, "commit", "-q", "--allow-empty", "-m", "seed")
+
+            def state() -> tuple[str, str, str]:
+                return (
+                    _git(scratch, "config", "--local", "--list"),
+                    _git(scratch, "log", "--all", "--format=%H %s"),
+                    _git(scratch, "status", "--porcelain"),
+                )
+
+            before = state()
+            env = dict(os.environ)
+            env["GIT_DIR"] = str(scratch / ".git")
+            env["GIT_WORK_TREE"] = str(scratch)
+            proc = subprocess.run(
+                [sys.executable, str(REPO / "spec" / "audit.py"), "--selftest"],
+                env=env,
+                capture_output=True,
+                text=True,
+                encoding="utf-8",
+                check=False,
+                timeout=600,
+            )
+
+            self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr)
+            self.assertIn("SELFTEST PASS", proc.stdout)
+            self.assertEqual(state(), before)
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_pr_review.py

@@ -34,6 +34,9 @@
 sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "scripts"))
 import pr_review
 
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "spec"))
+from git_env import without_git_location
+
 REPO = Path(__file__).resolve().parent.parent
 RUNBOOK = REPO / ".github" / "copilot-instructions.md"
 GOVERNANCE = REPO / "GOVERNANCE.md"
@@ -11467,21 +11470,14 @@ class TestOriginOwnerIgnoresInheritedEnvironment(unittest.TestCase):
     covered here.
     """
 
-    # The same four discovery names origin_owner() strips, kept local rather than imported from production.
-    _DISCOVERY_VARS = ("GIT_DIR", "GIT_WORK_TREE", "GIT_COMMON_DIR", "GIT_OBJECT_DIRECTORY")
-
     def _clean_env(self) -> dict[str, str]:
         """The caller's environment minus every channel the probe itself drops.
 
         A fixture that left them in place would build its repositories under the same influence
-        the cases exist to rule out, and the two halves have to match for that reason rather
-        than because the setup commands are known to read each one.
+        the cases exist to rule out, so it drops at least what the probe drops, and every other
+        repository-local variable git names besides.
         """
-        return {
-            k: v
-            for k, v in os.environ.items()
-            if k not in self._DISCOVERY_VARS and not k.startswith("GIT_CONFIG")
-        }
+        return {k: v for k, v in without_git_location().items() if not k.startswith("GIT_CONFIG")}
 
     def make_repo(self, parent: Path, name: str, owner_slash_repo: str) -> Path:
         """Build one throwaway repo under `parent`, immune to the caller's own git environment.