fix(dashboard): let an explicit api_key win over the provider entry's stored one

POST /api/model/set accepts an api_key and threads it into
_apply_main_model_assignment. The custom-endpoint work then added a
provider-entry fallback right after it — but unconditionally:

    if not base_url and provider_entry.get("base_url"):
        base_url = provider_entry["base_url"]          # explicit wins
    model_cfg = _apply_main_model_assignment(..., base_url, api_key)
    if provider_entry.get("api_key"):
        model_cfg["api_key"] = provider_entry["api_key"]   # explicit LOSES

The two lines disagree about precedence. base_url fills only a gap; api_key
overwrites whatever the caller sent.

So rotating a key through this endpoint returns 200 and silently keeps the
old one:

    request api_key : sk-NEW-ROTATED-KEY
    stored  api_key : sk-STORED-OLD-KEY

That matters beyond the write itself: model.api_key outranks the environment
at client construction, so the stale key keeps authenticating and shadows
anything the operator configures next — the persistent-401 shape
credential_lifecycle.py documents as #62269.

A regression, not long-standing. Against 3d9789357^ the same request stores
sk-NEW-ROTATED-KEY.

Gate the fallback on `not api_key`, matching the base_url line directly above
it. Switching to a configured provider with no key in the request still adopts
the entry's key — pinned by its own test so the feature's intent doesn't
regress in the other direction.
This commit is contained in:
Frowtek 2026-07-20 04:48:51 +03:00 committed by Teknium
parent 31c08a9aad
commit 38a274b297
2 changed files with 66 additions and 1 deletions

View file

@ -6421,7 +6421,16 @@ def _apply_model_assignment_sync(
model_cfg = _apply_main_model_assignment(
cfg.get("model", {}), provider, model, base_url, api_key
)
if isinstance(provider_entry, dict) and provider_entry.get("api_key"):
# Fall back to the provider entry's stored key only when the request
# didn't carry one — same precedence as the base_url fill above. An
# unconditional overwrite silently discards a key the caller is
# rotating in, and model.api_key outranks the environment at client
# construction (#62269), so the stale key keeps authenticating.
if (
not api_key
and isinstance(provider_entry, dict)
and provider_entry.get("api_key")
):
model_cfg["api_key"] = provider_entry["api_key"]
cfg["model"] = model_cfg

View file

@ -4252,6 +4252,62 @@ class TestWebServerEndpoints:
assert cfg["model"]["default"] == "gpt-5.4"
assert cfg["model"]["base_url"] == "http://127.0.0.1:8081/v1"
def _seed_custom_provider_with_key(self):
from hermes_cli.config import load_config, save_config
cfg = load_config()
cfg["providers"] = {
"acme": {
"name": "Acme",
"base_url": "https://llm.acme.corp/v1",
"model": "acme/m1",
"api_key": "sk-stored-old",
"models": {"acme/m1": {}},
}
}
save_config(cfg)
def test_set_model_main_honors_an_explicitly_supplied_api_key(self):
"""A key in the request must win over the provider entry's stored one.
The entry-key fallback exists so switching to a configured provider
picks up its credential. Applying it unconditionally discards a key the
caller is rotating in and ``model.api_key`` outranks the environment
at client construction (#62269), so the stale key keeps authenticating
while the UI reports the change saved.
"""
from hermes_cli.config import load_config
self._seed_custom_provider_with_key()
resp = self.client.post(
"/api/model/set",
json={
"scope": "main",
"provider": "acme",
"model": "acme/m1",
"api_key": "sk-new-rotated",
},
)
assert resp.status_code == 200
assert load_config()["model"]["api_key"] == "sk-new-rotated"
def test_set_model_main_falls_back_to_the_provider_entry_key(self):
"""With no key in the request the stored one is still adopted."""
from hermes_cli.config import load_config
self._seed_custom_provider_with_key()
resp = self.client.post(
"/api/model/set",
json={"scope": "main", "provider": "acme", "model": "acme/m1"},
)
assert resp.status_code == 200
model_cfg = load_config()["model"]
assert model_cfg["api_key"] == "sk-stored-old"
# The sibling base_url fill is unaffected.
assert model_cfg["base_url"] == "https://llm.acme.corp/v1"
def test_set_model_main_preserves_base_url_for_named_custom_provider(self):
"""Selecting a named custom endpoint from the Desktop model picker
should keep its endpoint URL attached to model config.