Skip to content

Commit ffd601c

Browse files
codexByron
authored andcommitted
fix(remote): reject option-shaped update targets
`Remote.update()` also crosses an internal Git command boundary: Git's `remote update` parses options, then forwards the remaining operands to `fetch --multiple` without an option terminator. A leading-dash remote name can therefore change the requested operation or its target set. Reject such names before dispatch. Document at `_call_process()` why high-level wrappers must protect operands, while the low-level runner must continue supporting deliberately positional options. Shell quoting does not affect Git's own option parsing. Add an unreleased changelog entry. Git reference: `builtin/remote.c:update()` at Git commit `12cb6293d6288865c1a133cf22accbaf99d13eb6` constructs the internal fetch command without preserving `--`. Validation: three new rejection cases failed before the fix. Positional, remote, and command tests pass 160 cases with one skip, excluding the previously reproduced baseline fetch failure in `TestRemote.test_base`. Ruff 0.16.5 lint and formatting checks pass for all eight changed Python files, and `git diff --check` passes.
1 parent 65ed3f8 commit ffd601c

4 files changed

Lines changed: 21 additions & 0 deletions

File tree

doc/source/changes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ Unreleased
1616
* Preserve leading-dash filenames in ``IndexFile.move()`` and ``Repo.ignored()``.
1717
Move operands can no longer enable overwrites or disable ``dry_run=True``.
1818

19+
* Reject option-shaped remote names in ``Remote.update()`` before Git can
20+
interpret them as options to an internal fetch.
21+
1922
3.1.63
2023
======
2124

git/cmd.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1795,6 +1795,12 @@ def _call_process(
17951795
This allows your commands to call git more conveniently, as ``None`` is
17961796
realized as non-existent.
17971797
1798+
Positional arguments may intentionally contain command options. Higher-level
1799+
APIs must separate their operands with ``--`` where the Git command supports
1800+
it, or reject option-shaped operands where Git reparses them internally (for
1801+
example, ``pull`` and ``remote update``). Shell quoting cannot prevent Git
1802+
from interpreting a leading-dash argument as an option.
1803+
17981804
:param kwargs:
17991805
Contains key-values for the following:
18001806

git/remote.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -871,6 +871,9 @@ def update(self, **kwargs: Any) -> "Remote":
871871
:return:
872872
self
873873
"""
874+
# Like pull, remote update forwards operands to fetch without `--`.
875+
if self.name.startswith("-"):
876+
raise UnsafeOptionError("Remote names used by update must not start with '-'.")
874877
scmd = "update"
875878
kwargs["insert_kwargs_after"] = scmd
876879
self.repo.git.remote(scmd, self.name, **kwargs)

test/test_positional_args.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,3 +118,12 @@ def test_move_cannot_override_dry_run(tmp_path):
118118
repo.index.move(["--no-dry-run", "source", "destination"], dry_run=True)
119119
assert (tmp_path / "source").read_text() == "source"
120120
assert not (tmp_path / "destination").exists()
121+
122+
123+
@pytest.mark.parametrize("name", ["--prune", "--all", "--upload-pack=helper"])
124+
def test_remote_update_rejects_option_shaped_name(tmp_path, name):
125+
repo = Repo.init(tmp_path)
126+
with mock.patch.object(Git, "_call_process", side_effect=AssertionError("Git must not run")) as run:
127+
with pytest.raises(UnsafeOptionError):
128+
Remote(repo, name).update()
129+
run.assert_not_called()

0 commit comments

Comments
 (0)