Skip to content

Commit 03b5db2

Browse files
Byroncodex
andcommitted
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

File tree

git/refs/head.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ def delete(cls, repo: "Repo", *heads: "Union[Head, str]", force: bool = False, *
166166
flag = "-d"
167167
if force:
168168
flag = "-D"
169-
repo.git.branch(flag, *heads)
169+
repo.git.branch(flag, "--", *heads)
170170

171171
def set_tracking_branch(self, remote_reference: Union["RemoteReference", None]) -> "Head":
172172
"""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":
241241
if force:
242242
flag = "-M"
243243

244-
self.repo.git.branch(flag, self, new_path)
244+
self.repo.git.branch(flag, "--", self, new_path)
245245
self.path = "%s/%s" % (self._common_path_default, new_path)
246246
return self
247247

git/refs/remote.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ def delete(cls, repo: "Repo", *refs: "RemoteReference", **kwargs: Any) -> None:
6161
for ref in refs:
6262
cls._check_ref_name_valid(ref.path)
6363

64-
repo.git.branch("-d", "-r", *refs)
64+
repo.git.branch("-d", "-r", "--", *refs)
6565
# The official deletion method will ignore remote symbolic refs - these are
6666
# generally ignored in the refs/ folder. We don't though and delete remainders
6767
# manually.

git/refs/tag.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,15 +155,15 @@ def create(
155155
if force:
156156
kwargs["f"] = True
157157

158-
args = (path, reference)
158+
args = ("--", path, reference)
159159

160160
repo.git.tag(*args, **kwargs)
161161
return TagReference(repo, "%s/%s" % (cls._common_path_default, path))
162162

163163
@classmethod
164164
def delete(cls, repo: "Repo", *tags: "TagReference") -> None: # type: ignore[override]
165165
"""Delete the given existing tag or tags."""
166-
repo.git.tag("-d", *tags)
166+
repo.git.tag("-d", "--", *tags)
167167

168168

169169
# Provide an alias.

test/test_positional_args.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import pytest
66

7-
from git import Git, Remote, Repo
7+
from git import Actor, Git, GitCommandError, Head, Remote, RemoteReference, Repo, TagReference
88
from git.exc import UnsafeOptionError
99

1010

@@ -40,3 +40,43 @@ def test_pull_preserves_operand_and_explicit_option_values(tmp_path):
4040
remote.pull("refs/heads/topic", upload_pack="helper with spaces", allow_unsafe_options=True)
4141
assert run.call_args[0] == ("pull", "--", remote, ["refs/heads/topic"])
4242
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

Comments
 (0)