Reject option-like VCS arguments a separator can't guard

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=<cmd>` 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 <commit>`, where `--output=<file>` 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

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)):

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] = []
 
         #

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)])

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:

tests/cmd/test_git.py

@@ -2785,3 +2785,54 @@ 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=<cmd>`` 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"
+
+
+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=<cmd>`` 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"
+    )

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=<cmd>`` -- 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")

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 <commit>`, which has no end-of-options `--`
+    before the operand. A value such as `--output=<file>` 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"