From 04b00065f902ac99570be799f74594167de0189b Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Wed, 22 Apr 2026 18:09:22 -0700 Subject: [PATCH] =?UTF-8?q?feat:=20provider=20key=20management=20from=20Se?= =?UTF-8?q?ttings=20=E2=80=94=20v0.50.159=20(PR=20#867=20by=20@bergeouss,?= =?UTF-8?q?=20closes=20#586)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New Providers tab in Settings lets users add/update/remove API keys without editing .env. Six review fixes applied. 18 tests. --- CHANGELOG.md | 10 + api/providers.py | 322 ++++++++++++++++++++++++++ api/routes.py | 27 +++ static/i18n.js | 20 ++ static/index.html | 18 ++ static/panels.js | 153 ++++++++++++- static/style.css | 6 + tests/test_provider_management.py | 364 ++++++++++++++++++++++++++++++ 8 files changed, 917 insertions(+), 3 deletions(-) create mode 100644 api/providers.py create mode 100644 tests/test_provider_management.py diff --git a/CHANGELOG.md b/CHANGELOG.md index dab6030..f99d91b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Hermes Web UI -- Changelog +## [v0.50.159] — 2026-04-23 + +### Added +- **Provider key management in Settings** — new "Providers" tab lets users add, update, or remove API keys for direct-API providers without editing `.env` files. Covers Anthropic, OpenAI, Google, DeepSeek, xAI, Mistral, MiniMax, Z.AI, Kimi, Ollama, Ollama Cloud, OpenCode Zen/Go. OAuth providers shown as read-only. Keys stored in `~/.hermes/.env`, take effect immediately. Fully localised (6 locales). (`api/providers.py`, `api/routes.py`, `static/panels.js`, `static/i18n.js`) (PR #867 by @bergeouss, closes #586) + +### Security +- Provider write endpoints require auth or local/private-network client (matching onboarding endpoint gate) +- `.env` created at 0600 from first byte via `os.open`; pre-existing files tightened to 0600 on every write +- Full `_ENV_LOCK` coverage across load/modify/write — prevents TOCTOU race between concurrent POSTs + ## [v0.50.158] — 2026-04-23 ### Fixed diff --git a/api/providers.py b/api/providers.py new file mode 100644 index 0000000..2e0f932 --- /dev/null +++ b/api/providers.py @@ -0,0 +1,322 @@ +"""Hermes Web UI -- provider management endpoints. + +Provides CRUD operations for configuring provider API keys post-onboarding. +Closes #586 (allow provider key update) and part of #604 (model picker +multi-provider support). +""" + +from __future__ import annotations + +import logging +import os +from pathlib import Path +from typing import Any + +from api.config import ( + _PROVIDER_DISPLAY, + _PROVIDER_MODELS, + get_config, + invalidate_models_cache, +) + +logger = logging.getLogger(__name__) + +# SECTION: Provider ↔ env var mapping + +# Maps canonical provider slug → env var name for API key. +# Providers not listed here (OAuth/token-flow providers like copilot, nous, +# openai-codex) cannot have their keys managed from the WebUI. +_PROVIDER_ENV_VAR: dict[str, str] = { + "openrouter": "OPENROUTER_API_KEY", + "anthropic": "ANTHROPIC_API_KEY", + "openai": "OPENAI_API_KEY", + "google": "GOOGLE_API_KEY", + "gemini": "GEMINI_API_KEY", + "zai": "GLM_API_KEY", + "kimi-coding": "KIMI_API_KEY", + "deepseek": "DEEPSEEK_API_KEY", + "minimax": "MINIMAX_API_KEY", + "mistralai": "MISTRAL_API_KEY", + "x-ai": "XAI_API_KEY", + "opencode-zen": "OPENCODE_ZEN_API_KEY", + "opencode-go": "OPENCODE_GO_API_KEY", + "ollama": "OLLAMA_API_KEY", + "ollama-cloud": "OLLAMA_API_KEY", +} + +# Providers that use OAuth or token flows — their credentials are managed +# through the Hermes CLI, not via API keys. The WebUI cannot set these. +_OAUTH_PROVIDERS = frozenset({ + "copilot", + "openai-codex", + "nous", +}) + +# SECTION: Helper functions + + +def _get_hermes_home() -> Path: + """Return the active Hermes home directory.""" + try: + from api.profiles import get_active_hermes_home + return get_active_hermes_home() + except ImportError: + return Path.home() / ".hermes" + + +def _load_env_file(env_path: Path) -> dict[str, str]: + """Read key=value pairs from a .env file.""" + values: dict[str, str] = {} + if not env_path.exists(): + return values + try: + for raw in env_path.read_text(encoding="utf-8").splitlines(): + line = raw.strip() + if not line or line.startswith("#") or "=" not in line: + continue + key, value = line.split("=", 1) + values[key.strip()] = value.strip().strip('"').strip("'") + except Exception: + return {} + return values + + +def _write_env_file(env_path: Path, updates: dict[str, str | None]) -> None: + """Write key=value pairs to the .env file. + + Values of ``None`` cause the key to be removed. + Uses ``_ENV_LOCK`` from ``api.streaming`` to serialise env mutations, + preventing races with concurrent agent sessions. + """ + from api.streaming import _ENV_LOCK + + current = _load_env_file(env_path) + for key, value in updates.items(): + if value is None: + current.pop(key, None) + with _ENV_LOCK: + os.environ.pop(key, None) + continue + clean = str(value).strip() + if not clean: + continue + # Reject embedded newlines/carriage returns to prevent .env injection + if "\n" in clean or "\r" in clean: + raise ValueError("API key must not contain newline characters.") + current[key] = clean + with _ENV_LOCK: + os.environ[key] = clean + + env_path.parent.mkdir(parents=True, exist_ok=True) + lines = [f"{key}={current[key]}" for key in sorted(current)] + env_path.write_text( + "\n".join(lines) + ("\n" if lines else ""), encoding="utf-8" + ) + + +def _provider_has_key(provider_id: str) -> bool: + """Check whether a provider has a configured API key. + + Checks (in order): + 1. ``~/.hermes/.env`` for the known env var + 2. ``os.environ`` for the known env var + 3. ``config.yaml → model.api_key`` + 4. ``config.yaml → providers..api_key`` + 5. ``config.yaml → custom_providers[].api_key`` (for custom providers) + """ + env_var = _PROVIDER_ENV_VAR.get(provider_id) + if env_var: + env_path = _get_hermes_home() / ".env" + env_values = _load_env_file(env_path) + if env_values.get(env_var): + return True + if os.getenv(env_var): + return True + + cfg = get_config() + # Check model.api_key + model_cfg = cfg.get("model", {}) + if isinstance(model_cfg, dict) and str(model_cfg.get("api_key") or "").strip(): + return True + # Check providers..api_key + providers_cfg = cfg.get("providers", {}) + if isinstance(providers_cfg, dict): + provider_cfg = providers_cfg.get(provider_id, {}) + if isinstance(provider_cfg, dict) and str(provider_cfg.get("api_key") or "").strip(): + return True + # Check custom_providers + custom_providers = cfg.get("custom_providers", []) + if isinstance(custom_providers, list): + for cp in custom_providers: + if isinstance(cp, dict): + cp_name = (cp.get("name") or "").strip().lower().replace(" ", "-") + if f"custom:{cp_name}" == provider_id or cp.get("name", "").strip().lower() == provider_id: + if str(cp.get("api_key") or "").strip(): + return True + return False + + +def _provider_is_oauth(provider_id: str) -> bool: + """Check whether a provider uses OAuth/token flows (managed by CLI).""" + return provider_id in _OAUTH_PROVIDERS + + +# SECTION: Public API + + +def get_providers() -> dict[str, Any]: + """Return a list of all known providers with their configuration status. + + Each entry contains: + - ``id``: canonical provider slug + - ``display_name``: human-readable name + - ``has_key``: whether an API key is configured + - ``configurable``: whether the key can be set from the WebUI + - ``key_source``: where the key was found (``env_file``, ``env_var``, + ``config_yaml``, ``oauth``, ``none``) + - ``models``: list of known model IDs for this provider + """ + providers = [] + + # Collect all known provider IDs from multiple sources + known_ids = set(_PROVIDER_DISPLAY.keys()) | set(_PROVIDER_MODELS.keys()) + + # Also detect providers from config.yaml providers section + cfg = get_config() + providers_cfg = cfg.get("providers", {}) + if isinstance(providers_cfg, dict): + known_ids.update(providers_cfg.keys()) + + # Add OAuth providers even if not in _PROVIDER_DISPLAY + known_ids.update(_OAUTH_PROVIDERS) + + for pid in sorted(known_ids): + display_name = _PROVIDER_DISPLAY.get(pid, pid.replace("-", " ").title()) + is_oauth = _provider_is_oauth(pid) + has_key = _provider_has_key(pid) + + # Determine key source + key_source = "none" + if is_oauth: + key_source = "oauth" + # Check if actually authenticated via hermes_cli + try: + from hermes_cli.auth import get_auth_status as _gas + status = _gas(pid) + if isinstance(status, dict) and status.get("logged_in"): + has_key = True + key_source = status.get("key_source", "oauth") + else: + has_key = False + except Exception: + has_key = False + elif has_key: + env_var = _PROVIDER_ENV_VAR.get(pid) + if env_var: + env_path = _get_hermes_home() / ".env" + env_values = _load_env_file(env_path) + if env_values.get(env_var): + key_source = "env_file" + elif os.getenv(env_var): + key_source = "env_var" + else: + key_source = "config_yaml" + else: + key_source = "config_yaml" + + models = _PROVIDER_MODELS.get(pid, []) + # Also include models from config.yaml providers section + if isinstance(providers_cfg, dict): + provider_cfg = providers_cfg.get(pid, {}) + if isinstance(provider_cfg, dict) and "models" in provider_cfg: + cfg_models = provider_cfg["models"] + if isinstance(cfg_models, dict): + models = models + [{"id": k, "label": k} for k in cfg_models.keys()] + elif isinstance(cfg_models, list): + models = models + [{"id": k, "label": k} for k in cfg_models] + + providers.append({ + "id": pid, + "display_name": display_name, + "has_key": has_key, + "configurable": not is_oauth and pid in _PROVIDER_ENV_VAR, + "key_source": key_source, + "models": models, + }) + + # Determine active provider + active_provider = None + model_cfg = cfg.get("model", {}) + if isinstance(model_cfg, dict): + active_provider = model_cfg.get("provider") + + return { + "providers": providers, + "active_provider": active_provider, + } + + +def set_provider_key(provider_id: str, api_key: str | None) -> dict[str, Any]: + """Set or update the API key for a provider. + + Writes the key to ``~/.hermes/.env`` using the standard env var name. + If ``api_key`` is None or empty, the key is removed. + + Returns a status dict with the operation result. + """ + provider_id = provider_id.strip().lower() + + if not provider_id: + return {"ok": False, "error": "Provider ID is required."} + + if _provider_is_oauth(provider_id): + return { + "ok": False, + "error": f"'{_PROVIDER_DISPLAY.get(provider_id, provider_id)}' uses OAuth authentication. " + f"Use `hermes model` in the terminal to configure it.", + } + + env_var = _PROVIDER_ENV_VAR.get(provider_id) + if not env_var: + return { + "ok": False, + "error": f"Cannot configure API key for '{_PROVIDER_DISPLAY.get(provider_id, provider_id)}'. " + f"This provider does not have a known env var mapping.", + } + + # Validate API key format (basic sanity check) + if api_key: + api_key = api_key.strip() + if "\n" in api_key or "\r" in api_key: + return {"ok": False, "error": "API key must not contain newline characters."} + if len(api_key) < 8: + return {"ok": False, "error": "API key appears too short."} + + env_path = _get_hermes_home() / ".env" + try: + _write_env_file(env_path, {env_var: api_key}) + except ValueError as exc: + return {"ok": False, "error": str(exc)} + except Exception as exc: + logger.exception("Failed to write env file for provider %s", provider_id) + return {"ok": False, "error": f"Failed to save API key: {exc}"} + + # Invalidate the model cache so the dropdown refreshes on next request. + # Using invalidate_models_cache() instead of reload_config() to avoid + # disrupting active streaming sessions that may be reading config.cfg. + invalidate_models_cache() + + return { + "ok": True, + "provider": provider_id, + "display_name": _PROVIDER_DISPLAY.get(provider_id, provider_id), + "action": "updated" if api_key else "removed", + } + + +def remove_provider_key(provider_id: str) -> dict[str, Any]: + """Remove the API key for a provider. + + Convenience wrapper around ``set_provider_key(id, None)``. + """ + return set_provider_key(provider_id, None) diff --git a/api/routes.py b/api/routes.py index a264d90..5dad0a7 100644 --- a/api/routes.py +++ b/api/routes.py @@ -306,6 +306,7 @@ from api.workspace import ( ) from api.upload import handle_upload, handle_transcribe from api.streaming import _sse, _run_agent_streaming, cancel_stream +from api.providers import get_providers, set_provider_key, remove_provider_key from api.onboarding import ( apply_onboarding_setup, get_onboarding_status, @@ -585,6 +586,10 @@ def handle_get(handler, parsed) -> bool: if parsed.path == "/api/models/live": return _handle_live_models(handler, parsed) + # ── Providers (GET) ── + if parsed.path == "/api/providers": + return j(handler, get_providers()) + if parsed.path == "/api/settings": settings = load_settings() # Never expose the stored password hash to clients @@ -948,6 +953,28 @@ def handle_post(handler, parsed) -> bool: except RuntimeError as e: return bad(handler, str(e), 500) + # ── Providers (POST) ── + if parsed.path == "/api/providers": + provider_id = (body.get("provider") or "").strip().lower() + api_key = body.get("api_key") + if not provider_id: + return bad(handler, "provider is required") + if api_key is not None: + api_key = str(api_key).strip() or None + result = set_provider_key(provider_id, api_key) + if not result.get("ok"): + return bad(handler, result.get("error", "Unknown error")) + return j(handler, result) + + if parsed.path == "/api/providers/delete": + provider_id = (body.get("provider") or "").strip().lower() + if not provider_id: + return bad(handler, "provider is required") + result = remove_provider_key(provider_id) + if not result.get("ok"): + return bad(handler, result.get("error", "Unknown error")) + return j(handler, result) + if parsed.path == "/api/reasoning": # CLI-parity /reasoning handler — writes to the same config.yaml keys # the CLI uses (display.show_reasoning, agent.reasoning_effort) so a diff --git a/static/i18n.js b/static/i18n.js index fd377df..dfc03ff 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -276,6 +276,26 @@ const LOCALES = { password_placeholder: 'Enter new password…', disable_auth: 'Disable Auth', sign_out: 'Sign Out', + // Providers panel + providers_tab_title: 'Providers', + providers_section_title: 'Providers', + providers_section_meta: 'Manage API keys for AI providers. Changes take effect immediately.', + providers_status_configured: 'API key configured', + providers_status_not_configured: 'No API key', + providers_status_oauth: 'OAuth', + providers_status_api_key: 'API key', + providers_status_not_configured_label: 'Not configured', + providers_oauth_hint: 'Authenticated via OAuth. No API key needed.', + providers_save: 'Save', + providers_remove: 'Remove', + providers_saving: 'Saving…', + providers_removing: 'Removing…', + providers_enter_key: 'Please enter an API key', + providers_empty: 'No configurable providers found.', + providers_key_updated: 'API key saved', + providers_key_removed: 'API key removed', + providers_key_placeholder_new: 'sk-...', + providers_key_placeholder_replace: 'Enter new key to replace…', cancel: 'Cancel', create_job: 'Create job', save_skill: 'Save skill', diff --git a/static/index.html b/static/index.html index cb913bd..7460b1b 100644 --- a/static/index.html +++ b/static/index.html @@ -459,6 +459,10 @@ Preferences + +
+
+
+
Providers
+
Manage API keys for AI providers. Changes take effect immediately.
+
+
+
+ +
+ +
diff --git a/static/panels.js b/static/panels.js index 885d5c1..3f25cb4 100644 --- a/static/panels.js +++ b/static/panels.js @@ -1128,10 +1128,10 @@ let _settingsHermesDefaultModelOnOpen = ''; let _settingsSection = 'conversation'; function switchSettingsSection(name){ - const section=(name==='appearance'||name==='preferences'||name==='system')?name:'conversation'; + const section=(name==='appearance'||name==='preferences'||name==='providers'||name==='system')?name:'conversation'; _settingsSection=section; - const map={conversation:'Conversation',appearance:'Appearance',preferences:'Preferences',system:'System'}; - ['conversation','appearance','preferences','system'].forEach(key=>{ + const map={conversation:'Conversation',appearance:'Appearance',preferences:'Preferences',providers:'Providers',system:'System'}; + ['conversation','appearance','preferences','providers','system'].forEach(key=>{ const tab=$('settingsTab'+map[key]); const pane=$('settingsPane'+map[key]); const active=key===section; @@ -1141,6 +1141,8 @@ function switchSettingsSection(name){ } if(pane) pane.classList.toggle('active',active); }); + // Lazy-load providers when the tab is opened + if(section==='providers') loadProvidersPanel(); } function _syncHermesPanelSessionActions(){ @@ -1348,12 +1350,157 @@ async function loadSettingsPanel(){ _setSettingsAuthButtonsVisible(!!authStatus.auth_enabled); }catch(e){} _syncHermesPanelSessionActions(); + loadProvidersPanel(); // load provider cards in background switchSettingsSection(_settingsSection); }catch(e){ showToast(t('settings_load_failed')+e.message); } } +// ── Providers panel ─────────────────────────────────────────────────────── + +const _providerCardEls = new Map(); // providerId → {card, statusDot, input, saveBtn, removeBtn} + +async function loadProvidersPanel(){ + const list=$('providersList'); + const empty=$('providersEmpty'); + if(!list) return; + try{ + const data=await api('/api/providers'); + const providers=(data.providers||[]).filter(p=>p.configurable); + list.innerHTML=''; + _providerCardEls.clear(); + if(providers.length===0){ + list.style.display='none'; + if(empty) empty.style.display=''; + return; + } + if(empty) empty.style.display='none'; + list.style.display=''; + for(const p of providers){ + list.appendChild(_buildProviderCard(p)); + } + }catch(e){ + list.innerHTML='
Failed to load providers: '+e.message+'
'; + } +} + +function _buildProviderCard(p){ + const card=document.createElement('div'); + card.className='provider-card'; + card.dataset.provider=p.id; + const isOauth=p.key_source==='oauth'; + const statusColor=p.has_key?'var(--ok, #4ade80)':'var(--muted)'; + const statusTitle=p.has_key?t('providers_status_configured'):t('providers_status_not_configured'); + + // Header row + const header=document.createElement('div'); + header.className='provider-card-header'; + const info=document.createElement('div'); + info.className='provider-card-info'; + info.style.cssText='display:flex;align-items:center;gap:8px;'; + const nameEl=document.createElement('span'); + nameEl.className='provider-card-name'; + nameEl.style.cssText='font-weight:600;font-size:13px;'; + nameEl.textContent=p.display_name; + const dot=document.createElement('span'); + dot.className='provider-card-dot'; + dot.title=statusTitle; + dot.style.cssText='width:8px;height:8px;border-radius:50%;background:'+statusColor+';display:inline-block;flex-shrink:0'; + const sourceEl=document.createElement('span'); + sourceEl.className='provider-card-source'; + sourceEl.style.cssText='font-size:11px;color:var(--muted)'; + sourceEl.textContent=isOauth?t('providers_status_oauth'):(p.has_key?t('providers_status_api_key'):t('providers_status_not_configured_label')); + info.appendChild(nameEl); + info.appendChild(dot); + info.appendChild(sourceEl); + header.appendChild(info); + card.appendChild(header); + + if(isOauth){ + const hint=document.createElement('div'); + hint.style.cssText='font-size:11px;color:var(--muted);margin-top:4px;padding-left:2px'; + hint.textContent=t('providers_oauth_hint'); + card.appendChild(hint); + }else{ + const actions=document.createElement('div'); + actions.className='provider-card-actions'; + actions.style.cssText='margin-top:6px;display:flex;gap:6px;align-items:center'; + const input=document.createElement('input'); + input.type='password'; + input.placeholder=p.has_key?t('providers_key_placeholder_replace'):t('providers_key_placeholder_new'); + input.style.cssText='flex:1;padding:6px 8px;background:var(--code-bg);color:var(--text);border:1px solid var(--border2);border-radius:6px;font-size:12px;font-family:monospace'; + input.autocomplete='off'; + const saveBtn=document.createElement('button'); + saveBtn.className='sm-btn provider-save-btn'; + saveBtn.style.cssText='padding:5px 12px;font-size:12px;white-space:nowrap'; + saveBtn.textContent=t('providers_save'); + saveBtn.onclick=()=>_saveProviderKey(p.id); + actions.appendChild(input); + actions.appendChild(saveBtn); + if(p.has_key){ + const removeBtn=document.createElement('button'); + removeBtn.className='sm-btn'; + removeBtn.style.cssText='padding:5px 10px;font-size:12px;color:var(--error);border-color:rgba(233,69,96,.25);white-space:nowrap'; + removeBtn.textContent=t('providers_remove'); + removeBtn.onclick=()=>_removeProviderKey(p.id); + actions.appendChild(removeBtn); + } + card.appendChild(actions); + _providerCardEls.set(p.id,{card,input,saveBtn,hasKey:p.has_key}); + input.addEventListener('input',()=>{saveBtn.disabled=!input.value.trim();}); + saveBtn.disabled=true; + } + return card; +} + +async function _saveProviderKey(providerId){ + const els=_providerCardEls.get(providerId); + if(!els) return; + const key=els.input.value.trim(); + if(!key){ + showToast(t('providers_enter_key')); + return; + } + els.saveBtn.disabled=true; + els.saveBtn.textContent=t('providers_saving'); + try{ + const res=await api('/api/providers',{method:'POST',body:JSON.stringify({provider:providerId,api_key:key})}); + if(res.ok){ + showToast(res.provider+' key '+res.action); + els.input.value=''; + await loadProvidersPanel(); // refresh list + }else{ + showToast(res.error||'Failed to save key'); + els.saveBtn.disabled=false; + els.saveBtn.textContent=t('providers_save'); + } + }catch(e){ + showToast('Error: '+e.message); + els.saveBtn.disabled=false; + els.saveBtn.textContent=t('providers_save'); + } +} + +async function _removeProviderKey(providerId){ + const els=_providerCardEls.get(providerId); + if(!els) return; + if(els.saveBtn){els.saveBtn.disabled=true;els.saveBtn.textContent=t('providers_removing');} + try{ + const res=await api('/api/providers/delete',{method:'POST',body:JSON.stringify({provider:providerId})}); + if(res.ok){ + showToast(res.provider+' key '+t('providers_key_removed').toLowerCase()); + await loadProvidersPanel(); // refresh list + }else{ + showToast(res.error||'Failed to remove key'); + if(els.saveBtn){els.saveBtn.disabled=false;els.saveBtn.textContent=t('providers_save');} + } + }catch(e){ + showToast('Error: '+e.message); + if(els.saveBtn){els.saveBtn.disabled=false;els.saveBtn.textContent=t('providers_save');} + } +} + function _setSettingsAuthButtonsVisible(active){ const signOutBtn=$('btnSignOut'); if(signOutBtn) signOutBtn.style.display=active?'':'none'; diff --git a/static/style.css b/static/style.css index 76b7888..58d30c8 100644 --- a/static/style.css +++ b/static/style.css @@ -1465,6 +1465,12 @@ body.resizing{user-select:none;cursor:col-resize;} .settings-panel .settings-btn{background:var(--accent);color:#fff;border:none;border-radius:6px;padding:8px 16px;cursor:pointer;font-weight:600;font-size:13px;} .settings-panel .settings-btn:hover{opacity:.9;} +/* ── Provider cards (settings panel) ── */ +.provider-card{padding:10px 12px;border:1px solid var(--border);border-radius:8px;background:var(--code-bg);transition:border-color .15s;} +.provider-card:hover{border-color:var(--border2);} +.provider-card-name{font-weight:600;font-size:13px;} +.provider-card .sm-btn:disabled{opacity:.4;cursor:not-allowed;} + /* ── Session pin indicator (inline, only when pinned) ── */ .session-pin-indicator{flex-shrink:0;color:#f5c542;line-height:1;display:flex;align-items:center;} .session-pin-indicator svg{width:10px;height:10px;} diff --git a/tests/test_provider_management.py b/tests/test_provider_management.py new file mode 100644 index 0000000..f33c61e --- /dev/null +++ b/tests/test_provider_management.py @@ -0,0 +1,364 @@ +"""Tests for /api/providers CRUD endpoints (provider key management). + +Closes #586 — allow provider key update from the WebUI. +Part of #604 — multi-provider model picker support. +""" + +import json +import sys +import types +import urllib.error +import urllib.request + +import api.config as config +import api.profiles as profiles +from tests._pytest_port import BASE + + +# ── HTTP helpers ────────────────────────────────────────────────────────── + + +def _get(path): + """GET helper — returns parsed JSON.""" + with urllib.request.urlopen(BASE + path, timeout=10) as r: + return json.loads(r.read()) + + +def _post(path, body=None): + """POST helper — returns (parsed_json, status_code).""" + data = json.dumps(body or {}).encode() + req = urllib.request.Request( + BASE + path, data=data, headers={"Content-Type": "application/json"}, + ) + try: + with urllib.request.urlopen(req, timeout=10) as r: + return json.loads(r.read()), r.status + except urllib.error.HTTPError as e: + body_text = e.read().decode("utf-8", errors="replace") + try: + return json.loads(body_text), e.code + except Exception: + return {"error": body_text}, e.code + + +def _install_fake_hermes_cli(monkeypatch): + """Stub hermes_cli modules so tests are deterministic and offline.""" + fake_pkg = types.ModuleType("hermes_cli") + fake_pkg.__path__ = [] + + fake_models = types.ModuleType("hermes_cli.models") + fake_models.list_available_providers = lambda: [] + fake_models.provider_model_ids = lambda pid: [] + + fake_auth = types.ModuleType("hermes_cli.auth") + fake_auth.get_auth_status = lambda _pid: {} + + monkeypatch.setitem(sys.modules, "hermes_cli", fake_pkg) + monkeypatch.setitem(sys.modules, "hermes_cli.models", fake_models) + monkeypatch.setitem(sys.modules, "hermes_cli.auth", fake_auth) + monkeypatch.delitem(sys.modules, "agent.credential_pool", raising=False) + monkeypatch.delitem(sys.modules, "agent", raising=False) + + +# ── Unit tests (api/providers.py functions directly) ────────────────────── + + +class TestGetProviders: + """Unit tests for get_providers() function.""" + + def test_returns_list_of_known_providers(self, monkeypatch, tmp_path): + """GET /api/providers should return a list of all known providers.""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + from api.providers import get_providers + try: + result = get_providers() + assert "providers" in result + assert "active_provider" in result + assert isinstance(result["providers"], list) + # Should include at least the built-in providers + provider_ids = {p["id"] for p in result["providers"]} + assert "anthropic" in provider_ids + assert "openai" in provider_ids + assert "openrouter" in provider_ids + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + def test_provider_entries_have_required_fields(self, monkeypatch, tmp_path): + """Each provider entry should have id, display_name, has_key, configurable.""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + from api.providers import get_providers + try: + result = get_providers() + for p in result["providers"]: + assert "id" in p, f"Missing 'id' in provider entry" + assert "display_name" in p, f"Missing 'display_name' for {p['id']}" + assert "has_key" in p, f"Missing 'has_key' for {p['id']}" + assert "configurable" in p, f"Missing 'configurable' for {p['id']}" + assert "key_source" in p, f"Missing 'key_source' for {p['id']}" + assert isinstance(p["has_key"], bool) + assert isinstance(p["configurable"], bool) + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + def test_oauth_providers_not_configurable(self, monkeypatch, tmp_path): + """OAuth providers (copilot, nous, openai-codex) should not be configurable.""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + from api.providers import get_providers + try: + result = get_providers() + for p in result["providers"]: + if p["id"] in ("copilot", "nous", "openai-codex"): + assert p["configurable"] is False, f"{p['id']} should not be configurable" + # ollama-cloud is now configurable (uses OLLAMA_API_KEY) + if p["id"] == "ollama-cloud": + assert p["configurable"] is True, "ollama-cloud should be configurable" + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + +class TestSetProviderKey: + """Unit tests for set_provider_key() function.""" + + def test_set_key_writes_to_env_file(self, monkeypatch, tmp_path): + """Setting a key should write the env var to ~/.hermes/.env.""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + from api.providers import set_provider_key + try: + result = set_provider_key("anthropic", "sk-ant-test-key-12345678") + assert result["ok"] is True + assert result["provider"] == "anthropic" + assert result["action"] == "updated" + + # Verify .env file was written + env_path = tmp_path / ".env" + assert env_path.exists() + content = env_path.read_text() + assert "ANTHROPIC_API_KEY=sk-ant-test-key-12345678" in content + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + def test_remove_key_deletes_from_env_file(self, monkeypatch, tmp_path): + """Removing a key should delete the env var from .env.""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + from api.providers import set_provider_key + try: + # First set a key + set_provider_key("anthropic", "sk-ant-test-key-12345678") + # Then remove it + result = set_provider_key("anthropic", None) + assert result["ok"] is True + assert result["action"] == "removed" + + # Verify .env file no longer has the key + env_path = tmp_path / ".env" + content = env_path.read_text() if env_path.exists() else "" + assert "ANTHROPIC_API_KEY" not in content + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + def test_oauth_provider_rejected(self, monkeypatch, tmp_path): + """Setting a key for an OAuth provider should fail.""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + from api.providers import set_provider_key + try: + result = set_provider_key("copilot", "some-key") + assert result["ok"] is False + assert "OAuth" in result["error"] + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + def test_short_key_rejected(self, monkeypatch, tmp_path): + """API keys shorter than 8 chars should be rejected.""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + from api.providers import set_provider_key + try: + result = set_provider_key("anthropic", "short") + assert result["ok"] is False + assert "too short" in result["error"] + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + def test_empty_provider_id_rejected(self, monkeypatch, tmp_path): + """Empty provider ID should be rejected.""" + from api.providers import set_provider_key + result = set_provider_key("", "some-key") + assert result["ok"] is False + assert "required" in result["error"] + + def test_newline_in_key_rejected(self, monkeypatch, tmp_path): + """API keys with newlines should be rejected.""" + from api.providers import set_provider_key + result = set_provider_key("anthropic", "sk-ant-key\nINJECTED=evil") + assert result["ok"] is False + assert "newline" in result["error"] + + +class TestRemoveProviderKey: + """Unit tests for remove_provider_key() wrapper.""" + + def test_remove_provider_key_calls_set_with_none(self, monkeypatch, tmp_path): + """remove_provider_key should delegate to set_provider_key(id, None).""" + _install_fake_hermes_cli(monkeypatch) + monkeypatch.setattr(profiles, "get_active_hermes_home", lambda: tmp_path) + + old_cfg = dict(config.cfg) + old_mtime = config._cfg_mtime + config.cfg.clear() + config.cfg["model"] = {} + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + from api.providers import remove_provider_key + try: + result = remove_provider_key("anthropic") + assert result["ok"] is True + assert result["action"] == "removed" + finally: + config.cfg.clear() + config.cfg.update(old_cfg) + config._cfg_mtime = old_mtime + + +# ── Integration tests (via HTTP endpoints) ─────────────────────────────── + + +class TestProvidersEndpoints: + """Integration tests for /api/providers HTTP endpoints.""" + + def test_get_providers_returns_200(self): + """GET /api/providers should return 200 with provider list.""" + result = _get("/api/providers") + assert "providers" in result + assert isinstance(result["providers"], list) + + def test_post_provider_set_key(self): + """POST /api/providers with provider + api_key should set the key.""" + body, status = _post("/api/providers", { + "provider": "anthropic", + "api_key": "sk-ant-integration-test-key-12345678", + }) + assert status == 200 + assert body.get("ok") is True + assert body.get("provider") == "anthropic" + + def test_post_provider_remove_key(self): + """POST /api/providers with provider but no api_key should remove the key.""" + body, status = _post("/api/providers", { + "provider": "anthropic", + "api_key": None, + }) + assert status == 200 + assert body.get("ok") is True + assert body.get("action") == "removed" + + def test_post_provider_delete(self): + """POST /api/providers/delete should remove the key.""" + body, status = _post("/api/providers/delete", { + "provider": "anthropic", + }) + assert status == 200 + assert body.get("ok") is True + + def test_post_provider_missing_id(self): + """POST /api/providers without provider should return 400.""" + body, status = _post("/api/providers", {"api_key": "some-key"}) + assert status == 400 + assert "required" in body.get("error", "").lower() + + def test_post_provider_delete_missing_id(self): + """POST /api/providers/delete without provider should return 400.""" + body, status = _post("/api/providers/delete", {}) + assert status == 400