diff --git a/agent/model_metadata.py b/agent/model_metadata.py index a638798a97..9a44b2c9bc 100644 --- a/agent/model_metadata.py +++ b/agent/model_metadata.py @@ -48,18 +48,37 @@ def __getattr__(name: str): raise AttributeError(f"module {__name__!r} has no attribute {name!r}") -def _resolve_requests_verify() -> bool | str: - """Resolve SSL verify setting for `requests` calls from env vars. +def _resolve_requests_verify(base_url: str = "") -> bool | str: + """Resolve SSL verify setting for `requests` calls. - The `requests` library only honours REQUESTS_CA_BUNDLE / CURL_CA_BUNDLE - by default. Hermes also honours HERMES_CA_BUNDLE (its own convention) - and SSL_CERT_FILE (used by the stdlib `ssl` module and by httpx), so - that a single env var can cover both `requests` and `httpx` callsites - inside the same process. + Priority (mirrors ``agent.ssl_verify.resolve_httpx_verify`` so the + ``requests``-based ``/models`` probes agree with the httpx chat client): - Returns either a filesystem path to a CA bundle, or True to defer to - the requests default (certifi). + 1. Per-provider ``ssl_verify: false`` for ``base_url`` — disable verification. + 2. Per-provider ``ssl_ca_cert`` for ``base_url`` — an explicit CA bundle. + Without this, a custom endpoint whose chain only verifies against the + provider's configured bundle (not the process ``SSL_CERT_FILE``) logs a + spurious CERTIFICATE_VERIFY_FAILED on every probe even though the chat + path succeeds (per-provider ``ssl_ca_cert`` was reaching only httpx). + 3. Env vars ``HERMES_CA_BUNDLE`` / ``REQUESTS_CA_BUNDLE`` / ``SSL_CERT_FILE`` + (a single var covers both ``requests`` and ``httpx`` in-process). + 4. ``True`` — defer to the requests default (certifi). + + ``base_url`` is optional so existing callers (OpenRouter, etc.) keep the + env-only behavior unchanged; only probes that pass a base_url pick up the + per-provider override. """ + if base_url: + try: + from hermes_cli.config import get_custom_provider_tls_settings + tls = get_custom_provider_tls_settings(base_url) + if tls.get("ssl_verify") is False: + return False + ca = tls.get("ssl_ca_cert") + if isinstance(ca, str) and ca and os.path.isfile(ca): + return ca + except Exception: + pass # fall through to env vars — never break a probe on config lookup for env_var in ("HERMES_CA_BUNDLE", "REQUESTS_CA_BUNDLE", "SSL_CERT_FILE"): val = os.getenv(env_var) if val and os.path.isfile(val): @@ -1261,7 +1280,7 @@ def fetch_endpoint_model_metadata( server_url.rstrip("/") + "/api/v1/models", headers=headers, timeout=(5, 10), - verify=_resolve_requests_verify(), + verify=_resolve_requests_verify(normalized), ) response.raise_for_status() payload = response.json() @@ -1324,7 +1343,7 @@ def fetch_endpoint_model_metadata( url, headers=headers, timeout=(5, 10), - verify=_resolve_requests_verify(), + verify=_resolve_requests_verify(normalized), stream=True, ) if response.status_code in (401, 403): @@ -1364,7 +1383,7 @@ def fetch_endpoint_model_metadata( try: # Try /v1/props first (current llama.cpp); fall back to /props for older builds base = request_candidate.rstrip("/").replace("/v1", "") - _verify = _resolve_requests_verify() + _verify = _resolve_requests_verify(normalized) props_resp = requests.get(base + "/v1/props", headers=headers, timeout=5, verify=_verify) if not props_resp.ok: props_resp = requests.get(base + "/props", headers=headers, timeout=5, verify=_verify) @@ -2284,7 +2303,7 @@ def _query_anthropic_context_length(model: str, base_url: str, api_key: str) -> "anthropic-version": "2023-06-01", } _ensure_requests() - resp = requests.get(url, headers=headers, timeout=(5, 10), verify=_resolve_requests_verify()) + resp = requests.get(url, headers=headers, timeout=(5, 10), verify=_resolve_requests_verify(base_url)) if resp.status_code != 200: return None data = resp.json() diff --git a/hermes_cli/models.py b/hermes_cli/models.py index 297f295a99..0687fc56e9 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -39,9 +39,41 @@ COPILOT_EDITOR_VERSION = "vscode/1.104.1" COPILOT_REASONING_EFFORTS_GPT5 = ["minimal", "low", "medium", "high"] COPILOT_REASONING_EFFORTS_O_SERIES = ["low", "medium", "high"] -def _urlopen_model_catalog_request(req: urllib.request.Request, *, timeout: float): +def _urlopen_model_catalog_request(req: urllib.request.Request, *, timeout: float, ssl_context=None): """Open catalog requests without forwarding headers across origins.""" - return open_credentialed_url(req, timeout=timeout) + return open_credentialed_url(req, timeout=timeout, ssl_context=ssl_context) + + +def _custom_provider_ssl_context(base_url: str): + """Build an ``ssl.SSLContext`` from a custom provider's TLS settings. + + Mirrors the httpx/requests TLS resolution so the urllib ``/models`` + discovery probe honors a provider's ``ssl_ca_cert`` / ``ssl_verify`` + instead of falling back to the process-wide ``SSL_CERT_FILE`` / certifi + bundle. Returns None when no per-provider TLS override applies, so the + caller keeps urllib's default policy for public/unconfigured endpoints. + """ + if not base_url: + return None + try: + from hermes_cli.config import get_custom_provider_tls_settings + + tls = get_custom_provider_tls_settings(base_url) + if not tls: + return None + import ssl + + if tls.get("ssl_verify") is False: + ctx = ssl.create_default_context() + ctx.check_hostname = False + ctx.verify_mode = ssl.CERT_NONE + return ctx + ca = tls.get("ssl_ca_cert") + if isinstance(ca, str) and ca and os.path.isfile(ca): + return ssl.create_default_context(cafile=ca) + except Exception: + return None # never break discovery on a TLS-config lookup + return None # Fallback OpenRouter snapshot used when the live catalog is unavailable. @@ -4630,12 +4662,19 @@ def probe_api_models( headers.update(normalize_extra_headers(request_headers)) + _ssl_context = _custom_provider_ssl_context(normalized) for candidate_base, is_fallback in candidates: url = candidate_base.rstrip("/") + "/models" tried.append(url) req = urllib.request.Request(url, headers=headers) + # Only thread ssl_context when a per-provider TLS override actually + # applies. Public/unconfigured endpoints keep the original 2-arg call, + # so nothing changes for them (and existing call-seam mocks stay valid). + _open_kwargs: dict[str, Any] = {"timeout": timeout} + if _ssl_context is not None: + _open_kwargs["ssl_context"] = _ssl_context try: - with _urlopen_model_catalog_request(req, timeout=timeout) as resp: + with _urlopen_model_catalog_request(req, **_open_kwargs) as resp: data = json.loads(resp.read().decode()) return { "models": [m.get("id", "") for m in data.get("data", [])], diff --git a/hermes_cli/urllib_security.py b/hermes_cli/urllib_security.py index 29c000985a..616ff7ade4 100644 --- a/hermes_cli/urllib_security.py +++ b/hermes_cli/urllib_security.py @@ -83,17 +83,31 @@ class _CrossOriginRequestSanitizer(urllib.request.BaseHandler): https_request = _sanitize -def _secure_opener_from_installed_policy(original_url: str): - """Clone the installed opener's handlers, replacing redirect policy only.""" +def _secure_opener_from_installed_policy(original_url: str, *, ssl_context=None): + """Clone the installed opener's handlers, replacing redirect policy only. + + When ``ssl_context`` is provided, the cloned HTTPS handler is replaced with + one bound to that context so per-provider TLS settings (``ssl_ca_cert`` / + ``ssl_verify``) apply to this request. When it is None, the installed + opener's TLS policy is preserved unchanged (env / certifi default). + """ installed = getattr(urllib.request, "_opener", None) if installed is None: installed = urllib.request.build_opener() + _https_handler_cls = getattr(urllib.request, "HTTPSHandler", None) handlers = [ copy.copy(handler) for handler in getattr(installed, "handlers", ()) if not isinstance(handler, urllib.request.HTTPRedirectHandler) + and not ( + ssl_context is not None + and _https_handler_cls is not None + and isinstance(handler, _https_handler_cls) + ) ] + if ssl_context is not None and _https_handler_cls is not None: + handlers.append(_https_handler_cls(context=ssl_context)) handlers.append(SafeCredentialRedirectHandler(original_url)) handlers.append(_CrossOriginRequestSanitizer(original_url)) secured = urllib.request.build_opener(*handlers) @@ -114,6 +128,7 @@ def open_credentialed_url( *, timeout: float, opener_factory: Callable[..., Any] | None = None, + ssl_context=None, ): """Open a request without forwarding credentials across origins. @@ -121,9 +136,16 @@ def open_credentialed_url( cookies, custom protocol handlers, and instrumentation while replacing its redirect handler. ``opener_factory`` is an explicit test seam; security is never disabled based on global ``urlopen`` identity. + + ``ssl_context`` (an ``ssl.SSLContext``) overrides the HTTPS handler's TLS + policy for this request only. It is used to honor a custom provider's + ``ssl_ca_cert`` / ``ssl_verify`` on the ``/models`` discovery path, which + otherwise falls back to the process-wide ``SSL_CERT_FILE`` / certifi bundle. """ if opener_factory is None: - opener = _secure_opener_from_installed_policy(request.full_url) + opener = _secure_opener_from_installed_policy( + request.full_url, ssl_context=ssl_context + ) for name, value in getattr(opener, "_hermes_initial_addheaders", ()): if not request.has_header(name): request.add_header(name, value) diff --git a/tests/agent/test_custom_provider_ca_probes.py b/tests/agent/test_custom_provider_ca_probes.py new file mode 100644 index 0000000000..c0c4b51750 --- /dev/null +++ b/tests/agent/test_custom_provider_ca_probes.py @@ -0,0 +1,312 @@ +"""Custom-provider TLS settings must reach the /models and pricing probes. + +Regression coverage for provider-scoped ``ssl_ca_cert`` / ``ssl_verify`` being +ignored by the discovery/pricing probes. Two probe families share the root +cause and are covered here: + +* ``requests``-based endpoint metadata / pricing probe + (``agent.model_metadata.fetch_endpoint_model_metadata`` via + ``_resolve_requests_verify``). +* ``urllib``-based ``/models`` catalog discovery probe + (``hermes_cli.models.probe_api_models`` via ``_custom_provider_ssl_context``). + +Both previously resolved TLS from process-wide env vars only, so a custom +endpoint whose chain verifies against the provider's configured bundle (but not +``SSL_CERT_FILE``) logged a spurious CERTIFICATE_VERIFY_FAILED on every probe +even though the chat client succeeded. + +No network I/O: real CA-bundle stand-in files via ``tmp_path`` plus a patched +provider list and a patched request seam. +""" + +from __future__ import annotations + +import ssl +import urllib.error +from unittest.mock import MagicMock, patch + +import certifi +import pytest + +from agent.model_metadata import _resolve_requests_verify +from hermes_cli.models import _custom_provider_ssl_context + +_CA_ENV_VARS = ( + "HERMES_CA_BUNDLE", + "REQUESTS_CA_BUNDLE", + "SSL_CERT_FILE", + "CURL_CA_BUNDLE", +) + +_BASE = "https://relay.example.invalid/v1" + + +@pytest.fixture +def clean_env(monkeypatch): + """Clear the CA env vars so each test starts from a known state.""" + for var in _CA_ENV_VARS: + monkeypatch.delenv(var, raising=False) + return monkeypatch + + +@pytest.fixture +def bundle_file(tmp_path): + path = tmp_path / "provider-ca.pem" + path.write_text("-----BEGIN CERTIFICATE-----\nstub\n-----END CERTIFICATE-----\n") + return str(path) + + +@pytest.fixture +def real_ca(): + """A real, parseable CA bundle on disk. + + ``ssl.create_default_context(cafile=...)`` parses the file eagerly, so the + urllib context path needs a genuine bundle rather than a stub. The + ``requests`` path only stores the path string (parsed lazily by requests at + call time), so it can use the ``bundle_file`` stub. + """ + return certifi.where() + + +def _providers(base_url, **tls): + entry = {"name": "relay", "base_url": base_url} + entry.update(tls) + return [entry] + + +class TestResolveRequestsVerifyProviderScoped: + """``_resolve_requests_verify(base_url)`` — the requests probe path.""" + + def test_provider_ca_used_for_matching_base_url(self, clean_env, bundle_file): + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers(_BASE, ssl_ca_cert=bundle_file), + ): + assert _resolve_requests_verify(_BASE) == bundle_file + + def test_provider_ca_overrides_env_ssl_cert_file(self, clean_env, tmp_path, bundle_file): + env_bundle = tmp_path / "env-ca.pem" + env_bundle.write_text("stub") + clean_env.setenv("SSL_CERT_FILE", str(env_bundle)) + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers(_BASE, ssl_ca_cert=bundle_file), + ): + assert _resolve_requests_verify(_BASE) == bundle_file + + def test_provider_ssl_verify_false_disables(self, clean_env): + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers(_BASE, ssl_verify=False), + ): + assert _resolve_requests_verify(_BASE) is False + + def test_no_base_url_does_not_consult_config(self, clean_env, bundle_file): + """Existing callers pass no base_url — env-only behavior, no config read.""" + clean_env.setenv("HERMES_CA_BUNDLE", bundle_file) + probe = MagicMock(return_value=[]) + with patch("hermes_cli.config.get_compatible_custom_providers", probe): + assert _resolve_requests_verify() == bundle_file + probe.assert_not_called() + + def test_unmatched_base_url_falls_through_to_env(self, clean_env, bundle_file): + clean_env.setenv("REQUESTS_CA_BUNDLE", bundle_file) + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers("https://other.example.invalid/v1", ssl_ca_cert="/nope.pem"), + ): + assert _resolve_requests_verify(_BASE) == bundle_file + + def test_unmatched_base_url_no_env_returns_true(self, clean_env): + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=[], + ): + assert _resolve_requests_verify(_BASE) is True + + def test_provider_ca_missing_file_falls_through_to_env(self, clean_env, bundle_file): + clean_env.setenv("SSL_CERT_FILE", bundle_file) + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers(_BASE, ssl_ca_cert="/does/not/exist.pem"), + ): + assert _resolve_requests_verify(_BASE) == bundle_file + + def test_config_lookup_failure_falls_through_to_env(self, clean_env, bundle_file): + clean_env.setenv("SSL_CERT_FILE", bundle_file) + with patch( + "hermes_cli.config.get_compatible_custom_providers", + side_effect=RuntimeError("config boom"), + ): + assert _resolve_requests_verify(_BASE) == bundle_file + + +class TestCustomProviderSSLContext: + """``_custom_provider_ssl_context`` — the urllib /models discovery path.""" + + def test_returns_verifying_context_with_provider_ca(self, real_ca): + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers(_BASE, ssl_ca_cert=real_ca), + ): + ctx = _custom_provider_ssl_context(_BASE) + assert isinstance(ctx, ssl.SSLContext) + assert ctx.verify_mode == ssl.CERT_REQUIRED + + def test_ssl_verify_false_returns_unverified_context(self): + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers(_BASE, ssl_verify=False), + ): + ctx = _custom_provider_ssl_context(_BASE) + assert isinstance(ctx, ssl.SSLContext) + assert ctx.check_hostname is False + assert ctx.verify_mode == ssl.CERT_NONE + + def test_no_base_url_returns_none(self): + assert _custom_provider_ssl_context("") is None + + def test_unmatched_returns_none(self): + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=[], + ): + assert _custom_provider_ssl_context(_BASE) is None + + def test_missing_ca_file_returns_none(self): + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers(_BASE, ssl_ca_cert="/does/not/exist.pem"), + ): + assert _custom_provider_ssl_context(_BASE) is None + + def test_config_lookup_failure_returns_none(self): + with patch( + "hermes_cli.config.get_compatible_custom_providers", + side_effect=RuntimeError("config boom"), + ): + assert _custom_provider_ssl_context(_BASE) is None + + +class TestMetadataProbeThreadsProviderCA: + """End-to-end: the requests metadata probe carries the provider CA to the wire.""" + + def test_fetch_endpoint_model_metadata_uses_provider_ca(self, clean_env, bundle_file): + import agent.model_metadata as mm + + captured = {} + + def fake_get(url, headers=None, timeout=None, verify=None): + captured["verify"] = verify + resp = MagicMock() + resp.raise_for_status.return_value = None + resp.json.return_value = {"data": []} + return resp + + mm._endpoint_model_metadata_cache.clear() + mm._endpoint_model_metadata_cache_time.clear() + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers(_BASE, ssl_ca_cert=bundle_file), + ), patch.object(mm.requests, "get", side_effect=fake_get): + mm.fetch_endpoint_model_metadata(_BASE, force_refresh=True) + + assert captured["verify"] == bundle_file + + def test_public_endpoint_keeps_env_default(self, clean_env): + import agent.model_metadata as mm + + captured = {} + + def fake_get(url, headers=None, timeout=None, verify=None): + captured["verify"] = verify + resp = MagicMock() + resp.raise_for_status.return_value = None + resp.json.return_value = {"data": []} + return resp + + mm._endpoint_model_metadata_cache.clear() + mm._endpoint_model_metadata_cache_time.clear() + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=[], + ), patch.object(mm.requests, "get", side_effect=fake_get): + mm.fetch_endpoint_model_metadata(_BASE, force_refresh=True) + + assert captured["verify"] is True + + +class TestCatalogProbeThreadsSSLContext: + """End-to-end: the urllib catalog probe carries the provider SSL context.""" + + def test_probe_api_models_passes_ssl_context(self, clean_env, real_ca): + import hermes_cli.models as models + + captured = {} + + def fake_open(req, *, timeout, ssl_context=None): + captured["ssl_context"] = ssl_context + raise urllib.error.URLError("stop after capture") + + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=_providers(_BASE, ssl_ca_cert=real_ca), + ), patch.object(models, "open_credentialed_url", side_effect=fake_open): + models.probe_api_models(None, _BASE, timeout=1) + + assert isinstance(captured["ssl_context"], ssl.SSLContext) + assert captured["ssl_context"].verify_mode == ssl.CERT_REQUIRED + + def test_probe_api_models_public_endpoint_uses_default_policy(self, clean_env): + import hermes_cli.models as models + + captured = {} + + def fake_open(req, *, timeout, ssl_context=None): + captured["ssl_context"] = ssl_context + raise urllib.error.URLError("stop after capture") + + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=[], + ), patch.object(models, "open_credentialed_url", side_effect=fake_open): + models.probe_api_models(None, _BASE, timeout=1) + + assert captured["ssl_context"] is None + + def test_public_endpoint_calls_seam_without_ssl_context_kwarg(self, clean_env): + """A public endpoint must not pass ssl_context to the call seam. + + Regression guard: threading ssl_context unconditionally broke existing + call-seam mocks whose signature is ``(req, timeout=...)``. The probe + must keep the original 2-arg call shape when no per-provider override + applies, so a strict 2-arg mock still works. + """ + import hermes_cli.models as models + + class _Resp: + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + def read(self): + return b'{"data": [{"id": "local-model"}]}' + + calls = [] + + def _strict_two_arg(req, timeout=5.0): + calls.append(req.full_url) + return _Resp() + + with patch( + "hermes_cli.config.get_compatible_custom_providers", + return_value=[], + ), patch.object( + models, "_urlopen_model_catalog_request", side_effect=_strict_two_arg + ): + probe = models.probe_api_models("key", "http://localhost:8000", timeout=1) + + assert probe["models"] == ["local-model"] + assert calls == ["http://localhost:8000/models"]