From bfe7460c79fabcd61c683d66f4ecbc45f0340c21 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 24 Jul 2026 21:11:26 -0500 Subject: [PATCH 1/5] fix(config): add a collision-safe env var name for custom endpoint keys Both the Desktop panel and the CLI setup flow need somewhere in .env to put a custom endpoint's API key. Deriving the name from the endpoint's hostname collapses two servers on one machine onto a single slot, and every IP-based local endpoint slugs to a digit-leading name that save_env_value rejects outright. Key off the endpoint's own identity and keep a fixed prefix. Co-authored-by: asorry75 <33794789+asorry75@users.noreply.github.com> Co-authored-by: webtecnica <75556242+webtecnica@users.noreply.github.com> --- hermes_cli/config.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index d4e9ed843caa6..cc03c5a57b5cc 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -8209,6 +8209,25 @@ def save_env_value(key: str, value: str): invalidate_env_cache() +def custom_endpoint_key_env(identity: str) -> str: + """Env var name holding a custom endpoint's API key. + + ``identity`` is whatever names the endpoint on the calling path — the + Desktop panel's endpoint id, or ``host:port`` for the CLI setup flow. + Two properties matter: + + - It keys off the endpoint's own identity, not just its hostname, so two + endpoints on one host (``127.0.0.1:8000`` and ``:8001``) get separate + slots instead of the second save clobbering the first's credential. + - The fixed ``HERMES_CUSTOM_`` prefix keeps the result a valid POSIX name + even when the slug starts with a digit, which every IP-based local + endpoint does (``127.0.0.1`` → ``127_0_0_1``). ``save_env_value`` + rejects digit-leading names outright. + """ + slug = re.sub(r"[^A-Z0-9]+", "_", str(identity or "").upper()).strip("_") + return f"HERMES_CUSTOM_{slug}_API_KEY" if slug else "HERMES_CUSTOM_API_KEY" + + def remove_env_value(key: str) -> bool: """Remove a key from ~/.hermes/.env and os.environ. From de6375ebc5e44e5d1441839d5c5506b60e42ba13 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 24 Jul 2026 21:12:36 -0500 Subject: [PATCH 2/5] fix(desktop): persist the whole discovered model list when saving an endpoint Test enumerates a custom provider's catalogue and the panel holds the result in discoveredModels, but the save payload never carried it, so only the one model the user hand-typed reached providers..models. Every downstream picker reads that map straight from config.yaml with no live probe, which is why a proxy serving 18 models offered exactly one. Send the discovered list and merge it onto the entry, so models already known keep their context lengths. Fixes #69988 Co-authored-by: asorry75 <33794789+asorry75@users.noreply.github.com> --- .../app/settings/custom-endpoints-settings.tsx | 7 ++++--- apps/desktop/src/types/hermes.ts | 1 + hermes_cli/web_server.py | 18 +++++++++++++----- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/apps/desktop/src/app/settings/custom-endpoints-settings.tsx b/apps/desktop/src/app/settings/custom-endpoints-settings.tsx index bea02e2bce796..b74b30a290905 100644 --- a/apps/desktop/src/app/settings/custom-endpoints-settings.tsx +++ b/apps/desktop/src/app/settings/custom-endpoints-settings.tsx @@ -58,7 +58,7 @@ function formFromEndpoint(endpoint: CustomEndpoint): EndpointForm { } } -function toPayload(form: EndpointForm): CustomEndpointUpdate { +function toPayload(form: EndpointForm, models?: string[]): CustomEndpointUpdate { const contextLength = Number.parseInt(form.contextLength, 10) return { @@ -69,7 +69,8 @@ function toPayload(form: EndpointForm): CustomEndpointUpdate { api_key: form.apiKey.trim() || undefined, context_length: Number.isFinite(contextLength) && contextLength > 0 ? contextLength : undefined, discover_models: form.discoverModels, - make_default: form.makeDefault + make_default: form.makeDefault, + models: models?.length ? models : undefined } } @@ -125,7 +126,7 @@ export function CustomEndpointsSettings({ onConfigSaved, onMainModelChanged }: C async function handleSave() { try { setSaving(true) - const response = await saveCustomEndpoint(toPayload(form)) + const response = await saveCustomEndpoint(toPayload(form, discoveredModels)) setEndpoints(response.endpoints) const saved = response.endpoints.find(endpoint => endpoint.id === response.id) diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index d811e07ed57c5..cd62848d73873 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/apps/desktop/src/types/hermes.ts @@ -182,6 +182,7 @@ export interface CustomEndpointUpdate { id?: string make_default?: boolean model: string + models?: string[] name: string } diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 1e5cf16551782..4c2690be16244 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -1271,6 +1271,7 @@ class CustomEndpointUpdate(BaseModel): context_length: Optional[int] = None discover_models: bool = True make_default: bool = False + models: Optional[List[str]] = None class MessagingPlatformUpdate(BaseModel): @@ -7700,13 +7701,20 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T "model": model, "discover_models": bool(body.discover_models), }) - # Same for the model map: the panel names one default model, it does not - # enumerate the provider's catalogue. Keep the other models (and their - # context lengths) and just ensure this one is present. + # Same for the model map: merge rather than replace, so existing models + # keep their context lengths. ``body.models`` is the catalogue the panel's + # Test button already discovered — without it only the one hand-typed + # model survived Save, and every picker showed a single-entry list for a + # provider serving dozens (#69988). A payload with no ``models`` (older + # UI) still just ensures the named default is present. existing_models = entry.get("models") models_map: Dict[str, Any] = dict(existing_models) if isinstance(existing_models, dict) else {} - current_model_entry = models_map.get(model) - models_map[model] = dict(current_model_entry) if isinstance(current_model_entry, dict) else {} + for candidate in (*(body.models or ()), model): + model_id = str(candidate).strip() + if not model_id: + continue + current = models_map.get(model_id) + models_map[model_id] = dict(current) if isinstance(current, dict) else {} entry["models"] = models_map if body.context_length and body.context_length > 0: entry["context_length"] = int(body.context_length) From bd2dcfe9ca40de8219eadc884c631033bbb96ed1 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 24 Jul 2026 21:12:46 -0500 Subject: [PATCH 3/5] fix(web_server): keep Desktop custom endpoint API keys out of config.yaml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Custom Endpoints panel wrote the raw key to providers..api_key, so the credential sat in plaintext in a file users routinely share and commit. The input is masked, so nothing warned them. Write the key to .env and reference it via key_env, the same indirection built-in providers use and that runtime_provider already resolves. The read side has to move with it: reporting has_api_key from api_key alone would show "no API key" for every migrated endpoint, and activate copying only api_key would drop the credential entirely. Delete now clears the .env slot too, and an entry still carrying a pre-fix plaintext key is migrated on its next save so existing users get cleaned up without re-entering anything — unless the key is a hand-written ${VAR} template, which is already safe and must not be duplicated into a second env var. Fixes #69449 Co-authored-by: webtecnica <75556242+webtecnica@users.noreply.github.com> Co-authored-by: asorry75 <33794789+asorry75@users.noreply.github.com> --- hermes_cli/web_server.py | 81 +++++++++++++++++++++++++++++++++++----- 1 file changed, 71 insertions(+), 10 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 4c2690be16244..a32f86d0c4cfb 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -72,6 +72,7 @@ from hermes_cli.config import ( save_config, save_env_value, remove_env_value, + custom_endpoint_key_env, check_config_version, detect_install_method, format_docker_update_message, @@ -7584,6 +7585,38 @@ def _models_from_custom_endpoint_entry(entry: Dict[str, Any]) -> List[str]: return [model for model in models if model and not (model in seen or seen.add(model))] +def _api_key_display(entry: Dict[str, Any]) -> Tuple[bool, Optional[str]]: + """Return ``(has_api_key, preview)`` for a provider or model config block. + + Keys live in ``.env`` behind ``key_env``; only entries written before + #69449 still carry a plaintext ``api_key``. Checking both keeps the panel + honest either way — reading only ``api_key`` reported "no API key" for + every endpoint whose key had been moved to ``.env``. + """ + plaintext = str(entry.get("api_key") or "").strip() + if plaintext: + return True, redact_key(plaintext) + key_env = str(entry.get("key_env") or "").strip() + if key_env: + return True, f"${{{key_env}}}" + return False, None + + +def _config_api_key_is_env_ref(endpoint_id: str) -> bool: + """True when this endpoint's on-disk ``api_key`` is a ``${VAR}`` template. + + ``load_config()`` expands env refs, so a hand-written + ``api_key: ${MY_KEY}`` is indistinguishable from a literal secret by the + time it reaches us. Such an entry is already keeping its secret out of + config.yaml, so migrating it would only copy that secret into a second + env var the user didn't ask for. + """ + providers = read_raw_config().get("providers") + entry = providers.get(endpoint_id) if isinstance(providers, dict) else None + raw_key = entry.get("api_key") if isinstance(entry, dict) else None + return bool(isinstance(raw_key, str) and re.search(r"\$\{[^}]+\}", raw_key)) + + def _custom_endpoint_response(cfg: Dict[str, Any]) -> Dict[str, Any]: model_cfg = cfg.get("model", {}) if isinstance(cfg.get("model"), dict) else {} current_provider = str(model_cfg.get("provider", "") or "") @@ -7602,6 +7635,7 @@ def _custom_endpoint_response(cfg: Dict[str, Any]) -> Dict[str, Any]: endpoint_id = str(provider_id) models = _models_from_custom_endpoint_entry(raw_entry) endpoint_model = str(raw_entry.get("model") or raw_entry.get("default_model") or (models[0] if models else "")) + has_api_key, api_key_preview = _api_key_display(raw_entry) endpoints.append({ "id": endpoint_id, "name": str(raw_entry.get("name") or endpoint_id), @@ -7610,13 +7644,14 @@ def _custom_endpoint_response(cfg: Dict[str, Any]) -> Dict[str, Any]: "models": models, "context_length": raw_entry.get("context_length"), "discover_models": bool(raw_entry.get("discover_models", True)), - "has_api_key": bool(str(raw_entry.get("api_key", "") or "").strip()), - "api_key_preview": redact_key(str(raw_entry.get("api_key", "") or "")) if raw_entry.get("api_key") else None, + "has_api_key": has_api_key, + "api_key_preview": api_key_preview, "is_current": endpoint_id == current_provider, "source": "providers", }) if current_provider.lower() == "custom" and current_base_url and not any(e["id"] == "custom" for e in endpoints): + has_api_key, api_key_preview = _api_key_display(model_cfg) endpoints.insert(0, { "id": "custom", "name": "Custom", @@ -7625,8 +7660,8 @@ def _custom_endpoint_response(cfg: Dict[str, Any]) -> Dict[str, Any]: "models": [current_model] if current_model else [], "context_length": model_cfg.get("context_length"), "discover_models": True, - "has_api_key": bool(str(model_cfg.get("api_key", "") or "").strip()), - "api_key_preview": redact_key(str(model_cfg.get("api_key", "") or "")) if model_cfg.get("api_key") else None, + "has_api_key": has_api_key, + "api_key_preview": api_key_preview, "is_current": True, "source": "direct-config", }) @@ -7659,7 +7694,7 @@ def _detach_main_model_from_provider(cfg: Dict[str, Any], provider_key: str) -> return if str(model_cfg.get("provider") or "").strip().lower() != provider_key: return - for field in ("provider", "base_url", "api_key"): + for field in ("provider", "base_url", "api_key", "key_env"): model_cfg.pop(field, None) cfg["model"] = model_cfg @@ -7719,8 +7754,29 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T if body.context_length and body.context_length > 0: entry["context_length"] = int(body.context_length) entry["models"][model]["context_length"] = int(body.context_length) - if body.api_key is not None and body.api_key.strip(): - entry["api_key"] = body.api_key.strip() + + # API keys never belong in config.yaml (#69449). Write to .env and + # reference it via ``key_env`` — the same indirection built-in providers + # use and that runtime_provider.py already resolves at load time. + env_var = custom_endpoint_key_env(endpoint_id) + submitted_key = body.api_key.strip() if body.api_key is not None else None + if submitted_key: + save_env_value(env_var, submitted_key) + entry["key_env"] = env_var + entry.pop("api_key", None) + elif submitted_key is not None: + # Blank field means "clear the key", not "leave it alone". + remove_env_value(env_var) + entry.pop("key_env", None) + entry.pop("api_key", None) + elif str(entry.get("api_key") or "").strip() and not _config_api_key_is_env_ref(endpoint_id): + # No new key submitted, but this entry still carries one an earlier + # release wrote in plaintext. Migrate it on the next save so endpoints + # configured before the fix get cleaned up too, without the user + # having to re-enter the key. + save_env_value(env_var, entry["api_key"].strip()) + entry["key_env"] = env_var + entry.pop("api_key", None) providers[endpoint_id] = entry cfg["providers"] = providers @@ -7729,8 +7785,9 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T cfg["model"] = _apply_main_model_assignment( cfg.get("model", {}), endpoint_id, model, base_url ) - if entry.get("api_key") and isinstance(cfg["model"], dict): - cfg["model"]["api_key"] = entry["api_key"] + if entry.get("key_env") and isinstance(cfg["model"], dict): + cfg["model"]["key_env"] = entry["key_env"] + cfg["model"].pop("api_key", None) return endpoint_id, entry @@ -7781,7 +7838,10 @@ def activate_custom_endpoint(endpoint_id: str): raise HTTPException(status_code=400, detail="custom endpoint is incomplete") model_cfg = _apply_main_model_assignment(cfg.get("model", {}), provider_key, model, base_url) - if entry.get("api_key"): + if entry.get("key_env"): + model_cfg["key_env"] = entry["key_env"] + model_cfg.pop("api_key", None) + elif entry.get("api_key"): model_cfg["api_key"] = entry["api_key"] cfg["model"] = model_cfg save_config(cfg) @@ -7805,6 +7865,7 @@ def delete_custom_endpoint(endpoint_id: str): providers.pop(provider_key, None) cfg["providers"] = providers _detach_main_model_from_provider(cfg, provider_key) + remove_env_value(custom_endpoint_key_env(provider_key)) save_config(cfg) response = _custom_endpoint_response(cfg) response["ok"] = True From 8a2925f794466a5d4487a1438b1b6c700b88a1d6 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 24 Jul 2026 21:12:53 -0500 Subject: [PATCH 4/5] fix(cli): store custom endpoint API key in .env instead of config.yaml hermes model's custom-endpoint flow is the other write path that produced a plaintext key, on both the model block and the custom_providers entry. Route it through the same .env indirection as the Desktop panel, and swap an existing entry's inline key for the reference when the URL is re-saved. Co-authored-by: liuhao1024 --- hermes_cli/main.py | 14 ++++++++++++-- hermes_cli/model_setup_flows.py | 30 +++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index 32292368fe056..1e2aa7b921b0e 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -3975,13 +3975,17 @@ def _custom_provider_base_url_config_value(provider_info, resolved_base_url=""): def _save_custom_provider( - base_url, api_key="", model="", context_length=None, name=None, api_mode=None + base_url, api_key="", model="", context_length=None, name=None, api_mode=None, + key_env="" ): """Save a custom endpoint to custom_providers in config.yaml. Deduplicates by base_url — if the URL already exists, updates the model name, context_length, and api_mode but doesn't add a duplicate entry. Uses *name* when provided, otherwise auto-generates from the URL. + + When *key_env* is set the caller has already written the key to ``.env``, + so the entry references it instead of inlining the secret (#69449). """ from hermes_cli.config import load_config, save_config @@ -4013,6 +4017,10 @@ def _save_custom_provider( elif "api_mode" in entry: entry.pop("api_mode", None) changed = True + if key_env and (entry.get("key_env") != key_env or entry.get("api_key")): + entry["key_env"] = key_env + entry.pop("api_key", None) + changed = True if changed: cfg["custom_providers"] = providers save_config(cfg) @@ -4023,7 +4031,9 @@ def _save_custom_provider( name = _auto_provider_name(base_url) entry = {"name": name, "base_url": base_url} - if api_key: + if key_env: + entry["key_env"] = key_env + elif api_key: entry["api_key"] = api_key if model: entry["model"] = model diff --git a/hermes_cli/model_setup_flows.py b/hermes_cli/model_setup_flows.py index 394360c160737..90f628578a785 100644 --- a/hermes_cli/model_setup_flows.py +++ b/hermes_cli/model_setup_flows.py @@ -23,6 +23,7 @@ from __future__ import annotations import argparse import os import subprocess +import urllib.parse from hermes_cli.config import clear_model_endpoint_credentials @@ -833,7 +834,13 @@ def _model_flow_custom(config): """ from hermes_cli.main import _auto_provider_name, _prompt_custom_api_mode_selection, _save_custom_provider from hermes_cli.auth import _save_model_choice, deactivate_provider - from hermes_cli.config import get_env_value, load_config, save_config + from hermes_cli.config import ( + custom_endpoint_key_env, + get_env_value, + load_config, + save_config, + save_env_value, + ) from hermes_cli.secret_prompt import masked_secret_prompt current_url = get_env_value("OPENAI_BASE_URL") or "" @@ -988,6 +995,18 @@ def _model_flow_custom(config): print(f"Invalid context length: {context_length_str} — will auto-detect.") context_length = None + # The key goes to .env and config.yaml only references it (#69449). Keyed + # on host:port so two servers on one machine keep separate credentials. + custom_key_env = "" + if effective_key: + _parsed = urllib.parse.urlparse(effective_url) + _identity = _parsed.hostname or "" + if _parsed.port: + _identity = f"{_identity}_{_parsed.port}" + custom_key_env = custom_endpoint_key_env(_identity) + save_env_value(custom_key_env, effective_key) + print(f" API key saved to .env as {custom_key_env}") + if model_name: _save_model_choice(model_name) @@ -999,8 +1018,8 @@ def _model_flow_custom(config): cfg["model"] = model model["provider"] = "custom" model["base_url"] = effective_url - if effective_key: - model["api_key"] = effective_key + if custom_key_env: + model["api_key"] = f"${{{custom_key_env}}}" if api_mode: model["api_mode"] = api_mode else: @@ -1025,8 +1044,8 @@ def _model_flow_custom(config): _caller_model = {"default": _caller_model} if _caller_model else {} _caller_model["provider"] = "custom" _caller_model["base_url"] = effective_url - if effective_key: - _caller_model["api_key"] = effective_key + if custom_key_env: + _caller_model["api_key"] = f"${{{custom_key_env}}}" if api_mode: _caller_model["api_mode"] = api_mode else: @@ -1042,6 +1061,7 @@ def _model_flow_custom(config): context_length=context_length, name=display_name, api_mode=api_mode, + key_env=custom_key_env, ) _prune_replaced_custom_model_config_credentials( effective_url, From 15f29d0b6fd385507d8feae37e4d76b2598ed8a5 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 24 Jul 2026 21:12:59 -0500 Subject: [PATCH 5/5] test: cover custom endpoint key storage and model-list persistence Bug-class coverage for both fixes: the full catalogue survives Save, context lengths are preserved, the key never lands in config.yaml on either write path, blank clears it, a pre-fix plaintext key migrates while a ${VAR} template is left alone, two endpoints on one host keep separate credentials, and an IP-derived name is still a valid POSIX env var. The two delete tests asserted on the plaintext mirror; they now assert the same invariants against the credential reference. --- tests/cli/test_cli_provider_resolution.py | 92 ++++++- tests/hermes_cli/test_web_server.py | 301 +++++++++++++++++++++- 2 files changed, 380 insertions(+), 13 deletions(-) diff --git a/tests/cli/test_cli_provider_resolution.py b/tests/cli/test_cli_provider_resolution.py index 2a04e2b8d252c..bf54224dd2d52 100644 --- a/tests/cli/test_cli_provider_resolution.py +++ b/tests/cli/test_cli_provider_resolution.py @@ -793,11 +793,16 @@ def test_model_flow_custom_persists_selected_api_mode(monkeypatch): "used_fallback": False, }, ) + saved_env = {} monkeypatch.setattr("hermes_cli.config.load_config", lambda: saved_cfg) monkeypatch.setattr("hermes_cli.config.save_config", lambda cfg: saved_cfg.update(cfg)) + monkeypatch.setattr( + "hermes_cli.config.save_env_value", + lambda key, value: saved_env.__setitem__(key, value), + ) monkeypatch.setattr( "hermes_cli.main._save_custom_provider", - lambda base_url, api_key="", model="", context_length=None, name=None, api_mode=None: captured_provider.update( + lambda base_url, api_key="", model="", context_length=None, name=None, api_mode=None, key_env="": captured_provider.update( { "base_url": base_url, "api_key": api_key, @@ -805,6 +810,7 @@ def test_model_flow_custom_persists_selected_api_mode(monkeypatch): "context_length": context_length, "name": name, "api_mode": api_mode, + "key_env": key_env, } ), ) @@ -825,10 +831,14 @@ def test_model_flow_custom_persists_selected_api_mode(monkeypatch): assert saved_cfg["model"]["provider"] == "custom" assert saved_cfg["model"]["base_url"] == "https://codex.example.com/v1" - assert saved_cfg["model"]["api_key"] == "test-key" assert saved_cfg["model"]["api_mode"] == "codex_responses" assert captured_provider["api_mode"] == "codex_responses" + # The key itself goes to .env; config.yaml only references it (#69449). + key_env = captured_provider["key_env"] + assert saved_cfg["model"]["api_key"] == f"${{{key_env}}}" + assert saved_env[key_env] == "test-key" + def test_cmd_model_forwards_nous_login_tls_options(monkeypatch): monkeypatch.setattr(hermes_main, "_require_tty", lambda *a: None) @@ -923,3 +933,81 @@ def test_save_custom_provider_uses_provided_name(monkeypatch, tmp_path): entries = saved.get("custom_providers", []) assert len(entries) == 1 assert entries[0]["name"] == "Ollama" + + +def test_save_custom_provider_references_the_key_instead_of_inlining_it(monkeypatch, tmp_path): + """With key_env set the entry must not carry the secret (#69449).""" + import yaml + from hermes_cli.main import _save_custom_provider + + cfg_path = tmp_path / "config.yaml" + cfg_path.write_text(yaml.dump({})) + monkeypatch.setattr( + "hermes_cli.config.load_config", lambda: yaml.safe_load(cfg_path.read_text()) or {}, + ) + saved = {} + monkeypatch.setattr("hermes_cli.config.save_config", lambda cfg: saved.update(cfg)) + + _save_custom_provider( + "http://localhost:11434/v1", + api_key="sk-secret", + name="Ollama", + key_env="HERMES_CUSTOM_LOCALHOST_11434_API_KEY", + ) + + entry = saved["custom_providers"][0] + assert entry["key_env"] == "HERMES_CUSTOM_LOCALHOST_11434_API_KEY" + assert "api_key" not in entry + assert "sk-secret" not in yaml.safe_dump(saved) + + +def test_save_custom_provider_migrates_an_existing_plaintext_entry(monkeypatch, tmp_path): + """Re-saving a known URL swaps its inline key for the .env reference.""" + import yaml + from hermes_cli.main import _save_custom_provider + + existing = { + "custom_providers": [ + { + "name": "Ollama", + "base_url": "http://localhost:11434/v1", + "api_key": "sk-legacy", + } + ] + } + monkeypatch.setattr("hermes_cli.config.load_config", lambda: existing) + saved = {} + monkeypatch.setattr("hermes_cli.config.save_config", lambda cfg: saved.update(cfg)) + + _save_custom_provider( + "http://localhost:11434/v1", + key_env="HERMES_CUSTOM_LOCALHOST_11434_API_KEY", + ) + + entry = saved["custom_providers"][0] + assert entry["key_env"] == "HERMES_CUSTOM_LOCALHOST_11434_API_KEY" + assert "api_key" not in entry + + +def test_custom_endpoint_key_env_is_a_valid_posix_name_for_ip_endpoints(): + """Every IP-based local endpoint slugs to a digit-leading name. + + ``save_env_value`` rejects names that don't match + ``[A-Za-z_][A-Za-z0-9_]*``, so deriving ``127_0_0_1_8080_API_KEY`` would + raise on exactly the local-proxy setups this is meant to protect. The + fixed prefix makes the result valid by construction. + """ + import re + + from hermes_cli.config import _ENV_VAR_NAME_RE, custom_endpoint_key_env + + for identity in ("127.0.0.1_8080", "0.0.0.0", "10.0.0.7:11434", "", "-–-"): + assert _ENV_VAR_NAME_RE.match(custom_endpoint_key_env(identity)), identity + + +def test_custom_endpoint_key_env_separates_ports_on_one_host(): + """Two servers on one machine must not collapse onto one .env slot.""" + from hermes_cli.config import custom_endpoint_key_env + + assert custom_endpoint_key_env("127.0.0.1_8000") != custom_endpoint_key_env("127.0.0.1_8001") + assert custom_endpoint_key_env("acme") == custom_endpoint_key_env("ACME") diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index 7b22dbf1318f4..277cb56b5cf3a 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -4664,15 +4664,15 @@ class TestWebServerEndpoints: assert models["acme/model-1"]["context_length"] == 200000 def test_deleting_the_active_custom_endpoint_clears_its_model_mirror(self): - """Deleting an endpoint must not leave its key running the agent. + """Deleting an endpoint must not leave its credential running the agent. - ``activate`` copies the endpoint's base_url + api_key onto ``model``, - and ``model.api_key`` outranks the environment at client construction - (#62269). Without clearing that mirror the agent keeps authenticating - to the deleted host with the deleted key, and the key the operator - just removed through the dashboard stays in config.yaml. + ``activate`` mirrors the endpoint's base_url + credential reference + onto ``model``, and that mirror outranks the environment at client + construction (#62269). Without clearing it the agent keeps + authenticating to the deleted host, and the credential the operator + just removed through the dashboard survives the delete. """ - from hermes_cli.config import load_config + from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config self.client.post( "/api/providers/custom-endpoints", @@ -4688,8 +4688,10 @@ class TestWebServerEndpoints: "/api/providers/custom-endpoints/acme/activate", json={} ).status_code == 200 + env_var = custom_endpoint_key_env("acme") cfg = load_config() - assert cfg["model"]["api_key"] == "sk-acme-secret" + assert cfg["model"]["key_env"] == env_var + assert get_env_value(env_var) == "sk-acme-secret" assert self.client.request( "DELETE", "/api/providers/custom-endpoints/acme" @@ -4699,12 +4701,14 @@ class TestWebServerEndpoints: assert "acme" not in (cfg.get("providers") or {}) model_cfg = cfg.get("model") or {} assert not model_cfg.get("api_key"), "deleted endpoint's key still in config.yaml" + assert not model_cfg.get("key_env"), "deleted endpoint's key ref still in config.yaml" assert not model_cfg.get("base_url"), "deleted endpoint's host still routed to" assert not model_cfg.get("provider") + assert not get_env_value(env_var), "deleted endpoint's key still in .env" def test_deleting_an_inactive_custom_endpoint_leaves_the_active_one_alone(self): - """Only the mirror of the DELETED provider is scrubbed.""" - from hermes_cli.config import load_config + """Only the DELETED provider's mirror and .env slot are scrubbed.""" + from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config for name, key in (("acme", "sk-acme"), ("other", "sk-other")): self.client.post( @@ -4723,8 +4727,283 @@ class TestWebServerEndpoints: model_cfg = load_config().get("model") or {} assert model_cfg.get("provider") == "other" - assert model_cfg.get("api_key") == "sk-other" + assert model_cfg.get("key_env") == custom_endpoint_key_env("other") assert model_cfg.get("base_url") == "https://llm.other.corp/v1" + assert get_env_value(custom_endpoint_key_env("other")) == "sk-other" + + def test_custom_endpoint_save_persists_the_whole_discovered_catalogue(self): + """Test discovers N models; Save must keep all N (#69988). + + Every downstream picker reads ``providers..models`` straight from + config.yaml with no live probe, so persisting only the one hand-typed + model left a provider serving dozens showing a single-entry list. + """ + from hermes_cli.config import load_config + + discovered = ["glm-5.2", "qwen3-max", "llama-4-405b", "deepseek-v4"] + resp = self.client.post( + "/api/providers/custom-endpoints", + json={ + "id": "proxy", + "name": "Proxy", + "base_url": "http://127.0.0.1:8000/v1", + "model": "glm-5.2", + "models": discovered, + }, + ) + + assert resp.status_code == 200 + assert sorted(load_config()["providers"]["proxy"]["models"]) == sorted(discovered) + endpoint = next(e for e in resp.json()["endpoints"] if e["id"] == "proxy") + assert sorted(endpoint["models"]) == sorted(discovered) + + def test_custom_endpoint_save_with_catalogue_keeps_known_context_lengths(self): + """A discovered list merges onto the entry; it doesn't reset it.""" + from hermes_cli.config import load_config, save_config + + cfg = load_config() + cfg["providers"] = { + "proxy": { + "name": "Proxy", + "base_url": "http://127.0.0.1:8000/v1", + "model": "a", + "models": {"a": {"context_length": 200000}}, + } + } + save_config(cfg) + + self.client.post( + "/api/providers/custom-endpoints", + json={ + "id": "proxy", + "name": "Proxy", + "base_url": "http://127.0.0.1:8000/v1", + "model": "a", + "models": ["a", "b"], + }, + ) + + models = load_config()["providers"]["proxy"]["models"] + assert sorted(models) == ["a", "b"] + assert models["a"]["context_length"] == 200000 + + def test_custom_endpoint_save_keeps_the_api_key_out_of_config(self): + """The key belongs in .env behind key_env, never in config.yaml (#69449).""" + from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config + + self.client.post( + "/api/providers/custom-endpoints", + json={ + "id": "proxy", + "name": "Proxy", + "base_url": "https://llm.example.com/v1", + "model": "m", + "api_key": "sk-super-secret", + "make_default": True, + }, + ) + + cfg = load_config() + entry = cfg["providers"]["proxy"] + env_var = custom_endpoint_key_env("proxy") + assert entry["key_env"] == env_var + assert "api_key" not in entry + assert "api_key" not in cfg["model"] + assert get_env_value(env_var) == "sk-super-secret" + assert "sk-super-secret" not in yaml.safe_dump(cfg) + + def test_custom_endpoint_edit_without_a_key_keeps_the_stored_one(self): + """The panel sends no api_key on an unrelated edit (the field is blank).""" + from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config + + common = { + "id": "proxy", + "name": "Proxy", + "base_url": "https://llm.example.com/v1", + } + self.client.post( + "/api/providers/custom-endpoints", + json={**common, "model": "m1", "api_key": "sk-keep-me"}, + ) + self.client.post("/api/providers/custom-endpoints", json={**common, "model": "m2"}) + + entry = load_config()["providers"]["proxy"] + assert entry["model"] == "m2" + assert entry["key_env"] == custom_endpoint_key_env("proxy") + assert get_env_value(custom_endpoint_key_env("proxy")) == "sk-keep-me" + + def test_custom_endpoint_save_migrates_a_legacy_plaintext_key(self): + """Entries written before #69449 get cleaned up on their next save. + + Requiring the user to re-type the key to get it out of config.yaml + would leave the plaintext sitting there for anyone who never edits the + endpoint again. + """ + from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config, save_config + + cfg = load_config() + cfg["providers"] = { + "proxy": { + "name": "Proxy", + "base_url": "https://llm.example.com/v1", + "model": "m", + "api_key": "sk-legacy-plaintext", + "models": {"m": {}}, + } + } + save_config(cfg) + + self.client.post( + "/api/providers/custom-endpoints", + json={ + "id": "proxy", + "name": "Proxy", + "base_url": "https://llm.example.com/v1", + "model": "m", + }, + ) + + entry = load_config()["providers"]["proxy"] + assert "api_key" not in entry + assert entry["key_env"] == custom_endpoint_key_env("proxy") + assert get_env_value(custom_endpoint_key_env("proxy")) == "sk-legacy-plaintext" + + def test_custom_endpoint_save_leaves_a_hand_written_env_ref_alone(self, monkeypatch): + """``api_key: ${MY_KEY}`` is already safe — don't copy it elsewhere. + + load_config() expands env refs, so such an entry looks like a literal + secret by the time Save sees it. Migrating it would duplicate the + user's secret into a second env var they never asked for. + """ + import yaml + + from hermes_cli.config import custom_endpoint_key_env, get_config_path, get_env_value + + monkeypatch.setenv("MY_PROXY_KEY", "sk-user-managed") + get_config_path().write_text( + yaml.safe_dump({ + "providers": { + "proxy": { + "name": "Proxy", + "base_url": "https://llm.example.com/v1", + "model": "m", + "api_key": "${MY_PROXY_KEY}", + } + }, + }), + encoding="utf-8", + ) + + self.client.post( + "/api/providers/custom-endpoints", + json={ + "id": "proxy", + "name": "Proxy", + "base_url": "https://llm.example.com/v1", + "model": "m", + }, + ) + + raw = yaml.safe_load(get_config_path().read_text(encoding="utf-8")) + assert raw["providers"]["proxy"]["api_key"] == "${MY_PROXY_KEY}" + assert not get_env_value(custom_endpoint_key_env("proxy")) + + def test_custom_endpoint_blank_api_key_clears_the_credential(self): + """An explicitly emptied field means "remove the key", not "keep it".""" + from hermes_cli.config import custom_endpoint_key_env, get_env_value, load_config + + common = { + "id": "proxy", + "name": "Proxy", + "base_url": "https://llm.example.com/v1", + "model": "m", + } + self.client.post( + "/api/providers/custom-endpoints", json={**common, "api_key": "sk-drop-me"} + ) + self.client.post("/api/providers/custom-endpoints", json={**common, "api_key": ""}) + + entry = load_config()["providers"]["proxy"] + assert "api_key" not in entry + assert "key_env" not in entry + assert not get_env_value(custom_endpoint_key_env("proxy")) + + def test_two_endpoints_on_one_host_keep_separate_credentials(self): + """Two local servers must not share an .env slot. + + Deriving the env var from the hostname collapses ``127.0.0.1:8000`` + and ``:8001`` onto one name, so saving the second silently overwrites + the first's key. + """ + from hermes_cli.config import custom_endpoint_key_env, get_env_value + + for port, key in ((8000, "sk-first"), (8001, "sk-second")): + self.client.post( + "/api/providers/custom-endpoints", + json={ + "id": f"local-{port}", + "name": f"Local {port}", + "base_url": f"http://127.0.0.1:{port}/v1", + "model": "m", + "api_key": key, + }, + ) + + assert get_env_value(custom_endpoint_key_env("local-8000")) == "sk-first" + assert get_env_value(custom_endpoint_key_env("local-8001")) == "sk-second" + + def test_custom_endpoint_response_reports_a_key_held_in_env(self): + """has_api_key must follow key_env, not just a plaintext api_key. + + Reading only ``api_key`` made the panel report "no API key" for every + endpoint whose credential had been moved to .env. + """ + resp = self.client.post( + "/api/providers/custom-endpoints", + json={ + "id": "proxy", + "name": "Proxy", + "base_url": "https://llm.example.com/v1", + "model": "m", + "api_key": "sk-in-env", + }, + ) + + endpoint = next(e for e in resp.json()["endpoints"] if e["id"] == "proxy") + assert endpoint["has_api_key"] is True + assert "sk-in-env" not in (endpoint["api_key_preview"] or "") + + def test_activating_an_endpoint_carries_its_credential_either_way(self): + """Activate must work for both key_env and pre-#69449 plaintext entries.""" + from hermes_cli.config import load_config, save_config + + cfg = load_config() + cfg["providers"] = { + "legacy": { + "name": "Legacy", + "base_url": "https://llm.legacy.com/v1", + "model": "m", + "api_key": "sk-legacy", + "models": {"m": {}}, + }, + "modern": { + "name": "Modern", + "base_url": "https://llm.modern.com/v1", + "model": "m", + "key_env": "MODERN_API_KEY", + "models": {"m": {}}, + }, + } + save_config(cfg) + + self.client.post("/api/providers/custom-endpoints/modern/activate", json={}) + model_cfg = load_config()["model"] + assert model_cfg["key_env"] == "MODERN_API_KEY" + assert "api_key" not in model_cfg + + self.client.post("/api/providers/custom-endpoints/legacy/activate", json={}) + model_cfg = load_config()["model"] + assert model_cfg["api_key"] == "sk-legacy" def test_set_model_main_preserves_base_url_for_named_custom_provider(self): """Selecting a named custom endpoint from the Desktop model picker