mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix: clear stale api_key on provider switch in _model_flow_api_key_provider
When switching from one API-key provider to another via 'hermes model',
the old provider's api_key was left in config.yaml's model.api_key field.
Built-in providers get their keys from env vars / credential pool, so the
stale key caused credential drift — the new provider would try to
authenticate with the old key and fail with 401.
The sister function in auth.py (set_provider_in_config) correctly pops
both api_key and api_mode on provider switch. This adds the missing
model.pop('api_key', None) call, mirroring auth.py:2764.
Fixes #14134
This commit is contained in:
parent
4fade39c90
commit
fd13002d99
2 changed files with 196 additions and 0 deletions
|
|
@ -4191,6 +4191,11 @@ def _model_flow_api_key_provider(config, provider_id, current_model=""):
|
|||
model["api_mode"] = opencode_model_api_mode(provider_id, selected)
|
||||
else:
|
||||
model.pop("api_mode", None)
|
||||
# Clear stale api_key from a previous provider. Built-in providers
|
||||
# get their keys from env vars / credential pool — a leftover key
|
||||
# from a prior provider causes credential drift (401 errors).
|
||||
# Mirrors auth.py set_provider_in_config (line ~2764). (#14134)
|
||||
model.pop("api_key", None)
|
||||
save_config(cfg)
|
||||
deactivate_provider()
|
||||
|
||||
|
|
|
|||
191
tests/hermes_cli/test_api_key_drift_provider_switch.py
Normal file
191
tests/hermes_cli/test_api_key_drift_provider_switch.py
Normal file
|
|
@ -0,0 +1,191 @@
|
|||
"""Tests that switching providers via _model_flow_api_key_provider
|
||||
clears stale api_key from the model config dict.
|
||||
|
||||
Regression test for #14134: when switching from one API-key provider
|
||||
to another, the old provider's api_key was left in model.api_key,
|
||||
causing credential drift — the new provider would try to use the
|
||||
old provider's key and fail with 401.
|
||||
|
||||
The sister function in auth.py (set_provider_in_config) correctly
|
||||
pops both api_key and api_mode on provider switch. This test ensures
|
||||
_model_flow_api_key_provider does the same.
|
||||
"""
|
||||
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def config_home(tmp_path, monkeypatch):
|
||||
"""Isolated HERMES_HOME with config that has a stale api_key."""
|
||||
home = tmp_path / "hermes"
|
||||
home.mkdir()
|
||||
env_file = home / ".env"
|
||||
env_file.write_text("")
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
# Clear env vars that could interfere
|
||||
monkeypatch.delenv("HERMES_MODEL", raising=False)
|
||||
monkeypatch.delenv("LLM_MODEL", raising=False)
|
||||
monkeypatch.delenv("HERMES_INFERENCE_PROVIDER", raising=False)
|
||||
monkeypatch.delenv("GITHUB_TOKEN", raising=False)
|
||||
monkeypatch.delenv("GH_TOKEN", raising=False)
|
||||
monkeypatch.delenv("OPENAI_BASE_URL", raising=False)
|
||||
monkeypatch.delenv("OPENAI_API_KEY", raising=False)
|
||||
monkeypatch.delenv("OPENROUTER_API_KEY", raising=False)
|
||||
monkeypatch.delenv("STEPFUN_API_KEY", raising=False)
|
||||
monkeypatch.delenv("STEPFUN_BASE_URL", raising=False)
|
||||
return home
|
||||
|
||||
|
||||
def _write_config(home, model_dict):
|
||||
"""Write a config.yaml with the given model dict."""
|
||||
import yaml
|
||||
config_yaml = home / "config.yaml"
|
||||
config_yaml.write_text(yaml.dump({"model": model_dict}))
|
||||
|
||||
|
||||
def _read_model_config(home):
|
||||
"""Read the model section from config.yaml."""
|
||||
import yaml
|
||||
config = yaml.safe_load((home / "config.yaml").read_text()) or {}
|
||||
return config.get("model", {})
|
||||
|
||||
|
||||
class TestApiKeyDriftOnProviderSwitch:
|
||||
"""Switching from one api-key provider to another must clear the
|
||||
stale api_key from the model config dict."""
|
||||
|
||||
def test_api_key_cleared_on_provider_switch(self, config_home, monkeypatch):
|
||||
"""Start with model.api_key = 'sk-old-key' from provider A,
|
||||
switch to provider B — api_key must be popped."""
|
||||
from hermes_cli.auth import PROVIDER_REGISTRY
|
||||
|
||||
pconfig = PROVIDER_REGISTRY.get("zai")
|
||||
if not pconfig:
|
||||
pytest.skip("zai not in PROVIDER_REGISTRY")
|
||||
|
||||
# Start with a config from a *previous* provider that had an api_key
|
||||
_write_config(config_home, {
|
||||
"default": "some-old-model",
|
||||
"provider": "ollama-cloud",
|
||||
"base_url": "https://api.ola.cloud/v1",
|
||||
"api_key": "sk-old-provider-key-12345",
|
||||
})
|
||||
|
||||
monkeypatch.setenv("GLM_API_KEY", "test-key")
|
||||
|
||||
from hermes_cli.main import _model_flow_api_key_provider
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
with patch("hermes_cli.auth._prompt_model_selection", return_value="glm-5"), \
|
||||
patch("hermes_cli.auth.deactivate_provider"), \
|
||||
patch("builtins.input", return_value=""):
|
||||
_model_flow_api_key_provider(load_config(), "zai", "some-old-model")
|
||||
|
||||
model = _read_model_config(config_home)
|
||||
assert isinstance(model, dict)
|
||||
assert model.get("provider") == "zai", (
|
||||
f"provider should be 'zai', got {model.get('provider')}"
|
||||
)
|
||||
assert "api_key" not in model, (
|
||||
f"api_key should be cleared on provider switch, but found: {model.get('api_key')}"
|
||||
)
|
||||
|
||||
def test_api_mode_also_cleared_on_non_opencode_switch(self, config_home, monkeypatch):
|
||||
"""A stale api_mode from a previous custom provider must also
|
||||
be cleared when switching to a non-opencode provider."""
|
||||
from hermes_cli.auth import PROVIDER_REGISTRY
|
||||
|
||||
pconfig = PROVIDER_REGISTRY.get("zai")
|
||||
if not pconfig:
|
||||
pytest.skip("zai not in PROVIDER_REGISTRY")
|
||||
|
||||
# Start with custom-provider config that had api_mode and api_key
|
||||
_write_config(config_home, {
|
||||
"default": "custom-model",
|
||||
"provider": "custom",
|
||||
"base_url": "https://custom.old/v1",
|
||||
"api_key": "sk-stale-custom-key",
|
||||
"api_mode": "anthropic_messages",
|
||||
})
|
||||
|
||||
monkeypatch.setenv("GLM_API_KEY", "test-key")
|
||||
|
||||
from hermes_cli.main import _model_flow_api_key_provider
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
with patch("hermes_cli.auth._prompt_model_selection", return_value="glm-5"), \
|
||||
patch("hermes_cli.auth.deactivate_provider"), \
|
||||
patch("builtins.input", return_value=""):
|
||||
_model_flow_api_key_provider(load_config(), "zai", "custom-model")
|
||||
|
||||
model = _read_model_config(config_home)
|
||||
assert isinstance(model, dict)
|
||||
assert "api_key" not in model, (
|
||||
f"api_key should be cleared, got: {model.get('api_key')}"
|
||||
)
|
||||
assert "api_mode" not in model, (
|
||||
f"api_mode should be cleared for non-opencode, got: {model.get('api_mode')}"
|
||||
)
|
||||
|
||||
def test_switch_preserves_default_model(self, config_home, monkeypatch):
|
||||
"""The model.default should be updated to the new selection even
|
||||
when there was a stale api_key."""
|
||||
from hermes_cli.auth import PROVIDER_REGISTRY
|
||||
|
||||
pconfig = PROVIDER_REGISTRY.get("zai")
|
||||
if not pconfig:
|
||||
pytest.skip("zai not in PROVIDER_REGISTRY")
|
||||
|
||||
_write_config(config_home, {
|
||||
"default": "old-model-from-previous-provider",
|
||||
"provider": "ollama-cloud",
|
||||
"api_key": "sk-orphaned-key",
|
||||
})
|
||||
|
||||
monkeypatch.setenv("GLM_API_KEY", "test-key")
|
||||
|
||||
from hermes_cli.main import _model_flow_api_key_provider
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
with patch("hermes_cli.auth._prompt_model_selection", return_value="glm-5"), \
|
||||
patch("hermes_cli.auth.deactivate_provider"), \
|
||||
patch("builtins.input", return_value=""):
|
||||
_model_flow_api_key_provider(load_config(), "zai", "old-model-from-previous-provider")
|
||||
|
||||
model = _read_model_config(config_home)
|
||||
assert model.get("default") == "glm-5", (
|
||||
f"model.default should be 'glm-5', got {model.get('default')}"
|
||||
)
|
||||
assert "api_key" not in model
|
||||
|
||||
def test_no_api_key_no_error(self, config_home, monkeypatch):
|
||||
"""If config has no stale api_key, switching should still work fine."""
|
||||
from hermes_cli.auth import PROVIDER_REGISTRY
|
||||
|
||||
pconfig = PROVIDER_REGISTRY.get("zai")
|
||||
if not pconfig:
|
||||
pytest.skip("zai not in PROVIDER_REGISTRY")
|
||||
|
||||
# Clean config, no api_key
|
||||
_write_config(config_home, {
|
||||
"default": "old-model",
|
||||
"provider": "ollama-cloud",
|
||||
})
|
||||
|
||||
monkeypatch.setenv("GLM_API_KEY", "test-key")
|
||||
|
||||
from hermes_cli.main import _model_flow_api_key_provider
|
||||
from hermes_cli.config import load_config
|
||||
|
||||
with patch("hermes_cli.auth._prompt_model_selection", return_value="glm-5"), \
|
||||
patch("hermes_cli.auth.deactivate_provider"), \
|
||||
patch("builtins.input", return_value=""):
|
||||
_model_flow_api_key_provider(load_config(), "zai", "old-model")
|
||||
|
||||
model = _read_model_config(config_home)
|
||||
assert model.get("provider") == "zai"
|
||||
assert model.get("default") == "glm-5"
|
||||
assert "api_key" not in model
|
||||
Loading…
Add table
Add a link
Reference in a new issue