From 38a274b297f62160caf07ebc16eecde5d69ae8bb Mon Sep 17 00:00:00 2001 From: Frowtek Date: Mon, 20 Jul 2026 04:48:51 +0300 Subject: [PATCH] fix(dashboard): let an explicit api_key win over the provider entry's stored one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- hermes_cli/web_server.py | 11 +++++- tests/hermes_cli/test_web_server.py | 56 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 5050c6651db1..10510251ff3b 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -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 diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index dbbfe6f6d070..ac4a7e04ca41 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -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.