Skip to content

Commit 671ea00

Browse files
codexByron
authored andcommitted
test(refs): cover all option-shaped checkout branch operands
Pursue the repository-owner follow-up to `GHSA-f8wj-prgh-h9j7`, reported against released `GitPython` 3.2.1. Current `main` already rejects every leading-dash branch name through the unconditional `Git._check_operand()` guard in `Head.checkout()`, introduced in `212573bf`. Preserve that shared prevention and make its coverage and public contract explicit. Expand the existing operand tests to short options, attached option values, and terminators, with both `allow_unsafe_options` settings. Extend the scripted cloned-branch fixture with uncommitted work, and verify rejection does not launch Git or change the branch, commit, index status, file content, or untracked-file status. Document the exception and the independent branch validation in the API and unreleased changes. Git's baseline agrees: `check_branch_ref()` rejects leading-dash branch names, and `t/t1402-check-ref-format.sh` tests that rule. Reference source: `6de20f6092dcf9bdb1c8efe03db4b70c82b423dd` (`v2.56.0-116-g6de20f6092`). Placing `--` before the branch would instead select path checkout semantics. Validation used Python 3.12.14, Git 2.54.0, and GixPython 0.3.0 on macOS: - The three new cloned-branch cases failed with the unaltered `Head.checkout()` method extracted from the `3.2.1` tag at `c179357b` and loaded into the current test harness in a disposable process. This was method-level reproduction, not a full installed-release test. - `test/test_positional_args.py` and `test/test_refs.py` passed 184 tests and 24 subtests on each backend through `test/run-local.py`. - The final cloned-branch fixture passed all four cases on each backend. - Ruff lint/format, ShellCheck, and `git diff --check` passed. No new runtime guard is needed. A patched release and advisory publication remain repository-owner decisions; neither is performed by this follow-up. Agent-Model: unknown
1 parent 8899d19 commit 671ea00

5 files changed

Lines changed: 38 additions & 13 deletions

File tree

‎doc/source/changes.rst‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ Git executable and safety
2323
option/protocol checks remain, and operands and stdin records receive additional
2424
validation. NUL-delimited ``cat-file`` requests prevent newline injection.
2525
Previously accepted option-like revision/ref arguments can now be rejected.
26+
``Head.checkout()`` rejects every branch name starting with a dash, including
27+
when ``allow_unsafe_options=True``. This setting permits explicit keyword
28+
options while branch names remain operands.
2629
* New plumbing calls suppress implicit hooks, filesystem monitors, automatic
2730
maintenance, and lazy network fetches. Index staging continues to store raw
2831
content rather than introduce clean filters. Diffs disable external diff and

‎git/refs/head.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ def checkout(
279279
:param allow_unsafe_options:
280280
Allow unsafe options such as ``--pathspec-from-file`` to be passed to
281281
:manpage:`git-checkout(1)`.
282+
Branch names are validated regardless of this setting.
282283
283284
:param kwargs:
284285
Additional keyword arguments to be passed to git checkout, e.g.
@@ -290,6 +291,9 @@ def checkout(
290291
If there is no active branch, as the HEAD is now detached, the HEAD
291292
reference will be returned instead.
292293
294+
:raise git.exc.UnsafeOptionError:
295+
If the branch name starts with a dash or contains NUL, CR, or LF.
296+
293297
:note:
294298
By default it is only allowed to checkout heads - everything else will leave
295299
the HEAD detached which is allowed and possible, but remains a special state

‎test/test_positional_args.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ def test_rename_head_cannot_select_current_branch(repo, tmp_path):
7373

7474

7575
@pytest.mark.parametrize("allow_unsafe_options", [False, True])
76-
@pytest.mark.parametrize("name", ["--force", "--detach", "--ours"])
76+
@pytest.mark.parametrize(
77+
"name", ["-f", "-Bcheckout-target", "--orphan=checkout-target", "--force", "--detach", "--ours", "-", "--"]
78+
)
7779
def test_checkout_rejects_option_shaped_branch_names(repo, tmp_path, name, allow_unsafe_options):
7880
branch = Head(repo, "refs/heads/" + name)
7981
with mock.patch.object(Git, "_call_process_safe", side_effect=AssertionError("Git must not run")) as run:

‎test/test_refs.py‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,30 @@
4343
from test.lib.fixtures import fixture_repo, read_only_fixture, writable_fixture
4444

4545

46+
@pytest.mark.parametrize(
47+
"branch_name", ["--pathspec-from-file=pathspecs", "-f", "--orphan=checkout-target", "-Bcheckout-target"]
48+
)
49+
def test_cloned_head_checkout_rejects_option_shaped_branch_names(branch_name):
50+
with fixture_repo("refs.sh", "unsafe-branch", "sha1", branch_name, write=True, subdir="clone") as cloned:
51+
branch = cloned.active_branch
52+
assert branch.name == branch_name
53+
commit = cloned.head.commit
54+
path = Path(cloned.working_tree_dir) / "file.txt"
55+
content = path.read_bytes()
56+
status = cloned.git.status("--porcelain")
57+
assert status.startswith(" M file.txt")
58+
for allow_unsafe_options in (False, True):
59+
with patch.object(Git, "execute", autospec=True, side_effect=Git.execute) as run:
60+
with pytest.raises(UnsafeOptionError) as error:
61+
branch.checkout(force=False, allow_unsafe_options=allow_unsafe_options)
62+
run.assert_not_called()
63+
assert "unmatched-private-content" not in str(error.value)
64+
assert cloned.active_branch == branch
65+
assert cloned.head.commit == commit
66+
assert path.read_bytes() == content
67+
assert cloned.git.status("--porcelain") == status
68+
69+
4670
@pytest.mark.parametrize("object_format", ["sha1", "sha256"])
4771
def test_reference_names_preserve_unicode_line_separators(object_format):
4872
names = [
@@ -658,16 +682,6 @@ def test_head_checkout_rejects_pathspec_from_file(self, rw_repo):
658682
with self.assertRaises(UnsafeOptionError):
659683
branch.checkout()
660684

661-
def test_cloned_head_checkout_rejects_pathspec_from_file(self):
662-
with fixture_repo("refs.sh", "unsafe-branch", write=True, subdir="clone") as cloned:
663-
branch_name = "--pathspec-from-file=pathspecs"
664-
assert cloned.active_branch.name == branch_name
665-
for allow_unsafe_options in (False, True):
666-
with self.assertRaises(UnsafeOptionError) as error:
667-
cloned.active_branch.checkout(allow_unsafe_options=allow_unsafe_options)
668-
assert "unmatched-private-content" not in str(error.exception)
669-
assert cloned.active_branch.name == branch_name
670-
671685
@with_rw_repo("HEAD")
672686
def test_head_reset_rejects_pathspec_from_file(self, rw_repo):
673687
pathspecs = read_only_fixture("refs-input.sh") / "pathspecs"

‎tests/fixtures/refs.sh‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,12 @@ case "$1" in
3939
git rev-parse HEAD > ../outside-ref
4040
;;
4141
unsafe-branch)
42-
git update-ref 'refs/heads/--pathspec-from-file=pathspecs' HEAD
43-
git symbolic-ref HEAD 'refs/heads/--pathspec-from-file=pathspecs'
42+
branch_name="$3"
43+
git update-ref "refs/heads/$branch_name" HEAD
44+
git symbolic-ref HEAD "refs/heads/$branch_name"
4445
cd ..
4546
git clone repo clone
4647
printf 'unmatched-private-content\n' > clone/pathspecs
48+
printf 'uncommitted\n' >> clone/file.txt
4749
;;
4850
esac

0 commit comments

Comments
 (0)