From 493dc177bc470f996e185e44cf8cd7f129ddc59c Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 30 Aug 2026 07:44:02 -0500 Subject: [PATCH 1/5] fix(cmd/git[pull]): Reject option-like repository and reftag git pull re-invokes git fetch without an end-of-options --, so the separator libvcs places before pull's positionals never reached the child fetch. A repository or reftag beginning with - was parsed there as an option; --upload-pack= ran an arbitrary command. Reject a - prefix in both positions via a shared reject_option_like() helper in _internal/run, raising LibVCSException before the command is built. A regression test drives the injection through the real Git.pull API and asserts the canary is never created. --- src/libvcs/_internal/run.py | 17 +++++++++++++++++ src/libvcs/cmd/git.py | 11 ++++++++--- tests/cmd/test_git.py | 26 ++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/libvcs/_internal/run.py b/src/libvcs/_internal/run.py index c559116f..c74481cf 100644 --- a/src/libvcs/_internal/run.py +++ b/src/libvcs/_internal/run.py @@ -115,6 +115,23 @@ def _normalize_command_args(args: _CMD) -> list[StrOrBytesPath]: return [os.fspath(arg) for arg in args] +def reject_option_like(value: str, *, name: str) -> str: + """Return ``value`` unless it would be parsed as a command-line option. + + A value beginning with ``-`` lands in the argv where the tool reads an + operand and is instead parsed as an option. Where an end-of-options ``--`` + cannot neutralize that — ``git pull`` re-spawns ``git fetch`` without one, + and a rev separator means a pathspec — the value must be rejected instead. + """ + if value.startswith("-"): + msg = ( + f"{name} may not begin with '-': {value!r} would be parsed as a " + "command-line option (argument injection)." + ) + raise exc.LibVCSException(msg) + return value + + def _stringify_command(args: _CMD) -> str | list[str]: """Return a human-readable command for CommandError.""" if isinstance(args, (str, bytes, os.PathLike)): diff --git a/src/libvcs/cmd/git.py b/src/libvcs/cmd/git.py index e6f695f1..bdf225a9 100644 --- a/src/libvcs/cmd/git.py +++ b/src/libvcs/cmd/git.py @@ -12,7 +12,12 @@ from collections.abc import Sequence from libvcs._internal.query_list import QueryList -from libvcs._internal.run import ProgressCallbackProtocol, _normalize_command_args, run +from libvcs._internal.run import ( + ProgressCallbackProtocol, + _normalize_command_args, + reject_option_like, + run, +) from libvcs._internal.types import StrOrBytesPath, StrPath _CMD = StrOrBytesPath | Sequence[StrOrBytesPath] @@ -914,9 +919,9 @@ def pull( """ required_flags: list[str] = [] if repository: - required_flags.insert(0, repository) + required_flags.insert(0, reject_option_like(repository, name="repository")) if reftag: - required_flags.insert(0, reftag) + required_flags.insert(0, reject_option_like(str(reftag), name="reftag")) local_flags: list[str] = [] # diff --git a/tests/cmd/test_git.py b/tests/cmd/test_git.py index 09e9978f..edb8d764 100644 --- a/tests/cmd/test_git.py +++ b/tests/cmd/test_git.py @@ -2785,3 +2785,29 @@ def test_run_failure_preserves_stderr_lines(git_repo: GitSync) -> None: # localize, rather than on translatable English error text. assert "\n" in output assert "no-such-remote" in output + + +def test_pull_rejects_option_like_repository( + create_git_remote_repo: CreateRepoFn, + git_repo: GitSync, + tmp_path: pathlib.Path, +) -> None: + """Reject a repository argument that git would parse as an option. + + ``git pull`` re-spawns ``git fetch`` without an end-of-options ``--``, so + the separator libvcs places before its positionals does not protect this + command. A ``reftag`` beginning with ``-`` reaches the child ``git fetch`` + as an option; ``--upload-pack=`` there is arbitrary command execution. + """ + remote_repo = create_git_remote_repo() + git_repo.cmd.remotes.add(name="origin", url=f"file://{remote_repo}") + canary = tmp_path / "PULL_PWNED" + + with pytest.raises(exc.LibVCSException): + git_repo.cmd.pull( + reftag=f"--upload-pack=touch {canary}", + repository="origin", + check_returncode=True, + ) + + assert not canary.exists(), "Prevent argument injection via git pull" From b40f30576c676d4957f32c38fded48d70c90fd5f Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 30 Aug 2026 07:46:21 -0500 Subject: [PATCH 2/5] fix(sync/git[update_repo]): Reject an option-like rev A configured rev flowed into git rev-list , which places the operand with no end-of-options --. A rev of --output= was parsed there as git's diff --output option, truncating the file during option parsing -- file destruction from a config-supplied revision. Validate rev with reject_option_like() before use and record a "rev" SyncResult error instead of running the command. A regression test drives a malicious rev through update_repo and asserts the victim file is left intact. --- src/libvcs/sync/git.py | 8 ++++++++ tests/sync/test_git.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/libvcs/sync/git.py b/src/libvcs/sync/git.py index c56c1486..2d14c3b5 100644 --- a/src/libvcs/sync/git.py +++ b/src/libvcs/sync/git.py @@ -26,6 +26,7 @@ from urllib import parse as urlparse from libvcs import exc +from libvcs._internal.run import reject_option_like from libvcs._internal.types import StrPath from libvcs.cmd.git import Git from libvcs.sync.base import ( @@ -491,6 +492,13 @@ def update_repo( # Get requested revision or tag url, git_tag = self.url, getattr(self, "rev", None) + if git_tag: + try: + reject_option_like(str(git_tag), name="rev") + except exc.LibVCSException as e: + result.add_error("rev", str(e), exception=e) + return result + if not git_tag: self.log.debug("No git revision set, defaulting to origin/master") try: diff --git a/tests/sync/test_git.py b/tests/sync/test_git.py index 235eae3f..1fb8abff 100644 --- a/tests/sync/test_git.py +++ b/tests/sync/test_git.py @@ -1847,3 +1847,24 @@ def test_remote_swallows_libvcs_exception( ) assert git_repo.remote("origin") is None + + +def test_update_repo_rejects_option_like_rev( + git_repo: GitSync, + tmp_path: pathlib.Path, +) -> None: + """Reject a revision that git would parse as an option. + + A `rev` reaches `git rev-list `, which has no end-of-options `--` + before the operand. A value such as `--output=` is parsed there as + the diff `--output` option, whose callback truncates the file during + option parsing -- arbitrary file destruction from a config-supplied rev. + """ + victim = tmp_path / "victim.txt" + victim.write_text("important\n") + git_repo.rev = f"--output={victim}" + + result = git_repo.update_repo() + + assert not result.ok, "update_repo() should fail for an option-like rev" + assert victim.read_text() == "important\n", "Prevent rev argument injection" From f23def8337335a600fa6b0fb5b81c526a21e40b6 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 30 Aug 2026 07:48:07 -0500 Subject: [PATCH 3/5] fix(cmd/svn[checkout]): Reject an option-like URL svn checkout takes the URL as its first positional with no end-of-options separator, so a URL beginning with - was parsed as an option; svn's --config-option can set a tunnel command, an execution primitive. Reject a - prefix via reject_option_like(), matching the protection the git and hg clone paths already carry. relocate() and switch() are unaffected: they convert a non-file:// target through pathlib as_uri(), so the value can never reach argv option-like. --- src/libvcs/cmd/svn.py | 9 +++++++-- tests/cmd/test_svn.py | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/libvcs/cmd/svn.py b/src/libvcs/cmd/svn.py index 8c32a821..13d11416 100644 --- a/src/libvcs/cmd/svn.py +++ b/src/libvcs/cmd/svn.py @@ -16,7 +16,12 @@ from collections.abc import Sequence from libvcs import exc -from libvcs._internal.run import ProgressCallbackProtocol, _normalize_command_args, run +from libvcs._internal.run import ( + ProgressCallbackProtocol, + _normalize_command_args, + reject_option_like, + run, +) from libvcs._internal.types import StrOrBytesPath, StrPath _CMD: t.TypeAlias = StrOrBytesPath | Sequence[StrOrBytesPath] @@ -212,7 +217,7 @@ def checkout( >>> svn.checkout(url=f'file://{svn_remote_repo}', revision=10) 'svn: E160006: No such revision 10...' """ - local_flags: list[str] = [url, str(self.path)] + local_flags: list[str] = [reject_option_like(url, name="url"), str(self.path)] if revision is not None: local_flags.extend(["--revision", str(revision)]) diff --git a/tests/cmd/test_svn.py b/tests/cmd/test_svn.py index 0ecf0604..452b04fe 100644 --- a/tests/cmd/test_svn.py +++ b/tests/cmd/test_svn.py @@ -8,6 +8,7 @@ import pytest +from libvcs import exc from libvcs.cmd.svn import Svn if t.TYPE_CHECKING: @@ -38,3 +39,16 @@ def test_svn_run_timeout_propagates_to_runner( _args, kwargs = mock_run.call_args assert kwargs.get("timeout") == 2.5 + + +def test_checkout_rejects_option_like_url(tmp_path: pathlib.Path) -> None: + """Reject a checkout URL that svn would parse as an option. + + ``svn checkout`` takes the URL as its first positional. A value beginning + with ``-`` -- e.g. ``--config-option=config:tunnels:ssh=`` -- is parsed + as an option instead, and svn's tunnel config is an execution primitive. + """ + svn = Svn(path=tmp_path) + + with pytest.raises(exc.LibVCSException, match="may not begin with"): + svn.checkout(url="--config-option=config:tunnels:ssh=touch ./PWNED") From a3d2dd8f584474a6bcb08cb1b40d5b45132424d6 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 30 Aug 2026 07:50:00 -0500 Subject: [PATCH 4/5] test(cmd/git[clone]): Pin the end-of-options separator Assert git clone emits -- immediately before the URL. This was the only git-side separator with no test; a refactor dropping it would else pass, because the single-positional clone shape never runs the injected --upload-pack helper, making an end-to-end canary test vacuous here (unlike the hg alias path, whose regression test does execute). --- tests/cmd/test_git.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/cmd/test_git.py b/tests/cmd/test_git.py index edb8d764..2851ce4b 100644 --- a/tests/cmd/test_git.py +++ b/tests/cmd/test_git.py @@ -2811,3 +2811,28 @@ def test_pull_rejects_option_like_repository( ) assert not canary.exists(), "Prevent argument injection via git pull" + + +def test_clone_places_end_of_options_before_url( + tmp_path: pathlib.Path, + mocker: MockerFixture, +) -> None: + """Pin the ``--`` separator ahead of the clone URL. + + ``git clone`` reads the URL as a positional; the ``--`` libvcs emits before + it is what stops a ``--upload-pack=`` URL from being parsed as an + option. No other test covers this separator, so a refactor dropping it would + otherwise pass -- the single-positional clone shape does not execute the + payload, making an end-to-end canary test vacuous here. + """ + repo = git.Git(path=tmp_path) + mock_run = mocker.patch("libvcs.cmd.git.run", return_value="") + + repo.clone(url="https://example.com/repo.git") + + _args, kwargs = mock_run.call_args + argv = [os.fspath(a) for a in kwargs["args"]] + assert "--" in argv, "clone must emit an end-of-options separator" + assert argv[argv.index("--") + 1] == "https://example.com/repo.git", ( + "URL must follow the -- separator, not precede it" + ) From 1e40a0ece973e237c7b605a2b372e62b022c7198 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 30 Aug 2026 08:03:16 -0500 Subject: [PATCH 5/5] docs(CHANGES): Record backend argument-injection guards Regenerated as a single CHANGES-only commit: the guards for Git.pull, GitSync rev, and Svn.checkout, crediting the report that prompted the audit. The entries were previously entangled in the three fix commits. --- CHANGES | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CHANGES b/CHANGES index ba893f33..d58c6b36 100644 --- a/CHANGES +++ b/CHANGES @@ -22,6 +22,28 @@ _Notes on the upcoming release will go here._ ### Fixes +#### Reject option-like URLs, revisions, and refs across VCS backends + +libvcs now rejects an argument that a VCS binary would parse as an option in a +position where an end-of-options `--` cannot neutralize it: + +- {meth}`~libvcs.cmd.git.Git.pull` raises {exc}`~libvcs.exc.LibVCSException` + when `repository` or `reftag` begins with `-`. `git pull` re-invokes `git + fetch` without a `--`, so the separator before pull's positionals never + reached the child; a value such as `--upload-pack=` was parsed there and + ran an arbitrary command. +- {meth}`~libvcs.sync.git.GitSync.update_repo` records a `rev` error instead of + running the command when the configured revision begins with `-`. The + revision reached `git rev-list `, where `--output=` was parsed + as `git`'s diff `--output` option and truncated that file. +- {meth}`~libvcs.cmd.svn.Svn.checkout` raises when `url` begins with `-`; svn's + `--config-option` can set a tunnel command, an execution primitive. This + matches the protection the git and hg clone paths already carried. + +A report from [Harsh Raj Singhania](https://github.com/HarshRajSinghania) +prompted this audit of how arguments reach the git, hg, and svn binaries. The +reported issue did not reproduce; the fixes above are what the audit found. + #### Progress callback timestamps carry a time zone (#549) The `timestamp` passed to a