From a4e55be0720fec82d0c553c3e52a83342cd1dd06 Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Mon, 5 Oct 2026 10:50:22 +0530 Subject: [PATCH 1/4] fix(security): isolate Git credentials for HTTP MCP scans Signed-off-by: yashrajbasav --- src/skillspector/input_handler.py | 57 ++++++++++++++++- src/skillspector/mcp_server.py | 11 +++- src/skillspector/nodes/resolve_input.py | 5 +- src/skillspector/state.py | 2 + tests/nodes/test_resolve_input.py | 20 +++++- tests/unit/test_input_handler.py | 83 +++++++++++++++++++++++++ tests/unit/test_mcp_server.py | 22 +++++++ 7 files changed, 194 insertions(+), 6 deletions(-) diff --git a/src/skillspector/input_handler.py b/src/skillspector/input_handler.py index 21c83b71e..bde17f318 100644 --- a/src/skillspector/input_handler.py +++ b/src/skillspector/input_handler.py @@ -767,7 +767,10 @@ class InputHandler: Normalizes all inputs to a local directory path for scanning. """ - def __init__(self, transitive_budget: object | None = None) -> None: + def __init__( + self, transitive_budget: object | None = None, *, allow_git_credentials: bool = True + ) -> None: + self._allow_git_credentials = allow_git_credentials self._temp_dir: Path | None = None self._transitive_budget = transitive_budget self.primary_file_path: str | None = None @@ -1129,23 +1132,70 @@ def _resolve_tree_ref(self, repository_url: str, segments: list[str]) -> tuple[s f"{repository_url} ({'/'.join(segments)})" ) + def _git_invocation(self, url: str) -> tuple[list[str], dict[str, str] | None]: + """Build a Git policy for both ref discovery and cloning. + + Remote MCP callers must not borrow the server's SSH keys, helpers, + authorization headers, URL rewrites, client certificates, or netrc. + Trusted local callers retain the operator's configured Git identity. + """ + if self._allow_git_credentials: + return ["git"], None + parsed = urlparse(url) + if parsed.scheme != "https" or parsed.username is not None or parsed.password is not None: + raise ValueError("Untrusted Git scans require an unauthenticated HTTPS URL") + isolated_home = self._get_temp_dir() / "git-home" + isolated_home.mkdir(mode=0o700, exist_ok=True) + env = { + key: value + for key, value in os.environ.items() + if key.upper() in {"PATH", "SYSTEMROOT", "WINDIR", "TEMP", "TMP", "TMPDIR"} + } + env.update( + HOME=str(isolated_home), + USERPROFILE=str(isolated_home), + XDG_CONFIG_HOME=str(isolated_home), + GIT_CONFIG_GLOBAL=os.devnull, + GIT_CONFIG_SYSTEM=os.devnull, + GIT_CONFIG_NOSYSTEM="1", + GIT_TERMINAL_PROMPT="0", + GIT_ALLOW_PROTOCOL="https", + ) + return [ + "git", + "-C", + str(isolated_home), + "-c", + "credential.helper=", + "-c", + "core.askPass=", + "-c", + f"core.hooksPath={os.devnull}", + "-c", + "http.extraHeader=", + "-c", + "http.followRedirects=false", + ], env + def _list_remote_refs(self, repository_url: str) -> set[str]: """Return the branch/tag names advertised by the remote repository. Bounded by the ingest deadline; the host allowlist and private-IP checks from URL validation apply. """ + git_argv, git_env = self._git_invocation(repository_url) self._validate_url_host(repository_url, ALLOWED_GIT_HOSTS) deadline = self._deadline() self._check_deadline(deadline, "git") timeout = max(1.0, deadline - monotonic()) try: process = subprocess.run( - ["git", "ls-remote", repository_url], + [*git_argv, "ls-remote", repository_url], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, timeout=timeout, check=False, + **({"env": git_env} if git_env is not None else {}), ) except subprocess.TimeoutExpired as exc: raise IngestLimitExceededError("Git ref listing exceeded its time limit") from exc @@ -1206,6 +1256,7 @@ def _clone_git(self, url: str, *, branch: str | None = None) -> Path: self._truncate("byte_budget_exhausted", "git") if remaining_artifacts is not None and remaining_artifacts <= 0: self._truncate("artifact_budget_exhausted", "git") + git_argv, git_env = self._git_invocation(url) self._validate_url_host(url, ALLOWED_GIT_HOSTS) deadline = self._deadline() self._check_deadline(deadline, "git") @@ -1225,6 +1276,7 @@ def _clone_git(self, url: str, *, branch: str | None = None) -> Path: clone_command[6:6] = ["--branch", branch] if remaining_bytes is not None: clone_command.insert(6, f"--filter=blob:limit={remaining_bytes}") + clone_command[:1] = git_argv process: subprocess.Popen[bytes] | None = None final_measurement: _TreeMeasurement | None = None try: @@ -1233,6 +1285,7 @@ def _clone_git(self, url: str, *, branch: str | None = None) -> Path: stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, shell=False, + **({"env": git_env} if git_env is not None else {}), ) while True: self._check_deadline(deadline, "git") diff --git a/src/skillspector/mcp_server.py b/src/skillspector/mcp_server.py index 57699867b..6a13a90e5 100644 --- a/src/skillspector/mcp_server.py +++ b/src/skillspector/mcp_server.py @@ -30,6 +30,7 @@ from collections.abc import Mapping from pathlib import Path from typing import TYPE_CHECKING, Any +from urllib.parse import urlparse from skillspector import __version__ from skillspector.cleanup import cleanup_result @@ -115,7 +116,7 @@ async def run_scan( :data:`VALID_FORMATS`. allow_local_targets: Whether local filesystem targets are allowed. HTTP MCP calls set this to ``False`` so routable servers do not - accept caller-controlled local paths. + accept caller-controlled local paths or use ambient Git credentials. yara_rules_dir: Optional directory of additional YARA rules. Returns: @@ -132,12 +133,20 @@ async def run_scan( local_yara_rules = yara_rules_dir is not None and _is_local_target(yara_rules_dir) if local_target or local_yara_rules: raise ValueError("local targets are disabled for this MCP transport") + parsed_target = urlparse(target.strip()) + if ( + parsed_target.scheme != "https" + or parsed_target.username is not None + or parsed_target.password is not None + ): + raise ValueError("this MCP transport requires an unauthenticated HTTPS target") llm_preflight_available, _ = is_llm_available() llm_enabled = use_llm and llm_preflight_available state: dict[str, Any] = { "input_path": target, + "allow_git_credentials": allow_local_targets, "output_format": output_format, "use_llm": llm_enabled, "llm_requested": use_llm, diff --git a/src/skillspector/nodes/resolve_input.py b/src/skillspector/nodes/resolve_input.py index cead1255a..b153b1246 100644 --- a/src/skillspector/nodes/resolve_input.py +++ b/src/skillspector/nodes/resolve_input.py @@ -56,7 +56,10 @@ def resolve_input(state: SkillspectorState) -> dict[str, object]: workflow_budget = ensure_workflow_resource_budget(state) if input_path and isinstance(input_path, str) and input_path.strip(): - handler = InputHandler(transitive_budget=workflow_budget) + handler = InputHandler( + transitive_budget=workflow_budget, + allow_git_credentials=state.get("allow_git_credentials", True), + ) try: resolved, source_type = handler.resolve(input_path.strip()) temp_dir = handler.temp_dir_for_cleanup() diff --git a/src/skillspector/state.py b/src/skillspector/state.py index fdd43b246..2bb310a50 100644 --- a/src/skillspector/state.py +++ b/src/skillspector/state.py @@ -235,6 +235,8 @@ class SkillspectorState(TypedDict, total=False): # Input: resolve_input node consumes input_path or skill_path, sets skill_path input_path: str | None + # Only trusted local transports may use the operator's Git credentials. + allow_git_credentials: bool skill_path: str | None # Relative identity of a wrapped/downloaded explicit file, distinct from passive assets. primary_file_path: str | None diff --git a/tests/nodes/test_resolve_input.py b/tests/nodes/test_resolve_input.py index e8af70f95..26fde723b 100644 --- a/tests/nodes/test_resolve_input.py +++ b/tests/nodes/test_resolve_input.py @@ -91,7 +91,10 @@ def test_workflow_budget_starts_before_input_materialization( class CapturingHandler: primary_file_path = None - def __init__(self, transitive_budget: object | None = None) -> None: + def __init__( + self, transitive_budget: object | None = None, *, allow_git_credentials: bool = True + ) -> None: + assert allow_git_credentials is True assert transitive_budget is not None assert getattr(transitive_budget, "started_at", None) is not None captured.append(transitive_budget) @@ -116,7 +119,10 @@ def test_transitive_truncation_is_typed_sanitized_and_cleaned( cleaned: list[bool] = [] class TruncatedHandler: - def __init__(self, transitive_budget: object | None = None) -> None: + def __init__( + self, transitive_budget: object | None = None, *, allow_git_credentials: bool = True + ) -> None: + assert allow_git_credentials is True assert transitive_budget is not None def resolve(self, _input_path: str) -> tuple[Path, str]: @@ -323,3 +329,13 @@ def wait(self, timeout: float | None = None) -> int: assert [clone.terminated for clone in InterruptedClone.instances] == [True] assert created assert [path for path in created if path.exists()] == [] + + +def test_resolve_input_preserves_anonymous_git_policy() -> None: + with pytest.raises(ValueError, match="unauthenticated HTTPS"): + resolve_input( + { + "input_path": "git@github.com:org/private.git", + "allow_git_credentials": False, + } + ) diff --git a/tests/unit/test_input_handler.py b/tests/unit/test_input_handler.py index 663274b12..e8634f49c 100644 --- a/tests/unit/test_input_handler.py +++ b/tests/unit/test_input_handler.py @@ -660,3 +660,86 @@ def test_scp_ssrf_gate_fires() -> None: with patch("skillspector.input_handler._is_private_ip", return_value=True): with pytest.raises(ValueError, match="private/internal IP"): InputHandler()._validate_url_host("git@github.com:org/repo.git", ALLOWED_GIT_HOSTS) + + +@pytest.mark.parametrize("operation", ["_clone_git", "_list_remote_refs"]) +def test_anonymous_git_drops_ambient_auth_and_config( + operation: str, monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + hostile = { + "HOME": str(tmp_path), + "SSH_AUTH_SOCK": "/synthetic/agent", + "GIT_ASKPASS": "/synthetic/askpass", + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "http.extraHeader", + "GIT_CONFIG_VALUE_0": "Authorization: synthetic-secret", + "GIT_CONFIG_GLOBAL": str(tmp_path / "credentials.config"), + "GITHUB_TOKEN": "synthetic-token", + "CURL_HOME": str(tmp_path), + "SSLKEYLOGFILE": str(tmp_path / "tls-keys"), + } + for key, value in hostile.items(): + monkeypatch.setenv(key, value) + handler = InputHandler(allow_git_credentials=False) + calls: list[tuple[list[str], dict]] = [] + + def fake_git(command: list[str], **kwargs: object) -> MagicMock: + calls.append((command, kwargs)) + if "clone" in command: + Path(command[-1]).mkdir(parents=True) + process = MagicMock() + process.poll.return_value = 0 + process.wait.return_value = 0 + process.returncode = 0 + process.stdout = b"abc\trefs/heads/main\n" + return process + + try: + with ( + patch.object(handler, "_validate_url_host", return_value="github.com"), + patch("skillspector.input_handler.subprocess.Popen", side_effect=fake_git), + patch("skillspector.input_handler.subprocess.run", side_effect=fake_git), + ): + getattr(handler, operation)("https://github.com/org/public.git") + assert len(calls) == 1 + command, kwargs = calls[0] + env = kwargs["env"] + isolated_home = handler.temp_dir_for_cleanup() / "git-home" + assert command[:3] == ["git", "-C", str(isolated_home)] + assert "credential.helper=" in command + assert "core.askPass=" in command + assert "http.extraHeader=" in command + assert "http.followRedirects=false" in command + assert env["HOME"] == env["USERPROFILE"] == str(isolated_home) + assert env["GIT_CONFIG_GLOBAL"] == env["GIT_CONFIG_SYSTEM"] == os.devnull + assert env["GIT_CONFIG_NOSYSTEM"] == "1" + assert env["GIT_ALLOW_PROTOCOL"] == "https" + assert env["GIT_TERMINAL_PROMPT"] == "0" + assert list(isolated_home.iterdir()) == [] + for key in hostile.keys() - {"HOME", "GIT_CONFIG_GLOBAL"}: + assert key not in env + assert os.environ["HOME"] == str(tmp_path) + finally: + handler.cleanup() + + +@pytest.mark.parametrize("operation", ["_clone_git", "_list_remote_refs"]) +@pytest.mark.parametrize( + "target", + [ + "git@github.com:org/private.git", + "ssh://git@github.com/org/private.git", + "https://user:password@github.com/org/private.git", + ], +) +def test_anonymous_git_rejects_auth_targets_before_start(operation: str, target: str) -> None: + handler = InputHandler(allow_git_credentials=False) + with ( + patch("skillspector.input_handler.subprocess.Popen") as popen, + patch("skillspector.input_handler.subprocess.run") as run, + ): + with pytest.raises(ValueError, match="unauthenticated HTTPS"): + getattr(handler, operation)(target) + popen.assert_not_called() + run.assert_not_called() + assert handler.temp_dir_for_cleanup() is None diff --git a/tests/unit/test_mcp_server.py b/tests/unit/test_mcp_server.py index 23cf6733f..232c3ba17 100644 --- a/tests/unit/test_mcp_server.py +++ b/tests/unit/test_mcp_server.py @@ -1066,6 +1066,26 @@ def fail_to_expanduser(self: Path) -> Path: assert mcp_server._is_local_target("~nosuchuser/skill") is True +@pytest.mark.parametrize( + "target", + [ + "git@github.com:org/private.git", + "ssh://git@github.com/org/private.git", + "git+ssh://git@github.com/org/private.git", + "https://token@github.com/org/private.git", + "https://user:password@github.com/org/private.git", + ], +) +async def test_http_rejects_credentialed_targets_before_graph( + target: str, monkeypatch: pytest.MonkeyPatch +) -> None: + graph_ainvoke = AsyncMock() + monkeypatch.setattr(mcp_server.graph, "ainvoke", graph_ainvoke) + with pytest.raises(ValueError, match="unauthenticated HTTPS"): + await run_scan(target, allow_local_targets=False) + graph_ainvoke.assert_not_awaited() + + async def test_run_scan_allows_remote_target_when_local_targets_disallowed( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -1088,6 +1108,7 @@ async def test_run_scan_allows_remote_target_when_local_targets_disallowed( assert result["target"] == target assert graph_ainvoke.await_count == 1 assert graph_ainvoke.await_args.args[0]["input_path"] == target + assert graph_ainvoke.await_args.args[0]["allow_git_credentials"] is False async def test_run_scan_keeps_default_local_target_compatibility( @@ -1111,6 +1132,7 @@ async def test_run_scan_keeps_default_local_target_compatibility( assert result["target"] == str(tmp_path) assert graph_ainvoke.await_count == 1 assert graph_ainvoke.await_args.args[0]["input_path"] == str(tmp_path) + assert graph_ainvoke.await_args.args[0]["allow_git_credentials"] is True @pytest.mark.parametrize( From f34cda027707b3f137d49cc0c3212ae738d555f3 Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Mon, 5 Oct 2026 21:23:19 +0530 Subject: [PATCH 2/4] fix: preserve anonymous Git transport compatibility and isolation Signed-off-by: yashrajbasav --- README.md | 4 ++ src/skillspector/input_handler.py | 26 +++++++++--- tests/unit/test_input_handler.py | 69 ++++++++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ac9daf1ff..2125efbe6 100644 --- a/README.md +++ b/README.md @@ -449,6 +449,10 @@ claude mcp add skillspector -- skillspector mcp > - Local paths and `file://` URLs are **automatically rejected** over HTTP to > prevent unauthenticated callers from reading arbitrary host files. Only > remote Git and `.zip` URLs are accepted. +> +> HTTP Git scans accept only credential-free `https://` targets and never use +> the server's Git login. Use stdio or the CLI for private repositories. +> Redirects are rejected; use the current repository URL after a rename or transfer. ## Vulnerability Patterns diff --git a/src/skillspector/input_handler.py b/src/skillspector/input_handler.py index f1c46ddca..019a6ea65 100644 --- a/src/skillspector/input_handler.py +++ b/src/skillspector/input_handler.py @@ -1132,7 +1132,7 @@ def _resolve_tree_ref(self, repository_url: str, segments: list[str]) -> tuple[s f"{repository_url} ({'/'.join(segments)})" ) - def _git_invocation(self, url: str) -> tuple[list[str], dict[str, str] | None]: + def _git_invocation(self, url: str) -> tuple[list[str], dict[str, str] | None, str]: """Build a Git policy for both ref discovery and cloning. Remote MCP callers must not borrow the server's SSH keys, helpers, @@ -1140,16 +1140,29 @@ def _git_invocation(self, url: str) -> tuple[list[str], dict[str, str] | None]: Trusted local callers retain the operator's configured Git identity. """ if self._allow_git_credentials: - return ["git"], None + return ["git"], None, url parsed = urlparse(url) if parsed.scheme != "https" or parsed.username is not None or parsed.password is not None: raise ValueError("Untrusted Git scans require an unauthenticated HTTPS URL") + if parsed.hostname in {"gitlab.com", "bitbucket.org"}: + # Browser URLs need the Git endpoint without following a redirect + # whose destination has not passed the host/IP checks. + path = parsed.path.rstrip("/") + if path and not path.endswith(".git"): + path += ".git" + url = parsed._replace(path=path).geturl() isolated_home = self._get_temp_dir() / "git-home" isolated_home.mkdir(mode=0o700, exist_ok=True) env = { key: value for key, value in os.environ.items() - if key.upper() in {"PATH", "SYSTEMROOT", "WINDIR", "TEMP", "TMP", "TMPDIR"} + if key.upper() + in { + "PATH", "SYSTEMROOT", "WINDIR", "TEMP", "TMP", "TMPDIR", + "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", + "GIT_SSL_CAINFO", "GIT_SSL_CAPATH", "SSL_CERT_FILE", "SSL_CERT_DIR", + "CURL_CA_BUNDLE", + } } env.update( HOME=str(isolated_home), @@ -1160,6 +1173,7 @@ def _git_invocation(self, url: str) -> tuple[list[str], dict[str, str] | None]: GIT_CONFIG_NOSYSTEM="1", GIT_TERMINAL_PROMPT="0", GIT_ALLOW_PROTOCOL="https", + GIT_CEILING_DIRECTORIES=str(isolated_home.parent), ) return [ "git", @@ -1175,7 +1189,7 @@ def _git_invocation(self, url: str) -> tuple[list[str], dict[str, str] | None]: "http.extraHeader=", "-c", "http.followRedirects=false", - ], env + ], env, url def _list_remote_refs(self, repository_url: str) -> set[str]: """Return the branch/tag names advertised by the remote repository. @@ -1183,7 +1197,7 @@ def _list_remote_refs(self, repository_url: str) -> set[str]: Bounded by the ingest deadline; the host allowlist and private-IP checks from URL validation apply. """ - git_argv, git_env = self._git_invocation(repository_url) + git_argv, git_env, repository_url = self._git_invocation(repository_url) self._validate_url_host(repository_url, ALLOWED_GIT_HOSTS) deadline = self._deadline() self._check_deadline(deadline, "git") @@ -1256,7 +1270,7 @@ def _clone_git(self, url: str, *, branch: str | None = None) -> Path: self._truncate("byte_budget_exhausted", "git") if remaining_artifacts is not None and remaining_artifacts <= 0: self._truncate("artifact_budget_exhausted", "git") - git_argv, git_env = self._git_invocation(url) + git_argv, git_env, url = self._git_invocation(url) self._validate_url_host(url, ALLOWED_GIT_HOSTS) deadline = self._deadline() self._check_deadline(deadline, "git") diff --git a/tests/unit/test_input_handler.py b/tests/unit/test_input_handler.py index 2ab6db3d4..eb791afd7 100644 --- a/tests/unit/test_input_handler.py +++ b/tests/unit/test_input_handler.py @@ -17,6 +17,7 @@ import ctypes import os +import subprocess import sys from errno import ENOENT from pathlib import Path, PurePosixPath @@ -718,8 +719,19 @@ def test_anonymous_git_drops_ambient_auth_and_config( "GITHUB_TOKEN": "synthetic-token", "CURL_HOME": str(tmp_path), "SSLKEYLOGFILE": str(tmp_path / "tls-keys"), + "GIT_SSL_CERT": "/synthetic/client.pem", + "GIT_SSL_KEY": "/synthetic/client.key", + "GIT_CEILING_DIRECTORIES": "/synthetic/ceiling", } - for key, value in hostile.items(): + transport = { + key: "/synthetic/ca" if "CA" in key or key.startswith("SSL_") else "http://proxy.example:8080" + for key in ( + "HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy", "ALL_PROXY", "all_proxy", + "NO_PROXY", "no_proxy", "GIT_SSL_CAINFO", "GIT_SSL_CAPATH", "SSL_CERT_FILE", + "SSL_CERT_DIR", "CURL_CA_BUNDLE", + ) + } + for key, value in (hostile | transport).items(): monkeypatch.setenv(key, value) handler = InputHandler(allow_git_credentials=False) calls: list[tuple[list[str], dict]] = [] @@ -755,9 +767,11 @@ def fake_git(command: list[str], **kwargs: object) -> MagicMock: assert env["GIT_CONFIG_GLOBAL"] == env["GIT_CONFIG_SYSTEM"] == os.devnull assert env["GIT_CONFIG_NOSYSTEM"] == "1" assert env["GIT_ALLOW_PROTOCOL"] == "https" + assert env["GIT_CEILING_DIRECTORIES"] == str(isolated_home.parent) + assert {key: env[key] for key in transport} == transport assert env["GIT_TERMINAL_PROMPT"] == "0" assert list(isolated_home.iterdir()) == [] - for key in hostile.keys() - {"HOME", "GIT_CONFIG_GLOBAL"}: + for key in hostile.keys() - {"HOME", "GIT_CONFIG_GLOBAL", "GIT_CEILING_DIRECTORIES"}: assert key not in env assert os.environ["HOME"] == str(tmp_path) finally: @@ -784,3 +798,54 @@ def test_anonymous_git_rejects_auth_targets_before_start(operation: str, target: popen.assert_not_called() run.assert_not_called() assert handler.temp_dir_for_cleanup() is None + + +@pytest.mark.parametrize("operation", ["_clone_git", "_list_remote_refs"]) +@pytest.mark.parametrize( + "target, expected", + [ + ("https://gitlab.com/group/subgroup/repo", "https://gitlab.com/group/subgroup/repo.git"), + ("https://gitlab.com/group/repo.git/", "https://gitlab.com/group/repo.git"), + ("https://bitbucket.org/team/repo/", "https://bitbucket.org/team/repo.git"), + ("https://github.com/org/repo", "https://github.com/org/repo"), + ], +) +def test_anonymous_git_normalizes_repository_urls_without_redirects(operation, target, expected): + handler = InputHandler(allow_git_credentials=False) + calls = [] + + def fake_git(command, **kwargs): + calls.append(command) + if "clone" in command: + Path(command[-1]).mkdir(parents=True) + return SimpleNamespace(poll=lambda: 0, returncode=0, stdout=b"abc\trefs/heads/main\n") + + try: + with ( + patch.object(handler, "_validate_url_host"), + patch("skillspector.input_handler.subprocess.Popen", side_effect=fake_git), + patch("skillspector.input_handler.subprocess.run", side_effect=fake_git), + ): + getattr(handler, operation)(target) + assert expected in calls[0] + assert "http.followRedirects=false" in calls[0] + finally: + handler.cleanup() + + +def test_anonymous_git_cannot_read_enclosing_repository_config(tmp_path): + subprocess.run(["git", "init", "--quiet", str(tmp_path)], check=True) + subprocess.run( + ["git", "-C", str(tmp_path), "config", "http.https://github.com/.extraheader", "synthetic-header"], + check=True, + ) + handler = InputHandler(allow_git_credentials=False) + with patch.object(handler, "_get_temp_dir", return_value=tmp_path / "scan"): + (tmp_path / "scan").mkdir() + argv, env, _ = handler._git_invocation("https://github.com/org/repo") + result = subprocess.run( + [*argv, "config", "--get-urlmatch", "http.extraheader", "https://github.com/org/repo"], + env=env, capture_output=True, check=False, + ) + assert b"synthetic-header" not in result.stdout + assert result.stdout.strip() == b"" From 81a85fe362622ef5355724ef286cdc37d20883d0 Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Mon, 5 Oct 2026 21:26:41 +0530 Subject: [PATCH 3/4] style: format review regression coverage Signed-off-by: yashrajbasav --- src/skillspector/input_handler.py | 51 ++++++++++++++++++++----------- tests/unit/test_input_handler.py | 33 ++++++++++++++++---- 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/skillspector/input_handler.py b/src/skillspector/input_handler.py index 019a6ea65..847b4ba6a 100644 --- a/src/skillspector/input_handler.py +++ b/src/skillspector/input_handler.py @@ -1158,9 +1158,20 @@ def _git_invocation(self, url: str) -> tuple[list[str], dict[str, str] | None, s for key, value in os.environ.items() if key.upper() in { - "PATH", "SYSTEMROOT", "WINDIR", "TEMP", "TMP", "TMPDIR", - "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", - "GIT_SSL_CAINFO", "GIT_SSL_CAPATH", "SSL_CERT_FILE", "SSL_CERT_DIR", + "PATH", + "SYSTEMROOT", + "WINDIR", + "TEMP", + "TMP", + "TMPDIR", + "HTTP_PROXY", + "HTTPS_PROXY", + "ALL_PROXY", + "NO_PROXY", + "GIT_SSL_CAINFO", + "GIT_SSL_CAPATH", + "SSL_CERT_FILE", + "SSL_CERT_DIR", "CURL_CA_BUNDLE", } } @@ -1175,21 +1186,25 @@ def _git_invocation(self, url: str) -> tuple[list[str], dict[str, str] | None, s GIT_ALLOW_PROTOCOL="https", GIT_CEILING_DIRECTORIES=str(isolated_home.parent), ) - return [ - "git", - "-C", - str(isolated_home), - "-c", - "credential.helper=", - "-c", - "core.askPass=", - "-c", - f"core.hooksPath={os.devnull}", - "-c", - "http.extraHeader=", - "-c", - "http.followRedirects=false", - ], env, url + return ( + [ + "git", + "-C", + str(isolated_home), + "-c", + "credential.helper=", + "-c", + "core.askPass=", + "-c", + f"core.hooksPath={os.devnull}", + "-c", + "http.extraHeader=", + "-c", + "http.followRedirects=false", + ], + env, + url, + ) def _list_remote_refs(self, repository_url: str) -> set[str]: """Return the branch/tag names advertised by the remote repository. diff --git a/tests/unit/test_input_handler.py b/tests/unit/test_input_handler.py index eb791afd7..bb75ae7ef 100644 --- a/tests/unit/test_input_handler.py +++ b/tests/unit/test_input_handler.py @@ -724,11 +724,23 @@ def test_anonymous_git_drops_ambient_auth_and_config( "GIT_CEILING_DIRECTORIES": "/synthetic/ceiling", } transport = { - key: "/synthetic/ca" if "CA" in key or key.startswith("SSL_") else "http://proxy.example:8080" + key: "/synthetic/ca" + if "CA" in key or key.startswith("SSL_") + else "http://proxy.example:8080" for key in ( - "HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy", "ALL_PROXY", "all_proxy", - "NO_PROXY", "no_proxy", "GIT_SSL_CAINFO", "GIT_SSL_CAPATH", "SSL_CERT_FILE", - "SSL_CERT_DIR", "CURL_CA_BUNDLE", + "HTTPS_PROXY", + "https_proxy", + "HTTP_PROXY", + "http_proxy", + "ALL_PROXY", + "all_proxy", + "NO_PROXY", + "no_proxy", + "GIT_SSL_CAINFO", + "GIT_SSL_CAPATH", + "SSL_CERT_FILE", + "SSL_CERT_DIR", + "CURL_CA_BUNDLE", ) } for key, value in (hostile | transport).items(): @@ -836,7 +848,14 @@ def fake_git(command, **kwargs): def test_anonymous_git_cannot_read_enclosing_repository_config(tmp_path): subprocess.run(["git", "init", "--quiet", str(tmp_path)], check=True) subprocess.run( - ["git", "-C", str(tmp_path), "config", "http.https://github.com/.extraheader", "synthetic-header"], + [ + "git", + "-C", + str(tmp_path), + "config", + "http.https://github.com/.extraheader", + "synthetic-header", + ], check=True, ) handler = InputHandler(allow_git_credentials=False) @@ -845,7 +864,9 @@ def test_anonymous_git_cannot_read_enclosing_repository_config(tmp_path): argv, env, _ = handler._git_invocation("https://github.com/org/repo") result = subprocess.run( [*argv, "config", "--get-urlmatch", "http.extraheader", "https://github.com/org/repo"], - env=env, capture_output=True, check=False, + env=env, + capture_output=True, + check=False, ) assert b"synthetic-header" not in result.stdout assert result.stdout.strip() == b"" From f44ffbb280301965c98108640fd3c5592b975b7c Mon Sep 17 00:00:00 2001 From: yashrajbasav Date: Tue, 6 Oct 2026 12:40:10 +0530 Subject: [PATCH 4/4] docs: clarify HTTP scan transport environment settings Signed-off-by: yashrajbasav --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c280c22ca..7c1042ca9 100644 --- a/README.md +++ b/README.md @@ -477,9 +477,11 @@ claude mcp add skillspector -- skillspector mcp > prevent unauthenticated callers from reading arbitrary host files. Only > remote Git and `.zip` URLs are accepted. > -> HTTP Git scans accept only credential-free `https://` targets and never use -> the server's Git login. Use stdio or the CLI for private repositories. -> Redirects are rejected; use the current repository URL after a rename or transfer. +> HTTP scans accept only credential-free `https://` targets. Git scans never use +> the server's Git login or Git config. If a proxy or custom CA is needed, set +> `HTTPS_PROXY`, `NO_PROXY` or `GIT_SSL_CAINFO` in the server environment. +> Use stdio or the CLI for private repositories. Git redirects are rejected; +> use the current repository URL after a rename or transfer. ## Vulnerability Patterns