diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ef4721095..4bc3badfe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** after consumers re-vendor the shared gh-aw `apm.md`, its import requires an explicit `target` instead of deprecated `all`; `apm-action` otherwise writes `all` into the isolated `apm.yml`, where it degrades to auto-detection without harness markers. Set the workflow engine's target and recompile; see the [gh-aw migration recipe](https://microsoft.github.io/apm/integrations/gh-aw/#shared-apmmd-import-recommended). (#2706) - Re-vendored shared gh-aw workflows now default to APM 0.28.0 for both pack and restore, the version used for the recorded `microsoft/apm-action@v1.10.0` compatibility proof, not the latest CLI release; an explicit `apm-version` still overrides it. (#2706) +### Fixed + +- Installing a private Git dependency no longer fails with `Unable to verify Git URL rewrite safety` when a `url..insteadOf` rule rewrites the fetched HTTPS URL to SSH. The `http.extraHeader` URL-match probe now runs only for HTTP(S) effective URLs, because Git rejects SCP-style targets such as `git@github.com:owner/repo`. (#2906) + ### Security - The shared gh-aw APM pack job now declares `contents: read` (previously `permissions: {}`), the minimum the explicit built-in-token path needs. No write scope is added, and the token is not forwarded to restore or agent jobs. (#2706) diff --git a/docs/src/content/docs/getting-started/authentication.md b/docs/src/content/docs/getting-started/authentication.md index 96c9da447a..0e27d2398f 100644 --- a/docs/src/content/docs/getting-started/authentication.md +++ b/docs/src/content/docs/getting-started/authentication.md @@ -41,15 +41,25 @@ every cross-host network target, regardless of host class. A managed HTTPS credential also cannot cross a scheme, host, or port boundary. Same-host SSH and local-mirror selections remain credential-free. -If a rewrite is rejected, find its source and remove or replace it: +If a rewrite is rejected, inspect the matching rules first: ```bash git config --show-origin --get-regexp '^url\..*\.insteadOf$' ``` +APM only runs the `http.extraHeader` URL-match probe when the effective +rewritten URL is HTTP(S). Safe same-host SSH rewrites remain +credential-free, so a same-host SSH target does not use that probe. +Confirm that the longest matching rule keeps the target on the same host +and does not introduce credentials or an insecure transport, then retry. +If a rewrite that should be safe still cannot be verified, fix the +matching rule or the config that sets it before removing a rule that may +be safe. + If the selected rewrite is a `file://` mirror and the clone fails, verify that -the local path exists and is readable. Fix or remove that rewrite; configuring -an SSH key or token does not repair a missing local mirror. +the local path exists and is readable. Fix that rewrite or remove it if the +mirror is stale; configuring an SSH key or token does not repair a missing +local mirror. APM snapshots the effective Git config, validates the longest matching rewrite, and freezes the result for the child process. It drops malformed ambient HTTP diff --git a/packages/apm-guide/.apm/skills/apm-usage/authentication.md b/packages/apm-guide/.apm/skills/apm-usage/authentication.md index e9792fc74c..1561a2f9c2 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/authentication.md +++ b/packages/apm-guide/.apm/skills/apm-usage/authentication.md @@ -13,26 +13,32 @@ Only HTTP 401, 403, 404, or an equivalent Git authentication failure unlocks the Managed GitHub, GitLab, and Azure DevOps credentials use process-scoped Authorization headers. They are never embedded in Git URL userinfo. -Before each dependency Git operation that consumes a remote URL, APM rejects a -matching rewrite that embeds credentials, downgrades to insecure transports such -as `http://` or `git://`, selects remote-helper syntax such as `ext::` or -`https::`, or redirects any network remote to another host, regardless of host -class. A managed HTTPS credential cannot cross a scheme, host, or port boundary. -Same-host SSH and local-mirror selections remain credential-free. Inspect -rejected rules with: +Before each dependency Git operation that uses a remote URL, APM checks the +longest matching rewrite. It rejects rewrites that embed credentials, switch to +insecure transports such as `http://` or `git://`, use remote-helper syntax +such as `ext::` or `https::`, or send a network remote to a different host. A +managed HTTPS credential cannot cross a scheme, host, or port boundary. +Same-host SSH rewrites and local mirrors remain credential-free. + +If a rewrite is rejected and it should be safe, inspect the effective Git +config and matching `insteadOf` rules, then retry: ```bash git config --show-origin --get-regexp '^url\..*\.insteadOf$' ``` +Remove or replace the rule only if it is unsafe or misconfigured. + If the selected rewrite is a `file://` mirror and the clone fails, verify that the local path exists and is readable. Fix or remove that rewrite; host credentials cannot repair a missing local mirror. -APM snapshots effective Git config, validates the longest matching rewrite, and -freezes the result for the child. It drops malformed ambient HTTP headers before -applying an anonymous empty-header fence or one path-scoped AuthResolver header. -Dependency clones ignore Git templates and checkout hooks. +APM snapshots the effective Git config, validates the longest matching rewrite, +and passes that fixed result to the child Git process. It drops malformed +ambient HTTP headers before it applies either an anonymous empty-header fence or +one path-scoped AuthResolver header. For effective URLs outside HTTP(S), APM +skips the `http.extraHeader` URL-match probe. Dependency clones ignore Git +templates and checkout hooks. When fallback is required, APM checks these sources in order: diff --git a/src/apm_cli/utils/git_env.py b/src/apm_cli/utils/git_env.py index 1126950e23..f3a24b102d 100644 --- a/src/apm_cli/utils/git_env.py +++ b/src/apm_cli/utils/git_env.py @@ -134,11 +134,10 @@ _HTTP_HEADER_NAME_RE = re.compile(r"^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$") _GIT_CONFIG_PROBE_TIMEOUTS = (10, 30) -_URL_REWRITE_RECOVERY = ( - "inspect matching rules with " - "'git config --show-origin --get-regexp ^url\\..*\\.insteadOf$' " - "and remove the unsafe rule" +_URL_REWRITE_INSPECTION = ( + "inspect matching rules with 'git config --show-origin --get-regexp ^url\\..*\\.insteadOf$'" ) +_URL_REWRITE_RECOVERY = f"{_URL_REWRITE_INSPECTION} and remove the unsafe rule" class GitUrlRewriteError(ValueError): @@ -159,7 +158,7 @@ def __init__(self, category: str) -> None: self.category = category super().__init__( f"Unable to verify Git URL rewrite safety ({category}); " - f"check Git configuration and retry; {_URL_REWRITE_RECOVERY}" + f"check Git configuration and retry; {_URL_REWRITE_INSPECTION}" ) @@ -597,7 +596,13 @@ def _has_applicable_http_authorization( headers: Sequence[GitConfigEntry], env: dict[str, str], ) -> bool: - """Ask Git which URL-scoped extra headers apply to one remote.""" + """Ask Git which URL-scoped extra headers apply to one HTTP(S) remote.""" + try: + scheme = urlsplit(remote_url).scheme.lower() + except ValueError: + return False + if scheme not in {"http", "https"}: + return False direct_header = env.get("GIT_HTTP_EXTRAHEADER", "") if _is_valid_http_extraheader_value(direct_header) and _is_credential_bearing_http_header( direct_header @@ -657,7 +662,7 @@ def _urlmatched_header_group( if result.returncode == 1: return () if result.returncode != 0 or not isinstance(result.stdout, bytes): - raise GitUrlRewriteProbeError("Git URL-match probe failed") + raise GitUrlRewriteProbeError(f"Git URL-match probe exited with status {result.returncode}") selected = result.stdout.rstrip(b"\0\n") prefix = b"X-Apm-Config-Probe: " if not selected.startswith(prefix): diff --git a/tests/unit/cache/test_git_env.py b/tests/unit/cache/test_git_env.py index 352a2ed70c..1a5fa3753b 100644 --- a/tests/unit/cache/test_git_env.py +++ b/tests/unit/cache/test_git_env.py @@ -23,6 +23,7 @@ git_remote_refs, git_subprocess_env, git_subprocess_error_text, + git_url_has_authorization, reset_git_cache, set_git_authorization_header, ) @@ -341,6 +342,7 @@ def test_rewrite_probe_failure_has_safe_recovery(self) -> None: assert "Git config probe failed" in message assert "check Git configuration and retry" in message assert "--show-origin" in message + assert "remove the unsafe rule" not in message assert "private config detail" not in message def test_rewrite_probe_retries_once_after_timeout(self) -> None: @@ -918,6 +920,88 @@ def test_clone_rejects_ssh_remote_rewritten_to_http(self, tmp_path) -> None: env=env, ) + def test_clone_allows_scp_ssh_rewrite_while_a_header_is_injected(self, tmp_path) -> None: + env = { + "PATH": os.environ["PATH"], + "GIT_CONFIG_COUNT": "2", + "GIT_CONFIG_KEY_0": "http.extraheader", + "GIT_CONFIG_VALUE_0": "Authorization: Basic sentinel", + "GIT_CONFIG_KEY_1": "url.git@git.example.com:.insteadOf", + "GIT_CONFIG_VALUE_1": "https://git.example.com/", + } + with ( + patch.dict(os.environ, {"PATH": os.environ["PATH"]}, clear=True), + patch( + "apm_cli.utils.git_env.subprocess.run", + side_effect=_run_real_git_config_and_fake_clone, + ) as run, + ): + clone_git_worktree( + "https://git.example.com/acme/repo", + tmp_path / "clone", + env=env, + ) + + argv = run.call_args_list[-1].args[0] + assert "clone" in argv + assert [urlsplit(arg).hostname for arg in argv if urlsplit(arg).scheme == "https"] == [ + "git.example.com" + ] + + def test_scp_ssh_url_reports_no_http_authorization_without_probing_git(self) -> None: + headers = (GitConfigEntry("command", "http.extraheader", "Authorization: Basic sentinel"),) + with patch( + "apm_cli.utils.git_env._git_config_run", + side_effect=AssertionError("the URL-match probe must not run for a non-HTTP URL"), + ) as probe: + authorized = git_url_has_authorization("git@git.example.com:acme/repo", headers) + + assert authorized is False + probe.assert_not_called() + + def test_http_urlmatch_failure_reports_status_without_raw_config(self) -> None: + headers = (GitConfigEntry("command", "http.extraheader", "Authorization: Basic sentinel"),) + result = subprocess.CompletedProcess( + ["git", "config"], + 128, + stdout=b"", + stderr=b"private config detail", + ) + with ( + patch("apm_cli.utils.git_env._git_config_run", return_value=result), + pytest.raises(GitUrlRewriteProbeError) as raised, + ): + git_url_has_authorization("https://git.example.com/acme/repo", headers) + + message = str(raised.value) + assert "Git URL-match probe exited with status 128" in message + assert "check Git configuration and retry" in message + assert "remove the unsafe rule" not in message + assert "private config detail" not in message + + def test_malformed_rewrite_target_keeps_the_wrapped_safety_error(self, tmp_path) -> None: + env = { + "PATH": os.environ["PATH"], + "GIT_CONFIG_COUNT": "2", + "GIT_CONFIG_KEY_0": "http.extraheader", + "GIT_CONFIG_VALUE_0": "Authorization: Basic sentinel", + "GIT_CONFIG_KEY_1": "url.https://[::1/.insteadOf", + "GIT_CONFIG_VALUE_1": "https://git.example.com/", + } + with ( + patch.dict(os.environ, {"PATH": os.environ["PATH"]}, clear=True), + patch( + "apm_cli.utils.git_env.subprocess.run", + side_effect=_run_real_git_config_and_fake_clone, + ), + pytest.raises(ValueError, match="Unable to verify Git URL rewrite safety"), + ): + clone_git_worktree( + "https://git.example.com/acme/repo", + tmp_path / "clone", + env=env, + ) + @pytest.mark.parametrize( ("replacement", "message"), ( diff --git a/tests/unit/core/test_public_github_anonymous_first.py b/tests/unit/core/test_public_github_anonymous_first.py index 3bb51bf9d1..7db1aea5a3 100644 --- a/tests/unit/core/test_public_github_anonymous_first.py +++ b/tests/unit/core/test_public_github_anonymous_first.py @@ -24,6 +24,7 @@ TransportSelector, ) from apm_cli.models.dependency.reference import DependencyReference +from apm_cli.utils.git_env import get_git_executable, git_network_env _GITHUB_TOKEN_ENV_NAMES = { "GH_TOKEN", @@ -230,6 +231,88 @@ def operation(token: str | None, env: dict[str, str]) -> str: assert "GITHUB_APM_PAT_ACME" not in env +def test_public_github_authenticated_retry_drops_header_after_real_ssh_rewrite( + tmp_path: Path, +) -> None: + """The authenticated retry keeps the rewrite but loses HTTP auth on SSH.""" + git_config = tmp_path / "gitconfig" + git_config.write_text( + '[url "git@github.com:"]\n\tinsteadOf = https://github.com/\n', + encoding="ascii", + ) + base_env = { + "GIT_CONFIG_GLOBAL": str(git_config), + "GIT_CONFIG_NOSYSTEM": "1", + "PATH": os.environ["PATH"], + } + resolver = AuthResolver() + attempts: list[tuple[str | None, dict[str, str], dict[str, str]]] = [] + remote_url = "https://github.com/acme/widgets" + + def operation(token: str | None, env: dict[str, str]) -> str: + child = git_network_env(remote_url, env) + attempts.append((token, env, child)) + if token is None: + raise _HttpStatusError(404) + return "private-ok" + + with patch.dict( + os.environ, + {"GITHUB_APM_PAT_ACME": "private-token", "PATH": os.environ["PATH"]}, + clear=True, + ): + result = resolver.try_with_fallback( + "github.com", + operation, + org="acme", + path="acme/widgets", + unauth_first=True, + base_env=base_env, + ) + + assert result == "private-ok" + assert [token for token, _env, _child in attempts] == [None, "private-token"] + auth_retry_env = attempts[1][1] + auth_retry_child = attempts[1][2] + assert any( + key.lower().endswith("extraheader") and value.startswith("Authorization:") + for key, value in _indexed_git_config(auth_retry_env) + ) + assert _git_auth_entries(auth_retry_child) == [] + + headers = subprocess.run( + ( + get_git_executable(), + "config", + "--get-urlmatch", + "http.extraHeader", + remote_url, + ), + check=False, + capture_output=True, + text=True, + env=auth_retry_child, + cwd=tmp_path, + ) + rewrites = subprocess.run( + ( + get_git_executable(), + "config", + "--null", + "--get-regexp", + r"^url\..*\.insteadOf$", + ), + check=True, + capture_output=True, + env=auth_retry_child, + cwd=tmp_path, + ) + + assert headers.returncode == 1 + assert headers.stdout == "" + assert rewrites.stdout == b"url.git@github.com:.insteadof\nhttps://github.com/\0" + + @pytest.mark.parametrize("secondary_source", ("gh", "git")) def test_public_github_secondary_fallback_preserves_caller_git_config( tmp_path: Path, diff --git a/tests/unit/test_transport_selection.py b/tests/unit/test_transport_selection.py index cb26ef9eab..4b0aaea308 100644 --- a/tests/unit/test_transport_selection.py +++ b/tests/unit/test_transport_selection.py @@ -18,6 +18,8 @@ from __future__ import annotations import os +import subprocess +from pathlib import Path from typing import Dict, List, Optional # noqa: F401, UP035 from unittest.mock import patch @@ -38,6 +40,7 @@ protocol_pref_from_env, ) from apm_cli.models.dependency.reference import DependencyReference +from apm_cli.utils.git_env import GitUrlRewriteError, get_git_executable, git_network_env # --------------------------------------------------------------------------- # Helpers @@ -75,6 +78,64 @@ def _scheme_labels(plan: TransportPlan) -> list[str]: return [a.scheme for a in plan.attempts] +def _real_gitconfig_resolver_plan( + tmp_path: Path, + *, + rewrite_base: str, + rewrite_prefix: str, + candidate_url: str = "https://github.com/owner/repo", + command_header: str | None = None, +) -> TransportPlan: + """Return one transport plan using the production Git config resolver.""" + home = tmp_path / "home" + home.mkdir() + config = tmp_path / "gitconfig" + config.write_text( + f'[url "{rewrite_base}"]\n\tinsteadOf = {rewrite_prefix}\n', + encoding="ascii", + ) + env = { + "HOME": str(home), + "PATH": os.environ["PATH"], + "GIT_CONFIG_GLOBAL": str(config), + "GIT_CONFIG_NOSYSTEM": "1", + } + if command_header is not None: + env.update( + { + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "http.extraheader", + "GIT_CONFIG_VALUE_0": command_header, + } + ) + with patch.dict(os.environ, env, clear=True): + return TransportSelector(insteadof_resolver=GitConfigInsteadOfResolver()).select( + dep_ref=_dep("owner/repo"), + has_token=True, + candidate_url=candidate_url, + ) + + +def _real_git_headers(env: dict[str, str], remote_url: str, cwd: Path) -> list[str]: + """Return the header values real Git selects for one URL.""" + result = subprocess.run( + ( + get_git_executable(), + "config", + "--get-urlmatch", + "http.extraHeader", + remote_url, + ), + check=False, + capture_output=True, + text=True, + env=env, + cwd=cwd, + ) + assert result.returncode in {0, 1}, result.stderr + return result.stdout.splitlines() + + # --------------------------------------------------------------------------- # Selection matrix # --------------------------------------------------------------------------- @@ -500,3 +561,116 @@ def test_resolve_returns_none_when_no_rewrites(self): run.return_value.returncode = 0 run.return_value.stdout = b"" assert resolver.resolve("https://github.com/owner/repo") is None + + @pytest.mark.parametrize("command_header", [None, "Authorization: Basic sentinel"]) + def test_real_resolver_allows_https_to_scp_ssh_rewrite( + self, + tmp_path: Path, + command_header: str | None, + ) -> None: + """A command-scoped HTTP header does not block same-host SCP rewrites.""" + plan = _real_gitconfig_resolver_plan( + tmp_path, + rewrite_base="git@github.com:", + rewrite_prefix="https://github.com/", + command_header=command_header, + ) + + assert _scheme_labels(plan) == ["ssh"] + assert plan.strict is True + assert plan.attempts[0].requested_url == "https://github.com/owner/repo" + assert plan.attempts[0].effective_url == "git@github.com:owner/repo" + assert plan.attempts[0].use_token is False + + def test_real_resolver_unmatched_command_header_leaves_https_plan( + self, + tmp_path: Path, + ) -> None: + """A non-matching header-only config stays on authenticated HTTPS.""" + plan = _real_gitconfig_resolver_plan( + tmp_path, + rewrite_base="git@github.com:", + rewrite_prefix="https://github.com/acme/", + command_header="Authorization: Basic sentinel", + ) + + assert _scheme_labels(plan) == ["https"] + assert plan.strict is True + assert plan.attempts[0].requested_url is None + assert plan.attempts[0].effective_url is None + assert plan.attempts[0].use_token is True + + def test_real_resolver_allows_https_to_ssh_url_rewrite( + self, + tmp_path: Path, + ) -> None: + """The real resolver also accepts same-host ssh:// rewrites.""" + plan = _real_gitconfig_resolver_plan( + tmp_path, + rewrite_base="ssh://git@github.com/", + rewrite_prefix="https://github.com/", + command_header="Authorization: Basic sentinel", + ) + + assert _scheme_labels(plan) == ["ssh"] + assert plan.strict is True + assert plan.attempts[0].requested_url == "https://github.com/owner/repo" + assert plan.attempts[0].effective_url == "ssh://git@github.com/owner/repo" + assert plan.attempts[0].use_token is False + + def test_real_resolver_checks_http_authorization_for_https_rewrite( + self, + tmp_path: Path, + ) -> None: + """HTTP(S) rewrites still enforce the credential origin boundary.""" + with pytest.raises(GitUrlRewriteError, match="different HTTPS origin"): + _real_gitconfig_resolver_plan( + tmp_path, + rewrite_base="https://github.com:8443/", + rewrite_prefix="https://github.com/", + command_header="Authorization: Basic sentinel", + ) + + def test_real_resolver_consumer_drops_dummy_http_header_after_scp_rewrite( + self, + tmp_path: Path, + ) -> None: + """A real selector + git_network_env consumer keeps rewrite but drops HTTP auth.""" + plan = _real_gitconfig_resolver_plan( + tmp_path, + rewrite_base="git@github.com:", + rewrite_prefix="https://github.com/", + command_header="Authorization: Basic sentinel", + ) + + attempt = plan.attempts[0] + assert attempt.requested_url == "https://github.com/owner/repo" + assert attempt.effective_url == "git@github.com:owner/repo" + + env = { + "HOME": str(tmp_path / "home"), + "PATH": os.environ["PATH"], + "GIT_CONFIG_GLOBAL": str(tmp_path / "gitconfig"), + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "http.extraheader", + "GIT_CONFIG_VALUE_0": "Authorization: Basic sentinel", + } + with patch.dict(os.environ, env, clear=True): + child = git_network_env(attempt.requested_url, env) + + assert _real_git_headers(child, attempt.requested_url, tmp_path) == [] + rewrites = subprocess.run( + ( + get_git_executable(), + "config", + "--null", + "--get-regexp", + r"^url\..*\.insteadOf$", + ), + check=True, + capture_output=True, + env=child, + cwd=tmp_path, + ) + assert rewrites.stdout == b"url.git@github.com:.insteadof\nhttps://github.com/\0"