diff --git a/repo_policy_sync/docs/explanation/execution-model.md b/repo_policy_sync/docs/explanation/execution-model.md index 507967f..bb09268 100644 --- a/repo_policy_sync/docs/explanation/execution-model.md +++ b/repo_policy_sync/docs/explanation/execution-model.md @@ -48,7 +48,10 @@ owns one deterministic branch and one pull request per repository. mode leaves the pull request open. Pre-commit runs use a credential-reduced environment and temporary home - directory. Hooks are still arbitrary repository code, so apply mode requires + directory. To keep authenticated nested Git fetches working, the runner + copies only global `url.*.insteadOf` rewrites into a temporary Git config; + GitHub token variables and unrelated user configuration are not passed to + hooks. Hooks are still arbitrary repository code, so apply mode requires trusted target repositories. Checkout synchronization and each policy's repository processing run in diff --git a/repo_policy_sync/docs/how-to/run-a-policy.md b/repo_policy_sync/docs/how-to/run-a-policy.md index 92e2842..5419fa1 100644 --- a/repo_policy_sync/docs/how-to/run-a-policy.md +++ b/repo_policy_sync/docs/how-to/run-a-policy.md @@ -120,10 +120,11 @@ workflow must use the `plan` command and must not use `apply`. Apply mode runs the target repository's configured pre-commit hooks on the policy-changed paths before publishing changes. Treat apply mode as -trusted-repository execution: -repository hooks can execute arbitrary code. The runner removes the usual -GitHub token and user configuration environment, disables Git prompts, and -uses a temporary home directory, but this is not a sandbox. +trusted-repository execution: repository hooks can execute arbitrary code. The +runner removes the usual GitHub token and user configuration environment, +disables Git prompts, and uses a temporary home directory. It copies only +global `url.*.insteadOf` rewrites into a temporary Git config so authenticated +nested Git fetches continue to work; this is not a sandbox. For CI or another programmatic consumer, write the versioned JSON report to a file while retaining the standard table output: diff --git a/repo_policy_sync/src/github.py b/repo_policy_sync/src/github.py index 20c8acd..5376c77 100644 --- a/repo_policy_sync/src/github.py +++ b/repo_policy_sync/src/github.py @@ -46,6 +46,8 @@ "USER", "LOGNAME", } +_GIT_URL_REWRITE_KEY = re.compile(r"^url\..*\.insteadof$", re.IGNORECASE) +_GIT_URL_REWRITE_PATTERN = r"^url\..*\.insteadof$" PULL_REQUEST_TEMPLATE_PLACEHOLDERS = ( "policy_id", "policy_description", @@ -59,6 +61,81 @@ _PULL_REQUEST_TEMPLATE_PLACEHOLDER = re.compile(r"\{\{([^{}]*)\}\}") +def _copy_global_git_url_rewrites(home: Path) -> Path | None: + """Copy global Git URL rewrites into an isolated configuration file. + + Apply workflows commonly authenticate private dependencies by configuring + a token-bearing `url.*.insteadOf` rule globally. Pre-commit gets a fresh + home directory and system configuration is disabled, so those rules would + otherwise disappear before a hook starts Bazel or another nested Git + client. Only URL rewrites are copied: user identity, aliases, credential + helpers, and unrelated Git configuration remain unavailable to hooks. + """ + + rewrites = _read_global_git_url_rewrites() + if not rewrites: + return None + + config = home / ".gitconfig" + for key, value in rewrites: + result = subprocess.run( + ["git", "config", "--file", str(config), "--add", key, value], + check=False, + capture_output=True, + text=True, + ) + if result.returncode != 0: + detail = result.stderr.strip() or result.stdout.strip() + detail = redact_sensitive_text(detail) + raise CommandError( + "could not prepare Git URL rewrites for pre-commit" + + (f": {detail}" if detail else "") + ) + return config + + +def _read_global_git_url_rewrites() -> tuple[tuple[str, str], ...]: + """Read only global Git URL rewrites without inheriting user config.""" + + try: + result = subprocess.run( + [ + "git", + "config", + "--global", + "--get-regexp", + _GIT_URL_REWRITE_PATTERN, + ], + check=False, + capture_output=True, + text=True, + ) + except OSError: + # The regular pre-commit command will report a missing Git executable + # in the usual way. A missing executable must not turn this optional + # credential hand-off into a less useful error. + return () + + # `git config --get-regexp` returns 1 when no key matches. Other failures + # indicate that the existing Git configuration could not be inspected and + # should not be silently converted into an authentication failure later. + if result.returncode == 1: + return () + if result.returncode != 0: + detail = result.stderr.strip() or result.stdout.strip() + detail = redact_sensitive_text(detail) + raise CommandError( + "could not read global Git URL rewrites" + (f": {detail}" if detail else "") + ) + + rewrites: list[tuple[str, str]] = [] + for line in result.stdout.splitlines(): + key, separator, value = line.partition(" ") + if separator and _GIT_URL_REWRITE_KEY.fullmatch(key): + rewrites.append((key, value)) + return tuple(rewrites) + + @dataclass(frozen=True) class PullRequest: number: int @@ -420,6 +497,12 @@ def run_pre_commit( if key in _PRE_COMMIT_ENVIRONMENT_KEYS or key.startswith("LC_") } with tempfile.TemporaryDirectory(prefix=f"{TOOL_SLUG}-pre-commit-") as home: + # The normal apply workflow authenticates nested Git fetches by + # installing a narrowly scoped `url.*.insteadOf` rewrite in the + # runner's global Git configuration. Keep that mechanism + # available to trusted hooks while preserving the isolated home + # directory and reduced environment used for pre-commit. + git_config = _copy_global_git_url_rewrites(Path(home)) environment.update( { "HOME": home, @@ -429,6 +512,8 @@ def run_pre_commit( "GIT_TERMINAL_PROMPT": "0", } ) + if git_config is not None: + environment["GIT_CONFIG_GLOBAL"] = str(git_config) command = ["pre-commit", "run", "--all-files"] if paths is not None: command = ["pre-commit", "run", "--files", *paths] diff --git a/repo_policy_sync/tests/test_github.py b/repo_policy_sync/tests/test_github.py index 5d6ba8b..8baf49e 100644 --- a/repo_policy_sync/tests/test_github.py +++ b/repo_policy_sync/tests/test_github.py @@ -232,6 +232,86 @@ def record( assert observed["GIT_TERMINAL_PROMPT"] == "0" +def test_pre_commit_carries_only_global_git_url_rewrites( + monkeypatch, tmp_path: Path +) -> None: + """Authenticated nested Git fetches work without exposing user config.""" + + (tmp_path / ".pre-commit-config.yaml").write_text("repos: []\n") + user_home = tmp_path / "user-home" + user_home.mkdir() + global_config = user_home / ".gitconfig" + subprocess.run( + [ + "git", + "config", + "--file", + str(global_config), + "--add", + "url.https://x-access-token:github-token@github.com/.insteadOf", + "https://github.com/", + ], + check=True, + ) + subprocess.run( + [ + "git", + "config", + "--file", + str(global_config), + "user.name", + "test user", + ], + check=True, + ) + monkeypatch.setenv("HOME", str(user_home)) + monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "1") + monkeypatch.delenv("GIT_CONFIG_GLOBAL", raising=False) + observed: dict[str, str] = {} + + def record( + command: list[str], + *, + cwd: Path | None = None, + env: dict[str, str] | None = None, + ) -> str: + if command[0] == "pre-commit": + assert env is not None + observed.update(env) + config = Path(env["GIT_CONFIG_GLOBAL"]) + rewrites = subprocess.run( + [ + "git", + "config", + "--file", + str(config), + "--get-regexp", + "^url\\..*\\.insteadof$", + ], + check=True, + capture_output=True, + text=True, + ) + assert "github-token" in rewrites.stdout + assert ( + subprocess.run( + ["git", "config", "--file", str(config), "--get", "user.name"], + check=False, + capture_output=True, + text=True, + ).returncode + != 0 + ) + return "" + + monkeypatch.setattr(GitHubCli, "_run", staticmethod(record)) + + assert GitHubCli().run_pre_commit(checkout=tmp_path) + + assert observed["GIT_CONFIG_NOSYSTEM"] == "1" + assert observed["GIT_TERMINAL_PROMPT"] == "0" + + def test_pre_commit_failure_stops_commit_and_push(monkeypatch, tmp_path: Path) -> None: (tmp_path / ".pre-commit-config.yaml").write_text("repos: []\n") (tmp_path / ".gitignore").write_text("\n")