From 71abc44cce1aae6b11dcd39d7decd0d4fe96eace Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Sat, 16 May 2026 13:42:42 +0100 Subject: [PATCH 1/2] fix: skip direct GitHub API validation when PROXY_REGISTRY_ONLY is set When PROXY_REGISTRY_ONLY=1 (enterprise proxy-only mode), apm install fails because _validate_package_exists probes github.com directly. Add is_enforce_only() guards to all three validation paths: 1. Virtual-package path: skip validate_virtual_package_exists() 2. GitHub.com path: skip _check_repo API call 3. Parse-failure fallback: skip _check_repo_fallback API call In proxy-only mode the download step is the authoritative existence check -- the proxy will return 404 for absent packages. Closes #615 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/install/validation.py | 15 + tests/unit/install/test_validation_proxy.py | 314 ++++++++++++++++++++ 2 files changed, 329 insertions(+) create mode 100644 tests/unit/install/test_validation_proxy.py diff --git a/src/apm_cli/install/validation.py b/src/apm_cli/install/validation.py index 7bcaa8874..379c9a2c7 100644 --- a/src/apm_cli/install/validation.py +++ b/src/apm_cli/install/validation.py @@ -183,6 +183,8 @@ def _validate_package_exists(package, verbose=False, auth_resolver=None, logger= from apm_cli.utils.github_host import is_azure_devops_hostname, is_github_hostname + from ..deps.registry_proxy import is_enforce_only + virtual_subdir_repo_probe = ( dep_ref.is_virtual and dep_ref.is_virtual_subdirectory() @@ -194,6 +196,8 @@ def _validate_package_exists(package, verbose=False, auth_resolver=None, logger= # the virtual path is a subdirectory on a non-GitHub host. Those should # validate the clone root with git, preserving SSH/credential-helper flows. if dep_ref.is_virtual and not virtual_subdir_repo_probe: + if is_enforce_only(): + return True ctx = auth_resolver.resolve_for_dep(dep_ref) host = dep_ref.host or default_host() org = ( @@ -509,6 +513,12 @@ def _log_attempt_result(probe_url: str, run_result): ) host_info = auth_resolver.classify_host(host, port=port) + if is_enforce_only(): + # PROXY_REGISTRY_ONLY=1: skip the GitHub API probe. + # Marketplace/lockfile resolution already ran through the proxy; + # the download step will surface a proxy 404 if absent. + return True + if verbose_log: ctx = auth_resolver.resolve(host, org=org, port=port) verbose_log( @@ -596,6 +606,11 @@ def _check_repo(token, git_env): if not re.fullmatch(r"[A-Za-z0-9._-]+/[A-Za-z0-9._-]+", repo_path): return False + from ..deps.registry_proxy import is_enforce_only + + if is_enforce_only(): + return True + def _check_repo_fallback(token, git_env): host_info = auth_resolver.classify_host(host) api_url = f"{host_info.api_base}/repos/{repo_path}" diff --git a/tests/unit/install/test_validation_proxy.py b/tests/unit/install/test_validation_proxy.py new file mode 100644 index 000000000..3f8af2fcd --- /dev/null +++ b/tests/unit/install/test_validation_proxy.py @@ -0,0 +1,314 @@ +"""Tests for proxy validation bypass fix (PROXY_REGISTRY_ONLY guard). + +PR #615 adds two `is_enforce_only()` guards to skip GitHub API calls when +PROXY_REGISTRY_ONLY=1 is set: + +1. Guard 1 (~line 514): GitHub.com path — skips _check_repo API call +2. Guard 2 (~line 609): Parse-failure fallback path — skips _check_repo_fallback API call + +This test suite validates that when PROXY_REGISTRY_ONLY=1, API calls are +skipped and the function returns True (allowing the download step to enforce +proxy access and surface a proxy 404 if needed). +""" + +from __future__ import annotations + +import os +from unittest.mock import MagicMock, patch + +from apm_cli.install import validation + + +class TestProxyBypassGuard: + """Test that is_enforce_only() guards skip GitHub API calls in both code paths.""" + + def _setup_resolver(self): + """Build a minimal AuthResolver mock for GitHub.com packages.""" + resolver = MagicMock() + host_info = MagicMock() + host_info.api_base = "https://api.github.com" + host_info.display_name = "github.com" + host_info.kind = "github" + host_info.has_public_repos = True + resolver.classify_host.return_value = host_info + ctx = MagicMock(source="env", token_type="pat", token=None) + resolver.resolve.return_value = ctx + resolver.resolve_for_dep.return_value = ctx + + # Single-call shim: invoke the operation once unauth. + def _fake_fallback(host, op, **kwargs): + return op(None, {}) + + resolver.try_with_fallback.side_effect = _fake_fallback + return resolver + + def test_github_path_skipped_when_enforce_only(self): + """Guard 1: GitHub.com path skips API when PROXY_REGISTRY_ONLY=1.""" + resolver = self._setup_resolver() + + with patch.dict(os.environ, {"PROXY_REGISTRY_ONLY": "1"}): + with patch("apm_cli.install.validation.requests.get") as mock_get: + result = validation._validate_package_exists( + "microsoft/apm", + verbose=False, + auth_resolver=resolver, + logger=None, + ) + + # Should return True (proxy-only mode: download step handles enforcement) + assert result is True + + # API call must be skipped entirely + assert mock_get.call_count == 0 + + def test_github_path_calls_api_without_enforce_only(self): + """GitHub.com path calls API when PROXY_REGISTRY_ONLY is NOT set.""" + resolver = self._setup_resolver() + + # Explicitly clear PROXY_REGISTRY_ONLY to ensure API path is taken. + env = {k: v for k, v in os.environ.items() if k != "PROXY_REGISTRY_ONLY"} + + with patch.dict(os.environ, env, clear=True): + with patch("apm_cli.install.validation.requests.get") as mock_get: + # Mock a successful API response (200 OK). + mock_response = MagicMock() + mock_response.status_code = 200 + mock_get.return_value = mock_response + + result = validation._validate_package_exists( + "microsoft/apm", + verbose=False, + auth_resolver=resolver, + logger=None, + ) + + # Should return True (API succeeded). + assert result is True + + # API call must have been made (to probe the repo). + assert mock_get.call_count >= 1 + + def test_fallback_path_skipped_when_enforce_only(self): + """Guard 2: Fallback path skips API when PROXY_REGISTRY_ONLY=1. + + The fallback path is triggered when DependencyReference.parse() raises + an error but the input matches owner/repo format. We mock the parse + failure to exercise this code path. + """ + resolver = self._setup_resolver() + + # Mock DependencyReference.parse to raise, triggering the fallback path. + # The fallback will extract owner/repo from the package string directly. + with patch.dict(os.environ, {"PROXY_REGISTRY_ONLY": "1"}): + with patch( + "apm_cli.models.apm_package.DependencyReference.parse", + side_effect=ValueError("Simulated parse error"), + ): + with patch("apm_cli.install.validation.requests.get") as mock_get: + # Use a valid owner/repo format for the fallback path. + result = validation._validate_package_exists( + "owner/repo", + verbose=False, + auth_resolver=resolver, + logger=None, + ) + + # Guard 2 should return True (proxy-only mode). + assert result is True + + # API call must be skipped. + assert mock_get.call_count == 0 + + def test_fallback_path_calls_api_without_enforce_only(self): + """Fallback path calls API when PROXY_REGISTRY_ONLY is NOT set. + + Same setup as above, but with PROXY_REGISTRY_ONLY unset, the fallback + should proceed to make an API call. + """ + resolver = self._setup_resolver() + + # Explicitly clear PROXY_REGISTRY_ONLY. + env = {k: v for k, v in os.environ.items() if k != "PROXY_REGISTRY_ONLY"} + + with patch.dict(os.environ, env, clear=True): + with patch( + "apm_cli.models.apm_package.DependencyReference.parse", + side_effect=ValueError("Simulated parse error"), + ): + with patch("apm_cli.install.validation.requests.get") as mock_get: + # Mock a successful API response. + mock_response = MagicMock() + mock_response.status_code = 200 + mock_get.return_value = mock_response + + # Use a valid owner/repo format for the fallback path. + result = validation._validate_package_exists( + "owner/repo", + verbose=False, + auth_resolver=resolver, + logger=None, + ) + + # Should return True (API succeeded). + assert result is True + + # API call must have been made. + assert mock_get.call_count >= 1 + + def test_fallback_path_rejects_invalid_owner_repo_format(self): + """Fallback path rejects inputs that don't match owner/repo format. + + Even in the fallback path, we have a strict regex check to prevent + path-confusion attacks. Inputs like "../", embedded slashes, or control + bytes should be rejected outright. + """ + resolver = self._setup_resolver() + + # Mock DependencyReference.parse to raise. + with patch( + "apm_cli.models.apm_package.DependencyReference.parse", + side_effect=ValueError("Simulated parse error"), + ): + with patch("apm_cli.install.validation.requests.get") as mock_get: + # Invalid formats should be rejected (return False) WITHOUT + # making any API call. + test_cases = [ + "../../../evil", # Path traversal + "owner/repo/extra", # Too many segments + "owner", # No slash + "owner/", # Missing repo + "/repo", # Missing owner + ] + + for invalid_input in test_cases: + mock_get.reset_mock() + result = validation._validate_package_exists( + invalid_input, + verbose=False, + auth_resolver=resolver, + logger=None, + ) + + # Should be rejected without an API call. + assert result is False, f"Expected False for {invalid_input}" + assert mock_get.call_count == 0, f"Unexpected API call for {invalid_input}" + + def test_enforce_only_variants(self): + """is_enforce_only() recognizes multiple truthy env var values. + + The function accepts "1", "true", "yes" (case-insensitive). + """ + resolver = self._setup_resolver() + + # Test multiple truthy variants. + truthy_values = ["1", "true", "True", "TRUE", "yes", "YES"] + + for value in truthy_values: + with patch.dict(os.environ, {"PROXY_REGISTRY_ONLY": value}): + with patch("apm_cli.install.validation.requests.get") as mock_get: + result = validation._validate_package_exists( + "microsoft/apm", + verbose=False, + auth_resolver=resolver, + logger=None, + ) + + # Each variant should skip the API call. + assert mock_get.call_count == 0, ( + f"API was called with PROXY_REGISTRY_ONLY={value}" + ) + assert result is True + + def test_virtual_path_skipped_when_enforce_only(self): + """Guard 3: Virtual package path skips downloader when PROXY_REGISTRY_ONLY=1. + + When a dep_ref has is_virtual=True and is not a subdirectory-on- + non-GitHub-host, the validator would normally call + GitHubPackageDownloader.validate_virtual_package_exists(). The new + is_enforce_only() guard at line 199-200 skips that call and returns True. + """ + resolver = self._setup_resolver() + + # Create a virtual dep_ref mock (simulating a virtual package). + virtual_ref = MagicMock() + virtual_ref.is_virtual = True + virtual_ref.virtual_path = "prompts/code-review.prompt.md" + virtual_ref.is_virtual_subdirectory.return_value = False # Not a subdirectory + virtual_ref.is_local = False # Not local + virtual_ref.local_path = None + virtual_ref.repo_url = "owner/repo" + virtual_ref.host = "github.com" + virtual_ref.reference = "main" + virtual_ref.is_azure_devops.return_value = False + + with patch.dict(os.environ, {"PROXY_REGISTRY_ONLY": "1"}): + with patch("apm_cli.utils.github_host.is_github_hostname") as mock_is_github: + mock_is_github.return_value = True # github.com is GitHub + with patch( + "apm_cli.deps.github_downloader.GitHubPackageDownloader" + ) as mock_downloader_class: + # The mock downloader instance's validate_virtual_package_exists should NOT be called. + mock_downloader_instance = MagicMock() + mock_downloader_class.return_value = mock_downloader_instance + + result = validation._validate_package_exists( + "owner/repo/prompts/code-review.prompt.md", + verbose=False, + auth_resolver=resolver, + logger=None, + dep_ref=virtual_ref, + ) + + # Guard 3 should return True (proxy-only mode). + assert result is True + + # validate_virtual_package_exists must NOT be called. + mock_downloader_instance.validate_virtual_package_exists.assert_not_called() + + def test_virtual_path_calls_downloader_without_enforce_only(self): + """Virtual package path calls downloader when PROXY_REGISTRY_ONLY is NOT set. + + Same setup as above, but without PROXY_REGISTRY_ONLY, the virtual + package validation should proceed to call validate_virtual_package_exists. + """ + resolver = self._setup_resolver() + + # Create a virtual dep_ref mock (simulating a virtual package). + virtual_ref = MagicMock() + virtual_ref.is_virtual = True + virtual_ref.virtual_path = "src/agent" + virtual_ref.is_virtual_subdirectory.return_value = True # Is a subdirectory + virtual_ref.is_local = False # Not local + virtual_ref.local_path = None + virtual_ref.repo_url = "owner/repo" + virtual_ref.host = "github.com" + virtual_ref.reference = "main" + virtual_ref.is_azure_devops.return_value = False + + # Explicitly clear PROXY_REGISTRY_ONLY. + env = {k: v for k, v in os.environ.items() if k != "PROXY_REGISTRY_ONLY"} + + with patch.dict(os.environ, env, clear=True): + with patch("apm_cli.utils.github_host.is_github_hostname") as mock_is_github: + mock_is_github.return_value = True # github.com is GitHub + with patch( + "apm_cli.deps.github_downloader.GitHubPackageDownloader" + ) as mock_downloader_class: + # Mock the downloader instance and its validate_virtual_package_exists method. + mock_downloader_instance = MagicMock() + mock_downloader_instance.validate_virtual_package_exists.return_value = True + mock_downloader_class.return_value = mock_downloader_instance + + result = validation._validate_package_exists( + "owner/repo/src/agent", + verbose=False, + auth_resolver=resolver, + logger=None, + dep_ref=virtual_ref, + ) + + # Should return True (downloader validation succeeded). + assert result is True + + # validate_virtual_package_exists MUST be called. + mock_downloader_instance.validate_virtual_package_exists.assert_called_once() From a4a137b342f6b0aa1f9dfd9bb0dc5f7faa3cc0aa Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Sat, 16 May 2026 20:48:28 +0100 Subject: [PATCH 2/2] fix: address review panel findings for PR #1357 (issue #615) B1: add is_enforce_only() guard to the ADO/GHES git ls-remote path so that PROXY_REGISTRY_ONLY=1 skips the subprocess call and returns True, matching the behaviour of all other network call sites. B2: fix contradictory mock in test_virtual_path_calls_downloader_without_enforce_only (is_virtual_subdirectory=True was irrelevant when is_github_hostname=True). N1: replace UTF-8 em dashes in module docstring with ASCII hyphens. N2: update class docstring to reference all four network call paths. R1: clear ARTIFACTORY_ONLY as well as PROXY_REGISTRY_ONLY in all 'without enforce' tests; add test_deprecated_artifactory_only_skips_api to cover the deprecated alias path through is_enforce_only(). Panel R1: add logger.info() skip-notice at all four guard sites. Panel R2: add CHANGELOG entry under ## [Unreleased] / ### Fixed. Tests: add test_ado_ghes_path_skipped_when_enforce_only (guard active) and test_ado_ghes_path_calls_git_without_enforce_only (guard inactive, subprocess.run IS invoked); patch GitHubPackageDownloader at source module to avoid real URL-building internals in the unit test. --- CHANGELOG.md | 1 + src/apm_cli/install/validation.py | 28 ++++ tests/unit/install/test_validation_proxy.py | 171 ++++++++++++++++++-- 3 files changed, 189 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5cc53e42..fa0885dea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed direct GitHub API and ADO/GHES `git ls-remote` calls not respecting `PROXY_REGISTRY_ONLY` mode; all four validation code paths (virtual package, GitHub.com API, ADO/GHES git, and parse-failure fallback) now skip outbound network probes and return `True` when proxy-only mode is active. (#615) - MCP server installation now respects the `targets:` whitelist exactly like `apm install`: drop a non-listed runtime even when its `.cursor/`, `.codex/`, or other on-disk signal exists. Previously the MCP install path called `active_targets()` reading the singular `target:` key only, so projects whitelisting `targets: [copilot]` could still receive `~/.codex/config.toml` and `.cursor/mcp.json` writes from foreign signals. The fix audits both paths: (a) the call site at `local_bundle_handler.py` now forwards the canonical plural list; (b) the gate now delegates to the same `resolve_targets` resolver that backs `apm install` skills, so a malformed `targets:` field (conflicting `target:` + `targets:`, `targets: []`, or unknown target name) fails closed with the same `[x]` red error voice + remediation block. The same delegation closes a related asymmetry: a greenfield project (no `targets:`, no `--target` flag, no detected signals) used to silently fall back to `[copilot]` for MCP-only invocations, while `apm install` raised `NoHarnessError` on the same input -- both surfaces now error consistently. Drop lines now use the `[i] Skipped MCP config for X (active targets: Y)` format mirroring the canonical `Targets: X (source: Y)` provenance line. The `-g`/`--global` carve-out is unchanged: `apm install -g --mcp NAME` writes to user-scope (`~/.config/...`, `~/.codex/`, etc.) bypassing the project-scope gate by design. (#1335) - Gemini CLI: `apm install -g --mcp NAME` now correctly writes to `~/.gemini/settings.json` (user scope) and `apm install` from outside the target project writes to `/.gemini/settings.json` instead of `cwd`. Previously `--global` had no effect on Gemini and project-scope writes silently landed in the wrong directory. The matching opt-in gate and cleanup paths in `MCPIntegrator` are aligned in the same change. (#1299) - `apm install --target claude` now preserves self-defined stdio MCP `env` values from `apm.yml` and writes non-string values such as `PORT: 3000` and `DEBUG: false` as MCP-compatible strings. (#1222) diff --git a/src/apm_cli/install/validation.py b/src/apm_cli/install/validation.py index 379c9a2c7..06fe69f80 100644 --- a/src/apm_cli/install/validation.py +++ b/src/apm_cli/install/validation.py @@ -197,6 +197,13 @@ def _validate_package_exists(package, verbose=False, auth_resolver=None, logger= # validate the clone root with git, preserving SSH/credential-helper flows. if dep_ref.is_virtual and not virtual_subdir_repo_probe: if is_enforce_only(): + # PROXY_REGISTRY_ONLY=1: skip virtual package validation probe. + # The download step will surface a proxy 404 if the package is absent. + if logger: + logger.info( + "Skipping virtual package validation for" + f" {dep_ref.host or 'remote'}: proxy-only mode is active" + ) return True ctx = auth_resolver.resolve_for_dep(dep_ref) host = dep_ref.host or default_host() @@ -264,6 +271,16 @@ def _warn(msg: str) -> None: or dep_ref.is_azure_devops() or (dep_ref.host and dep_ref.host != "github.com") ): + if is_enforce_only(): + # PROXY_REGISTRY_ONLY=1: skip direct git ls-remote probe for ADO/GHES. + # The download step will surface a proxy 404 if the package is absent. + if logger: + logger.info( + "Skipping direct git ls-remote for" + f" {dep_ref.host or 'remote'}: proxy-only mode is active" + ) + return True + # Determine host type before building the URL so we know whether to # embed a token. Generic (non-GitHub, non-ADO) hosts are excluded # from APM-managed auth; they rely on git credential helpers via the @@ -517,6 +534,10 @@ def _log_attempt_result(probe_url: str, run_result): # PROXY_REGISTRY_ONLY=1: skip the GitHub API probe. # Marketplace/lockfile resolution already ran through the proxy; # the download step will surface a proxy 404 if absent. + if logger: + logger.info( + f"Skipping direct GitHub API probe for {host}: proxy-only mode is active" + ) return True if verbose_log: @@ -609,6 +630,13 @@ def _check_repo(token, git_env): from ..deps.registry_proxy import is_enforce_only if is_enforce_only(): + # PROXY_REGISTRY_ONLY=1: skip the GitHub API fallback probe. + # The download step will surface a proxy 404 if the package is absent. + if logger: + logger.info( + f"Skipping direct GitHub API fallback probe for {host}:" + " proxy-only mode is active" + ) return True def _check_repo_fallback(token, git_env): diff --git a/tests/unit/install/test_validation_proxy.py b/tests/unit/install/test_validation_proxy.py index 3f8af2fcd..8a5365b37 100644 --- a/tests/unit/install/test_validation_proxy.py +++ b/tests/unit/install/test_validation_proxy.py @@ -1,10 +1,12 @@ """Tests for proxy validation bypass fix (PROXY_REGISTRY_ONLY guard). -PR #615 adds two `is_enforce_only()` guards to skip GitHub API calls when +PR #615 adds `is_enforce_only()` guards to skip GitHub API calls when PROXY_REGISTRY_ONLY=1 is set: -1. Guard 1 (~line 514): GitHub.com path — skips _check_repo API call -2. Guard 2 (~line 609): Parse-failure fallback path — skips _check_repo_fallback API call +1. Guard 1 (~line 199): Virtual package path -- skips validate_virtual_package_exists +2. Guard 2 (~line 514): GitHub.com path -- skips _check_repo API call +3. Guard 3 (~line 609): Parse-failure fallback path -- skips _check_repo_fallback API call +4. Guard 4 (~line 273): ADO/GHES path -- skips git ls-remote probe This test suite validates that when PROXY_REGISTRY_ONLY=1, API calls are skipped and the function returns True (allowing the download step to enforce @@ -20,7 +22,7 @@ class TestProxyBypassGuard: - """Test that is_enforce_only() guards skip GitHub API calls in both code paths.""" + """Test that is_enforce_only() guards skip network calls on all code paths.""" def _setup_resolver(self): """Build a minimal AuthResolver mock for GitHub.com packages.""" @@ -65,8 +67,13 @@ def test_github_path_calls_api_without_enforce_only(self): """GitHub.com path calls API when PROXY_REGISTRY_ONLY is NOT set.""" resolver = self._setup_resolver() - # Explicitly clear PROXY_REGISTRY_ONLY to ensure API path is taken. - env = {k: v for k, v in os.environ.items() if k != "PROXY_REGISTRY_ONLY"} + # Explicitly clear both PROXY_REGISTRY_ONLY and the deprecated ARTIFACTORY_ONLY + # alias to ensure API path is taken regardless of test environment state. + env = { + k: v + for k, v in os.environ.items() + if k not in ("PROXY_REGISTRY_ONLY", "ARTIFACTORY_ONLY") + } with patch.dict(os.environ, env, clear=True): with patch("apm_cli.install.validation.requests.get") as mock_get: @@ -127,8 +134,13 @@ def test_fallback_path_calls_api_without_enforce_only(self): """ resolver = self._setup_resolver() - # Explicitly clear PROXY_REGISTRY_ONLY. - env = {k: v for k, v in os.environ.items() if k != "PROXY_REGISTRY_ONLY"} + # Explicitly clear both PROXY_REGISTRY_ONLY and the deprecated ARTIFACTORY_ONLY + # alias to ensure API path is taken regardless of test environment state. + env = { + k: v + for k, v in os.environ.items() + if k not in ("PROXY_REGISTRY_ONLY", "ARTIFACTORY_ONLY") + } with patch.dict(os.environ, env, clear=True): with patch( @@ -277,7 +289,12 @@ def test_virtual_path_calls_downloader_without_enforce_only(self): virtual_ref = MagicMock() virtual_ref.is_virtual = True virtual_ref.virtual_path = "src/agent" - virtual_ref.is_virtual_subdirectory.return_value = True # Is a subdirectory + # B2 fix: is_virtual_subdirectory=False is consistent with is_github_hostname=True + # (GitHub hosts never use the subdirectory probe path). Setting it True alongside + # is_github_hostname=True previously created a contradictory state where + # virtual_subdir_repo_probe evaluated to False anyway, making the subdirectory + # flag irrelevant and the test misleading. + virtual_ref.is_virtual_subdirectory.return_value = False virtual_ref.is_local = False # Not local virtual_ref.local_path = None virtual_ref.repo_url = "owner/repo" @@ -285,8 +302,13 @@ def test_virtual_path_calls_downloader_without_enforce_only(self): virtual_ref.reference = "main" virtual_ref.is_azure_devops.return_value = False - # Explicitly clear PROXY_REGISTRY_ONLY. - env = {k: v for k, v in os.environ.items() if k != "PROXY_REGISTRY_ONLY"} + # Explicitly clear both PROXY_REGISTRY_ONLY and the deprecated ARTIFACTORY_ONLY + # alias to ensure the downloader path is taken regardless of test environment state. + env = { + k: v + for k, v in os.environ.items() + if k not in ("PROXY_REGISTRY_ONLY", "ARTIFACTORY_ONLY") + } with patch.dict(os.environ, env, clear=True): with patch("apm_cli.utils.github_host.is_github_hostname") as mock_is_github: @@ -312,3 +334,130 @@ def test_virtual_path_calls_downloader_without_enforce_only(self): # validate_virtual_package_exists MUST be called. mock_downloader_instance.validate_virtual_package_exists.assert_called_once() + + def test_ado_ghes_path_skipped_when_enforce_only(self): + """Guard 4: ADO/GHES git ls-remote path skips probe when PROXY_REGISTRY_ONLY=1. + + When a dep has a non-github.com host the validator would normally run + ``git ls-remote`` against ADO or GHES. The new is_enforce_only() guard + skips that subprocess call and returns True immediately. + """ + resolver = self._setup_resolver() + + ado_ref = MagicMock() + ado_ref.is_virtual = False + ado_ref.is_local = False + ado_ref.local_path = None + ado_ref.host = "dev.azure.com" + ado_ref.repo_url = "myorg/myrepo" + ado_ref.port = None + ado_ref.is_azure_devops.return_value = True + ado_ref.is_virtual_subdirectory.return_value = False + ado_ref.explicit_scheme = None + ado_ref.is_insecure = False + + with patch.dict(os.environ, {"PROXY_REGISTRY_ONLY": "1"}): + with patch("subprocess.run") as mock_run: + result = validation._validate_package_exists( + "myorg/myrepo", + verbose=False, + auth_resolver=resolver, + logger=None, + dep_ref=ado_ref, + ) + + # Guard 4 should return True (proxy-only mode). + assert result is True + + # git ls-remote must NOT be called. + assert mock_run.call_count == 0, ( + "subprocess.run (git ls-remote) was called in proxy-only mode" + ) + + def test_ado_ghes_path_calls_git_without_enforce_only(self): + """ADO/GHES path runs git ls-remote when PROXY_REGISTRY_ONLY is NOT set. + + Patches GitHubPackageDownloader at its source module so the per-function + ``from apm_cli.deps.github_downloader import GitHubPackageDownloader`` + picks up the mock, avoiding real auth/URL-building internals while still + exercising the subprocess.run call site. + """ + resolver = self._setup_resolver() + + ado_ref = MagicMock() + ado_ref.is_virtual = False + ado_ref.is_local = False + ado_ref.local_path = None + ado_ref.host = "dev.azure.com" + ado_ref.repo_url = "myorg/myrepo" + ado_ref.port = None + ado_ref.is_azure_devops.return_value = True + ado_ref.is_virtual_subdirectory.return_value = False + ado_ref.explicit_scheme = None + ado_ref.is_insecure = False + + # Stub the downloader so _build_repo_url and .git_env don't hit real code. + mock_downloader = MagicMock() + mock_downloader._build_repo_url.return_value = "https://dev.azure.com/myorg/myrepo" + mock_downloader.git_env = {} + mock_downloader._sanitize_git_error.return_value = "" + mock_dl_cls = MagicMock(return_value=mock_downloader) + + # Explicitly clear both enforce env vars. + env = { + k: v + for k, v in os.environ.items() + if k not in ("PROXY_REGISTRY_ONLY", "ARTIFACTORY_ONLY") + } + + with patch.dict(os.environ, env, clear=True): + with patch("apm_cli.deps.github_downloader.GitHubPackageDownloader", mock_dl_cls): + with patch("subprocess.run") as mock_run: + mock_proc = MagicMock() + mock_proc.returncode = 0 + mock_proc.stderr = "" + mock_run.return_value = mock_proc + + result = validation._validate_package_exists( + "myorg/myrepo", + verbose=False, + auth_resolver=resolver, + logger=None, + dep_ref=ado_ref, + ) + + # Should return True (git ls-remote succeeded). + assert result is True + + # subprocess.run (git ls-remote) MUST have been called. + assert mock_run.call_count >= 1, "subprocess.run (git ls-remote) was not called" + + def test_deprecated_artifactory_only_skips_api(self): + """ARTIFACTORY_ONLY (deprecated alias) also triggers proxy-only bypass. + + is_enforce_only() falls back to ARTIFACTORY_ONLY when PROXY_REGISTRY_ONLY + is unset. This test verifies the deprecated alias is honoured so existing + deployments that have not yet migrated keep working correctly. + """ + resolver = self._setup_resolver() + + # Set the deprecated alias and ensure the canonical var is absent. + env = { + k: v + for k, v in os.environ.items() + if k not in ("PROXY_REGISTRY_ONLY", "ARTIFACTORY_ONLY") + } + env["ARTIFACTORY_ONLY"] = "1" + + with patch.dict(os.environ, env, clear=True): + with patch("apm_cli.install.validation.requests.get") as mock_get: + result = validation._validate_package_exists( + "microsoft/apm", + verbose=False, + auth_resolver=resolver, + logger=None, + ) + + # The deprecated alias must trigger the same bypass. + assert result is True + assert mock_get.call_count == 0, "API was called despite ARTIFACTORY_ONLY=1"