Repository navigation
Commit edcd66f
fix(remote): reject option-shaped update targets
<!-- agent -->
`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 `--`.
Assisted-by: GPT 6.0
Co-authored-by: GPT 6.0 <codex@openai.com>1 parent 0d8d845 commit edcd66f
3 files changed
Lines changed: 18 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1795,6 +1795,12 @@ def _call_process( | |||
| 1795 | 1795 | This allows your commands to call git more conveniently, as ``None`` is | |
| 1796 | 1796 | realized as non-existent. | |
| 1797 | 1797 | ||
| 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 | + | ||
| 1798 | 1804 | :param kwargs: | |
| 1799 | 1805 | Contains key-values for the following: | |
| 1800 | 1806 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -871,6 +871,9 @@ def update(self, **kwargs: Any) -> "Remote": | |||
| 871 | 871 | :return: | |
| 872 | 872 | self | |
| 873 | 873 | """ | |
| 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 '-'.") | ||
| 874 | 877 | scmd = "update" | |
| 875 | 878 | kwargs["insert_kwargs_after"] = scmd | |
| 876 | 879 | self.repo.git.remote(scmd, self.name, **kwargs) | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -118,3 +118,12 @@ def test_move_cannot_override_dry_run(tmp_path): | |||
| 118 | 118 | repo.index.move(["--no-dry-run", "source", "destination"], dry_run=True) | |
| 119 | 119 | assert (tmp_path / "source").read_text() == "source" | |
| 120 | 120 | 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