fix(models): restore authenticated Actual discovery on macOS

contributors/emails/justin@actual.inc

@@ -0,0 +1,2 @@
+somewheresy
+# PR #83554 (Actual authenticated model discovery)

hermes_cli/urllib_security.py

@@ -3,15 +3,28 @@
 from __future__ import annotations
 
 import copy
+import logging
+import os
+import ssl
+import sys
 import urllib.parse
 import urllib.request
 from collections.abc import Callable, Iterable
+from pathlib import Path
 from typing import Any
 
+logger = logging.getLogger(__name__)
+
 # Headers safe to forward to a different origin. Everything else is dropped:
 # custom provider headers routinely carry credentials under arbitrary names.
 _CROSS_ORIGIN_SAFE_HEADERS = frozenset({"accept", "user-agent"})
 _DEFAULT_PORTS = {"http": 80, "https": 443}
+_CA_BUNDLE_ENV_VARS = (
+    "HERMES_CA_BUNDLE",
+    "SSL_CERT_FILE",
+    "REQUESTS_CA_BUNDLE",
+    "CURL_CA_BUNDLE",
+)
 
 
 def url_origin(url: str) -> tuple[str, str, int | None]:
@@ -83,17 +96,67 @@ def _sanitize(self, request: urllib.request.Request):
     https_request = _sanitize
 
 
+def _resolved_https_context() -> ssl.SSLContext | None:
+    """Return the explicit CA context for Hermes-owned urllib openers."""
+    ca_bundle = next(
+        (
+            value
+            for name in _CA_BUNDLE_ENV_VARS
+            if (value := os.getenv(name, "").strip())
+        ),
+        "",
+    )
+    if ca_bundle:
+        ca_path = Path(ca_bundle).expanduser()
+        if ca_path.is_file():
+            try:
+                return ssl.create_default_context(cafile=str(ca_path))
+            except (OSError, ssl.SSLError) as exc:
+                logger.warning(
+                    "CA bundle could not be loaded from %s: %s — falling back to default certificates",
+                    ca_bundle,
+                    exc,
+                )
+        else:
+            logger.warning(
+                "CA bundle path does not exist: %s — falling back to default certificates",
+                ca_bundle,
+            )
+
+    if sys.platform != "darwin":
+        return None
+
+    try:
+        import certifi
+
+        return ssl.create_default_context(cafile=certifi.where())
+    except (ImportError, OSError, ssl.SSLError) as exc:
+        logger.warning(
+            "Could not load certifi for urllib HTTPS verification: %s — falling back to default certificates",
+            exc,
+        )
+        return None
+
+
 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).
+    ``ssl_verify``) apply to this request. When it is None, Hermes-owned
+    openers get an explicit CA default via ``_resolved_https_context`` (env
+    bundle first, certifi on macOS); an application-installed opener's TLS
+    policy is preserved unchanged.
     """
     installed = getattr(urllib.request, "_opener", None)
     if installed is None:
-        installed = urllib.request.build_opener()
+        context = _resolved_https_context()
+        if context is None:
+            installed = urllib.request.build_opener()
+        else:
+            installed = urllib.request.build_opener(
+                urllib.request.HTTPSHandler(context=context)
+            )
 
     _https_handler_cls = getattr(urllib.request, "HTTPSHandler", None)
     handlers = [

plugins/model-providers/actual/__init__.py

@@ -61,8 +61,10 @@ def fetch_models(
         req.add_header("Accept", "application/json")
         req.add_header("User-Agent", _profile_user_agent())
 
+        from hermes_cli.urllib_security import open_credentialed_url
+
         try:
-            with urllib.request.urlopen(req, timeout=timeout) as resp:
+            with open_credentialed_url(req, timeout=timeout) as resp:
                 data = json.loads(resp.read().decode())
             items = data if isinstance(data, list) else data.get("data", [])
             return [m["id"] for m in items if isinstance(m, dict) and "id" in m]

tests/hermes_cli/test_actual_provider.py

@@ -48,11 +48,25 @@ def test_actual_aliases_and_profile_metadata():
 
 
 def test_actual_base_url_normalization():
-    assert normalize_actual_base_url("https://api.actual.inc") == DEFAULT_ACTUAL_BASE_URL
-    assert normalize_actual_base_url("https://api.actual.inc/v1") == DEFAULT_ACTUAL_BASE_URL
-    assert normalize_actual_base_url("http://127.0.0.1:8080") == DEFAULT_ACTUAL_LOCAL_BASE_URL
-    assert normalize_actual_base_url("http://127.0.0.1:8080/v1") == DEFAULT_ACTUAL_LOCAL_BASE_URL
-    assert normalize_actual_base_url("http://localhost:8080/") == "http://localhost:8080/v1"
+    assert (
+        normalize_actual_base_url("https://api.actual.inc") == DEFAULT_ACTUAL_BASE_URL
+    )
+    assert (
+        normalize_actual_base_url("https://api.actual.inc/v1")
+        == DEFAULT_ACTUAL_BASE_URL
+    )
+    assert (
+        normalize_actual_base_url("http://127.0.0.1:8080")
+        == DEFAULT_ACTUAL_LOCAL_BASE_URL
+    )
+    assert (
+        normalize_actual_base_url("http://127.0.0.1:8080/v1")
+        == DEFAULT_ACTUAL_LOCAL_BASE_URL
+    )
+    assert (
+        normalize_actual_base_url("http://localhost:8080/")
+        == "http://localhost:8080/v1"
+    )
 
 
 def test_actual_credentials_default_to_hosted_api(monkeypatch):
@@ -173,26 +187,107 @@ def __exit__(self, *args):
         def read(self):
             return json.dumps({"data": [{"id": "actual/local-model"}]}).encode()
 
-    def _urlopen(req, timeout=0):
+    def _open(req, timeout=0):
         seen["url"] = req.full_url
         seen["auth"] = req.get_header("Authorization")
         seen["timeout"] = timeout
         return _Response()
 
-    monkeypatch.setattr("urllib.request.urlopen", _urlopen)
+    monkeypatch.setattr("hermes_cli.urllib_security.open_credentialed_url", _open)
 
     assert profile.fetch_models(api_key=None, timeout=1.5) == ["actual/local-model"]
     assert seen["url"] == DEFAULT_ACTUAL_LOCAL_BASE_URL + "/models"
     assert seen["auth"] is None
     assert seen["timeout"] == 1.5
 
 
+def test_actual_profile_fetch_models_sends_credential_only_to_original_origin(
+    monkeypatch,
+):
+    """fetch_models must route through the shared redirect-credential guard.
+
+    ActualProfile overrides ProviderProfile.fetch_models with its own
+    base_url resolution, and previously called raw urllib.request.urlopen
+    directly instead of the base class's open_credentialed_url — losing the
+    protection that strips the Authorization header when a redirect leaves
+    the original host. Exercises the real SafeCredentialRedirectHandler
+    (no mocking of open_credentialed_url itself) against a local HTTP
+    server that 302s to a different origin, mirroring
+    test_urllib_security.py's end-to-end redirect tests.
+    """
+    import http.server
+    import threading
+
+    _clear_actual_env(monkeypatch)
+    profile = get_provider_profile("actual")
+
+    source_auth_headers: list[str | None] = []
+    target_auth_headers: list[str | None] = []
+
+    class _RedirectTargetHandler(http.server.BaseHTTPRequestHandler):
+        def do_GET(self):
+            target_auth_headers.append(self.headers.get("Authorization"))
+            body = json.dumps({"data": [{"id": "should-not-be-trusted"}]}).encode()
+            self.send_response(200)
+            self.send_header("Content-Type", "application/json")
+            self.send_header("Content-Length", str(len(body)))
+            self.end_headers()
+            self.wfile.write(body)
+
+        def log_message(self, *_args):
+            pass
+
+    target_server = http.server.HTTPServer(("127.0.0.1", 0), _RedirectTargetHandler)
+    target_thread = threading.Thread(target=target_server.serve_forever, daemon=True)
+    target_thread.start()
+    target_port = target_server.server_address[1]
+
+    class _RedirectingHandler(http.server.BaseHTTPRequestHandler):
+        def do_GET(self):
+            source_auth_headers.append(self.headers.get("Authorization"))
+            self.send_response(302)
+            self.send_header("Location", f"http://127.0.0.1:{target_port}/models")
+            self.end_headers()
+
+        def log_message(self, *_args):
+            pass
+
+    redirect_server = http.server.HTTPServer(("127.0.0.1", 0), _RedirectingHandler)
+    redirect_thread = threading.Thread(
+        target=redirect_server.serve_forever, daemon=True
+    )
+    redirect_thread.start()
+    redirect_port = redirect_server.server_address[1]
+
+    try:
+        result = profile.fetch_models(
+            api_key="actual-secret-token",
+            base_url=f"http://127.0.0.1:{redirect_port}",
+            timeout=5.0,
+        )
+    finally:
+        redirect_server.shutdown()
+        target_server.shutdown()
+        redirect_thread.join(timeout=2.0)
+        target_thread.join(timeout=2.0)
+
+    assert result == ["should-not-be-trusted"], (
+        "sanity check: the redirect must actually have been followed"
+    )
+    assert source_auth_headers == ["Bearer actual-secret-token"]
+    assert target_auth_headers == [None], (
+        "Authorization header leaked to a different origin after a redirect"
+    )
+
+
 def test_actual_provider_model_ids_use_local_profile_catalog(monkeypatch):
     _clear_actual_env(monkeypatch)
     monkeypatch.setenv("ACTUAL_BASE_URL", "http://127.0.0.1:8080")
     profile = get_provider_profile("actual")
 
-    with patch.object(profile, "fetch_models", return_value=["actual/local-model"]) as fetch:
+    with patch.object(
+        profile, "fetch_models", return_value=["actual/local-model"]
+    ) as fetch:
         assert provider_model_ids("actual") == ["actual/local-model"]
 
     fetch.assert_called_once_with(
@@ -201,6 +296,32 @@ def test_actual_provider_model_ids_use_local_profile_catalog(monkeypatch):
     )
 
 
+def test_actual_hosted_model_ids_send_resolved_credential(monkeypatch):
+    _clear_actual_env(monkeypatch)
+    monkeypatch.setenv("ACTUAL_API_KEY", "actual-test-key")
+    profile = get_provider_profile("actual")
+
+    with patch.object(
+        profile, "fetch_models", return_value=["actual/hosted-model"]
+    ) as fetch:
+        assert provider_model_ids("actual") == ["actual/hosted-model"]
+
+    fetch.assert_called_once_with(
+        api_key="actual-test-key",
+        base_url=DEFAULT_ACTUAL_BASE_URL,
+    )
+
+
+def test_actual_hosted_model_ids_do_not_probe_without_credentials(monkeypatch):
+    _clear_actual_env(monkeypatch)
+    profile = get_provider_profile("actual")
+
+    with patch.object(profile, "fetch_models") as fetch:
+        assert provider_model_ids("actual") == []
+
+    fetch.assert_not_called()
+
+
 def test_actual_codex_transport_clamps_reasoning_effort():
     """Actual's SGLang/vLLM backends only accept none/low/medium/high/max.