mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(agent): honor custom CA certs on aux client + harden TLS resolution
The salvaged fix wired per-provider ssl_ca_cert / ssl_verify (and HERMES_CA_BUNDLE) into the MAIN OpenAI client. This follow-up: - Auxiliary client parity: process_bootstrap.build_keepalive_http_client accepts and forwards verify; auxiliary_client._resolve_aux_verify mirrors the main-client TLS resolution (via load_config_readonly, the read-only fast path) so compression/vision/web_extract/title-gen/session_search honor the same per-provider CA. Without this, chat worked against a private-CA endpoint but every auxiliary call still failed APIConnectionError. - switch_model now reads custom_providers from live config (load_config_readonly) instead of the init-time agent._custom_providers snapshot, so ssl_ca_cert / ssl_verify edits are honored on mid-session model switch — matching the context-length reload (#15779). - Drop the dead client-level verify= where a custom httpx transport is used (httpx ignores it there); verify lives on the transport. Fix docstrings. Applies to both run_agent._build_keepalive_http_client and process_bootstrap. - resolve_httpx_verify: add CURL_CA_BUNDLE to the env chain (consistency with agent/ssl_guard._CA_BUNDLE_ENV_VARS) and emit a loud logger.warning naming the endpoint whenever ssl_verify:false disables verification. - get_custom_provider_tls_settings: case-insensitive base_url match (config dedup already lowercases; scheme/host are case-insensitive) so a mixed-case entry doesn't silently drop its CA. Exact match preserved — no prefix bypass. - Demote best-effort except Exception: pass in agent_init/switch_model to logger.debug(exc_info=True). - Tests for aux verify forwarding, _resolve_aux_verify, case-insensitive match, and prefix-bypass rejection.
This commit is contained in:
parent
3a2ba959ce
commit
676236bb1d
9 changed files with 189 additions and 11 deletions
|
|
@ -987,7 +987,7 @@ def init_agent(
|
|||
get_compatible_custom_providers(load_config()),
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
logger.debug("custom-provider TLS resolution skipped", exc_info=True)
|
||||
|
||||
agent.api_key = client_kwargs.get("api_key", "")
|
||||
agent.base_url = client_kwargs.get("base_url", agent.base_url)
|
||||
|
|
|
|||
|
|
@ -1787,15 +1787,23 @@ def switch_model(agent, new_model, new_provider, api_key='', base_url='', api_mo
|
|||
"base_url": effective_base,
|
||||
}
|
||||
try:
|
||||
from hermes_cli.config import apply_custom_provider_tls_to_client_kwargs
|
||||
from hermes_cli.config import (
|
||||
apply_custom_provider_tls_to_client_kwargs,
|
||||
get_compatible_custom_providers,
|
||||
load_config_readonly,
|
||||
)
|
||||
|
||||
# Read custom_providers from live config (not the init-time
|
||||
# snapshot on ``agent._custom_providers``) so ssl_ca_cert /
|
||||
# ssl_verify edits are honored when switching mid-session,
|
||||
# matching the context-length reload below (#15779).
|
||||
apply_custom_provider_tls_to_client_kwargs(
|
||||
agent._client_kwargs,
|
||||
str(effective_base or ""),
|
||||
getattr(agent, "_custom_providers", None),
|
||||
get_compatible_custom_providers(load_config_readonly()),
|
||||
)
|
||||
except Exception:
|
||||
pass
|
||||
logger.debug("custom-provider TLS resolution skipped on switch_model", exc_info=True)
|
||||
_sm_timeout = get_provider_request_timeout(agent.provider, agent.model)
|
||||
if _sm_timeout is not None:
|
||||
agent._client_kwargs["timeout"] = _sm_timeout
|
||||
|
|
|
|||
|
|
@ -128,13 +128,45 @@ _LOGGED_UNSUPPORTED_EXTPROC_KEYS: set = set()
|
|||
_LOGGED_UNSUPPORTED_OAUTH_KEYS: set = set()
|
||||
|
||||
|
||||
def _resolve_aux_verify(base_url: Optional[str]) -> Any:
|
||||
"""Resolve httpx ``verify`` for an auxiliary-client base_url.
|
||||
|
||||
Mirrors the main client's TLS resolution so auxiliary calls (compression,
|
||||
vision, web_extract, title generation, etc.) honor per-provider
|
||||
``ssl_ca_cert`` / ``ssl_verify`` config and the ``HERMES_CA_BUNDLE`` /
|
||||
``SSL_CERT_FILE`` env conventions. Best-effort: any failure falls back to
|
||||
the httpx/certifi default (``True``).
|
||||
"""
|
||||
try:
|
||||
from agent.ssl_verify import resolve_httpx_verify
|
||||
from hermes_cli.config import (
|
||||
get_custom_provider_tls_settings,
|
||||
load_config_readonly,
|
||||
)
|
||||
|
||||
tls = get_custom_provider_tls_settings(
|
||||
str(base_url or ""), config=load_config_readonly()
|
||||
)
|
||||
return resolve_httpx_verify(
|
||||
ca_bundle=tls.get("ssl_ca_cert"),
|
||||
ssl_verify=tls.get("ssl_verify"),
|
||||
base_url=str(base_url or ""),
|
||||
)
|
||||
except Exception:
|
||||
return True
|
||||
|
||||
|
||||
def _openai_http_client_kwargs(
|
||||
base_url: Optional[str],
|
||||
*,
|
||||
async_mode: bool = False,
|
||||
) -> Dict[str, Any]:
|
||||
"""Inject keepalive httpx client with env-only proxy (not macOS system proxy)."""
|
||||
client = build_keepalive_http_client(str(base_url or ""), async_mode=async_mode)
|
||||
client = build_keepalive_http_client(
|
||||
str(base_url or ""),
|
||||
async_mode=async_mode,
|
||||
verify=_resolve_aux_verify(base_url),
|
||||
)
|
||||
if client is None:
|
||||
return {}
|
||||
return {"http_client": client}
|
||||
|
|
|
|||
|
|
@ -146,6 +146,7 @@ def build_keepalive_http_client(
|
|||
base_url: str = "",
|
||||
*,
|
||||
async_mode: bool = False,
|
||||
verify: Any = True,
|
||||
) -> Optional[Any]:
|
||||
"""Build an httpx client for OpenAI SDK calls with env-only proxy policy.
|
||||
|
||||
|
|
@ -154,6 +155,13 @@ def build_keepalive_http_client(
|
|||
``trust_env`` path, so macOS system proxy settings from
|
||||
``urllib.request.getproxies()`` (which omit the ExceptionsList) are not
|
||||
applied. Mirrors ``AIAgent._build_keepalive_http_client``.
|
||||
|
||||
``verify`` is forwarded to httpx so auxiliary-client calls (compression,
|
||||
vision, web_extract, title generation, etc.) honor the same per-provider
|
||||
``ssl_ca_cert`` / ``ssl_verify`` and ``HERMES_CA_BUNDLE`` settings the main
|
||||
client uses. It is passed on the ``HTTPTransport`` (which owns the SSL
|
||||
context when a custom transport is supplied) and, for the copilot branch
|
||||
that has no custom transport, on the client itself.
|
||||
"""
|
||||
try:
|
||||
import httpx
|
||||
|
|
@ -161,7 +169,7 @@ def build_keepalive_http_client(
|
|||
|
||||
if "api.githubcopilot.com" in str(base_url or "").lower():
|
||||
client_cls = httpx.AsyncClient if async_mode else httpx.Client
|
||||
return client_cls()
|
||||
return client_cls(verify=verify)
|
||||
|
||||
sock_opts = [(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)]
|
||||
if hasattr(socket, "TCP_KEEPIDLE"):
|
||||
|
|
@ -174,8 +182,10 @@ def build_keepalive_http_client(
|
|||
proxy = _get_proxy_for_base_url(base_url)
|
||||
transport_cls = httpx.AsyncHTTPTransport if async_mode else httpx.HTTPTransport
|
||||
client_cls = httpx.AsyncClient if async_mode else httpx.Client
|
||||
# verify lives on the transport: httpx ignores the client-level
|
||||
# ``verify`` when a custom ``transport=`` is supplied.
|
||||
return client_cls(
|
||||
transport=transport_cls(socket_options=sock_opts),
|
||||
transport=transport_cls(socket_options=sock_opts, verify=verify),
|
||||
proxy=proxy,
|
||||
)
|
||||
except Exception:
|
||||
|
|
|
|||
|
|
@ -23,16 +23,26 @@ def resolve_httpx_verify(
|
|||
*,
|
||||
ca_bundle: Optional[str] = None,
|
||||
ssl_verify: Any = None,
|
||||
base_url: str = "",
|
||||
) -> bool | ssl.SSLContext:
|
||||
"""Resolve httpx ``verify`` for provider HTTP clients.
|
||||
|
||||
Priority:
|
||||
1. ``ssl_verify: false`` — disable verification (local dev only)
|
||||
2. explicit ``ca_bundle`` (per-provider ``ssl_ca_cert`` config field)
|
||||
3. ``HERMES_CA_BUNDLE``, ``SSL_CERT_FILE``, ``REQUESTS_CA_BUNDLE`` env vars
|
||||
3. ``HERMES_CA_BUNDLE``, ``SSL_CERT_FILE``, ``REQUESTS_CA_BUNDLE``,
|
||||
``CURL_CA_BUNDLE`` env vars
|
||||
4. ``True`` (httpx/certifi default)
|
||||
|
||||
``base_url`` is used only for the insecure-mode warning message.
|
||||
"""
|
||||
if _coerce_insecure(ssl_verify):
|
||||
logger.warning(
|
||||
"TLS certificate verification DISABLED (ssl_verify: false) for %s — "
|
||||
"this is intended for local development only and is unsafe on any "
|
||||
"network you do not fully control.",
|
||||
base_url or "a custom provider endpoint",
|
||||
)
|
||||
return False
|
||||
|
||||
effective_ca = (
|
||||
|
|
@ -40,6 +50,7 @@ def resolve_httpx_verify(
|
|||
or os.getenv("HERMES_CA_BUNDLE", "").strip()
|
||||
or os.getenv("SSL_CERT_FILE", "").strip()
|
||||
or os.getenv("REQUESTS_CA_BUNDLE", "").strip()
|
||||
or os.getenv("CURL_CA_BUNDLE", "").strip()
|
||||
)
|
||||
if effective_ca:
|
||||
ca_path = str(Path(effective_ca).expanduser())
|
||||
|
|
|
|||
|
|
@ -4728,11 +4728,16 @@ def get_custom_provider_tls_settings(
|
|||
if not base_url or not isinstance(custom_providers, list):
|
||||
return {}
|
||||
|
||||
target_url = (base_url or "").rstrip("/")
|
||||
# Case-insensitive compare: elsewhere custom_providers are keyed on a
|
||||
# lowercased base_url (see get_compatible_custom_providers dedup), and
|
||||
# scheme/host are case-insensitive anyway — so a config entry written as
|
||||
# https://Ollama.Example.com/v1 must still match a lowercased runtime
|
||||
# base_url. Exact match after rstrip('/') + lower() (no prefix/substring).
|
||||
target_url = (base_url or "").rstrip("/").lower()
|
||||
for entry in custom_providers:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
entry_url = (entry.get("base_url") or "").rstrip("/")
|
||||
entry_url = (entry.get("base_url") or "").rstrip("/").lower()
|
||||
if not entry_url or entry_url != target_url:
|
||||
continue
|
||||
out: Dict[str, Any] = {}
|
||||
|
|
|
|||
|
|
@ -3904,10 +3904,11 @@ class AIAgent:
|
|||
# Explicitly read proxy settings while still honoring NO_PROXY for
|
||||
# loopback / local endpoints such as a locally hosted sub2api.
|
||||
_proxy = _get_proxy_for_base_url(base_url)
|
||||
# verify lives on the transport: httpx ignores the client-level
|
||||
# ``verify`` when a custom ``transport=`` is supplied.
|
||||
return _httpx.Client(
|
||||
transport=_httpx.HTTPTransport(socket_options=_sock_opts, verify=verify),
|
||||
proxy=_proxy,
|
||||
verify=verify,
|
||||
)
|
||||
except Exception:
|
||||
return None
|
||||
|
|
|
|||
79
tests/agent/test_auxiliary_client_ssl_verify.py
Normal file
79
tests/agent/test_auxiliary_client_ssl_verify.py
Normal file
|
|
@ -0,0 +1,79 @@
|
|||
"""Regression: auxiliary-client keepalive httpx client must honor custom CA bundles.
|
||||
|
||||
The main OpenAI client resolves per-provider ``ssl_ca_cert`` / ``ssl_verify`` and
|
||||
``HERMES_CA_BUNDLE`` via ``agent.ssl_verify.resolve_httpx_verify``. Auxiliary calls
|
||||
(compression, vision, web_extract, title generation, session_search) build their own
|
||||
keepalive client through ``agent.process_bootstrap.build_keepalive_http_client`` and must
|
||||
apply the same TLS settings — otherwise an HTTPS custom_providers endpoint signed by a
|
||||
private CA works for chat but fails ``APIConnectionError`` on every auxiliary task.
|
||||
"""
|
||||
|
||||
import ssl
|
||||
|
||||
import certifi
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from agent.process_bootstrap import build_keepalive_http_client
|
||||
|
||||
_CA_ENV_VARS = ("HERMES_CA_BUNDLE", "SSL_CERT_FILE", "REQUESTS_CA_BUNDLE", "HTTPS_PROXY")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def clean_tls_env(monkeypatch):
|
||||
for var in _CA_ENV_VARS:
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
|
||||
|
||||
def test_build_keepalive_http_client_forwards_verify_context(clean_tls_env):
|
||||
ctx = ssl.create_default_context(cafile=certifi.where())
|
||||
client = build_keepalive_http_client("https://ollama.example.com/v1", verify=ctx)
|
||||
assert isinstance(client, httpx.Client)
|
||||
assert client._transport._pool._ssl_context is ctx
|
||||
|
||||
|
||||
def test_build_keepalive_http_client_verify_false_disables_hostname_check(clean_tls_env):
|
||||
client = build_keepalive_http_client("https://ollama.example.com/v1", verify=False)
|
||||
assert isinstance(client, httpx.Client)
|
||||
assert client._transport._pool._ssl_context.check_hostname is False
|
||||
|
||||
|
||||
def test_build_keepalive_http_client_default_verify_true(clean_tls_env):
|
||||
client = build_keepalive_http_client("https://ollama.example.com/v1")
|
||||
assert isinstance(client, httpx.Client)
|
||||
|
||||
|
||||
def test_resolve_aux_verify_uses_per_provider_ssl_ca_cert(clean_tls_env, monkeypatch):
|
||||
"""_resolve_aux_verify should mirror the main-client resolution for a matched base_url."""
|
||||
import hermes_cli.config as cfg
|
||||
from agent import auxiliary_client
|
||||
|
||||
# get_custom_provider_tls_settings is imported inside the function from
|
||||
# hermes_cli.config, so patch it at the source module.
|
||||
monkeypatch.setattr(
|
||||
cfg,
|
||||
"get_custom_provider_tls_settings",
|
||||
lambda *a, **k: {"ssl_ca_cert": certifi.where()},
|
||||
)
|
||||
verify = auxiliary_client._resolve_aux_verify("https://ollama.example.com/v1")
|
||||
assert isinstance(verify, ssl.SSLContext)
|
||||
|
||||
|
||||
def test_resolve_aux_verify_ssl_verify_false(clean_tls_env, monkeypatch):
|
||||
import hermes_cli.config as cfg
|
||||
from agent import auxiliary_client
|
||||
|
||||
monkeypatch.setattr(
|
||||
cfg,
|
||||
"get_custom_provider_tls_settings",
|
||||
lambda *a, **k: {"ssl_verify": False},
|
||||
)
|
||||
assert auxiliary_client._resolve_aux_verify("https://ollama.example.com/v1") is False
|
||||
|
||||
|
||||
def test_resolve_aux_verify_no_match_defaults_true(clean_tls_env, monkeypatch):
|
||||
import hermes_cli.config as cfg
|
||||
from agent import auxiliary_client
|
||||
|
||||
monkeypatch.setattr(cfg, "get_custom_provider_tls_settings", lambda *a, **k: {})
|
||||
assert auxiliary_client._resolve_aux_verify("https://openrouter.ai/api/v1") is True
|
||||
|
|
@ -38,3 +38,35 @@ def test_apply_custom_provider_tls_to_client_kwargs():
|
|||
)
|
||||
assert client_kwargs["ssl_ca_cert"] == "/etc/ssl/mkcert-root.pem"
|
||||
assert client_kwargs["ssl_verify"] is True
|
||||
|
||||
|
||||
def test_get_custom_provider_tls_settings_matches_case_insensitively():
|
||||
"""A config base_url with mixed case must still match a lowercased runtime base_url."""
|
||||
providers = [
|
||||
{
|
||||
"name": "Ollama",
|
||||
"base_url": "https://Ollama.Example.com/v1",
|
||||
"ssl_ca_cert": "/etc/ssl/mkcert-root.pem",
|
||||
}
|
||||
]
|
||||
tls = get_custom_provider_tls_settings(
|
||||
"https://ollama.example.com/v1",
|
||||
custom_providers=providers,
|
||||
)
|
||||
assert tls == {"ssl_ca_cert": "/etc/ssl/mkcert-root.pem"}
|
||||
|
||||
|
||||
def test_get_custom_provider_tls_settings_no_substring_bypass():
|
||||
"""A base_url that is only a prefix of an entry must NOT match."""
|
||||
providers = [
|
||||
{
|
||||
"name": "Ollama",
|
||||
"base_url": "https://ollama.example.com/v1",
|
||||
"ssl_verify": False,
|
||||
}
|
||||
]
|
||||
# A different host that shares a prefix must not pick up ssl_verify:false.
|
||||
assert get_custom_provider_tls_settings(
|
||||
"https://ollama.example.com.attacker.test/v1",
|
||||
custom_providers=providers,
|
||||
) == {}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue