mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(providers): refuse only a synthesized placeholder for the selected entry
Review findings on #147761: the missing-profile-key refusal fired for a custom provider on model.openai_runtime=codex_app_server (which owns its auth), for a caller-supplied explicit_api_key='no-key-required', and lost the selected entry when explicit_base_url replaced its URL (back to the misleading 401). The check now runs on the final api_mode and skips transports that never send the key (codex_app_server, bedrock_converse), skips any caller-supplied key, and identifies a named-entry runtime by its selected entry rather than by URL.
This commit is contained in:
1 parent
139716ae63
commit
30375b51bc
3 files changed
+51
-13
No files matched your search
@@ -1072,8 +1072,6 @@ def resolve_runtime_provider(*, requested: Optional[str] = None, explicit_api_ke
|
||||
_raise_if_local_alias_missing_endpoint(requested_provider, explicit_base_url)
|
||||
runtime = next(r for r in _ladder_rungs(requested_provider, explicit_api_key, explicit_base_url, target_model) if r)
|
||||
_raise_for_credentialless_bare_custom(requested_provider, runtime)
|
||||
from hermes_cli.runtime_provider_missing_key import raise_if_declared_key_missing
|
||||
raise_if_declared_key_missing(requested_provider, runtime, _get_model_config(), _FROZEN_RUNTIME_CONFIG.get())
|
||||
# model.openai_runtime is applied ONCE, after the ladder: every rung (pool, OAuth store,
|
||||
# explicit --api-key/--base-url, env key) hardcodes the wire api_mode for openai/openai-codex,
|
||||
# so applying the opt-in inside one rung left the others on codex_responses (#115169).
|
||||
@@ -1084,6 +1082,9 @@ def resolve_runtime_provider(*, requested: Optional[str] = None, explicit_api_ke
|
||||
logger.info("model.openai_runtime=codex_app_server overrides the %s runtime (source=%s); its credential/endpoint "
|
||||
"is not used — the app-server authenticates with its own login", runtime.get("provider"), runtime.get("source"))
|
||||
runtime["api_mode"] = api_mode
|
||||
from hermes_cli.runtime_provider_missing_key import raise_if_declared_key_missing
|
||||
raise_if_declared_key_missing(requested_provider, runtime, _get_model_config(), _FROZEN_RUNTIME_CONFIG.get(),
|
||||
explicit_api_key=explicit_api_key)
|
||||
return runtime
|
||||
|
||||
|
||||
|
||||
@@ -25,14 +25,19 @@ def _declared_key_env(entry: Optional[dict[str, Any]]) -> str:
|
||||
return str(entry.get("key_env") or entry.get("api_key_env") or "").strip()
|
||||
|
||||
|
||||
def _declared_custom_key(requested_provider: str, base_url: str, model_cfg: dict[str, Any],
|
||||
def _declared_custom_key(requested_provider: str, runtime: dict[str, Any], model_cfg: dict[str, Any],
|
||||
config: Optional[dict[str, Any]]) -> tuple[str, str]:
|
||||
"""``(label, key_env)`` of the config block that declared a key variable for this custom
|
||||
endpoint, else ``("", "")``: the owning ``providers:`` / ``custom_providers:`` entry (by name,
|
||||
then URL), then ``model.key_env`` when ``model.base_url`` is this endpoint."""
|
||||
from agent.custom_entry_pool import custom_entry_for_endpoint
|
||||
endpoint, else ``("", "")``: the owning ``providers:`` / ``custom_providers:`` entry, then
|
||||
``model.key_env`` when ``model.base_url`` is this endpoint. A runtime the named-entry rung built
|
||||
(``source: custom_provider:<name>``) took its credential from the SELECTED entry even when an
|
||||
explicit base_url replaced the entry's URL, so that entry is found by name alone; any other
|
||||
runtime (direct alias, URL-matched bare custom) is owned by whichever entry serves its URL."""
|
||||
from agent.custom_entry_pool import custom_entry_for_endpoint, named_custom_entry
|
||||
base_url = str(runtime.get("base_url") or "")
|
||||
name = requested_provider if requested_provider not in ("", "auto", "custom") else None
|
||||
match = custom_entry_for_endpoint(base_url, name, config)
|
||||
from_named_entry = str(runtime.get("source") or "").startswith("custom_provider:")
|
||||
match = named_custom_entry(name, config) if name and from_named_entry else custom_entry_for_endpoint(base_url, name, config)
|
||||
if match is not None and (key_env := _declared_key_env(match[1])):
|
||||
return f"custom provider '{match[1].get('name') or match[0]}'", key_env
|
||||
cfg_url = str(model_cfg.get("base_url") or "").strip().rstrip("/")
|
||||
@@ -68,24 +73,33 @@ def named_profile_key_hint(env_vars) -> str:
|
||||
return f" {where_to_set_key(names[0])}"
|
||||
|
||||
|
||||
# Transports that never send the resolved key: the codex app-server authenticates with its own login,
|
||||
# Bedrock with the AWS SDK credential chain (agent_init builds neither client from ``api_key``).
|
||||
_KEYLESS_API_MODES = frozenset({"codex_app_server", "bedrock_converse"})
|
||||
|
||||
|
||||
def raise_if_declared_key_missing(requested_provider: Optional[str], runtime: dict[str, Any], model_cfg: dict[str, Any],
|
||||
config: Optional[dict[str, Any]] = None) -> None:
|
||||
config: Optional[dict[str, Any]] = None, *, explicit_api_key: Optional[str] = None) -> None:
|
||||
"""Refuse a custom runtime carrying the placeholder when its config declared a key variable
|
||||
that resolves to nothing in a ROUTED profile's scope (a profile served by a process launched
|
||||
for another, which never reads that process's environment). A single-profile process keeps
|
||||
the placeholder plus ``_key_env_secret``'s warning: there the variable is read from the
|
||||
process environment too, and callers rely on the placeholder rather than an error. A block
|
||||
that declares no key variable keeps the placeholder: that IS the keyless local-server
|
||||
configuration. ``config`` is a frozen
|
||||
configuration. Only a placeholder the resolver SYNTHESIZED is refused: a caller-supplied key
|
||||
(even the literal ``no-key-required``, the same value a set key variable may hold) and a
|
||||
transport that never sends the key (``_KEYLESS_API_MODES``, judged on the final api_mode) pass.
|
||||
``config`` is a frozen
|
||||
route's config (the entry that owned the endpoint when the session froze it). Typed ``AuthError``
|
||||
(``missing_api_key``) so every caller's fallback chain still advances."""
|
||||
from agent.secret_scope import serves_routed_profile
|
||||
if runtime.get("provider") != "custom" or runtime.get("api_key") != NO_KEY_PLACEHOLDER:
|
||||
return
|
||||
if str(explicit_api_key or "").strip() or runtime.get("api_mode") in _KEYLESS_API_MODES:
|
||||
return
|
||||
if not serves_routed_profile():
|
||||
return
|
||||
label, key_env = _declared_custom_key(str(requested_provider or "").strip().lower(),
|
||||
str(runtime.get("base_url") or ""), model_cfg, config)
|
||||
label, key_env = _declared_custom_key(str(requested_provider or "").strip().lower(), runtime, model_cfg, config)
|
||||
if not key_env or get_secret_str(key_env, "").strip():
|
||||
return
|
||||
from hermes_cli.auth import AuthError
|
||||
|
||||
@@ -15,10 +15,10 @@ import pytest
|
||||
from agent import secret_scope
|
||||
|
||||
|
||||
def _named_profile(tmp_path, monkeypatch, *, key_env: bool, dotenv: str = ""):
|
||||
def _named_profile(tmp_path, monkeypatch, *, key_env: bool, dotenv: str = "", model_extra=None):
|
||||
home = tmp_path / ".hermes" / "profiles" / "beta"
|
||||
home.mkdir(parents=True)
|
||||
model = {"default": "fake-model", "provider": "custom", "base_url": "http://127.0.0.1:18991/v1"}
|
||||
model = {"default": "fake-model", "provider": "custom", "base_url": "http://127.0.0.1:18991/v1", **(model_extra or {})}
|
||||
entry = {"name": "FakeUp", "base_url": "http://127.0.0.1:18991/v1"}
|
||||
if key_env:
|
||||
entry["key_env"] = "CANARY_KEY"
|
||||
@@ -74,3 +74,26 @@ def test_keyless_endpoint_and_profile_key_still_resolve(tmp_path, monkeypatch, m
|
||||
runtime = resolve_runtime_provider()
|
||||
assert runtime["provider"] == "custom"
|
||||
assert runtime["api_key"] == expected_key
|
||||
|
||||
|
||||
@pytest.mark.parametrize("model_extra,kwargs,expect", [
|
||||
# The codex app-server authenticates with its own login and never sends the resolved key.
|
||||
({"provider": "custom:FakeUp", "openai_runtime": "codex_app_server"}, {}, "codex_app_server"),
|
||||
# A caller-supplied placeholder is a choice, not a synthesized stand-in for the unset variable.
|
||||
({}, {"explicit_api_key": "no-key-required"}, "chat_completions"),
|
||||
# An explicit base_url replaces the selected entry's URL, not its key declaration.
|
||||
({"provider": "custom:FakeUp"}, {"explicit_base_url": "http://127.0.0.1:18992/v1"}, "missing_api_key"),
|
||||
], ids=["codex-app-server-owns-auth", "explicit-placeholder", "explicit-base-url-keeps-entry"])
|
||||
def test_only_a_synthesized_placeholder_for_the_selected_entry_is_refused(tmp_path, monkeypatch, multiplexed,
|
||||
model_extra, kwargs, expect):
|
||||
from hermes_cli.auth import AuthError
|
||||
from hermes_cli.runtime_provider import resolve_runtime_provider
|
||||
|
||||
multiplexed(_named_profile(tmp_path, monkeypatch, key_env=True, model_extra=model_extra))
|
||||
if expect == "missing_api_key":
|
||||
with pytest.raises(AuthError) as excinfo:
|
||||
resolve_runtime_provider(**kwargs)
|
||||
assert excinfo.value.code == "missing_api_key" and "FakeUp" in str(excinfo.value)
|
||||
return
|
||||
runtime = resolve_runtime_provider(**kwargs)
|
||||
assert (runtime["provider"], runtime["api_mode"]) == ("custom", expect)
|
||||
Reference in new issue
Block a user