Skip to content

Commit 790bb31

Browse files
Byroncodex
andcommitted
fix(remote): reject option-shaped pull operands
<!-- agent --> `Remote.pull()` validated keyword options but forwarded refspecs through `git pull`, whose internal fetch invocation loses the `--` separator. GHSA-f9j4-qggq-h239 reports the resulting positional validation bypass. Reject all leading-dash refspecs and remote names before spawning Git, rather than trying to enumerate dangerous option spellings. This also applies with `allow_unsafe_options=True`: explicit options still belong in keyword arguments. Document the behavior and add an unreleased changelog entry. No shell quoting change is needed; the problem is Git's own option parsing, not splitting arguments in Python. Git reference: `builtin/pull.c:run_fetch()` at Git commit `12cb6293d6288865c1a133cf22accbaf99d13eb6` forwards the remote and refspecs without an option terminator. Tested with Apple Git 2.54.0. Assisted-by: GPT 6.0 Co-authored-by: GPT 6.0 <codex@openai.com>
1 parent cf43820 commit 790bb31

3 files changed

Lines changed: 52 additions & 2 deletions

File tree

doc/source/changes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Security fixes for
99

1010
* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-gq48-pqfc-9p58
1111
* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-23mf-xhv8-69c2
12+
* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-f9j4-qggq-h239
1213

1314
If you can, also try and provide feedback on the upcoming v4 branch
1415
https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome.

git/remote.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from git.cmd import Git, handle_process_output
1515
from git.compat import defenc, force_text
1616
from git.config import GitConfigParser, SectionConstraint, cp
17-
from git.exc import GitCommandError
17+
from git.exc import GitCommandError, UnsafeOptionError
1818
from git.refs import Head, Reference, RemoteReference, SymbolicReference, TagReference
1919
from git.util import (
2020
CallableRemoteProgress,
@@ -1101,7 +1101,8 @@ def pull(
11011101
merge of branch with your local branch.
11021102
11031103
:param refspec:
1104-
See :meth:`fetch` method.
1104+
See :meth:`fetch` method. Values starting with ``-`` are rejected,
1105+
even when ``allow_unsafe_options`` is enabled. Pass options as keywords.
11051106
11061107
:param progress:
11071108
See :meth:`push` method.
@@ -1127,6 +1128,12 @@ def pull(
11271128
kwargs = add_progress(kwargs, self.repo.git, progress)
11281129

11291130
refspec = Git._unpack_args(refspec or [])
1131+
# Git pull forwards these operands to fetch without preserving `--`.
1132+
# Reject every option-shaped operand, including with unsafe options enabled:
1133+
# opting into an explicit option must not turn a refspec into an option.
1134+
for operand in [self.name, *refspec]:
1135+
if operand.startswith("-"):
1136+
raise UnsafeOptionError("Remote names and pull refspecs must not start with '-'.")
11301137
if not allow_unsafe_protocols:
11311138
for ref in refspec:
11321139
Git.check_unsafe_protocols(ref)

test/test_positional_args.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
"""High-level operands must not become Git options."""
2+
3+
from unittest import mock
4+
5+
import pytest
6+
7+
from git import Git, Remote, Repo
8+
from git.exc import UnsafeOptionError
9+
10+
11+
@pytest.mark.parametrize("allow_unsafe_options", [False, True])
12+
@pytest.mark.parametrize(
13+
"refspec",
14+
["--upload-pack=helper", ["--upl=helper"], ["main", "--dry-run"], "-uhelper", "--arg value", "--"],
15+
)
16+
def test_pull_rejects_option_shaped_refspec(tmp_path, refspec, allow_unsafe_options):
17+
repo = Repo.init(tmp_path)
18+
remote = Remote(repo, "origin")
19+
with mock.patch.object(Git, "_call_process", side_effect=AssertionError("Git must not run")) as run:
20+
with pytest.raises(UnsafeOptionError):
21+
remote.pull(refspec, allow_unsafe_options=allow_unsafe_options)
22+
run.assert_not_called()
23+
24+
25+
def test_pull_rejects_option_shaped_remote(tmp_path):
26+
repo = Repo.init(tmp_path)
27+
remote = Remote(repo, "--upload-pack=helper")
28+
with mock.patch.object(Git, "_call_process", side_effect=AssertionError("Git must not run")) as run:
29+
with pytest.raises(UnsafeOptionError):
30+
remote.pull("main")
31+
run.assert_not_called()
32+
33+
34+
def test_pull_preserves_operand_and_explicit_option_values(tmp_path):
35+
repo = Repo.init(tmp_path)
36+
remote = Remote(repo, "origin")
37+
with mock.patch.object(Git, "_call_process") as run, mock.patch.object(
38+
Remote, "_get_fetch_info_from_stderr", return_value=[]
39+
):
40+
remote.pull("refs/heads/topic", upload_pack="helper with spaces", allow_unsafe_options=True)
41+
assert run.call_args[0] == ("pull", "--", remote, ["refs/heads/topic"])
42+
assert run.call_args[1]["upload_pack"] == "helper with spaces"

0 commit comments

Comments
 (0)