From d0b012e909590053b4d7289150a8a4403929a0ea Mon Sep 17 00:00:00 2001 From: Arseniy Dugin Date: Mon, 20 Jul 2026 15:46:10 +0000 Subject: [PATCH] Strip credentials from normalized Git remote URLs Git remote URLs can embed credentials in their userinfo (https://user:token@host/...), most often from CI checkout URLs or a caller-supplied --git-remote-url. Normalization previously rewrote only SSH/SCP forms and stripped the .git suffix, so credentials in an http(s) URL flowed unchanged into published component annotations, CLI output, logs, and browse links. Make credential removal a guarantee of URL normalization. _normalize_git_url now parses the URL and rebuilds the netloc from host and port only, dropping all userinfo, converts ssh:// to https://, drops userinfo from SCP-style remotes, strips the .git suffix, and redacts known sensitive query parameters. The function is idempotent. ComponentPublisher sanitizes a caller-supplied git_remote_url at construction, and component_inspector sanitizes before emitting transparency reasons and building browse links. Scheme, host, port, and path are preserved; only credential material is removed, and clone/fetch authentication is unaffected. --- .../src/tangle_cli/component_inspector.py | 8 +- .../src/tangle_cli/component_publisher.py | 10 +- packages/tangle-cli/src/tangle_cli/utils.py | 97 ++++++++++--- tests/test_component_inspector.py | 21 +++ tests/test_component_publisher.py | 14 ++ tests/test_utils.py | 131 ++++++++++++++++++ 6 files changed, 258 insertions(+), 23 deletions(-) diff --git a/packages/tangle-cli/src/tangle_cli/component_inspector.py b/packages/tangle-cli/src/tangle_cli/component_inspector.py index 92f16c5..8adc722 100644 --- a/packages/tangle-cli/src/tangle_cli/component_inspector.py +++ b/packages/tangle-cli/src/tangle_cli/component_inspector.py @@ -11,6 +11,7 @@ from tangle_cli.handler import TangleCliHandler from tangle_cli.models import ComponentInfo, ComponentSpec +from tangle_cli.utils import _normalize_git_url if TYPE_CHECKING: from tangle_cli.client import TangleApiClient @@ -416,7 +417,8 @@ def transparency_check(spec: ComponentSpec) -> tuple[bool, str]: if ann.get("git_remote_url") and ( ann.get("component_yaml_path") or ann.get("git_relative_dir") ): - return True, f"git source metadata links to {ann['git_remote_url']}" + safe_url = _normalize_git_url(ann["git_remote_url"]) + return True, f"git source metadata links to {safe_url}" impl = spec.implementation or {} container = impl.get("container", {}) @@ -447,6 +449,10 @@ def _resolve_git_source(spec: ComponentSpec) -> dict[str, Any] | None: if not git_url: return None + # Server-supplied annotations may still carry credentials in the remote + # URL; strip them before building any browsable/emitted links. + git_url = _normalize_git_url(git_url) + sha = annotations.get("git_remote_sha", "") branch = annotations.get("git_remote_branch", "main") component_yaml = annotations.get("component_yaml_path") diff --git a/packages/tangle-cli/src/tangle_cli/component_publisher.py b/packages/tangle-cli/src/tangle_cli/component_publisher.py index 5db339d..be7923b 100644 --- a/packages/tangle-cli/src/tangle_cli/component_publisher.py +++ b/packages/tangle-cli/src/tangle_cli/component_publisher.py @@ -164,7 +164,15 @@ def __init__( self._git_root = str(git_root or git_info.get("_git_root") or "") or None self.git_remote_sha = git_remote_sha or git_info.get("git_remote_sha") self.git_remote_branch = git_remote_branch or git_info.get("git_remote_branch") - self.git_remote_url = git_remote_url or git_info.get("git_remote_url") + resolved_git_remote_url = git_remote_url or git_info.get("git_remote_url") + # Sanitize here as well as in get_git_info: a caller-supplied + # --git-remote-url bypasses get_git_info's normalization, so this is the + # single choke point that guarantees no credentials are persisted. + self.git_remote_url = ( + utils._normalize_git_url(resolved_git_remote_url) + if resolved_git_remote_url + else resolved_git_remote_url + ) self.git_repo = git_repo def _component_spec_model(self) -> type[Any]: diff --git a/packages/tangle-cli/src/tangle_cli/utils.py b/packages/tangle-cli/src/tangle_cli/utils.py index 0449d45..fa722ad 100644 --- a/packages/tangle-cli/src/tangle_cli/utils.py +++ b/packages/tangle-cli/src/tangle_cli/utils.py @@ -870,31 +870,86 @@ def normalize_annotation_paths( _CI_REPO_URL_VARS: tuple[str, ...] = ("BUILDKITE_REPO", "GITHUB_SERVER_URL", "CI_REPOSITORY_URL") -def _normalize_git_url(url: str) -> str: - """Normalize a git remote URL to a browsable HTTPS URL. +# Query-string parameter names that carry authentication material. When a +# remote URL retains any of these they are dropped during normalization so +# secrets are never persisted, displayed, logged, or embedded in error text. +_SENSITIVE_QUERY_KEYS: frozenset[str] = frozenset({ + "access_token", "personal_access_token", "private_token", "token", + "api_key", "apikey", "key", "auth", "authorization", + "password", "passwd", "pwd", "secret", + "sig", "signature", "x-access-token", +}) + + +def _redact_sensitive_query(query: str) -> str: + """Drop known credential-bearing parameters from a URL query string. + + A query with no sensitive keys is returned byte-for-byte unchanged so that + ordinary URLs are not silently re-encoded. + """ + if not query: + return query - Handles common formats: - - ``git@github.com:Org/repo.git`` -> ``https://github.com/Org/repo`` - - ``ssh://git@github.com/Org/repo.git`` -> ``https://github.com/Org/repo`` - - ``https://github.com/Org/repo.git`` -> ``https://github.com/Org/repo`` - - ``https://github.com/Org/repo`` -> unchanged + from urllib.parse import parse_qsl, urlencode - The ``.git`` suffix is stripped so the result can be used directly to - build ``/blob/{ref}/{path}`` links without an extra ``.removesuffix``. - """ - import re + pairs = parse_qsl(query, keep_blank_values=True) + if not any(key.lower() in _SENSITIVE_QUERY_KEYS for key, _ in pairs): + return query + kept = [(key, value) for key, value in pairs if key.lower() not in _SENSITIVE_QUERY_KEYS] + return urlencode(kept) - # SCP-style: git@host:path - m = re.match(r"^git@([^:]+):(.+)$", url) - if m: - url = f"https://{m.group(1)}/{m.group(2)}" - else: - # ssh://git@host/path - m = re.match(r"^ssh://(?:[^@]+@)?([^/]+)/(.+)$", url) - if m: - url = f"https://{m.group(1)}/{m.group(2)}" - return url.removesuffix(".git") +def _normalize_git_url(url: str) -> str: + """Normalize a git remote URL to a browsable, credential-free HTTPS URL. + + Converts SSH/SCP forms to HTTPS and strips the ``.git`` suffix so the + result can build ``/blob/{ref}/{path}`` links directly: + + - ``git@github.com:Org/repo.git`` -> ``https://github.com/Org/repo`` + - ``ssh://git@github.com/Org/repo.git`` -> ``https://github.com/Org/repo`` + - ``https://github.com/Org/repo.git`` -> ``https://github.com/Org/repo`` + - ``https://github.com/Org/repo`` -> unchanged + + Any embedded credentials are removed: ``user:password@`` / ``token@`` + userinfo is stripped from URL-form remotes and dropped entirely from + SCP-style remotes, and known sensitive query parameters are redacted. This + guarantees secrets never reach persisted annotations, CLI output, logs, or + error messages. Host, port, path, and fragment are preserved. Local + filesystem paths and unrecognized schemes are returned unchanged (aside + from ``.git`` stripping). The function is idempotent. + """ + from urllib.parse import urlsplit, urlunsplit + + if not url: + return url + + stripped = url.strip() + + # SCP-style syntax: ``[user@]host:path`` (no scheme). We drop any userinfo + # since it is authentication material, and rewrite to https so the result + # is browsable. Guard against Windows drive paths (``C:\...``) and against + # anything that already carries an explicit scheme. + if "://" not in stripped and not re.match(r"^[A-Za-z]:[\\/]", stripped): + scp = re.match(r"^(?:[^@/]+@)?([^/:]+):(?!//)(.+)$", stripped) + if scp: + stripped = f"https://{scp.group(1)}/{scp.group(2)}" + + parts = urlsplit(stripped) + if parts.scheme and parts.hostname is not None: + host = parts.hostname + # Re-bracket IPv6 literals, which ``hostname`` returns without brackets. + if ":" in host and not host.startswith("["): + host = f"[{host}]" + netloc = host if parts.port is None else f"{host}:{parts.port}" + out_scheme = "https" if parts.scheme.lower() == "ssh" else parts.scheme.lower() + query = _redact_sensitive_query(parts.query) + # Strip ``.git`` from the path itself so it is removed even when a query + # or fragment follows it. + path = parts.path.removesuffix(".git") + return urlunsplit((out_scheme, netloc, path, query, parts.fragment)) + + # No host to sanitize (e.g. ``file:///path``) or a bare local path. + return stripped.removesuffix(".git") def _fill_from_ci_env(info: dict[str, str]) -> None: diff --git a/tests/test_component_inspector.py b/tests/test_component_inspector.py index 7d5c068..f3dc465 100644 --- a/tests/test_component_inspector.py +++ b/tests/test_component_inspector.py @@ -97,6 +97,27 @@ def test_unknown_container_is_opaque(self): assert transparent is False assert "no inline source" in reason + def test_git_source_reason_does_not_leak_credentials(self): + spec = ComponentSpec.from_dict({ + "spec": { + "name": "demo", + "implementation": {"container": {"image": "registry.example.com/private/demo:latest"}}, + "metadata": { + "annotations": { + "git_remote_url": "https://user:s3cr3tTOKEN@github.com/Org/repo.git", + "component_yaml_path": "comp.yaml", + }, + }, + }, + }) + + transparent, reason = ComponentInspector.transparency_check(spec) + + assert transparent is True + assert "s3cr3tTOKEN" not in reason + assert "@github.com" not in reason + assert "https://github.com/Org/repo" in reason + class TestComponentLibrary: def test_standard_library_does_not_fetch_cross_origin_component_urls(self): diff --git a/tests/test_component_publisher.py b/tests/test_component_publisher.py index a6ac801..0f56dc9 100644 --- a/tests/test_component_publisher.py +++ b/tests/test_component_publisher.py @@ -396,6 +396,20 @@ def test_publish_components_passes_structured_context_to_context_aware_hooks(tmp assert kwargs_hook.contexts == [after_context] +def test_publisher_strips_credentials_from_supplied_git_remote_url(tmp_path: Path) -> None: + # A caller-supplied --git-remote-url bypasses get_git_info's normalization, + # so the publisher itself must sanitize it before it reaches annotations. + publisher = ComponentPublisher( + dry_run=True, + git_remote_url="https://gitlab-ci-token:glcbt-SECRETVALUE@gitlab.com/Org/repo.git", + git_root=tmp_path, + ) + + assert publisher.git_remote_url == "https://gitlab.com/Org/repo" + assert "glcbt-SECRETVALUE" not in (publisher.git_remote_url or "") + assert "@" not in (publisher.git_remote_url or "") + + def test_publish_components_batches_configs_and_runs_hooks(tmp_path: Path) -> None: first = write_component(tmp_path / "one.yaml", name="one", version="1.0") second = write_component(tmp_path / "two.yaml", name="two", version="2.0") diff --git a/tests/test_utils.py b/tests/test_utils.py index 24f6a4c..464e0b5 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -177,3 +177,134 @@ class TestNormalizeGitUrl: ]) def test_normalization(self, input_url, expected): assert _normalize_git_url(input_url) == expected + + @pytest.mark.parametrize("input_url,expected", [ + # user:password userinfo is stripped from http(s) URLs + ( + "https://user:s3cr3t@github.com/Org/repo.git", + "https://github.com/Org/repo", + ), + # token-style single-field userinfo (personal access token) + ( + "https://ghp_ABC123token@github.com/Org/repo.git", + "https://github.com/Org/repo", + ), + # username-only userinfo + ( + "https://alice@example.com/Org/repo.git", + "https://example.com/Org/repo", + ), + # password-only / empty username + ( + "https://:onlypassword@example.com/Org/repo", + "https://example.com/Org/repo", + ), + # percent-encoded userinfo (e.g. an email-style username and @ in secret) + ( + "https://user%40corp.com:p%40ss%2Fword@gitlab.com/Org/repo.git", + "https://gitlab.com/Org/repo", + ), + # plain http is preserved as http (scheme not silently upgraded) + ( + "http://user:pw@internal.example/Org/repo.git", + "http://internal.example/Org/repo", + ), + # host + port is preserved while credentials are removed + ( + "https://user:pw@example.com:8443/Org/repo.git", + "https://example.com:8443/Org/repo", + ), + # GitLab CI token URL (a very common real-world leak vector) + ( + "https://gitlab-ci-token:glcbt-xxxxxxxx@gitlab.com/Org/repo.git", + "https://gitlab.com/Org/repo", + ), + # ssh:// with userinfo -> https, credentials dropped + ( + "ssh://git@github.com/Org/repo.git", + "https://github.com/Org/repo", + ), + # ssh:// with an explicit port keeps the port + ( + "ssh://git@github.com:2222/Org/repo.git", + "https://github.com:2222/Org/repo", + ), + # scp-style with a username other than git + ( + "deploy@example.com:Org/repo.git", + "https://example.com/Org/repo", + ), + # IPv6 literal host with credentials and port + ( + "https://user:pw@[2001:db8::1]:8443/Org/repo.git", + "https://[2001:db8::1]:8443/Org/repo", + ), + # fragment is preserved + ( + "https://user:pw@github.com/Org/repo.git#readme", + "https://github.com/Org/repo#readme", + ), + ]) + def test_credentials_are_stripped(self, input_url, expected): + assert _normalize_git_url(input_url) == expected + + @pytest.mark.parametrize("secret,input_url", [ + ("s3cr3t", "https://user:s3cr3t@github.com/Org/repo.git"), + ("ghp_ABC123token", "https://ghp_ABC123token@github.com/Org/repo.git"), + ("glcbt-xxxxxxxx", "https://gitlab-ci-token:glcbt-xxxxxxxx@gitlab.com/Org/repo.git"), + ("p%40ss%2Fword", "https://u:p%40ss%2Fword@gitlab.com/Org/repo.git"), + ("onlypassword", "https://:onlypassword@example.com/Org/repo"), + ]) + def test_no_secret_material_survives(self, secret, input_url): + result = _normalize_git_url(input_url) + assert secret not in result + assert "@" not in result + + @pytest.mark.parametrize("input_url,expected", [ + # sensitive query parameters are redacted + ( + "https://github.com/Org/repo?access_token=abc123", + "https://github.com/Org/repo", + ), + ( + "https://example.com/Org/repo.git?private_token=tok&ref=main", + "https://example.com/Org/repo?ref=main", + ), + ( + "https://example.com/Org/repo?password=hunter2&x=1", + "https://example.com/Org/repo?x=1", + ), + ]) + def test_sensitive_query_params_redacted(self, input_url, expected): + assert _normalize_git_url(input_url) == expected + + @pytest.mark.parametrize("input_url", [ + "https://github.com/Org/repo", + "https://github.com/Org/repo?ref=main&path=a/b", + "/local/path/to/repo", + "./relative/repo", + "file:///home/user/repo", + "git@github.com:Org/repo.git", + ]) + def test_credential_free_urls_are_preserved(self, input_url): + # Non-sensitive query strings and local paths must not be corrupted. + result = _normalize_git_url(input_url) + assert _normalize_git_url(result) == result # idempotent + + def test_local_paths_not_corrupted(self): + assert _normalize_git_url("/abs/path/repo") == "/abs/path/repo" + assert _normalize_git_url("./rel/repo") == "./rel/repo" + assert _normalize_git_url("file:///home/user/repo.git") == "file:///home/user/repo" + + def test_windows_path_not_treated_as_scp(self): + assert _normalize_git_url(r"C:\Users\me\repo") == r"C:\Users\me\repo" + + def test_empty_and_whitespace(self): + assert _normalize_git_url("") == "" + assert _normalize_git_url(" https://user:pw@github.com/Org/repo.git ") == ( + "https://github.com/Org/repo" + ) + + def test_idempotent(self): + once = _normalize_git_url("https://user:token@github.com/Org/repo.git") + assert _normalize_git_url(once) == once