From c253b073809f75fefa55dcd5bbe41b5faee8ca9c Mon Sep 17 00:00:00 2001 From: helix4u <4317663+helix4u@users.noreply.github.com> Date: Fri, 19 Jun 2026 20:36:09 -0600 Subject: [PATCH] fix(model): clear stale endpoint credentials across switches --- gateway/slash_commands.py | 6 +- hermes_cli/auth.py | 16 ++-- hermes_cli/config.py | 24 +++++ hermes_cli/model_setup_flows.py | 13 +++ hermes_cli/web_server.py | 13 ++- tests/cli/test_cli_provider_resolution.py | 93 +++++++++++++++++++ tests/gateway/test_model_picker_persist.py | 10 +- ...test_update_config_clears_custom_fields.py | 19 +++- tests/hermes_cli/test_web_server.py | 10 +- 9 files changed, 187 insertions(+), 17 deletions(-) diff --git a/gateway/slash_commands.py b/gateway/slash_commands.py index afb5737151b..c528f82e440 100644 --- a/gateway/slash_commands.py +++ b/gateway/slash_commands.py @@ -34,7 +34,7 @@ from agent.i18n import t from gateway.config import HomeChannel, Platform, PlatformConfig from gateway.platforms.base import EphemeralReply, MessageEvent, MessageType from gateway.session import SessionSource, build_session_key -from hermes_cli.config import cfg_get +from hermes_cli.config import cfg_get, clear_model_endpoint_credentials from utils import ( atomic_json_write, atomic_yaml_write, @@ -1239,6 +1239,8 @@ class GatewaySlashCommandsMixin: _persist_model_cfg["provider"] = result.target_provider if result.base_url: _persist_model_cfg["base_url"] = result.base_url + if str(result.target_provider or "").strip().lower() != "custom": + clear_model_endpoint_credentials(_persist_model_cfg) from hermes_cli.config import save_config save_config(_persist_cfg) except Exception as e: @@ -1429,6 +1431,8 @@ class GatewaySlashCommandsMixin: model_cfg["provider"] = result.target_provider if result.base_url: model_cfg["base_url"] = result.base_url + if str(result.target_provider or "").strip().lower() != "custom": + clear_model_endpoint_credentials(model_cfg) from hermes_cli.config import save_config save_config(cfg) except Exception as e: diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index d0c70a48def..7a08e2165bf 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -6386,16 +6386,12 @@ def _update_config_for_provider( # Clear stale base_url to prevent contamination when switching providers model_cfg.pop("base_url", None) - # Clear stale api_key/api_mode left over from a previous custom provider. - # When the user switches from e.g. a MiniMax custom endpoint - # (api_mode=anthropic_messages, api_key=mxp-...) to a built-in provider - # (e.g. OpenRouter), the stale api_key/api_mode would override the new - # provider's credentials and transport choice. Built-in providers that - # need a specific api_mode (copilot, xai) set it at request-resolution - # time via `_copilot_runtime_api_mode` / `_detect_api_mode_for_url`, so - # removing the persisted value here is safe. - model_cfg.pop("api_key", None) - model_cfg.pop("api_mode", None) + # Clear stale endpoint credentials left over from a previous custom provider. + # Built-in providers resolve credentials from env/auth state, not inline + # model.api_key. + from hermes_cli.config import clear_model_endpoint_credentials + + clear_model_endpoint_credentials(model_cfg) # When switching to a non-OpenRouter provider, ensure model.default is # valid for the new provider. An OpenRouter-formatted name like diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 80a4bc70901..ea87623d8fb 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -3915,6 +3915,30 @@ def _set_nested(config, dotted_key: str, value): current[last] = value +def clear_model_endpoint_credentials( + model_cfg: Dict[str, Any], + *, + clear_api_key: bool = True, + clear_api_mode: bool = True, +) -> Dict[str, Any]: + """Remove stale inline endpoint credentials from a model config. + + ``model.api_key`` is valid only for explicit custom endpoint assignments. + Built-in providers resolve credentials from env vars, auth.json, or the + credential pool. When switching away from a custom endpoint, leaving these + fields behind keeps secrets in config.yaml and can contaminate later custom + resolution paths. + """ + if not isinstance(model_cfg, dict): + return model_cfg + if clear_api_key: + model_cfg.pop("api_key", None) + model_cfg.pop("api", None) + if clear_api_mode: + model_cfg.pop("api_mode", None) + return model_cfg + + def get_missing_config_fields() -> List[Dict[str, Any]]: """ Check which config fields are missing or outdated (recursive). diff --git a/hermes_cli/model_setup_flows.py b/hermes_cli/model_setup_flows.py index 18776fd0678..8148abba0f0 100644 --- a/hermes_cli/model_setup_flows.py +++ b/hermes_cli/model_setup_flows.py @@ -24,6 +24,8 @@ import argparse import os import subprocess +from hermes_cli.config import clear_model_endpoint_credentials + def _prompt_auth_credentials_choice(title: str) -> str: """Prompt for reuse / reauthenticate / cancel with the standard radio UI. @@ -123,6 +125,7 @@ def _model_flow_openrouter(config, current_model=""): model["provider"] = "openrouter" model["base_url"] = OPENROUTER_BASE_URL model["api_mode"] = "chat_completions" + clear_model_endpoint_credentials(model, clear_api_mode=False) save_config(cfg) deactivate_provider() print(f"Default model set to: {selected} (via OpenRouter)") @@ -341,6 +344,7 @@ def _model_flow_nous(config, current_model="", args=None): model_cfg["base_url"] = inference_url.rstrip("/") else: model_cfg.pop("base_url", None) + clear_model_endpoint_credentials(model_cfg) config["model"] = model_cfg # Clear any custom endpoint that might conflict if get_env_value("OPENAI_BASE_URL"): @@ -1249,6 +1253,7 @@ def _model_flow_azure_foundry(config, current_model=""): model["api_mode"] = api_mode model["default"] = effective_model model["auth_mode"] = auth_mode_label + clear_model_endpoint_credentials(model, clear_api_mode=False) if use_entra: # Persist only the non-default Entra scope so config.yaml stays tidy. # Azure identity selection stays in standard AZURE_* env vars. @@ -1670,6 +1675,7 @@ def _model_flow_copilot(config, current_model=""): catalog=catalog, api_key=api_key, ) + clear_model_endpoint_credentials(model, clear_api_mode=False) if selected_effort is not None: _set_reasoning_effort(cfg, selected_effort) save_config(cfg) @@ -1795,6 +1801,7 @@ def _model_flow_copilot_acp(config, current_model=""): model["provider"] = provider_id model["base_url"] = effective_base model["api_mode"] = "chat_completions" + clear_model_endpoint_credentials(model, clear_api_mode=False) save_config(cfg) deactivate_provider() @@ -1884,6 +1891,7 @@ def _model_flow_kimi(config, current_model=""): model["provider"] = provider_id model["base_url"] = effective_base model.pop("api_mode", None) # let runtime auto-detect from URL + clear_model_endpoint_credentials(model, clear_api_mode=False) save_config(cfg) deactivate_provider() @@ -1997,6 +2005,7 @@ def _model_flow_stepfun(config, current_model=""): model["provider"] = provider_id model["base_url"] = effective_base model.pop("api_mode", None) + clear_model_endpoint_credentials(model, clear_api_mode=False) save_config(cfg) deactivate_provider() @@ -2080,6 +2089,7 @@ def _model_flow_bedrock_api_key(config, region, current_model=""): model["provider"] = "custom" model["base_url"] = mantle_base_url model.pop("api_mode", None) # chat_completions is the default + clear_model_endpoint_credentials(model, clear_api_mode=False) # Also save region in bedrock config for reference bedrock_cfg = cfg.get("bedrock", {}) @@ -2273,6 +2283,7 @@ def _model_flow_bedrock(config, current_model=""): model["provider"] = "bedrock" model["base_url"] = f"https://bedrock-runtime.{region}.amazonaws.com" model.pop("api_mode", None) # bedrock_converse is auto-detected + clear_model_endpoint_credentials(model, clear_api_mode=False) bedrock_cfg = cfg.get("bedrock", {}) if not isinstance(bedrock_cfg, dict): @@ -2566,6 +2577,7 @@ def _model_flow_api_key_provider(config, provider_id, current_model=""): cfg["model"] = model model["provider"] = provider_id model["base_url"] = effective_base + clear_model_endpoint_credentials(model, clear_api_mode=False) if provider_id in {"opencode-zen", "opencode-go"}: model["api_mode"] = opencode_model_api_mode(provider_id, selected) else: @@ -2720,6 +2732,7 @@ def _model_flow_anthropic(config, current_model=""): cfg["model"] = model model["provider"] = "anthropic" model.pop("base_url", None) + clear_model_endpoint_credentials(model) save_config(cfg) deactivate_provider() diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 6701d67394f..398e61772f0 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -48,6 +48,7 @@ from hermes_cli.config import ( cfg_get, DEFAULT_CONFIG, OPTIONAL_ENV_VARS, + clear_model_endpoint_credentials, get_config_path, get_env_path, get_hermes_home, @@ -901,8 +902,11 @@ def _apply_main_model_assignment( # same-provider re-pick so re-selecting a model doesn't wipe the key. if api_key.strip(): model_cfg["api_key"] = api_key.strip() + model_cfg.pop("api", None) elif model_cfg.get("api_key") and new_provider != prev_provider: - model_cfg["api_key"] = "" + clear_model_endpoint_credentials(model_cfg, clear_api_mode=False) + if new_provider != prev_provider: + clear_model_endpoint_credentials(model_cfg, clear_api_key=False) model_cfg.pop("context_length", None) return model_cfg @@ -3871,6 +3875,8 @@ def _apply_model_assignment_sync( slot_cfg = {} slot_cfg["provider"] = "auto" slot_cfg["model"] = "" + slot_cfg.pop("base_url", None) + clear_model_endpoint_credentials(slot_cfg) aux[slot] = slot_cfg cfg["auxiliary"] = aux save_config(cfg) @@ -3886,8 +3892,13 @@ def _apply_model_assignment_sync( slot_cfg = aux.get(slot) if not isinstance(slot_cfg, dict): slot_cfg = {} + prev_provider = str(slot_cfg.get("provider") or "").strip().lower() + new_provider = provider.strip().lower() slot_cfg["provider"] = provider slot_cfg["model"] = model + if new_provider != prev_provider and new_provider != "custom": + slot_cfg.pop("base_url", None) + clear_model_endpoint_credentials(slot_cfg) aux[slot] = slot_cfg cfg["auxiliary"] = aux diff --git a/tests/cli/test_cli_provider_resolution.py b/tests/cli/test_cli_provider_resolution.py index 5dbfca1ae6f..a5b37742ad6 100644 --- a/tests/cli/test_cli_provider_resolution.py +++ b/tests/cli/test_cli_provider_resolution.py @@ -378,6 +378,99 @@ def test_model_flow_nous_does_not_restore_stale_custom_api_key(tmp_path, monkeyp assert "api_mode" not in model +def _seed_stale_custom_model(tmp_path, monkeypatch): + import yaml + + config_home = tmp_path / "hermes" + config_home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(config_home)) + config_path = config_home / "config.yaml" + config_path.write_text( + yaml.safe_dump( + { + "model": { + "provider": "custom", + "default": "glm-5.2", + "base_url": "https://api.neuralwatt.com/v1", + "api_key": "${NEURALWATT_API_KEY}", + "api": "legacy-stale-key", + "api_mode": "anthropic_messages", + } + }, + sort_keys=False, + ) + ) + (config_home / ".env").write_text("") + return config_path + + +def test_model_flow_openrouter_clears_stale_custom_key(tmp_path, monkeypatch): + import yaml + + config_path = _seed_stale_custom_model(tmp_path, monkeypatch) + + monkeypatch.setattr( + "hermes_cli.main._prompt_api_key", + lambda *args, **kwargs: ("sk-openrouter", False), + ) + monkeypatch.setattr( + "hermes_cli.models.model_ids", + lambda **kwargs: ["anthropic/claude-sonnet-4.6"], + ) + monkeypatch.setattr("hermes_cli.models.get_pricing_for_provider", lambda *a, **k: {}) + monkeypatch.setattr( + "hermes_cli.auth._prompt_model_selection", + lambda *args, **kwargs: "anthropic/claude-sonnet-4.6", + ) + monkeypatch.setattr("hermes_cli.auth.deactivate_provider", lambda: None) + + hermes_main._model_flow_openrouter({}, current_model="glm-5.2") + + config = yaml.safe_load(config_path.read_text()) or {} + model = config["model"] + assert model["provider"] == "openrouter" + assert model["default"] == "anthropic/claude-sonnet-4.6" + assert model["api_mode"] == "chat_completions" + assert "api_key" not in model + assert "api" not in model + + +def test_model_flow_anthropic_clears_stale_custom_key_and_mode(tmp_path, monkeypatch): + import yaml + + config_path = _seed_stale_custom_model(tmp_path, monkeypatch) + + monkeypatch.setattr("hermes_cli.auth.get_anthropic_key", lambda: "sk-ant-api03-test") + monkeypatch.setattr( + "agent.anthropic_adapter.read_claude_code_credentials", + lambda: None, + ) + monkeypatch.setattr( + "agent.anthropic_adapter.is_claude_code_token_valid", + lambda creds: False, + ) + monkeypatch.setattr( + "hermes_cli.model_setup_flows._prompt_auth_credentials_choice", + lambda title: "use", + ) + monkeypatch.setattr( + "hermes_cli.auth._prompt_model_selection", + lambda *args, **kwargs: "claude-sonnet-4-6", + ) + monkeypatch.setattr("hermes_cli.auth.deactivate_provider", lambda: None) + + hermes_main._model_flow_anthropic({}, current_model="glm-5.2") + + config = yaml.safe_load(config_path.read_text()) or {} + model = config["model"] + assert model["provider"] == "anthropic" + assert model["default"] == "claude-sonnet-4-6" + assert "base_url" not in model + assert "api_key" not in model + assert "api" not in model + assert "api_mode" not in model + + def test_model_flow_nous_offers_tool_gateway_prompt_when_unconfigured(monkeypatch, capsys): from hermes_cli.nous_account import NousPortalAccountInfo diff --git a/tests/gateway/test_model_picker_persist.py b/tests/gateway/test_model_picker_persist.py index ff74fd53de8..ca9498389b1 100644 --- a/tests/gateway/test_model_picker_persist.py +++ b/tests/gateway/test_model_picker_persist.py @@ -140,7 +140,13 @@ async def _drive_picker(runner, event): "seed_model", [ # Already-nested dict (common case). - {"default": "old-model", "provider": "openai-codex"}, + { + "default": "old-model", + "provider": "custom", + "base_url": "https://api.custom.example/v1", + "api_key": "sk-stale", + "api_mode": "anthropic_messages", + }, # Flat-string model: must be coerced to a nested dict on a tap (same # scalar-``model:`` guard the text path has) instead of raising # ``TypeError`` on assignment. @@ -166,6 +172,8 @@ async def test_picker_tap_persists_by_default(tmp_path, monkeypatch, seed_model) assert written["model"]["default"] == "gpt-5.5" assert written["model"]["provider"] == "openrouter" assert written["model"]["base_url"] == "https://openrouter.ai/api/v1" + assert "api_key" not in written["model"] + assert "api_mode" not in written["model"] @pytest.mark.asyncio diff --git a/tests/hermes_cli/test_update_config_clears_custom_fields.py b/tests/hermes_cli/test_update_config_clears_custom_fields.py index 6d74a1c0373..99dc8261c37 100644 --- a/tests/hermes_cli/test_update_config_clears_custom_fields.py +++ b/tests/hermes_cli/test_update_config_clears_custom_fields.py @@ -16,7 +16,7 @@ from __future__ import annotations import yaml from hermes_cli.auth import _update_config_for_provider -from hermes_cli.config import get_config_path +from hermes_cli.config import clear_model_endpoint_credentials, get_config_path def _read_model_cfg() -> dict: @@ -49,6 +49,23 @@ def _seed_custom_provider_config(api_mode: str = "anthropic_messages") -> None: class TestUpdateConfigForProviderClearsStaleCustomFields: + def test_clear_model_endpoint_credentials_removes_key_alias_and_mode(self): + model_cfg = { + "provider": "openrouter", + "default": "anthropic/claude-sonnet-4.6", + "api_key": "sk-stale", + "api": "sk-legacy-stale", + "api_mode": "anthropic_messages", + } + + returned = clear_model_endpoint_credentials(model_cfg) + + assert returned is model_cfg + assert "api_key" not in model_cfg + assert "api" not in model_cfg + assert "api_mode" not in model_cfg + assert model_cfg["provider"] == "openrouter" + def test_switching_to_openrouter_clears_api_key_and_api_mode(self): _seed_custom_provider_config() diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index 0a5319a0518..99969e29dc6 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -2327,9 +2327,10 @@ class TestWebServerEndpoints: # api_key follows the same lifecycle as base_url: # supplied → persisted. out = _apply_main_model_assignment( - {}, "custom", "m", "http://x/v1", "sk-secret" + {"api": "sk-legacy-old"}, "custom", "m", "http://x/v1", "sk-secret" ) assert out["api_key"] == "sk-secret" + assert "api" not in out # same provider, no new key → existing key preserved (re-picking a model # on the same custom endpoint must not wipe the saved key). @@ -2342,9 +2343,12 @@ class TestWebServerEndpoints: # switching providers without a new key → stale key cleared. out = _apply_main_model_assignment( - {"provider": "custom", "api_key": "sk-old"}, "openrouter", "m" + {"provider": "custom", "api_key": "sk-old", "api_mode": "anthropic_messages"}, + "openrouter", + "m", ) - assert out["api_key"] == "" + assert "api_key" not in out + assert "api_mode" not in out def test_parse_model_ids_handles_openai_and_bare_shapes(self): """Model discovery must tolerate the common /v1/models shapes and