From 719fa90e8053c21f130b14b30ee00fa9a426ff0a Mon Sep 17 00:00:00 2001 From: Abbas Jafari Date: Thu, 23 Apr 2026 15:32:07 +0200 Subject: [PATCH 1/2] fix(install): honour REQUESTS_CA_BUNDLE in package validator The validator probed the GitHub API via stdlib urllib, which only honours SSL_CERT_FILE. Behind a TLS-intercepting proxy this surfaced as a misleading "package not accessible" error, sending users down the PAT troubleshooting path instead of the CA-trust one. Switch both _check_repo helpers to requests.get, catch SSLError explicitly, log a CA-trust hint to --verbose, and skip the auth-error context render on TLS failures. --- src/apm_cli/install/validation.py | 108 ++++++++++---- tests/unit/install/test_validation_tls.py | 169 ++++++++++++++++++++++ 2 files changed, 248 insertions(+), 29 deletions(-) create mode 100644 tests/unit/install/test_validation_tls.py diff --git a/src/apm_cli/install/validation.py b/src/apm_cli/install/validation.py index e67baec8c..dbb9fc6c7 100644 --- a/src/apm_cli/install/validation.py +++ b/src/apm_cli/install/validation.py @@ -25,9 +25,51 @@ from pathlib import Path +import requests + from ..utils.console import _rich_echo, _rich_info from ..utils.github_host import default_host +# --------------------------------------------------------------------------- +# TLS failure helpers +# --------------------------------------------------------------------------- + +# Marker prefix used on RuntimeError messages raised when the underlying +# network probe fails TLS verification. Lets the caller distinguish trust +# failures from auth / 404 / network errors so the user is not pushed down +# the PAT troubleshooting path for a CA-trust problem. +_TLS_ERROR_PREFIX = "TLS verification failed" + + +def _is_tls_failure(exc: BaseException) -> bool: + """Return True if exc (or any cause in its chain) is a TLS verification failure.""" + cur: BaseException | None = exc + seen = 0 + while cur is not None and seen < 8: + msg = str(cur) + if _TLS_ERROR_PREFIX in msg or "CERTIFICATE_VERIFY_FAILED" in msg: + return True + # requests.exceptions.SSLError is the canonical type once we stop + # using urllib; check by name to avoid importing requests at module + # import time. + if type(cur).__name__ == "SSLError": + return True + cur = cur.__cause__ or cur.__context__ + seen += 1 + return False + + +def _log_tls_failure(host_display: str, exc: BaseException, verbose_log) -> None: + """Log a TLS verification failure to the verbose stream with a CA-trust hint.""" + if not verbose_log: + return + verbose_log(f"TLS verification failed for {host_display}: {exc}") + verbose_log( + " hint: the CA bundle does not trust the certificate presented " + "(common behind a corporate TLS-intercepting proxy). " + "Set REQUESTS_CA_BUNDLE to your organisation's CA bundle and retry." + ) + # --------------------------------------------------------------------------- # Validation helpers @@ -306,9 +348,6 @@ def _validate_package_exists(package, verbose=False, auth_resolver=None, logger= def _check_repo(token, git_env): """Check repo accessibility via GitHub API (or git ls-remote for non-GitHub).""" - import urllib.request - import urllib.error - api_base = host_info.api_base api_url = f"{api_base}/repos/{dep_ref.repo_url}" headers = { @@ -318,24 +357,27 @@ def _check_repo(token, git_env): if token: headers["Authorization"] = f"Bearer {token}" - req = urllib.request.Request(api_url, headers=headers) try: - resp = urllib.request.urlopen(req, timeout=15) - if verbose_log: - verbose_log(f"API {api_url} -> {resp.status}") - return True - except urllib.error.HTTPError as e: - if verbose_log: - verbose_log(f"API {api_url} -> {e.code} {e.reason}") - if e.code == 404 and token: - # 404 with token could mean no access -- raise to trigger fallback - raise RuntimeError(f"API returned {e.code}") - raise RuntimeError(f"API returned {e.code}: {e.reason}") - except Exception as e: + resp = requests.get(api_url, headers=headers, timeout=15) + except requests.exceptions.SSLError as e: + _log_tls_failure(host_info.display_name, e, verbose_log) + raise RuntimeError( + f"TLS verification failed for {host_info.display_name}" + ) from e + except requests.exceptions.RequestException as e: if verbose_log: verbose_log(f"API request failed: {e}") raise + if verbose_log: + verbose_log(f"API {api_url} -> {resp.status_code}") + if resp.ok: + return True + if resp.status_code == 404 and token: + # 404 with token could mean no access -- raise to trigger fallback + raise RuntimeError(f"API returned {resp.status_code}") + raise RuntimeError(f"API returned {resp.status_code}: {resp.reason}") + try: return auth_resolver.try_with_fallback( host, _check_repo, @@ -344,7 +386,11 @@ def _check_repo(token, git_env): unauth_first=True, verbose_callback=verbose_log, ) - except Exception: + except Exception as exc: + if _is_tls_failure(exc): + # TLS failures are not auth issues -- skip the auth context render + # so the user sees the CA-trust hint, not a PAT troubleshooting wall. + return False if verbose_log: try: ctx = auth_resolver.build_error_context( @@ -364,9 +410,6 @@ def _check_repo(token, git_env): repo_path = package # owner/repo format def _check_repo_fallback(token, git_env): - import urllib.request - import urllib.error - host_info = auth_resolver.classify_host(host) api_url = f"{host_info.api_base}/repos/{repo_path}" headers = { @@ -376,19 +419,24 @@ def _check_repo_fallback(token, git_env): if token: headers["Authorization"] = f"Bearer {token}" - req = urllib.request.Request(api_url, headers=headers) try: - resp = urllib.request.urlopen(req, timeout=15) - return True - except urllib.error.HTTPError as e: - if verbose_log: - verbose_log(f"API fallback -> {e.code} {e.reason}") - raise RuntimeError(f"API returned {e.code}") - except Exception as e: + resp = requests.get(api_url, headers=headers, timeout=15) + except requests.exceptions.SSLError as e: + _log_tls_failure(host_info.display_name, e, verbose_log) + raise RuntimeError( + f"TLS verification failed for {host_info.display_name}" + ) from e + except requests.exceptions.RequestException as e: if verbose_log: verbose_log(f"API fallback failed: {e}") raise + if resp.ok: + return True + if verbose_log: + verbose_log(f"API fallback -> {resp.status_code} {resp.reason}") + raise RuntimeError(f"API returned {resp.status_code}") + try: return auth_resolver.try_with_fallback( host, _check_repo_fallback, @@ -396,7 +444,9 @@ def _check_repo_fallback(token, git_env): unauth_first=True, verbose_callback=verbose_log, ) - except Exception: + except Exception as exc: + if _is_tls_failure(exc): + return False if verbose_log: try: ctx = auth_resolver.build_error_context(host, f"accessing {package}", org=org, dep_url=package) diff --git a/tests/unit/install/test_validation_tls.py b/tests/unit/install/test_validation_tls.py new file mode 100644 index 000000000..e2ef7bc9b --- /dev/null +++ b/tests/unit/install/test_validation_tls.py @@ -0,0 +1,169 @@ +"""Tests for the TLS-failure classification path in install.validation. + +Bug: behind a TLS-intercepting corporate proxy, the validator (which used +to use stdlib urllib) ignored REQUESTS_CA_BUNDLE and surfaced a misleading +"package not accessible" error. After the fix, validation goes through +``requests`` and an ``SSLError`` is logged with a CA-trust hint in the +verbose stream so users debugging the failure see the right cause. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +import pytest +import requests + +from apm_cli.install import validation + + +class TestTlsHelpers: + def test_is_tls_failure_detects_runtime_error_marker(self): + exc = RuntimeError("TLS verification failed for github.com") + assert validation._is_tls_failure(exc) is True + + def test_is_tls_failure_detects_certificate_verify_failed(self): + exc = RuntimeError("ssl error: CERTIFICATE_VERIFY_FAILED") + assert validation._is_tls_failure(exc) is True + + def test_is_tls_failure_detects_ssl_error_via_cause_chain(self): + original = requests.exceptions.SSLError("bad cert") + wrapped = RuntimeError("API request failed") + wrapped.__cause__ = original + assert validation._is_tls_failure(wrapped) is True + + def test_is_tls_failure_returns_false_for_generic_errors(self): + assert validation._is_tls_failure(RuntimeError("API returned 404")) is False + assert validation._is_tls_failure(ValueError("nope")) is False + + def test_is_tls_failure_bounded_chain_walk(self): + # Self-referential chain must not loop forever. + exc = RuntimeError("oops") + exc.__cause__ = exc + assert validation._is_tls_failure(exc) is False + + +class TestValidateTlsClassification: + """End-to-end: SSLError from requests.get -> False return + verbose hint.""" + + def _setup_resolver(self): + """Build an AuthResolver-like mock that exercises the unauth path only.""" + resolver = MagicMock() + host_info = MagicMock() + host_info.api_base = "https://api.github.com" + host_info.display_name = "github.com" + host_info.kind = "github" + 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 + + # try_with_fallback should call the operation once with token=None and + # let the SSLError propagate so the outer except can classify it. + def _fake_fallback(host, op, **kwargs): + return op(None, {}) + + resolver.try_with_fallback.side_effect = _fake_fallback + return resolver + + def _capture_verbose(self): + """Build a logger mock whose verbose_detail captures all messages.""" + captured: list[str] = [] + logger = MagicMock() + logger.verbose = True + logger.verbose_detail.side_effect = lambda msg: captured.append(msg) + return logger, captured + + def test_ssl_error_returns_false_and_logs_ca_hint_to_verbose(self): + resolver = self._setup_resolver() + logger, captured = self._capture_verbose() + + with patch( + "requests.get", + side_effect=requests.exceptions.SSLError("CERTIFICATE_VERIFY_FAILED"), + ): + result = validation._validate_package_exists( + "octocat/hello-world", + verbose=True, + auth_resolver=resolver, + logger=logger, + ) + + assert result is False + joined = "\n".join(captured) + assert "TLS verification failed" in joined + assert "REQUESTS_CA_BUNDLE" in joined + + def test_ssl_error_emits_nothing_when_not_verbose(self): + """Default (non-verbose) output must stay quiet -- the orchestrator + renders the user-facing 'not accessible' line; the TLS detail lives + behind --verbose.""" + resolver = self._setup_resolver() + + with patch( + "requests.get", + side_effect=requests.exceptions.SSLError("bad cert"), + ): + result = validation._validate_package_exists( + "octocat/hello-world", verbose=False, auth_resolver=resolver + ) + + assert result is False + + def test_ssl_error_skips_auth_error_context(self): + """TLS failures must not render the PAT/auth troubleshooting wall.""" + resolver = self._setup_resolver() + logger, _captured = self._capture_verbose() + + with patch( + "requests.get", + side_effect=requests.exceptions.SSLError("bad cert"), + ): + validation._validate_package_exists( + "octocat/hello-world", + verbose=True, + auth_resolver=resolver, + logger=logger, + ) + + # build_error_context emits PAT/SSO advice; on TLS failures we skip it. + resolver.build_error_context.assert_not_called() + + def test_http_404_still_returns_false(self): + """Regression guard: non-TLS failures keep the old behaviour.""" + resolver = self._setup_resolver() + fake_resp = MagicMock(ok=False, status_code=404, reason="Not Found") + + with patch("requests.get", return_value=fake_resp): + result = validation._validate_package_exists( + "octocat/missing", verbose=False, auth_resolver=resolver + ) + + assert result is False + + def test_http_200_returns_true(self): + resolver = self._setup_resolver() + fake_resp = MagicMock(ok=True, status_code=200, reason="OK") + + with patch("requests.get", return_value=fake_resp): + result = validation._validate_package_exists( + "octocat/hello-world", verbose=False, auth_resolver=resolver + ) + + assert result is True + + +class TestNoUrllibUrlopenInValidation: + """Regression guard: keep the validator on requests, not urllib.""" + + def test_validation_module_does_not_import_urllib_request_urlopen(self): + from pathlib import Path + + src = Path(validation.__file__).read_text(encoding="utf-8") + # Forbid the specific call form; importing urllib for other reasons + # remains acceptable. + assert "urllib.request.urlopen" not in src, ( + "install/validation.py must use 'requests' for HTTP probes so it " + "honours REQUESTS_CA_BUNDLE the same way the rest of the codebase " + "does. Replace urllib.request.urlopen with requests.get." + ) From 8d6566d11613c018e5c1ecb310b2ac4754f7bfc5 Mon Sep 17 00:00:00 2001 From: abi_jey Date: Sun, 26 Apr 2026 20:18:02 +0200 Subject: [PATCH 2/2] fix(install): address PR #911 review feedback for TLS validator isinstance(SSLError) check, dedup TLS hint across retry chain, surface CA-trust hint at default verbosity, drop dead fallback, tighten verbose output, add troubleshooting docs page and CHANGELOG entry. Assisted-by: Claude:claude-sonnet-4-6 --- CHANGELOG.md | 4 + docs/astro.config.mjs | 6 + .../docs/troubleshooting/ssl-issues.md | 40 +++++ src/apm_cli/install/validation.py | 35 +++-- tests/unit/install/test_validation_tls.py | 148 +++++++++++++----- 5 files changed, 181 insertions(+), 52 deletions(-) create mode 100644 docs/src/content/docs/troubleshooting/ssl-issues.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 48ac72cdb..7e7c4a41b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fixed TLS validation failure behind corporate TLS-intercepting proxies and firewalls: `install/validation.py` now uses `requests` (honouring `REQUESTS_CA_BUNDLE`) instead of stdlib `urllib`, and surfaces a single CA-trust hint at default verbosity instead of a misleading auth error. (#911) + ## [0.9.3] - 2026-04-26 ### Added diff --git a/docs/astro.config.mjs b/docs/astro.config.mjs index 364c7f188..f558c74fa 100644 --- a/docs/astro.config.mjs +++ b/docs/astro.config.mjs @@ -80,6 +80,12 @@ export default defineConfig({ { label: 'Agent Workflows (Experimental)', slug: 'guides/agent-workflows' }, ], }, + { + label: 'Troubleshooting', + items: [ + { label: 'SSL / TLS issues', slug: 'troubleshooting/ssl-issues' }, + ], + }, { label: 'Enterprise', items: [ diff --git a/docs/src/content/docs/troubleshooting/ssl-issues.md b/docs/src/content/docs/troubleshooting/ssl-issues.md new file mode 100644 index 000000000..284a0eb3d --- /dev/null +++ b/docs/src/content/docs/troubleshooting/ssl-issues.md @@ -0,0 +1,40 @@ +--- +title: "SSL / TLS issues" +description: "Fix SSL/TLS verification errors when running APM." +sidebar: + order: 1 +--- + +If `apm install` fails with a TLS error like: + +```text +[!] TLS verification failed -- if you're behind a corporate proxy or firewall, set the REQUESTS_CA_BUNDLE environment variable to the path of your organisation's CA bundle (a PEM file) and retry. +``` + +The most common cause is a corporate TLS-intercepting proxy or firewall (Zscaler, Netskope, Palo Alto, etc.) re-signing HTTPS traffic with an internal CA that APM doesn't trust. + +## Fix + +Point APM at the PEM file containing your organisation's CA. Ask your IT team for the path if you don't know it. + +**Linux / macOS:** + +```bash +export REQUESTS_CA_BUNDLE=/path/to/corporate-ca.pem +``` + +To persist across sessions, add the same line to your shell profile (`~/.bashrc`, `~/.zshrc`, `~/.profile`, etc.). + +**Windows (PowerShell):** + +```powershell +# Current session only +$env:REQUESTS_CA_BUNDLE = "C:\path\to\corporate-ca.pem" + +# Persist for future sessions (user-level) +[Environment]::SetEnvironmentVariable("REQUESTS_CA_BUNDLE", "C:\path\to\corporate-ca.pem", "User") +``` + +## Not behind a proxy or firewall? + +The root cause is likely somewhere else. Re-run with `--verbose` for the underlying exception. diff --git a/src/apm_cli/install/validation.py b/src/apm_cli/install/validation.py index dbb9fc6c7..f10b02f07 100644 --- a/src/apm_cli/install/validation.py +++ b/src/apm_cli/install/validation.py @@ -49,26 +49,28 @@ def _is_tls_failure(exc: BaseException) -> bool: msg = str(cur) if _TLS_ERROR_PREFIX in msg or "CERTIFICATE_VERIFY_FAILED" in msg: return True - # requests.exceptions.SSLError is the canonical type once we stop - # using urllib; check by name to avoid importing requests at module - # import time. - if type(cur).__name__ == "SSLError": + if isinstance(cur, requests.exceptions.SSLError): return True cur = cur.__cause__ or cur.__context__ seen += 1 return False -def _log_tls_failure(host_display: str, exc: BaseException, verbose_log) -> None: - """Log a TLS verification failure to the verbose stream with a CA-trust hint.""" - if not verbose_log: - return - verbose_log(f"TLS verification failed for {host_display}: {exc}") - verbose_log( - " hint: the CA bundle does not trust the certificate presented " - "(common behind a corporate TLS-intercepting proxy). " - "Set REQUESTS_CA_BUNDLE to your organisation's CA bundle and retry." +def _log_tls_failure(host_display: str, exc: BaseException, verbose_log, logger) -> None: + """Surface a TLS verification failure with an actionable CA-trust hint. + + Default verbosity: a single one-liner via ``logger.warning`` so users behind + a corporate proxy see the right next step without re-running with --verbose. + Verbose: also include the host name and the underlying exception text. + """ + logger.warning( + "TLS verification failed -- if you're behind a corporate proxy or " + "firewall, set the REQUESTS_CA_BUNDLE environment variable to the " + "path of your organisation's CA bundle (a PEM file) and retry. " + "See: https://microsoft.github.io/apm/troubleshooting/ssl-issues/" ) + if verbose_log: + verbose_log(f"underlying error from {host_display}: {exc}") # --------------------------------------------------------------------------- @@ -360,7 +362,6 @@ def _check_repo(token, git_env): try: resp = requests.get(api_url, headers=headers, timeout=15) except requests.exceptions.SSLError as e: - _log_tls_failure(host_info.display_name, e, verbose_log) raise RuntimeError( f"TLS verification failed for {host_info.display_name}" ) from e @@ -388,8 +389,7 @@ def _check_repo(token, git_env): ) except Exception as exc: if _is_tls_failure(exc): - # TLS failures are not auth issues -- skip the auth context render - # so the user sees the CA-trust hint, not a PAT troubleshooting wall. + _log_tls_failure(host_info.display_name, exc, verbose_log, logger) return False if verbose_log: try: @@ -422,7 +422,6 @@ def _check_repo_fallback(token, git_env): try: resp = requests.get(api_url, headers=headers, timeout=15) except requests.exceptions.SSLError as e: - _log_tls_failure(host_info.display_name, e, verbose_log) raise RuntimeError( f"TLS verification failed for {host_info.display_name}" ) from e @@ -446,6 +445,8 @@ def _check_repo_fallback(token, git_env): ) except Exception as exc: if _is_tls_failure(exc): + # See note above: logged once here, skip auth context render. + _log_tls_failure(host, exc, verbose_log, logger) return False if verbose_log: try: diff --git a/tests/unit/install/test_validation_tls.py b/tests/unit/install/test_validation_tls.py index e2ef7bc9b..b5ff995bc 100644 --- a/tests/unit/install/test_validation_tls.py +++ b/tests/unit/install/test_validation_tls.py @@ -3,8 +3,9 @@ Bug: behind a TLS-intercepting corporate proxy, the validator (which used to use stdlib urllib) ignored REQUESTS_CA_BUNDLE and surfaced a misleading "package not accessible" error. After the fix, validation goes through -``requests`` and an ``SSLError`` is logged with a CA-trust hint in the -verbose stream so users debugging the failure see the right cause. +``requests``, an ``SSLError`` raises a wrapped ``RuntimeError``, and the +outer handler logs a single CA-trust hint -- visible at default verbosity +so users behind a corporate proxy don't have to re-run with --verbose. """ from __future__ import annotations @@ -44,42 +45,45 @@ def test_is_tls_failure_bounded_chain_walk(self): class TestValidateTlsClassification: - """End-to-end: SSLError from requests.get -> False return + verbose hint.""" + """End-to-end: SSLError from requests.get -> False return + single CA hint.""" - def _setup_resolver(self): - """Build an AuthResolver-like mock that exercises the unauth path only.""" + def _setup_resolver(self, token=None): + """Build an AuthResolver-like mock that exercises the unauth path.""" 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) + ctx = MagicMock(source="env", token_type="pat", token=token) resolver.resolve.return_value = ctx resolver.resolve_for_dep.return_value = ctx - # try_with_fallback should call the operation once with token=None and - # let the SSLError propagate so the outer except can classify it. + # Single-call shim: invoke the operation once unauth and let the + # SSLError propagate so the outer except can classify it. def _fake_fallback(host, op, **kwargs): return op(None, {}) resolver.try_with_fallback.side_effect = _fake_fallback return resolver - def _capture_verbose(self): - """Build a logger mock whose verbose_detail captures all messages.""" - captured: list[str] = [] + def _capture_logger(self): + """Build a logger mock capturing both verbose_detail and warning.""" + verbose_msgs: list[str] = [] + warning_msgs: list[str] = [] logger = MagicMock() logger.verbose = True - logger.verbose_detail.side_effect = lambda msg: captured.append(msg) - return logger, captured + logger.verbose_detail.side_effect = lambda msg: verbose_msgs.append(msg) + logger.warning.side_effect = lambda msg: warning_msgs.append(msg) + return logger, verbose_msgs, warning_msgs def test_ssl_error_returns_false_and_logs_ca_hint_to_verbose(self): resolver = self._setup_resolver() - logger, captured = self._capture_verbose() + logger, verbose_msgs, warning_msgs = self._capture_logger() with patch( - "requests.get", + "apm_cli.install.validation.requests.get", side_effect=requests.exceptions.SSLError("CERTIFICATE_VERIFY_FAILED"), ): result = validation._validate_package_exists( @@ -90,33 +94,87 @@ def test_ssl_error_returns_false_and_logs_ca_hint_to_verbose(self): ) assert result is False - joined = "\n".join(captured) - assert "TLS verification failed" in joined - assert "REQUESTS_CA_BUNDLE" in joined - - def test_ssl_error_emits_nothing_when_not_verbose(self): - """Default (non-verbose) output must stay quiet -- the orchestrator - renders the user-facing 'not accessible' line; the TLS detail lives - behind --verbose.""" + joined_verbose = "\n".join(verbose_msgs) + joined_warning = "\n".join(warning_msgs) + # Verbose adds the host + underlying exception only; the actionable + # REQUESTS_CA_BUNDLE hint lives on the always-on warning channel so + # it isn't restated in the verbose detail. + assert "underlying error from github.com" in joined_verbose + assert "REQUESTS_CA_BUNDLE" not in joined_verbose + assert "REQUESTS_CA_BUNDLE" in joined_warning + + def test_ssl_error_emits_actionable_hint_at_default_verbosity(self): + """Default verbosity must surface a single one-liner so users behind + a corporate TLS-intercepting proxy don't need to re-run with --verbose + to see what to fix.""" resolver = self._setup_resolver() + logger, verbose_msgs, warning_msgs = self._capture_logger() + logger.verbose = False with patch( - "requests.get", + "apm_cli.install.validation.requests.get", side_effect=requests.exceptions.SSLError("bad cert"), ): result = validation._validate_package_exists( - "octocat/hello-world", verbose=False, auth_resolver=resolver + "octocat/hello-world", + verbose=False, + auth_resolver=resolver, + logger=logger, ) assert result is False + # One-liner via logger.warning at default verbosity, not buried in + # verbose detail. + assert len(warning_msgs) == 1 + assert "REQUESTS_CA_BUNDLE" in warning_msgs[0] + # Verbose-only details should not fire at default verbosity. + assert verbose_msgs == [] + + def test_ssl_error_logs_hint_exactly_once_when_token_present(self): + """Regression guard: try_with_fallback may retry (unauth -> token -> + credential-fill). TLS doesn't care about auth, so the inner SSLError + fires on every attempt -- but the user must see the CA-trust hint + only once, not 2-3 times.""" + # Token present means the unauth -> token retry path is exercised. + resolver = self._setup_resolver(token="ghp_fake") + + def _retrying_fallback(host, op, **kwargs): + try: + return op(None, {}) + except Exception: + try: + return op("ghp_fake", {}) + except Exception: + return op("ghp_credfill", {}) + + resolver.try_with_fallback.side_effect = _retrying_fallback + + logger, _verbose_msgs, warning_msgs = self._capture_logger() + + with patch( + "apm_cli.install.validation.requests.get", + side_effect=requests.exceptions.SSLError("bad cert"), + ): + result = validation._validate_package_exists( + "octocat/hello-world", + verbose=False, + auth_resolver=resolver, + logger=logger, + ) + + assert result is False + # Exactly one warning, regardless of how many internal retries fired. + assert len(warning_msgs) == 1, ( + f"expected single CA hint, got {len(warning_msgs)}: {warning_msgs}" + ) def test_ssl_error_skips_auth_error_context(self): """TLS failures must not render the PAT/auth troubleshooting wall.""" resolver = self._setup_resolver() - logger, _captured = self._capture_verbose() + logger, _verbose_msgs, _warning_msgs = self._capture_logger() with patch( - "requests.get", + "apm_cli.install.validation.requests.get", side_effect=requests.exceptions.SSLError("bad cert"), ): validation._validate_package_exists( @@ -134,7 +192,7 @@ def test_http_404_still_returns_false(self): resolver = self._setup_resolver() fake_resp = MagicMock(ok=False, status_code=404, reason="Not Found") - with patch("requests.get", return_value=fake_resp): + with patch("apm_cli.install.validation.requests.get", return_value=fake_resp): result = validation._validate_package_exists( "octocat/missing", verbose=False, auth_resolver=resolver ) @@ -145,7 +203,7 @@ def test_http_200_returns_true(self): resolver = self._setup_resolver() fake_resp = MagicMock(ok=True, status_code=200, reason="OK") - with patch("requests.get", return_value=fake_resp): + with patch("apm_cli.install.validation.requests.get", return_value=fake_resp): result = validation._validate_package_exists( "octocat/hello-world", verbose=False, auth_resolver=resolver ) @@ -156,14 +214,34 @@ def test_http_200_returns_true(self): class TestNoUrllibUrlopenInValidation: """Regression guard: keep the validator on requests, not urllib.""" - def test_validation_module_does_not_import_urllib_request_urlopen(self): - from pathlib import Path + def test_validation_does_not_call_urllib_request_urlopen(self): + """Runtime check: if validation.py ever reaches for + urllib.request.urlopen for HTTP probes, this assertion trips. + More robust than substring-matching the source.""" + 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 + resolver.try_with_fallback.side_effect = lambda host, op, **kw: op(None, {}) - src = Path(validation.__file__).read_text(encoding="utf-8") - # Forbid the specific call form; importing urllib for other reasons - # remains acceptable. - assert "urllib.request.urlopen" not in src, ( + fake_resp = MagicMock(ok=True, status_code=200, reason="OK") + forbidden = AssertionError( "install/validation.py must use 'requests' for HTTP probes so it " "honours REQUESTS_CA_BUNDLE the same way the rest of the codebase " "does. Replace urllib.request.urlopen with requests.get." ) + + with patch("urllib.request.urlopen", side_effect=forbidden) as urlopen_mock, \ + patch("apm_cli.install.validation.requests.get", return_value=fake_resp): + result = validation._validate_package_exists( + "octocat/hello-world", verbose=False, auth_resolver=resolver + ) + + assert result is True + urlopen_mock.assert_not_called()