fix(interface): Prevent argument injection in clone (closes #1134)

Prevent Git argument injection and option smuggling when cloning
repositories by inserting an end-of-options delimiter (--) before
positional target arguments in clone_repository and rejecting
leading-dash targets upfront in infer_target_type.

Fix the following gaps:
- Insert '--' separator before repo_url in git clone invocation
- Reject targets and repo URLs starting with '-' in infer_target_type
  and clone_repository
- Sanitize repo base names to strip leading dots/dashes and prevent
  directory traversal
- Add comprehensive test coverage in tests/test_clone_repository.py
This commit is contained in:
Tanmay9223 2026-10-15 00:53:46 +05:30
parent 87f2d783d6
commit 4d60330338
2 changed files with 119 additions and 0 deletions

View file

@ -1136,6 +1136,9 @@ def infer_target_type(target: str) -> tuple[str, dict[str, str]]: # noqa: PLR09
target = target.strip()
if target.startswith("-"):
raise ValueError(f"Invalid target: {target}. Targets cannot start with a dash.")
if target.startswith("git@"):
return "repository", {"target_repo": target}
@ -1254,6 +1257,7 @@ def read_target_list_file(path_str: str) -> list[str]:
def sanitize_name(name: str) -> str:
sanitized = re.sub(r"[^A-Za-z0-9._-]", "-", name.strip())
sanitized = sanitized.lstrip(".-")
return sanitized or "target"
@ -1555,6 +1559,11 @@ def stage_api_specs(targets_info: list[dict[str, Any]], run_name: str) -> list[d
def clone_repository(repo_url: str, run_name: str, dest_name: str | None = None) -> str:
console = Console()
if not repo_url or repo_url.strip().startswith("-"):
raise ValueError(
f"Invalid repository URL: {repo_url}. Repository URL cannot start with a dash."
)
git_executable = shutil.which("git")
if git_executable is None:
raise FileNotFoundError("Git executable not found in PATH")
@ -1567,8 +1576,23 @@ def clone_repository(repo_url: str, run_name: str, dest_name: str | None = None)
else:
repo_name = Path(repo_url).stem if repo_url.endswith(".git") else Path(repo_url).name
if (
not repo_name
or repo_name in (".", "..")
or "/" in repo_name
or "\\" in repo_name
or repo_name.startswith("-")
):
raise ValueError(f"Invalid repository destination name: {repo_name}")
clone_path = temp_dir / repo_name
if (
not clone_path.resolve().is_relative_to(temp_dir.resolve())
or clone_path.resolve() == temp_dir.resolve()
):
raise ValueError(f"Invalid clone path outside temporary directory: {clone_path}")
if clone_path.exists():
shutil.rmtree(clone_path)
@ -1578,6 +1602,7 @@ def clone_repository(repo_url: str, run_name: str, dest_name: str | None = None)
[
git_executable,
"clone",
"--",
repo_url,
str(clone_path),
],

View file

@ -0,0 +1,94 @@
"""Tests for clone_repository, infer_target_type, and sanitization in strix.interface.utils."""
from __future__ import annotations
from typing import TYPE_CHECKING
from unittest.mock import MagicMock, patch
import pytest
from strix.interface.utils import (
clone_repository,
derive_repo_base_name,
infer_target_type,
sanitize_name,
)
if TYPE_CHECKING:
from pathlib import Path
def test_clone_repository_includes_double_dash_separator(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
monkeypatch.setattr("tempfile.gettempdir", lambda: str(tmp_path))
monkeypatch.setattr("shutil.which", lambda cmd: "/usr/bin/git" if cmd == "git" else None)
mock_run = MagicMock()
with patch("subprocess.run", mock_run):
res = clone_repository("https://github.com/example/test-repo.git", "run_123")
expected_path = tmp_path / "strix_repos" / "run_123" / "test-repo"
assert res == str(expected_path.resolve())
mock_run.assert_called_once_with(
[
"/usr/bin/git",
"clone",
"--",
"https://github.com/example/test-repo.git",
str(expected_path),
],
capture_output=True,
text=True,
check=True,
)
def test_clone_repository_rejects_leading_dash_url() -> None:
with pytest.raises(ValueError, match="Repository URL cannot start with a dash"):
clone_repository("-u/upload-pack.git", "run_123")
with pytest.raises(ValueError, match="Repository URL cannot start with a dash"):
clone_repository("--upload-pack=touch /tmp/pwn", "run_123")
def test_clone_repository_rejects_invalid_dest_names(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
monkeypatch.setattr("tempfile.gettempdir", lambda: str(tmp_path))
monkeypatch.setattr("shutil.which", lambda cmd: "/usr/bin/git" if cmd == "git" else None)
with pytest.raises(ValueError, match="Invalid repository destination name"):
clone_repository("https://example.com/repo.git", "run_123", dest_name="..")
with pytest.raises(ValueError, match="Invalid repository destination name"):
clone_repository("https://example.com/repo.git", "run_123", dest_name=".")
with pytest.raises(ValueError, match="Invalid repository destination name"):
clone_repository("https://example.com/repo.git", "run_123", dest_name="-dashed")
with pytest.raises(ValueError, match="Invalid repository destination name"):
clone_repository("https://example.com/repo.git", "run_123", dest_name="sub/dir")
def test_infer_target_type_rejects_leading_dash() -> None:
with pytest.raises(ValueError, match="Targets cannot start with a dash"):
infer_target_type("-u/upload-pack.git")
with pytest.raises(ValueError, match="Targets cannot start with a dash"):
infer_target_type("--target-flag")
def test_sanitize_name_strips_leading_dots_and_dashes() -> None:
assert sanitize_name("..") == "target"
assert sanitize_name(".") == "target"
assert sanitize_name("-dashed") == "dashed"
assert sanitize_name(".hidden") == "hidden"
assert sanitize_name("normal_repo-1") == "normal_repo-1"
def test_derive_repo_base_name_handles_dot_git() -> None:
assert derive_repo_base_name("..git") == "target"
assert derive_repo_base_name("https://github.com/foo/bar.git") == "bar"
assert derive_repo_base_name("git@github.com:foo/bar.git") == "bar"