From 907b06fb283d0a6b418248499b9de5f907327cf7 Mon Sep 17 00:00:00 2001 From: ethernet Date: Thu, 24 Sep 2026 13:55:42 -0400 Subject: [PATCH] test(git-safety): parse the git config key instead of substring-matching argv The checkout guard decided whether `git config` writes a URL rewrite by substring-matching every argument, so `url..pushInsteadOf` and the Git 2.46 `git config set ` form slipped through, while a value that merely contained "url." could trip it. Find the key positionally (skipping option values and the subcommand) and match insteadOf/pushInsteadOf on it. --- tests/git_safety.py | 31 ++++++++++++++++++++++++++++--- tests/test_git_safety_boundary.py | 4 +++- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/tests/git_safety.py b/tests/git_safety.py index 1f828ff5c6..56e86814a1 100644 --- a/tests/git_safety.py +++ b/tests/git_safety.py @@ -12,6 +12,12 @@ _MUTATIONS = { "commit", "add", "rm", "update-ref", "branch", "worktree", "tag", } _TARGET_OPTIONS = {"--git-dir", "--work-tree"} +# `git config` flags that answer a query; any of them makes the call read-only. +_CONFIG_READ_FLAGS = {"--get", "--get-all", "--get-regexp", "--get-urlmatch", "--list", "-l"} +# `git config` options whose value is the next argv entry, so it is not the key. +_CONFIG_VALUE_OPTIONS = {"-f", "--file", "--blob", "--type", "--default", "--comment", "--value", "--url"} +# `git config ` (Git 2.46+): the key follows the subcommand. +_CONFIG_WRITE_SUBCOMMANDS = {"set", "unset", "rename-section", "remove-section"} _VALUE_OPTIONS = _TARGET_OPTIONS | {"-C", "-c", "--namespace", "--super-prefix"} def _command_name(token): @@ -80,6 +86,27 @@ def _git_verb_and_targets(tail, kwargs): explicit[name] = value return None, [], [] +def _config_writes_url_rewrite(after): + """True when `git config ` writes a url..insteadOf/pushInsteadOf key.""" + positional = [] + index = 0 + while index < len(after): + arg = after[index] + index += 1 + if arg in _CONFIG_READ_FLAGS: + return False + if arg.startswith("-"): + if arg in _CONFIG_VALUE_OPTIONS: + index += 1 + continue + positional.append(arg) + if positional and positional[0] in {"get", "list"}: + return False + if positional and positional[0] in _CONFIG_WRITE_SUBCOMMANDS: + positional = positional[1:] + key = positional[0].lower() if positional else "" + return key.startswith("url.") and key.endswith((".insteadof", ".pushinsteadof")) + def blocked_git_mutation(cmd, kwargs, protected_roots): tail = _git_argv_tail(cmd) if tail is None: @@ -87,9 +114,7 @@ def blocked_git_mutation(cmd, kwargs, protected_roots): verb, targets, after = _git_verb_and_targets(tail, kwargs or {}) if verb == "config": # Querying config is safe; writing URL rewrites into the checkout is not. - if any(arg in {"--get", "--get-all", "--get-regexp", "--list", "-l"} for arg in after): - return None - if not any("url." in arg and ".insteadof" in arg.lower() for arg in after): + if not _config_writes_url_rewrite(after): return None elif verb not in _MUTATIONS: return None diff --git a/tests/test_git_safety_boundary.py b/tests/test_git_safety_boundary.py index cc34255593..f0489f5379 100644 --- a/tests/test_git_safety_boundary.py +++ b/tests/test_git_safety_boundary.py @@ -57,13 +57,15 @@ def test_checkout_guard_covers_write_verbs_without_blocking_queries(tmp_path): for argv in ( ["commit", "-am", "change"], ["add", "-A"], ["rm", "-r", "."], ["config", "url.https://example.invalid/.insteadOf", "git@example.invalid:"], + ["config", "--file", ".git/config", "url.https://example.invalid/.pushInsteadOf", "git@x:"], + ["config", "set", "url.https://example.invalid/.insteadOf", "git@example.invalid:"], ["update-ref", "refs/heads/main", "abc"], ["branch", "-f", "main"], ["worktree", "add", "../other"], ["tag", "v1"], ["tag", "--delete", "--list", "v1"], ["tag", "--list", "--force", "v1"], ): assert blocked_git_mutation(["git", *argv], options, (root,)) == argv[0] assert blocked_git_mutation(["git", *argv], {"cwd": tmp_path}, (root,)) is None - for argv in (["status"], ["config", "--get", "url.x.insteadOf"], + for argv in (["status"], ["config", "--get", "url.x.insteadOf"], ["config", "get", "url.x.insteadOf"], ["worktree", "list"], ["branch", "--show-current"], ["tag", "-l"], ["tag", "--merged", "HEAD", "--list", "v[0-9]*"]): assert blocked_git_mutation(["git", *argv], options, (root,)) is None