Repository navigation
Commit 03b5db2
fix(refs): keep branch and tag operands out of option parsing
<!-- agent -->
A leading-dash name could change the operation requested by a high-level
API. In particular, `delete_head("--force", branch, force=False)` deleted
an unmerged branch, and renaming a `Head` named `--force` renamed the
current branch instead of rejecting the missing source.
Terminate options before names in branch deletion and rename, remote
reference deletion, and tag creation and deletion. Keep explicit keyword
options and the existing unsafe-option checks. This protects operands as
a class instead of enumerating individual dangerous flags.
Git reference: `builtin/branch.c` and `builtin/tag.c` use `parse_options()`
with support for `--` at Git commit
`12cb6293d6288865c1a133cf22accbaf99d13eb6`.
Assisted-by: GPT 6.0
Co-authored-by: GPT 6.0 <codex@openai.com>1 parent 790bb31 commit 03b5db2
4 files changed
Lines changed: 46 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -166,7 +166,7 @@ def delete(cls, repo: "Repo", *heads: "Union[Head, str]", force: bool = False, * | |||
| 166 | 166 | flag = "-d" | |
| 167 | 167 | if force: | |
| 168 | 168 | flag = "-D" | |
| 169 | - repo.git.branch(flag, *heads) | ||
| 169 | + repo.git.branch(flag, "--", *heads) | ||
| 170 | 170 | ||
| 171 | 171 | def set_tracking_branch(self, remote_reference: Union["RemoteReference", None]) -> "Head": | |
| 172 | 172 | """Configure this branch to track the given remote reference. This will | |
@@ -241,7 +241,7 @@ def rename(self, new_path: PathLike, force: bool = False) -> "Head": | |||
| 241 | 241 | if force: | |
| 242 | 242 | flag = "-M" | |
| 243 | 243 | ||
| 244 | - self.repo.git.branch(flag, self, new_path) | ||
| 244 | + self.repo.git.branch(flag, "--", self, new_path) | ||
| 245 | 245 | self.path = "%s/%s" % (self._common_path_default, new_path) | |
| 246 | 246 | return self | |
| 247 | 247 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -61,7 +61,7 @@ def delete(cls, repo: "Repo", *refs: "RemoteReference", **kwargs: Any) -> None: | |||
| 61 | 61 | for ref in refs: | |
| 62 | 62 | cls._check_ref_name_valid(ref.path) | |
| 63 | 63 | ||
| 64 | - repo.git.branch("-d", "-r", *refs) | ||
| 64 | + repo.git.branch("-d", "-r", "--", *refs) | ||
| 65 | 65 | # The official deletion method will ignore remote symbolic refs - these are | |
| 66 | 66 | # generally ignored in the refs/ folder. We don't though and delete remainders | |
| 67 | 67 | # manually. | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -155,15 +155,15 @@ def create( | |||
| 155 | 155 | if force: | |
| 156 | 156 | kwargs["f"] = True | |
| 157 | 157 | ||
| 158 | - args = (path, reference) | ||
| 158 | + args = ("--", path, reference) | ||
| 159 | 159 | ||
| 160 | 160 | repo.git.tag(*args, **kwargs) | |
| 161 | 161 | return TagReference(repo, "%s/%s" % (cls._common_path_default, path)) | |
| 162 | 162 | ||
| 163 | 163 | @classmethod | |
| 164 | 164 | def delete(cls, repo: "Repo", *tags: "TagReference") -> None: # type: ignore[override] | |
| 165 | 165 | """Delete the given existing tag or tags.""" | |
| 166 | - repo.git.tag("-d", *tags) | ||
| 166 | + repo.git.tag("-d", "--", *tags) | ||
| 167 | 167 | ||
| 168 | 168 | ||
| 169 | 169 | # Provide an alias. | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -4,7 +4,7 @@ | |||
| 4 | 4 | ||
| 5 | 5 | import pytest | |
| 6 | 6 | ||
| 7 | - from git import Git, Remote, Repo | ||
| 7 | + from git import Actor, Git, GitCommandError, Head, Remote, RemoteReference, Repo, TagReference | ||
| 8 | 8 | from git.exc import UnsafeOptionError | |
| 9 | 9 | ||
| 10 | 10 | ||
@@ -40,3 +40,43 @@ def test_pull_preserves_operand_and_explicit_option_values(tmp_path): | |||
| 40 | 40 | remote.pull("refs/heads/topic", upload_pack="helper with spaces", allow_unsafe_options=True) | |
| 41 | 41 | assert run.call_args[0] == ("pull", "--", remote, ["refs/heads/topic"]) | |
| 42 | 42 | assert run.call_args[1]["upload_pack"] == "helper with spaces" | |
| 43 | + | ||
| 44 | + | ||
| 45 | + def test_delete_head_cannot_override_force(tmp_path): | ||
| 46 | + repo = Repo.init(tmp_path) | ||
| 47 | + actor = Actor("Test", "test@example.com") | ||
| 48 | + initial = repo.index.commit("initial", author=actor, committer=actor) | ||
| 49 | + branch = repo.create_head("unmerged", initial) | ||
| 50 | + branch.commit = repo.index.commit("unmerged", head=False, author=actor, committer=actor) | ||
| 51 | + with pytest.raises(GitCommandError): | ||
| 52 | + repo.delete_head("--force", branch, force=False) | ||
| 53 | + assert branch.is_valid() | ||
| 54 | + repo.delete_head(branch, force=True) | ||
| 55 | + assert not branch.is_valid() | ||
| 56 | + | ||
| 57 | + | ||
| 58 | + def test_rename_head_cannot_select_current_branch(tmp_path): | ||
| 59 | + repo = Repo.init(tmp_path) | ||
| 60 | + actor = Actor("Test", "test@example.com") | ||
| 61 | + repo.index.commit("initial", author=actor, committer=actor) | ||
| 62 | + original = repo.active_branch.name | ||
| 63 | + with pytest.raises(GitCommandError): | ||
| 64 | + Head(repo, "refs/heads/--force").rename("renamed") | ||
| 65 | + assert repo.active_branch.name == original | ||
| 66 | + | ||
| 67 | + | ||
| 68 | + def test_tag_operands_follow_option_terminator(tmp_path): | ||
| 69 | + repo = Repo.init(tmp_path) | ||
| 70 | + with mock.patch.object(Git, "_call_process") as run: | ||
| 71 | + TagReference.create(repo, "topic", "HEAD") | ||
| 72 | + assert run.call_args[0] == ("tag", "--", "topic", "HEAD") | ||
| 73 | + TagReference.delete(repo, "--list") | ||
| 74 | + assert run.call_args[0] == ("tag", "-d", "--", "--list") | ||
| 75 | + | ||
| 76 | + | ||
| 77 | + def test_remote_ref_delete_preserves_operand(tmp_path): | ||
| 78 | + repo = Repo.init(tmp_path) | ||
| 79 | + ref = RemoteReference(repo, "refs/remotes/--force") | ||
| 80 | + with mock.patch.object(Git, "_call_process") as run: | ||
| 81 | + RemoteReference.delete(repo, ref) | ||
| 82 | + assert run.call_args[0] == ("branch", "-d", "-r", "--", ref) | ||
0 commit comments