diff --git a/hermes_cli/web_routers/config_env.py b/hermes_cli/web_routers/config_env.py index 924c2a600f..4b71ff92b5 100644 --- a/hermes_cli/web_routers/config_env.py +++ b/hermes_cli/web_routers/config_env.py @@ -1,9 +1,10 @@ """Config, env var and provider custom-endpoint dashboard routes. Extracted from ``hermes_cli.web_server``; helpers/state that tests monkeypatch on -``web_server`` stay there and are imported lazily at call time (cycle-safe). +``web_server`` stay there and are late-bound (cycle-safe). """ +import contextlib import logging import re import asyncio @@ -11,7 +12,7 @@ import time import urllib.parse from fastapi import APIRouter from hermes_cli.web_routers._common import http_failure -from hermes_cli.web_deps import late +from hermes_cli.web_deps import LateState, late from fastapi import HTTPException, Request from hermes_cli.config import DEFAULT_CONFIG, OPTIONAL_ENV_VARS, read_raw_config, custom_endpoint_key_env, coerce_provider_id, find_provider_entry, redact_key, _deep_merge from hermes_cli.web_models import ConfigUpdate, EnvVarUpdate, EnvVarDelete, EnvVarReveal, CustomEndpointUpdate @@ -39,18 +40,14 @@ load_env = late("load_env") remove_env_value = late("remove_env_value") save_config = late("save_config") save_env_value = late("save_env_value") +_CONFIG_MUTATION_LOCK = LateState("_CONFIG_MUTATION_LOCK") # Simple rate limiter for the reveal endpoint _reveal_timestamps: List[float] = [] - - _REVEAL_MAX_PER_WINDOW = 5 - - _REVEAL_WINDOW_SECONDS = 30 - # Display order for tabs — unlisted categories sort alphabetically after these. _CATEGORY_ORDER = [ "general", "agent", "terminal", "display", "delegation", @@ -59,13 +56,32 @@ _CATEGORY_ORDER = [ ] +@contextlib.contextmanager +def _env_write_errors(log_msg: str, *, http_passthrough: bool): + """``ValueError`` -> 400 with its message (save/remove_env_value reject + invalid names and denylisted keys — LD_PRELOAD, PATH, PYTHONPATH, …, and + the SPA needs the reason, not an opaque 500); anything else is logged and + becomes 500 "Internal server error".""" + try: + yield + except HTTPException: + if http_passthrough: + raise + _log.exception(log_msg) + raise HTTPException(status_code=500, detail="Internal server error") + except ValueError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + except Exception: + _log.exception(log_msg) + raise HTTPException(status_code=500, detail="Internal server error") + + @config_router.get("/api/config") async def get_config(profile: Optional[str] = None): # _profile_scope blocks on the process-wide _SKILLS_PROFILE_LOCK and - # load_config() reads from disk; on the event loop a slow lock-holder - # froze the whole gateway for >1s (observed via the loop watchdog). - # asyncio.to_thread copies the contextvar context, so the profile - # override stays scoped to the worker thread. + # load_config() reads from disk; a slow lock-holder on the event loop froze + # the whole gateway for >1s. asyncio.to_thread copies the contextvar + # context, so the profile override stays scoped to the worker thread. def _run(): with _profile_scope(profile): return _normalize_config_for_web(load_config()) @@ -100,28 +116,23 @@ async def get_egress_status(): @router.put("/api/config") async def update_config(body: ConfigUpdate, profile: Optional[str] = None): - from hermes_cli.web_server import _CONFIG_MUTATION_LOCK def _run(): approvals_mode_changed = False with _profile_scope(body.profile or profile): - # The dashboard form is schema-driven (see CONFIG_SCHEMA). Any root - # key absent from the schema — most visibly ``custom_providers``, but - # also ``agent.personalities``, ``terminal.lifetime_seconds``, etc. — - # is not sent in the PUT body. A full-replace save would silently - # drop those keys. Deep-merge incoming over what's on disk so the - # frontend can only overwrite what it explicitly sends. + # The dashboard form is schema-driven; root keys absent from the + # schema (``custom_providers``, ``agent.personalities``, ...) are not + # in the PUT body, so deep-merge incoming over disk rather than + # full-replace — the frontend can only overwrite what it sends. with _CONFIG_MUTATION_LOCK: existing = read_raw_config() incoming = _denormalize_config_from_web(body.config) merged = _deep_merge(existing, incoming) # Compare normalized approvals.mode across the in-memory - # documents, not config blocks and not cache re-reads: the - # settings page PUTs the defaulted GET record while disk - # holds sparse YAML, so a block compare is always-unequal - # (every autosave would broadcast), and reloading after the - # save can serve the pre-save cache on an (mtime_ns, size) - # key collision. Only approvals.mode feeds session.info, so - # it is the honest trigger. + # documents, not config blocks and not cache re-reads: the page + # PUTs the defaulted GET record while disk holds sparse YAML (a + # block compare is always-unequal), and a post-save reload can + # serve the pre-save cache on an (mtime_ns, size) collision. + # Only approvals.mode feeds session.info, so it is the trigger. approvals_mode_changed = _approval_mode_of(merged) != _approval_mode_of(existing) save_config(merged) # REST saves bypass the config.set RPC (which re-emits itself), so @@ -136,35 +147,38 @@ async def update_config(body: ConfigUpdate, profile: Optional[str] = None): return await asyncio.to_thread(_run) +def _provider_card(d, description: str, url, *, is_password: bool, advanced: bool) -> dict: + return { + "provider": d.slug, + "provider_label": d.label, + "description": description, + "url": url, + "is_password": is_password, + "advanced": advanced, + "category": "provider", + } + + def _catalog_provider_env_metadata() -> dict: - """Map provider env vars → desktop card metadata, derived from the catalog. + """Map provider env vars -> desktop card metadata, derived from the catalog. Returns ``{env_var: {provider, provider_label, description, url, is_password, advanced}}`` for every API-key provider in the unified ``provider_catalog()`` - (i.e. the ``hermes model`` universe). This is what lets the desktop Keys tab - render a card for a provider even when its env var was never hand-added to - ``OPTIONAL_ENV_VARS`` — closing the drift where CLI-configurable providers - (openai-api, kilocode, novita, tencent-tokenhub, copilot, …) were missing - from the GUI. - - Hand ``OPTIONAL_ENV_VARS`` prose is layered ON TOP of this in the endpoint; - this only supplies membership + grouping + sensible fallbacks. + (the ``hermes model`` universe), so the desktop Keys tab renders a card even + for providers never hand-added to ``OPTIONAL_ENV_VARS``. Hand + ``OPTIONAL_ENV_VARS`` prose is layered on top in the endpoint; this only + supplies membership + grouping + fallbacks. """ try: from hermes_cli.provider_catalog import provider_catalog except Exception: return {} - # Env vars already declared with a NON-provider category (e.g. the shared - # GITHUB_TOKEN, which is a Skills-Hub "tool" credential) must not be - # promoted into a provider card. Copilot lists GITHUB_TOKEN among its auth - # aliases, but its provider card uses the provider-owned COPILOT_GITHUB_TOKEN. - try: - from hermes_cli.config import OPTIONAL_ENV_VARS as _OPT - except Exception: - _OPT = {} + # Env vars declared with a NON-provider category (e.g. the shared + # GITHUB_TOKEN, a Skills-Hub "tool" credential) must not be promoted into a + # provider card even when a provider (Copilot) lists them as auth aliases. _non_provider_keys = { - k for k, v in _OPT.items() + k for k, v in OPTIONAL_ENV_VARS.items() if (v or {}).get("category") and (v or {}).get("category") != "provider" } @@ -172,74 +186,45 @@ def _catalog_provider_env_metadata() -> dict: for d in provider_catalog(): if d.tab != "keys": continue - # API-key vars: the first is the primary (password) field; any aliases - # are kept as additional password fields so users can clear them too. + # API-key vars: the first is the primary (password) field; aliases are + # kept as additional password fields so users can clear them too. for env_var in d.api_key_env_vars: if env_var in _non_provider_keys: continue # don't hijack a shared tool/messaging credential meta.setdefault( env_var, - { - "provider": d.slug, - "provider_label": d.label, - "description": d.description, - "url": d.signup_url or None, - "is_password": True, - "advanced": False, - "category": "provider", - }, + _provider_card(d, d.description, d.signup_url or None, is_password=True, advanced=False), ) # Base-URL override is an advanced, non-secret field for the same card. if d.base_url_env_var: meta.setdefault( d.base_url_env_var, - { - "provider": d.slug, - "provider_label": d.label, - "description": f"{d.label} base URL override", - "url": None, - "is_password": False, - "advanced": True, - "category": "provider", - }, + _provider_card(d, f"{d.label} base URL override", None, is_password=False, advanced=True), ) - # AWS-SDK providers (Bedrock) authenticate via the AWS credential chain - # rather than a pasted API key, so they have no api_key_env_vars. Tag - # their AWS_* settings to the provider card so they still appear on the - # Keys tab (otherwise Bedrock — a `hermes model` provider — would be - # invisible in the desktop app). + # Providers without api_key_env_vars would otherwise be invisible on + # the Keys tab: AWS-SDK providers (Bedrock) authenticate via the AWS + # credential chain, Vertex via OAuth2 (service-account JSON path or + # ADC — a path, not a secret). Tag their env vars to the card. if d.auth_type == "aws_sdk": for aws_var in ("AWS_REGION", "AWS_PROFILE"): existing = meta.get(aws_var, {}) - meta[aws_var] = { - "provider": d.slug, - "provider_label": d.label, - "description": existing.get("description") or f"{d.label} ({aws_var})", - "url": existing.get("url"), - "is_password": False, - "advanced": existing.get("advanced", True), - "category": "provider", - } - - # Vertex AI authenticates via OAuth2 (service-account JSON or ADC), not a - # pasted API key, so it also has no api_key_env_vars. Tag its credential - # env var to the provider card so it appears on the Keys tab (otherwise - # Vertex — a `hermes model` provider — would be invisible in the desktop - # app). The value is a filesystem path, not a secret string, so it is - # not a password field. + meta[aws_var] = _provider_card( + d, + existing.get("description") or f"{d.label} ({aws_var})", + existing.get("url"), + is_password=False, + advanced=existing.get("advanced", True), + ) if d.auth_type == "vertex": existing = meta.get("VERTEX_CREDENTIALS_PATH", {}) - meta["VERTEX_CREDENTIALS_PATH"] = { - "provider": d.slug, - "provider_label": d.label, - "description": existing.get("description") - or f"{d.label} — service account JSON path (or use ADC)", - "url": existing.get("url"), - "is_password": False, - "advanced": existing.get("advanced", True), - "category": "provider", - } + meta["VERTEX_CREDENTIALS_PATH"] = _provider_card( + d, + existing.get("description") or f"{d.label} — service account JSON path (or use ADC)", + existing.get("url"), + is_password=False, + advanced=existing.get("advanced", True), + ) return meta @@ -270,38 +255,29 @@ def _get_env_vars_sync(profile: Optional[str] = None): "is_password": info.get("password", cat_meta.get("is_password", False)), "tools": info.get("tools", []), "advanced": info.get("advanced", cat_meta.get("advanced", False)), - # True when this var is a messaging-platform credential owned by a - # Channels page card. The Keys/Env page uses this to hide it and - # avoid duplicating the (richer) Channels configuration UI. + # Messaging-platform credential owned by a Channels page card; the + # Keys/Env page hides it rather than duplicate the richer UI. "channel_managed": var_name in channel_keys, - # Provider grouping hints derived from the unified provider catalog - # so the desktop Keys tab groups by the SAME provider identity the - # CLI `hermes model` picker uses (not desktop-only prefix guesses). + # Provider grouping from the unified catalog, so the desktop groups + # by the SAME provider identity the CLI `hermes model` picker uses. "provider": cat_meta.get("provider", ""), "provider_label": cat_meta.get("provider_label", ""), - # True when this key exists in the user's .env but is NOT in any - # catalog (OPTIONAL_ENV_VARS or the provider catalog) — an - # arbitrary/custom env var the user added directly. Surfaced so the - # Keys page can list (and let the user manage) them instead of - # hiding everything it doesn't recognise. + # True for a .env key in no catalog at all — an arbitrary/custom var + # the user added directly, listed so the Keys page can manage it. "custom": custom, } result = {} for var_name, info in OPTIONAL_ENV_VARS.items(): result[var_name] = _row(var_name, info) - # Synthesize rows for catalog provider env vars that have no hand entry in - # OPTIONAL_ENV_VARS — these are the providers that were CLI-configurable but - # invisible in the desktop app until now. + # Catalog provider env vars with no hand entry in OPTIONAL_ENV_VARS. for var_name in catalog_meta: if var_name not in result: result[var_name] = _row(var_name, {}) - # Surface arbitrary/custom keys the user set in .env that aren't in any - # catalog. These are always "set" (they're on disk). Treated as secrets by - # default (is_password=True → redacted, reveal-gated) since an unrecognised - # key could hold anything. Channel-managed credentials are excluded — those - # belong to the Channels page. This makes the "add a custom key" surface - # round-trip: a key added there reappears here under its own section. + # Custom keys from .env: always "set" (on disk), treated as secrets by + # default (is_password=True -> redacted, reveal-gated) since an + # unrecognised key could hold anything. Channel-managed credentials belong + # to the Channels page. A key added via "add a custom key" round-trips here. for var_name in env_on_disk: if var_name in result or var_name in channel_keys: continue @@ -318,31 +294,22 @@ async def set_env_var(body: EnvVarUpdate, profile: Optional[str] = None): with _profile_scope(body.profile or profile): # Unified credential lifecycle: writes .env AND reconciles any # config.yaml mirror still holding the previous value of this var - # (model.api_key / auxiliary.*.api_key / custom_providers[*]), - # so a rotation can't leave a stale higher-precedence copy that - # keeps authenticating with the old key (#62269). + # (model.api_key / auxiliary.*.api_key / custom_providers[*]), so a + # rotation can't leave a stale higher-precedence copy that keeps + # authenticating with the old key. from hermes_cli.credential_lifecycle import save_provider_env_credential return save_provider_env_credential(body.key, body.value) - try: + with _env_write_errors("PUT /api/env failed", http_passthrough=False): return await asyncio.to_thread(_run) - except ValueError as exc: - # save_env_value raises ValueError for invalid names and for keys - # on the denylist (LD_PRELOAD, PATH, PYTHONPATH, …). Surface the - # message to the SPA so the user understands why the write was - # refused instead of seeing an opaque 500. - raise HTTPException(status_code=400, detail=str(exc)) from exc - except Exception: - _log.exception("PUT /api/env failed") - raise HTTPException(status_code=500, detail="Internal server error") -# Live credential probes keyed by env var. Each entry is (method, url, auth) -# where auth is "bearer" (Authorization header) or "query" (?key=). A cheap -# read-only models/key call that 401s on a bad token — enough to catch a -# mistyped key before it's persisted. Providers absent from this map (or local -# endpoints) are not network-validated; the client treats those as "unknown". +# Live credential probes keyed by env var: (url, auth) where auth is "bearer" +# (Authorization header) or "query" (?key=). A cheap read-only call that 401s +# on a bad token — enough to catch a mistyped key before it's persisted. +# Providers absent here (or local endpoints) are not network-validated; the +# client treats those as "unknown". _CREDENTIAL_PROBES: dict[str, tuple[str, str]] = { "OPENROUTER_API_KEY": ("https://openrouter.ai/api/v1/key", "bearer"), "OPENAI_API_KEY": ("https://api.openai.com/v1/models", "bearer"), @@ -359,9 +326,7 @@ def _custom_endpoint_id(raw: str, fallback: str = "custom") -> str: def _models_from_custom_endpoint_entry(entry: Dict[str, Any]) -> List[str]: models: List[str] = [] raw_models = entry.get("models") - if isinstance(raw_models, dict): - models.extend(str(model).strip() for model in raw_models.keys()) - elif isinstance(raw_models, list): + if isinstance(raw_models, (dict, list)): models.extend(str(model).strip() for model in raw_models) default_model = str(entry.get("model") or entry.get("default_model") or "").strip() @@ -375,10 +340,8 @@ def _models_from_custom_endpoint_entry(entry: Dict[str, Any]) -> List[str]: 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``. + Keys live in ``.env`` behind ``key_env``; only older entries still carry a + plaintext ``api_key``. Checking both keeps the panel honest either way. """ plaintext = str(entry.get("api_key") or "").strip() if plaintext: @@ -389,18 +352,21 @@ def _api_key_display(entry: Dict[str, Any]) -> Tuple[bool, Optional[str]]: return False, None +def _raw_provider_api_key(endpoint_id: str) -> str: + """The on-disk (un-expanded) ``api_key`` of a providers entry, or ``""``.""" + _stored, entry = find_provider_entry(read_raw_config().get("providers"), endpoint_id) + return str(entry.get("api_key") or "").strip() if isinstance(entry, dict) else "" + + 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. + ``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 already keeps 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. """ - _stored, entry = find_provider_entry(read_raw_config().get("providers"), endpoint_id) - raw_key = entry.get("api_key") if isinstance(entry, dict) else None - return bool(isinstance(raw_key, str) and re.search(r"\$\{[^}]+\}", raw_key)) + return bool(re.search(r"\$\{[^}]+\}", _raw_provider_api_key(endpoint_id))) def _custom_endpoint_response(cfg: Dict[str, Any]) -> Dict[str, Any]: @@ -466,14 +432,10 @@ def _detach_main_model_from_provider(cfg: Dict[str, Any], provider_key: str) -> """Drop the main-slot mirror of a provider that no longer exists. ``activate_custom_endpoint`` copies the endpoint's ``base_url`` and - ``api_key`` onto ``model``. That mirror outranks the environment at client - construction (#62269), so deleting the endpoint without clearing it leaves - the agent still authenticating to the deleted host with the deleted key — - and leaves that key sitting in config.yaml after the operator believes the - dashboard removed it. - - Only touches ``model`` when it actually names the deleted provider, so an - endpoint deleted while a *different* provider is active is left alone. + ``api_key`` onto ``model``; that mirror outranks the environment at client + construction, so deleting the endpoint without clearing it leaves the agent + authenticating to the deleted host with the deleted key (and the key in + config.yaml). Only touches ``model`` when it names the deleted provider. """ model_cfg = cfg.get("model") if not isinstance(model_cfg, dict): @@ -508,13 +470,11 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T if existing is None: existing = {} - # Merge onto the existing entry rather than replacing it. A providers. - # block is not owned by this panel: it can carry hand-written keys the - # dashboard has no field for — ``api_mode``, ``key_env``/``api_key_env``, - # ``extra_headers`` (which may themselves carry credentials), - # ``request_overrides`` — and rebuilding from scratch silently dropped every - # one of them on an unrelated edit, leaving a provider that no longer - # authenticates or speaks the right protocol. + # Merge onto the existing entry rather than replacing it: a providers. + # block can carry hand-written keys the dashboard has no field for + # (``api_mode``, ``key_env``/``api_key_env``, ``extra_headers`` — possibly + # with credentials — ``request_overrides``); rebuilding from scratch + # silently dropped them on an unrelated edit. entry: Dict[str, Any] = dict(existing) entry.update({ "name": name, @@ -522,12 +482,10 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T "model": model, "discover_models": bool(body.discover_models), }) - # 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. + # Same for the model map, so existing models keep their context lengths. + # ``body.models`` is the catalogue the panel's Test button discovered; + # without it only the hand-typed model survived Save. A payload with no + # ``models`` (older UI) still 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 {} for candidate in (*(body.models or ()), model): @@ -541,9 +499,9 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T entry["context_length"] = int(body.context_length) entry["models"][model]["context_length"] = int(body.context_length) - # 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. + # API keys never belong in config.yaml: write to .env and reference it via + # ``key_env`` — the indirection built-in providers use and that + # runtime_provider.py 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: @@ -556,10 +514,8 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T 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. + # Migrate a plaintext key an earlier release wrote, on the next save, + # without the user having to re-enter it. save_env_value(env_var, entry["api_key"].strip()) entry["key_env"] = env_var entry.pop("api_key", None) @@ -584,10 +540,9 @@ def _write_custom_endpoint(cfg: Dict[str, Any], body: CustomEndpointUpdate) -> T def list_custom_endpoints(profile: Optional[str] = None): """Return configured OpenAI-compatible custom endpoints for Desktop. - Scoped to the requested profile's config.yaml (issue: custom providers - only landing in the default profile): the desktop settings UI targets the - active profile, so read/write must resolve that profile's home rather than - the process-level HERMES_HOME. Mirrors ``/api/config``'s profile scoping. + Scoped to the requested profile's config.yaml: the desktop settings UI + targets the active profile, so read/write must resolve that profile's home + rather than the process-level HERMES_HOME (mirrors ``/api/config``). """ with http_failure("GET /api/providers/custom-endpoints failed", 500, detail="Failed to list custom endpoints"): with _config_profile_scope(profile): @@ -611,7 +566,10 @@ def upsert_custom_endpoint(body: CustomEndpointUpdate, profile: Optional[str] = @router.post("/api/providers/custom-endpoints/{endpoint_id}/activate") def activate_custom_endpoint(endpoint_id: str, profile: Optional[str] = None): """Set a configured custom endpoint as the default model provider.""" - try: + with http_failure( + f"POST /api/providers/custom-endpoints/{endpoint_id}/activate failed", 500, + detail="Failed to activate custom endpoint", + ): with _config_profile_scope(profile): cfg = load_config() provider_key = _custom_endpoint_id(endpoint_id) @@ -630,21 +588,12 @@ def activate_custom_endpoint(endpoint_id: str, profile: Optional[str] = None): model_cfg["key_env"] = entry["key_env"] model_cfg.pop("api_key", None) elif entry.get("api_key"): - # Same #88990 shape as /api/model/set: `cfg` is env-expanded, - # so a raw `${VAR}` api_key would land as plaintext. Copy the - # raw template when that's what's on disk. - _raw_entry = None + # `cfg` is env-expanded, so a raw `${VAR}` api_key would land as + # plaintext; copy the raw template when that's what's on disk. try: - _stored_raw, _raw_entry = find_provider_entry( - read_raw_config().get("providers"), provider_key - ) + _raw_key = _raw_provider_api_key(provider_key) except Exception: - _raw_entry = None - _raw_key = ( - str(_raw_entry.get("api_key") or "").strip() - if isinstance(_raw_entry, dict) - else "" - ) + _raw_key = "" if _raw_key.startswith("${") and _raw_key.endswith("}"): model_cfg["api_key"] = _raw_key else: @@ -652,17 +601,15 @@ def activate_custom_endpoint(endpoint_id: str, profile: Optional[str] = None): cfg["model"] = model_cfg save_config(cfg) return {"ok": True, "provider": provider_key, "model": model} - except HTTPException: - raise - except Exception: - _log.exception("POST /api/providers/custom-endpoints/%s/activate failed", endpoint_id) - raise HTTPException(status_code=500, detail="Failed to activate custom endpoint") @router.delete("/api/providers/custom-endpoints/{endpoint_id}") def delete_custom_endpoint(endpoint_id: str, profile: Optional[str] = None): """Remove a configured custom endpoint from ``providers``.""" - try: + with http_failure( + f"DELETE /api/providers/custom-endpoints/{endpoint_id} failed", 500, + detail="Failed to delete custom endpoint", + ): with _config_profile_scope(profile): cfg = load_config() provider_key = _custom_endpoint_id(endpoint_id) @@ -678,11 +625,6 @@ def delete_custom_endpoint(endpoint_id: str, profile: Optional[str] = None): response = _custom_endpoint_response(cfg) response["ok"] = True return response - except HTTPException: - raise - except Exception: - _log.exception("DELETE /api/providers/custom-endpoints/%s failed", endpoint_id) - raise HTTPException(status_code=500, detail="Failed to delete custom endpoint") @router.post("/api/providers/custom-endpoints/validate") @@ -731,14 +673,12 @@ async def validate_provider_credential(body: EnvVarUpdate, request: Request): return {"ok": False, "reachable": True, "message": "Enter a value first."} # Local / custom endpoint: validate connectivity, not auth — any HTTP - # response (even 401) proves the endpoint is up. Also surface the model - # ids the endpoint advertises (OpenAI ``/v1/models`` shape) so the GUI can - # auto-pick a default without asking the user to type a model name. + # response (even 401) proves the endpoint is up. Also surface the model ids + # it advertises (OpenAI ``/v1/models`` shape) so the GUI can auto-pick a + # default. The optional API key is sent so servers that require auth on + # ``/v1/models`` still enumerate instead of returning an empty list. if key == "OPENAI_BASE_URL": url = value.rstrip("/") + "/models" - # Send the optional API key so endpoints that require auth on - # ``/v1/models`` (many hosted OpenAI-compatible servers) still enumerate - # their models instead of returning an empty list behind a 401. api_key = (body.api_key or "").strip() headers = {"Authorization": f"Bearer {api_key}"} if api_key else None try: @@ -781,29 +721,19 @@ async def remove_env_var(body: EnvVarDelete, profile: Optional[str] = None): with _profile_scope(body.profile or profile): # Unified credential lifecycle: clears the .env entry AND every # mirror of the credential — env-seeded credential_pool entries in - # auth.json (stale ones kept providers alive in the model picker, - # #51071/#59761), the affected providers' model-cache rows, and - # value-matched config.yaml api_key mirrors. OAuth/device-code/ - # manual pool entries for the same provider are preserved. + # auth.json (stale ones kept providers alive in the model picker), + # the affected providers' model-cache rows, and value-matched + # config.yaml api_key mirrors. OAuth/device-code/manual pool entries + # for the same provider are preserved. from hermes_cli.credential_lifecycle import remove_provider_env_credential return remove_provider_env_credential(body.key) - try: + with _env_write_errors("DELETE /api/env failed", http_passthrough=True): result = await asyncio.to_thread(_run) if not result.get("found"): raise HTTPException(status_code=404, detail=f"{body.key} not found in .env") return result - except HTTPException: - raise - except ValueError as exc: - # remove_env_value raises ValueError for invalid key names. Surface - # the message to the SPA so the user understands why the delete was - # refused instead of seeing an opaque 500. Mirrors PUT /api/env. - raise HTTPException(status_code=400, detail=str(exc)) from exc - except Exception: - _log.exception("DELETE /api/env failed") - raise HTTPException(status_code=500, detail="Internal server error") @router.post("/api/env/reveal") @@ -812,15 +742,11 @@ async def reveal_env_var( ): """Return the real (unredacted) value of a single env var. - Protected by: - - Ephemeral session token (generated per server start, injected into SPA) - - Rate limiting (max 5 reveals per 30s window) - - Audit logging + Protected by the ephemeral session token (per server start, injected into + the SPA), rate limiting (max 5 reveals per 30s window) and audit logging. """ - # --- Token check --- _require_token(request) - # --- Rate limit --- now = time.time() cutoff = now - _REVEAL_WINDOW_SECONDS _reveal_timestamps[:] = [t for t in _reveal_timestamps if t > cutoff] @@ -828,7 +754,6 @@ async def reveal_env_var( raise HTTPException(status_code=429, detail="Too many reveal requests. Try again shortly.") _reveal_timestamps.append(now) - # --- Reveal --- def _run(): with _profile_scope(body.profile or profile): return load_env()