diff --git a/src/scanner/git_ops.py b/src/scanner/git_ops.py index 9a3e3b8..b1ecff4 100644 --- a/src/scanner/git_ops.py +++ b/src/scanner/git_ops.py @@ -59,6 +59,11 @@ def changed_files(self) -> List[str]: ] +def _validate_no_options(arg: str, arg_name: str) -> None: + if str(arg).startswith("-"): + raise ValueError(f"Invalid {arg_name}: cannot start with '-' to prevent command injection") + + def _run_git(args: List[str], cwd: str, timeout: int = 600) -> str: """Run a git command and return stdout.""" cmd = ["git"] + args @@ -89,6 +94,8 @@ def clone_repo( If ``token`` is provided, it is injected into the URL for private repos (format: ``https://{token}@github.com/org/repo.git``). """ + _validate_no_options(repo_url, "repo_url") + _validate_no_options(branch, "branch") if token and repo_url.startswith("https://"): repo_url = repo_url.replace("https://", f"https://{token}@") @@ -97,7 +104,7 @@ def clone_repo( args = ["clone", "--branch", branch, "--single-branch"] if depth: args += ["--depth", str(depth)] - args += [repo_url, local_path] + args += ["--", repo_url, local_path] _run_git(args, cwd=str(Path(local_path).parent), timeout=1800) sha = get_head_sha(local_path) @@ -107,6 +114,7 @@ def clone_repo( def pull_latest(local_path: str, branch: str = "main") -> str: """Pull the latest changes. Returns the new HEAD SHA.""" + _validate_no_options(branch, "branch") _run_git(["checkout", branch], cwd=local_path) _run_git(["pull", "origin", branch], cwd=local_path, timeout=600) sha = get_head_sha(local_path) diff --git a/tests/unit/scanner/test_git_ops.py b/tests/unit/scanner/test_git_ops.py new file mode 100644 index 0000000..afbffd8 --- /dev/null +++ b/tests/unit/scanner/test_git_ops.py @@ -0,0 +1,44 @@ +import unittest +from unittest.mock import patch +from src.scanner.git_ops import clone_repo, pull_latest + +class TestGitOps(unittest.TestCase): + def test_clone_repo_rejects_options(self): + with self.assertRaisesRegex(ValueError, "Invalid repo_url: cannot start with '-'"): + clone_repo("--upload-pack=malicious", "/tmp/local", "main") + + with self.assertRaisesRegex(ValueError, "Invalid branch: cannot start with '-'"): + clone_repo("https://github.com/repo.git", "/tmp/local", "--branch") + + def test_pull_latest_rejects_options(self): + with self.assertRaisesRegex(ValueError, "Invalid branch: cannot start with '-'"): + pull_latest("/tmp/local", "--branch") + + @patch("src.scanner.git_ops._run_git") + @patch("src.scanner.git_ops.get_head_sha") + @patch("src.scanner.git_ops.Path") + def test_clone_repo_happy_path(self, mock_path, mock_get_head, mock_run_git): + mock_get_head.return_value = "12345678" + sha = clone_repo("https://github.com/repo.git", "/tmp/local", "main") + self.assertEqual(sha, "12345678") + + mock_run_git.assert_called_with( + ["clone", "--branch", "main", "--single-branch", "--", "https://github.com/repo.git", "/tmp/local"], + cwd=mock_path.return_value.parent.__str__.return_value, + timeout=1800 + ) + + @patch("src.scanner.git_ops._run_git") + @patch("src.scanner.git_ops.get_head_sha") + def test_pull_latest_happy_path(self, mock_get_head, mock_run_git): + mock_get_head.return_value = "87654321" + sha = pull_latest("/tmp/local", "main") + self.assertEqual(sha, "87654321") + + # Verify checkout call + mock_run_git.assert_any_call(["checkout", "main"], cwd="/tmp/local") + # Verify pull call + mock_run_git.assert_any_call(["pull", "origin", "main"], cwd="/tmp/local", timeout=600) + +if __name__ == '__main__': + unittest.main()