From 998e35313a42f7d4fb82646ee57d0aea57f077c0 Mon Sep 17 00:00:00 2001 From: Rage Lopez Date: Thu, 16 Jul 2026 06:37:09 -0700 Subject: [PATCH] fix(auth): honor per-entry key_env when resolving fallback providers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fallback chain entry can name its API key via key_env (or the api_key_env alias) per the fallback-providers docs, but only the gateway path resolved it — TUI/desktop, cron, and CLI setup fallbacks ignored it, so a fallback provider whose key lives in a non-standard env var never resolved on those surfaces. Centralize the inline-api_key-then-key_env lookup in hermes_cli/fallback_config.resolve_entry_api_key() and use it at all four fallback resolution sites (tui_gateway, cron scheduler, gateway runner, CLI setup mixin); the CLI mixin also gains the base_url passthrough the other surfaces already had. Salvaged from PR #43861 (surgical reapply — the original branch predates the #65264 fallback restructuring). --- cron/scheduler.py | 7 +++-- gateway/run.py | 11 ++----- hermes_cli/cli_agent_setup_mixin.py | 10 +++++- hermes_cli/fallback_config.py | 20 ++++++++++++ tests/hermes_cli/test_fallback_config.py | 40 ++++++++++++++++++++++++ tests/test_tui_gateway_server.py | 36 +++++++++++++++++++++ tui_gateway/server.py | 7 +++-- 7 files changed, 118 insertions(+), 13 deletions(-) create mode 100644 tests/hermes_cli/test_fallback_config.py diff --git a/cron/scheduler.py b/cron/scheduler.py index 927fe736c2bb..63c45d898e4d 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -3083,14 +3083,17 @@ def run_job( if not fb_provider or not fb_model: continue try: + from hermes_cli.fallback_config import resolve_entry_api_key + fb_kwargs = { "requested": fb_provider, "target_model": fb_model, } if entry.get("base_url"): fb_kwargs["explicit_base_url"] = entry["base_url"] - if entry.get("api_key"): - fb_kwargs["explicit_api_key"] = entry["api_key"] + fb_api_key = resolve_entry_api_key(entry) + if fb_api_key: + fb_kwargs["explicit_api_key"] = fb_api_key runtime = resolve_runtime_provider(**fb_kwargs) model = fb_model logger.info( diff --git a/gateway/run.py b/gateway/run.py index 430704fbed21..c970ef93e582 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -2038,17 +2038,12 @@ def _try_resolve_fallback_provider() -> dict | None: return None for entry in fb_list: try: - explicit_api_key = entry.get("api_key") - if not explicit_api_key: - key_env = str( - entry.get("key_env") or entry.get("api_key_env") or "" - ).strip() - if key_env: - explicit_api_key = os.getenv(key_env, "").strip() or None + from hermes_cli.fallback_config import resolve_entry_api_key + runtime = resolve_runtime_provider( requested=entry.get("provider"), explicit_base_url=entry.get("base_url"), - explicit_api_key=explicit_api_key, + explicit_api_key=resolve_entry_api_key(entry), ) # Log the literal `provider` key from config, not the resolved # runtime category — an Ollama fallback resolves through the diff --git a/hermes_cli/cli_agent_setup_mixin.py b/hermes_cli/cli_agent_setup_mixin.py index 8c25e7a1fca3..b0a9e9e2fa10 100644 --- a/hermes_cli/cli_agent_setup_mixin.py +++ b/hermes_cli/cli_agent_setup_mixin.py @@ -57,7 +57,15 @@ class CLIAgentSetupMixin: if not _fb_provider or not _fb_model: continue try: - runtime = resolve_runtime_provider(requested=_fb_provider) + from hermes_cli.fallback_config import resolve_entry_api_key + + _fb_kwargs = {"requested": _fb_provider} + if _fb.get("base_url"): + _fb_kwargs["explicit_base_url"] = _fb["base_url"] + _fb_api_key = resolve_entry_api_key(_fb) + if _fb_api_key: + _fb_kwargs["explicit_api_key"] = _fb_api_key + runtime = resolve_runtime_provider(**_fb_kwargs) logger.warning( "Primary provider auth failed (%s). Falling through to fallback: %s/%s", _primary_exc, _fb_provider, _fb_model, diff --git a/hermes_cli/fallback_config.py b/hermes_cli/fallback_config.py index d7cfc952d2da..b18aecb6cb04 100644 --- a/hermes_cli/fallback_config.py +++ b/hermes_cli/fallback_config.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os from typing import Any @@ -11,6 +12,25 @@ def _normalized_base_url(value: Any) -> str: return value.strip().rstrip("/") +def resolve_entry_api_key(entry: dict[str, Any] | None) -> str | None: + """API key for one fallback entry: inline ``api_key``, else ``key_env``. + + Mirrors the custom-provider convention (``key_env`` names the env var + holding the key; ``api_key_env`` accepted as an alias). Returns None when + neither yields a non-empty value, letting ``resolve_runtime_provider`` + fall through to the provider's standard credential resolution. + """ + if not isinstance(entry, dict): + return None + inline = str(entry.get("api_key") or "").strip() + if inline: + return inline + key_env = str(entry.get("key_env") or entry.get("api_key_env") or "").strip() + if key_env: + return os.getenv(key_env, "").strip() or None + return None + + def _iter_fallback_entries(raw: Any) -> list[dict[str, Any]]: if isinstance(raw, dict): candidates = [raw] diff --git a/tests/hermes_cli/test_fallback_config.py b/tests/hermes_cli/test_fallback_config.py new file mode 100644 index 000000000000..b8da384cda9e --- /dev/null +++ b/tests/hermes_cli/test_fallback_config.py @@ -0,0 +1,40 @@ +"""Tests for hermes_cli/fallback_config.py — fallback entry API-key resolution.""" + +from hermes_cli.fallback_config import resolve_entry_api_key + + +class TestResolveEntryApiKey: + def test_inline_api_key_wins(self, monkeypatch): + monkeypatch.setenv("FB_KEY", "env-key") + entry = {"provider": "custom", "api_key": "inline-key", "key_env": "FB_KEY"} + assert resolve_entry_api_key(entry) == "inline-key" + + def test_key_env_resolves_from_environment(self, monkeypatch): + monkeypatch.setenv("FB_KEY", "env-key") + assert resolve_entry_api_key({"key_env": "FB_KEY"}) == "env-key" + + def test_api_key_env_alias(self, monkeypatch): + monkeypatch.setenv("FB_ALIAS_KEY", "alias-key") + assert resolve_entry_api_key({"api_key_env": "FB_ALIAS_KEY"}) == "alias-key" + + def test_unset_env_var_returns_none(self, monkeypatch): + monkeypatch.delenv("FB_MISSING", raising=False) + # None (not "") lets resolve_runtime_provider fall through to the + # provider's standard credential resolution. + assert resolve_entry_api_key({"key_env": "FB_MISSING"}) is None + + def test_empty_env_var_returns_none(self, monkeypatch): + monkeypatch.setenv("FB_EMPTY", " ") + assert resolve_entry_api_key({"key_env": "FB_EMPTY"}) is None + + def test_no_key_fields_returns_none(self): + assert resolve_entry_api_key({"provider": "openrouter", "model": "glm"}) is None + + def test_non_dict_returns_none(self): + assert resolve_entry_api_key(None) is None + assert resolve_entry_api_key("nope") is None # type: ignore[arg-type] + + def test_whitespace_inline_key_falls_through_to_env(self, monkeypatch): + monkeypatch.setenv("FB_KEY", "env-key") + entry = {"api_key": " ", "key_env": "FB_KEY"} + assert resolve_entry_api_key(entry) == "env-key" diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 6c0d57118fa7..be46a5e01d80 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -9597,6 +9597,42 @@ class TestResolveRuntimeWithFallback: assert resolution.selected_model == "z-ai/glm-5.2" assert resolution.used_fallback is True + def test_fallback_entry_key_env_resolves_api_key(self, monkeypatch): + """A fallback entry naming its key via key_env passes the resolved + env value as explicit_api_key (#43861, @VrtxOmega).""" + from hermes_cli.auth import AuthError + + monkeypatch.setenv("FB_TEST_KEY", "env-resolved-key") + captured = {} + fallback_runtime = {"provider": "openrouter", "api_key": "x"} + + def fake_resolve(**kwargs): + if kwargs.get("requested") == "openai-codex": + raise AuthError("No Codex credentials stored") + captured.update(kwargs) + return fallback_runtime + + monkeypatch.setattr( + "hermes_cli.runtime_provider.resolve_runtime_provider", + fake_resolve, + ) + monkeypatch.setattr( + server, + "_load_fallback_model", + lambda: [ + { + "provider": "openrouter", + "model": "z-ai/glm-5.2", + "key_env": "FB_TEST_KEY", + } + ], + ) + resolution = server._resolve_runtime_with_fallback( + {"requested": "openai-codex"} + ) + assert resolution.used_fallback is True + assert captured.get("explicit_api_key") == "env-resolved-key" + def test_auth_error_all_fallbacks_fail_raises(self, monkeypatch): """When all fallbacks also fail, re-raise the original AuthError.""" from hermes_cli.auth import AuthError diff --git a/tui_gateway/server.py b/tui_gateway/server.py index f0a554044d7d..4b9e7c190ab8 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -4507,14 +4507,17 @@ def _resolve_runtime_with_fallback( if not fb_provider or not fb_model: continue try: + from hermes_cli.fallback_config import resolve_entry_api_key + fb_kwargs: dict = { "requested": fb_provider, "target_model": fb_model, } if entry.get("base_url"): fb_kwargs["explicit_base_url"] = entry["base_url"] - if entry.get("api_key"): - fb_kwargs["explicit_api_key"] = entry["api_key"] + fb_api_key = resolve_entry_api_key(entry) + if fb_api_key: + fb_kwargs["explicit_api_key"] = fb_api_key runtime = resolve_runtime_provider(**fb_kwargs) import logging