Custom endpoint API keys stored in plaintext config.yaml

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #67213 fix(credentials): unified provider key delete/update across .env, auth.json, config.yaml

  merged to main

  files: hermes_cli/config.py, hermes_cli/credential_lifecycle.py, hermes_cli/web_server.py, tests/hermes_cli/test_credential_lifecycle.py, tests/hermes_cli/test_env_export_line_lifecycle.py, tui_gateway/server.py

  Show the diff

  • hermes_cli/config.py

    @@ -7846,6 +7846,20 @@ def _quote_env_value(value: str) -> str:
         return f'"{escaped}"'
     
     
    +def _env_line_defines_key(line: str, key: str) -> bool:
    +    """True when a .env line assigns ``key`` — plain or ``export``-prefixed.
    +
    +    ``load_env()`` accepts the bash-compatible ``export KEY=value`` form
    +    (#6659), so the writers must recognise the same shape. Otherwise a
    +    hand-added ``export`` line is invisible to save (duplicate appended) and
    +    remove (line survives → the value resurrects on the next load, #40041).
    +    """
    +    stripped = line.strip()
    +    if stripped.startswith("export "):
    +        stripped = stripped[7:].lstrip()
    +    return stripped.startswith(f"{key}=")
    +
    +
     def save_env_value(key: str, value: str):
         """Save or update a value in ~/.hermes/.env."""
         if is_managed():
    @@ -7887,10 +7901,15 @@ def save_env_value(key: str, value: str):
     
         serialized_value = _quote_env_value(value)
     
    -    # Find and update or append
    +    # Find and update or append. Match both ``KEY=`` and the bash-compatible
    +    # ``export KEY=`` form — load_env() parses export lines (#6659), so a
    +    # user-added ``export GITHUB_TOKEN=...`` shows as set in every UI. If the
    +    # writer didn't match it, a save would append a SECOND line and a later
    +    # delete of that line would silently resurrect the old exported value
    +    # (#40041: "token detected but cannot be replaced through the UI").
         found = False
         for i, line in enumerate(lines):
    -        if line.strip().startswith(f"{key}="):
    +        if _env_line_defines_key(line, key):
                 lines[i] = f"{key}={serialized_value}\n"
                 found = True
                 break
    @@ -7969,7 +7988,7 @@ def remove_env_value(key: str) -> bool:
             lines = f.readlines()
         lines = _sanitize_env_lines(lines)
     
    -    new_lines = [line for line in lines if not line.strip().startswith(f"{key}=")]
    +    new_lines = [line for line in lines if not _env_line_defines_key(line, key)]
         found = len(new_lines) < len(lines)
     
         if found:
    @@ -8030,7 +8049,12 @@ def save_anthropic_api_key(value: str, save_fn=None):
     
     
     def save_env_value_secure(key: str, value: str) -> Dict[str, Any]:
    -    save_env_value(key, value)
    +    # Route through the unified credential lifecycle so a rotation via the
    +    # secret-capture path also refreshes any config.yaml mirror of the old
    +    # value and lifts a prior env-source suppression (#62269 fix family).
    +    from hermes_cli.credential_lifecycle import save_provider_env_credential
    +
    +    save_provider_env_credential(key, value)
         return {
             "success": True,
             "stored_as": key,
    @@ -8450,7 +8474,11 @@ def set_config_value(key: str, value: str):
             sys.exit(1)
         # Check if it's an API key (goes to .env)
         if _is_env_config_key(key):
    -        save_env_value(key.upper(), value)
    +        # Unified lifecycle: also rotates any config.yaml mirror of the old
    +        # value so a stale higher-precedence copy can't win (#62269).
    +        from hermes_cli.credential_lifecycle import save_provider_env_credential
    +
    +        save_provider_env_credential(key.upper(), value)
             print(f"✓ Set {key} in {get_env_path()}")
             return
         
    @@ -8556,8 +8584,12 @@ def unset_config_value(key: str):
             sys.exit(1)
     
         if _is_env_config_key(key):
    -        removed = remove_env_value(key.upper())
    -        if not removed:
    +        # Unified lifecycle: prune env-seeded credential_pool entries and
    +        # model-cache rows too, so `hermes config unset <KEY>` fully removes
    +        # the provider instead of leaving it resurrectable (#51071 family).
    +        from hermes_cli.credential_lifecycle import remove_provider_env_credential
    +
    +        if not remove_provider_env_credential(key.upper()).get("found"):
                 print(f"Config key not set: {key}", file=sys.stderr)
                 sys.exit(1)
             print(f"✓ Unset {key} from {get_env_path()}")

  • hermes_cli/credential_lifecycle.py

    @@ -0,0 +1,272 @@
    +"""Unified provider-credential lifecycle across every store Hermes reads.
    +
    +A provider API key can live in up to THREE stores at once:
    +
    +    1. ``~/.hermes/.env``                     — the canonical secret store
    +    2. ``~/.hermes/auth.json`` →
    +       ``credential_pool.<provider>[*]``      — env-seeded pool entries
    +       (``source == "env:<VAR>"``) persisted by the pool loader
    +    3. ``~/.hermes/config.yaml``              — inline mirrors written by the
    +       custom-endpoint flows (``model.api_key``, ``auxiliary.<task>.api_key``,
    +       ``custom_providers[*].api_key``)
    +
    +Historically the desktop/dashboard endpoints (PUT/DELETE ``/api/env``) and the
    +TUI-gateway RPCs only mutated store 1. That divergence is the root cause of a
    +whole bug family:
    +
    +    * #51071 / #59761 — deleting a key removes it from ``.env`` but the stale
    +      ``credential_pool`` entry (and ``provider_models_cache.json`` row)
    +      survives, so the provider keeps appearing in the model picker, even
    +      across restarts (the pool loader is additive-only).
    +    * #62269 — updating a key rewrites ``.env`` but leaves the OLD key in a
    +      higher-precedence ``config.yaml`` mirror (``model.api_key`` wins over
    +      env at client construction), producing persistent 401s with a key the
    +      UI no longer shows.
    +
    +This module is the single choke point: every surface that saves or removes a
    +provider credential should route through :func:`save_provider_env_credential`
    +/ :func:`remove_provider_env_credential` so all three stores stay consistent.
    +
    +OAuth preservation contract: removal only prunes credential-pool entries whose
    +``source`` is exactly ``env:<VAR>``. OAuth/device-code/manual/borrowed entries
    +(``device_code``, ``manual*``, ``gh_cli``, ``claude_code``, ``oauth``, …) and
    +the ``providers.<id>`` OAuth token blocks in auth.json are never touched —
    +deleting an API key must not revoke an OAuth grant for the same provider.
    +
    +Secrecy contract: no function in this module logs, prints, or returns a
    +credential value. Results carry key NAMES and config PATHS only.
    +"""
    +
    +from __future__ import annotations
    +
    +from typing import Any, Dict, List
    +
    +__all__ = [
    +    "save_provider_env_credential",
    +    "remove_provider_env_credential",
    +    "purge_env_credential_references",
    +]
    +
    +
    +def _providers_for_env_var(env_var: str) -> List[str]:
    +    """Provider ids whose registered api_key_env_vars include ``env_var``."""
    +    try:
    +        from hermes_cli.auth import PROVIDER_REGISTRY
    +    except Exception:
    +        return []
    +    hits: List[str] = []
    +    for pid, cfg in PROVIDER_REGISTRY.items():
    +        try:
    +            if env_var in (cfg.api_key_env_vars or ()):
    +                hits.append(pid)
    +        except Exception:
    +            continue
    +    return hits
    +
    +
    +def _prune_env_pool_entries(env_var: str) -> List[str]:
    +    """Drop ``credential_pool`` entries seeded from ``env:<env_var>``.
    +
    +    Operates across ALL providers in the pool (the source string names the
    +    env var unambiguously, and shared vars like GITHUB_TOKEN may seed more
    +    than one provider). Entries with any other source — OAuth, device-code,
    +    manual, borrowed-CLI — are preserved verbatim, as are the
    +    ``providers.<id>`` OAuth blocks.
    +
    +    Returns the list of provider ids that had entries pruned.
    +    """
    +    from hermes_cli.auth import _auth_store_lock, _load_auth_store, _save_auth_store
    +
    +    source = f"env:{env_var}"
    +    pruned: List[str] = []
    +    with _auth_store_lock():
    +        auth_store = _load_auth_store()
    +        pool = auth_store.get("credential_pool")
    +        if not isinstance(pool, dict):
    +            return pruned
    +        changed = False
    +        for provider in list(pool.keys()):
    +            entries = pool[provider]
    +            if not isinstance(entries, list):
    +                continue
    +            kept = [
    +                entry
    +                for entry in entries
    +                if not (isinstance(entry, dict) and entry.get("source") == source)
    +            ]
    +            if len(kept) == len(entries):
    +                continue
    +            changed = True
    +            pruned.append(provider)
    +            if kept:
    +                pool[provider] = kept
    +            else:
    +                del pool[provider]
    +        if changed:
    +            _save_auth_store(auth_store)
    +    return pruned
    +
    +
    +def _scrub_config_yaml_mirrors(old_value: str, new_value: str | None) -> List[str]:
    +    """Reconcile config.yaml api_key mirrors that hold ``old_value``.
    +
    +    Value-matched on purpose: we only touch a config entry when it provably
    +    holds the SAME credential that just changed in ``.env`` — an independent
    +    key the user configured for a different endpoint is left alone.
    +
    +    ``new_value=None`` removes the mirror field; a string replaces it.
    +    Operates on the RAW user config (never the defaults-merged view) so the
    +    write doesn't bake defaults into the user's file. Returns the dotted
    +    paths that were updated (names only — never values).
    +    """
    +    if not old_value:
    +        return []
    +    from utils import atomic_yaml_write, fast_safe_load
    +
    +    from hermes_cli.config import (
    +        get_config_path,
    +        require_readable_config_before_write,
    +    )
    +
    +    config_path = get_config_path()
    +    if not config_path.exists():
    +        return []
    +    try:
    +        with open(config_path, encoding="utf-8") as f:
    +            user_config = fast_safe_load(f) or {}
    +    except Exception:
    +        return []
    +    if not isinstance(user_config, dict):
    +        return []
    +
    +    touched: List[str] = []
    +
    +    def _fix(section: Any, key_path: str) -> None:
    +        if not isinstance(section, dict):
    +            return
    +        # "api" is the legacy alias for model.api_key kept by older configs.
    +        for field in ("api_key", "api"):
    +            current = section.get(field)
    +            if isinstance(current, str) and current == old_value:
    +                if new_value:
    +                    section[field] = new_value
    +                else:
    +                    section.pop(field, None)
    +                touched.append(f"{key_path}.{field}")
    +
    +    _fix(user_config.get("model"), "model")
    +
    +    aux = user_config.get("auxiliary")
    +    if isinstance(aux, dict):
    +        for task, slot_cfg in aux.items():
    +            _fix(slot_cfg, f"auxiliary.{task}")
    +
    +    custom = user_config.get("custom_providers")
    +    if isinstance(custom, list):
    +        for idx, entry in enumerate(custom):
    +            _fix(entry, f"custom_providers.{idx}")
    +    elif isinstance(custom, dict):
    +        for name, entry in custom.items():
    +            _fix(entry, f"custom_providers.{name}")
    +
    +    if touched:
    +        require_readable_config_before_write(config_path)
    +        atomic_yaml_write(config_path, user_config, sort_keys=False)
    +    return touched
    +
    +
    +def purge_env_credential_references(
    +    env_var: str, *, clear_models_cache: bool = True
    +) -> Dict[str, Any]:
    +    """Remove non-.env references to an env-var credential.
    +
    +    Prunes ``credential_pool`` env-seeded entries and (optionally) the
    +    affected providers' rows in ``provider_models_cache.json`` so the model
    +    picker stops advertising a provider whose key is gone (#59761).
    +    """
    +    pruned = _prune_env_pool_entries(env_var)
    +    providers = sorted(set(pruned) | set(_providers_for_env_var(env_var)))
    +    # Make the removal sticky the same way `hermes auth remove` does: a
    +    # lingering shell export (or another live process's os.environ) would
    +    # otherwise re-seed the pool entry on the next load_pool(). The matching
    +    # save path lifts the suppression on an explicit re-add.
    +    try:
    +        from hermes_cli.auth import suppress_credential_source
    +
    +        for provider in providers:
    +            suppress_credential_source(provider, f"env:{env_var}")
    +    except Exception:
    +        pass
    +    if clear_models_cache and providers:
    +        try:
    +            from hermes_cli.models import clear_provider_models_cache
    +
    +            for provider in providers:
    +                clear_provider_models_cache(provider)
    +        except Exception:
    +            # Cache cleanup is best-effort — a failure here must not block
    +            # the credential removal itself.
    +            pass
    +    return {"pool_pruned": pruned, "providers": providers}
    +
    +
    +def save_provider_env_credential(env_var: str, value: str) -> Dict[str, Any]:
    +    """Save/update a credential in ``.env`` and reconcile every mirror.
    +
    +    After the ``.env`` write, any config.yaml mirror that held the PREVIOUS
    +    value of this var (``model.api_key`` etc.) is updated to the new value so
    +    a stale higher-precedence copy cannot shadow the rotation (#62269).
    +    Suppressed ``env:<VAR>`` pool sources are re-enabled so a deliberate
    +    re-add through the UI behaves like ``hermes auth add``.
    +    """
    +    from hermes_cli.config import load_env, save_env_value
    +
    +    old_value = load_env().get(env_var)
    +    save_env_value(env_var, value)
    +
    +    config_updates: List[str] = []
    +    if value and old_value and old_value != value:
    +        config_updates = _scrub_config_yaml_mirrors(old_value, value)
    +
    +    # A prior UI/CLI removal may have suppressed this env source; a fresh
    +    # save is an explicit re-add, so lift the suppression for every provider
    +    # that reads this var.
    +    try:
    +        from hermes_cli.auth import unsuppress_credential_source
    +
    +        for provider in _providers_for_env_var(env_var):
    +            unsuppress_credential_source(provider, f"env:{env_var}")
    +    except Exception:
    +        pass
    +
    +    return {"ok": True, "key": env_var, "config_updates": config_updates}
    +
    +
    +def remove_provider_env_credential(env_var: str) -> Dict[str, Any]:
    +    """Remove a credential from EVERY store it lives in.
    +
    +    Clears the ``.env`` entry (and process env), prunes env-seeded
    +    ``credential_pool`` entries, drops the affected providers' model-cache
    +    rows, and removes any config.yaml mirror holding the same value.
    +    OAuth/device-code/manual credentials are preserved (see module docstring).
    +
    +    ``found`` is True when ANY store held the credential — callers that
    +    previously 404'd on ".env miss" should key off this instead so a stale
    +    pool-only entry can still be cleaned up through the same button.
    +    """
    +    from hermes_cli.config import load_env, remove_env_value
    +
    +    old_value = load_env().get(env_var)
    +    removed_from_env = remove_env_value(env_var)
    +    refs = purge_env_credential_references(env_var)
    +    config_scrubbed = _scrub_config_yaml_mirrors(old_value, None) if old_value else []
    +
    +    return {
    +        "ok": True,
    +        "key": env_var,
    +        "removed": removed_from_env,
    +        "pool_pruned": refs["pool_pruned"],
    +        "providers": refs["providers"],
    +        "config_scrubbed": config_scrubbed,
    +        "found": bool(removed_from_env or refs["pool_pruned"] or config_scrubbed),
    +    }

  • hermes_cli/web_server.py

    @@ -6354,8 +6354,15 @@ def _row(var_name: str, info: dict, *, custom: bool = False) -> dict:
     async def set_env_var(body: EnvVarUpdate, profile: Optional[str] = None):
         try:
             with _profile_scope(body.profile or profile):
    -            save_env_value(body.key, body.value)
    -        return {"ok": True, "key": body.key}
    +            # 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).
    +            from hermes_cli.credential_lifecycle import save_provider_env_credential
    +
    +            result = save_provider_env_credential(body.key, body.value)
    +        return result
         except ValueError as exc:
             # save_env_value raises ValueError for invalid names and for keys
             # on the denylist (LD_PRELOAD, PATH, PYTHONPATH, …). Surface the
    @@ -6473,10 +6480,18 @@ async def validate_provider_credential(body: EnvVarUpdate, request: Request):
     async def remove_env_var(body: EnvVarDelete, profile: Optional[str] = None):
         try:
             with _profile_scope(body.profile or profile):
    -            removed = remove_env_value(body.key)
    -        if not removed:
    +            # 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.
    +            from hermes_cli.credential_lifecycle import remove_provider_env_credential
    +
    +            result = remove_provider_env_credential(body.key)
    +        if not result.get("found"):
                 raise HTTPException(status_code=404, detail=f"{body.key} not found in .env")
    -        return {"ok": True, "key": body.key}
    +        return result
         except HTTPException:
             raise
         except ValueError as exc:

  • tests/hermes_cli/test_credential_lifecycle.py

    @@ -0,0 +1,335 @@
    +"""E2E tests for the unified provider-credential lifecycle (#51071 #59761 #62269).
    +
    +A provider API key can live in .env, auth.json's credential_pool, and
    +config.yaml mirrors at once. These tests drive the REAL dashboard endpoint
    +handlers (PUT/DELETE /api/env) against real on-disk fixtures in a temp
    +HERMES_HOME (tests/conftest.py isolation) and assert every store agrees
    +afterwards.
    +
    +All fake secrets are constructed at runtime so no key-shaped literal ever
    +lands in the repo.
    +"""
    +
    +import json
    +
    +import pytest
    +from fastapi.testclient import TestClient
    +
    +from hermes_cli.web_server import _SESSION_TOKEN, app
    +
    +client = TestClient(app)
    +HEADERS = {"X-Hermes-Session-Token": _SESSION_TOKEN}
    +
    +# Runtime-constructed fake credentials (never literal key-shaped strings).
    +FAKE_ZAI_KEY = "zk-" + "a" * 24
    +FAKE_OAUTH_TOKEN = "oa-" + "b" * 24
    +NEW_KEY = "zk-" + "c" * 24
    +
    +
    [redacted:email]
    +def hermes_home(monkeypatch, tmp_path):
    +    """Fresh HERMES_HOME with .env + auth.json + config.yaml fixtures."""
    +    home = tmp_path / "cred_home"
    +    home.mkdir()
    +    monkeypatch.setenv("HERMES_HOME", str(home))
    +    from hermes_cli.config import invalidate_env_cache
    +
    +    invalidate_env_cache()
    +    return home
    +
    +
    +def _write_env(home, **pairs):
    +    home.joinpath(".env").write_text(
    +        "".join(f"{k}={v}\n" for k, v in pairs.items()), encoding="utf-8"
    +    )
    +    from hermes_cli.config import invalidate_env_cache
    +
    +    invalidate_env_cache()
    +
    +
    +def _write_auth(home, pool):
    +    home.joinpath("auth.json").write_text(
    +        json.dumps({"credential_pool": pool}), encoding="utf-8"
    +    )
    +
    +
    +def _read_auth(home):
    +    return json.loads(home.joinpath("auth.json").read_text(encoding="utf-8"))
    +
    +
    +def _zai_pool_fixture():
    +    """One env-seeded API-key entry plus one OAuth entry for the same provider."""
    +    return {
    +        "zai": [
    +            {
    +                "id": "e1",
    +                "label": "env",
    +                "auth_type": "api_key",
    +                "priority": 0,
    +                "source": "env:ZAI_API_KEY",
    +                "access_token": FAKE_ZAI_KEY,
    +            },
    +            {
    +                "id": "o1",
    +                "label": "oauth",
    +                "auth_type": "oauth",
    +                "priority": 0,
    +                "source": "device_code",
    +                "access_token": FAKE_OAUTH_TOKEN,
    +                "refresh_token": "rt-" + "d" * 16,
    +            },
    +        ]
    +    }
    +
    +
    +# ---------------------------------------------------------------------------
    +# DELETE — #51071 / #59761: stale credential_pool entries must be pruned
    +# ---------------------------------------------------------------------------
    +
    +
    +def test_delete_env_key_prunes_env_seeded_pool_entry(hermes_home):
    +    _write_env(hermes_home, ZAI_API_KEY=[redacted:secret-assignment]
    +    _write_auth(hermes_home, _zai_pool_fixture())
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "ZAI_API_KEY"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +    body = resp.json()
    +    assert body["ok"] is True
    +    assert "zai" in body["pool_pruned"]
    +
    +    # .env cleared
    +    from hermes_cli.config import load_env
    +
    +    assert "ZAI_API_KEY" not in load_env()
    +
    +    # auth.json: env-seeded entry gone, OAuth entry preserved
    +    store = _read_auth(hermes_home)
    +    sources = [e["source"] for e in store["credential_pool"]["zai"]]
    +    assert "env:ZAI_API_KEY" not in sources
    +    assert "device_code" in sources, "OAuth grant must survive an API-key delete"
    +
    +
    +def test_delete_env_key_removes_provider_pool_key_when_emptied(hermes_home):
    +    """A provider whose ONLY pool entry was env-seeded disappears entirely."""
    +    _write_env(hermes_home, ZAI_API_KEY=[redacted:secret-assignment]
    +    _write_auth(
    +        hermes_home,
    +        {"zai": [_zai_pool_fixture()["zai"][0]]},  # env entry only
    +    )
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "ZAI_API_KEY"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +    store = _read_auth(hermes_home)
    +    assert "zai" not in store.get("credential_pool", {}), (
    +        "provider must vanish from credential_pool so the model picker "
    +        "stops listing it (#51071)"
    +    )
    +
    +
    +def test_delete_survives_pool_reload(hermes_home):
    +    """#59761: the pool loader must not resurrect the entry after 'restart'."""
    +    _write_env(hermes_home, ZAI_API_KEY=[redacted:secret-assignment]
    +    _write_auth(hermes_home, {"zai": [_zai_pool_fixture()["zai"][0]]})
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "ZAI_API_KEY"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +
    +    # Simulate restart: reload the pool from disk the way startup does.
    +    from agent.credential_pool import load_pool
    +
    +    entries = load_pool("zai").entries()
    +    assert entries == [], f"stale entries resurrected: {[e.source for e in entries]}"
    +
    +
    +def test_delete_clears_provider_models_cache(hermes_home):
    +    _write_env(hermes_home, ZAI_API_KEY=[redacted:secret-assignment]
    +    _write_auth(hermes_home, {"zai": [_zai_pool_fixture()["zai"][0]]})
    +    cache_path = hermes_home / "provider_models_cache.json"
    +    cache_path.write_text(
    +        json.dumps({"zai": {"models": ["glm-5"], "ts": 0}}), encoding="utf-8"
    +    )
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "ZAI_API_KEY"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +    if cache_path.exists():
    +        cache = json.loads(cache_path.read_text(encoding="utf-8"))
    +        assert "zai" not in cache
    +
    +
    +def test_delete_pool_only_credential_still_cleans_up(hermes_home):
    +    """Stale pool entry with NO .env line (the #59761 restart state) is
    +    removable through the same delete button instead of 404ing."""
    +    _write_env(hermes_home)  # empty .env
    +    _write_auth(hermes_home, {"zai": [_zai_pool_fixture()["zai"][0]]})
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "ZAI_API_KEY"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +    store = _read_auth(hermes_home)
    +    assert "zai" not in store.get("credential_pool", {})
    +
    +
    +def test_delete_unknown_key_404s(hermes_home):
    +    _write_env(hermes_home)
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "NEVER_SET_KEY"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 404
    +
    +
    +def test_delete_does_not_touch_other_providers(hermes_home):
    +    _write_env(hermes_home, ZAI_API_KEY=[redacted:secret-assignment]
    +    other_key = "dk-" + "e" * 24
    +    pool = _zai_pool_fixture()
    +    pool["deepseek"] = [
    +        {
    +            "id": "d1",
    +            "label": "env",
    +            "auth_type": "api_key",
    +            "priority": 0,
    +            "source": "env:DEEPSEEK_API_KEY",
    +            "access_token": other_key,
    +        }
    +    ]
    +    _write_auth(hermes_home, pool)
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "ZAI_API_KEY"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +    store = _read_auth(hermes_home)
    +    assert [e["source"] for e in store["credential_pool"]["deepseek"]] == [
    +        "env:DEEPSEEK_API_KEY"
    +    ]
    +
    +
    +# ---------------------------------------------------------------------------
    +# UPDATE — #62269: config.yaml mirrors of the old key must rotate with .env
    +# ---------------------------------------------------------------------------
    +
    +
    +def _write_config(home, text):
    +    home.joinpath("config.yaml").write_text(text, encoding="utf-8")
    +
    +
    +def test_update_rotates_config_yaml_model_mirror(hermes_home):
    +    old = "sk-oe-" + "f" * 24
    +    new = "sk-oe-" + "g" * 24
    +    _write_env(hermes_home, OPENAI_API_KEY=old)
    +    _write_config(
    +        hermes_home,
    +        "model:\n"
    +        "  provider: custom\n"
    +        "  default: my-model\n"
    +        "  base_url: https://llm.example.test/v1\n"
    +        f"  api_key: [redacted:secret-assignment]",
    +    )
    +
    +    resp = client.put(
    +        "/api/env", json={"key": "OPENAI_API_KEY", "value": new}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +    assert "model.api_key" in resp.json().get("config_updates", [])
    +
    +    cfg_text = hermes_home.joinpath("config.yaml").read_text(encoding="utf-8")
    +    assert old not in cfg_text, "stale old key left in config.yaml (#62269)"
    +    assert new in cfg_text, "config.yaml mirror not rotated to the new key"
    +
    +    from hermes_cli.config import load_env
    +
    +    assert load_env()["OPENAI_API_KEY"] == new
    +
    +
    +def test_update_rotates_custom_provider_mirror(hermes_home):
    +    old = "sk-cp-" + "h" * 24
    +    new = "sk-cp-" + "i" * 24
    +    _write_env(hermes_home, OPENAI_API_KEY=old)
    +    _write_config(
    +        hermes_home,
    +        "custom_providers:\n"
    +        "  - name: myendpoint\n"
    +        "    base_url: https://llm.example.test/v1\n"
    +        f"    api_key: [redacted:secret-assignment]",
    +    )
    +
    +    resp = client.put(
    +        "/api/env", json={"key": "OPENAI_API_KEY", "value": new}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +    cfg_text = hermes_home.joinpath("config.yaml").read_text(encoding="utf-8")
    +    assert old not in cfg_text
    +    assert new in cfg_text
    +
    +
    +def test_update_leaves_unrelated_config_keys_alone(hermes_home):
    +    """A DIFFERENT key configured inline must not be rewritten by value-match."""
    +    old = "sk-un-" + "j" * 24
    +    unrelated = "sk-un-" + "k" * 24
    +    _write_env(hermes_home, OPENAI_API_KEY=old)
    +    _write_config(hermes_home, f"model:\n  provider: custom\n  api_key: [redacted:secret-assignment]")
    +
    +    resp = client.put(
    +        "/api/env",
    +        json={"key": "OPENAI_API_KEY", "value": "sk-un-" + "l" * 24},
    +        headers=HEADERS,
    +    )
    +    assert resp.status_code == 200
    +    cfg_text = hermes_home.joinpath("config.yaml").read_text(encoding="utf-8")
    +    assert unrelated in cfg_text, "unrelated inline key must be preserved"
    +
    +
    +def test_delete_scrubs_config_yaml_mirror(hermes_home):
    +    old = "sk-dl-" + "m" * 24
    +    _write_env(hermes_home, OPENAI_API_KEY=old)
    +    _write_config(hermes_home, f"model:\n  provider: custom\n  api_key: [redacted:secret-assignment]")
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "OPENAI_API_KEY"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +    assert "model.api_key" in resp.json()["config_scrubbed"]
    +    cfg_text = hermes_home.joinpath("config.yaml").read_text(encoding="utf-8")
    +    assert old not in cfg_text
    +
    +
    +# ---------------------------------------------------------------------------
    +# Suppression round-trip: delete sticks, re-add lifts it
    +# ---------------------------------------------------------------------------
    +
    +
    +def test_delete_then_resave_round_trip(hermes_home):
    +    _write_env(hermes_home, ZAI_API_KEY=[redacted:secret-assignment]
    +    _write_auth(hermes_home, {"zai": [_zai_pool_fixture()["zai"][0]]})
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "ZAI_API_KEY"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +
    +    from hermes_cli.auth import is_source_suppressed
    +
    +    assert is_source_suppressed("zai", "env:ZAI_API_KEY"), (
    +        "delete must suppress the env source so a lingering shell export "
    +        "can't re-seed the pool"
    +    )
    +
    +    resp = client.put(
    +        "/api/env", json={"key": "ZAI_API_KEY", "value": NEW_KEY}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +    assert not is_source_suppressed("zai", "env:ZAI_API_KEY"), (
    +        "an explicit re-save must lift the suppression (like `hermes auth add`)"
    +    )
    +
    +    from hermes_cli.config import load_env
    +
    +    assert load_env()["ZAI_API_KEY"] == NEW_KEY

  • tests/hermes_cli/test_env_export_line_lifecycle.py

    @@ -0,0 +1,125 @@
    +"""Regression tests for the Tools & Keys GitHub PAT save/remove path (#40041).
    +
    +Users following generic docs add ``export GITHUB_TOKEN=[redacted:secret-assignment] to
    +``~/.hermes/.env``. ``load_env()`` parses the export prefix (#6659), so every
    +UI shows the token as set (green light) — but ``save_env_value`` /
    +``remove_env_value`` only matched plain ``KEY=`` lines. Result: the UI could
    +neither replace nor remove the token (delete 404s as "not found in .env";
    +save appends a duplicate line that a later delete removes while the export
    +line silently resurrects the old value).
    +
    +Fake tokens are constructed at runtime — no key-shaped literals on disk.
    +"""
    +
    +import pytest
    +from fastapi.testclient import TestClient
    +
    +from hermes_cli.web_server import _SESSION_TOKEN, app
    +
    +client = TestClient(app)
    +HEADERS = {"X-Hermes-Session-Token": _SESSION_TOKEN}
    +
    +# Classic-PAT-shaped token, constructed at runtime (36 chars after prefix).
    +OLD_PAT = "ghp_" + "A" * 36
    +NEW_PAT = "ghp_" + "B" * 36
    +
    +
    [redacted:email]
    +def hermes_home(monkeypatch, tmp_path):
    +    home = tmp_path / "pat_home"
    +    home.mkdir()
    +    monkeypatch.setenv("HERMES_HOME", str(home))
    +    from hermes_cli.config import invalidate_env_cache
    +
    +    invalidate_env_cache()
    +    return home
    +
    +
    +def _write_env_raw(home, text):
    +    home.joinpath(".env").write_text(text, encoding="utf-8")
    +    from hermes_cli.config import invalidate_env_cache
    +
    +    invalidate_env_cache()
    +
    +
    +def test_classic_pat_save_via_endpoint_succeeds(hermes_home):
    +    """Saving a ghp_* classic PAT through the env endpoint must not 500 —
    +    GITHUB_TOKEN is a REST/Skills-Hub credential, not a Copilot one."""
    +    resp = client.put(
    +        "/api/env", json={"key": "GITHUB_TOKEN", "value": NEW_PAT}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200, resp.text
    +
    +    from hermes_cli.config import load_env
    +
    +    assert load_env()["GITHUB_TOKEN"] == NEW_PAT
    +
    +
    +def test_remove_export_prefixed_token(hermes_home):
    +    """DELETE must clear an ``export KEY=...`` line, not 404 on it."""
    +    _write_env_raw(hermes_home, f"export GITHUB_TOKEN=[redacted:secret-assignment]")
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "GITHUB_TOKEN"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200, (
    +        "export-prefixed lines are parsed by load_env (UI shows the token as "
    +        "set) so the delete path must recognise them too (#40041)"
    +    )
    +
    +    env_text = hermes_home.joinpath(".env").read_text(encoding="utf-8")
    +    assert OLD_PAT not in env_text
    +
    +    from hermes_cli.config import load_env
    +
    +    assert "GITHUB_TOKEN" not in load_env()
    +
    +
    +def test_update_export_prefixed_token_does_not_duplicate(hermes_home):
    +    """Saving over an ``export KEY=`` line must replace it in place."""
    +    _write_env_raw(hermes_home, f"export GITHUB_TOKEN=[redacted:secret-assignment]")
    +
    +    resp = client.put(
    +        "/api/env", json={"key": "GITHUB_TOKEN", "value": NEW_PAT}, headers=HEADERS
    +    )
    +    assert resp.status_code == 200
    +
    +    env_text = hermes_home.joinpath(".env").read_text(encoding="utf-8")
    +    assert OLD_PAT not in env_text, "old exported token line must be replaced"
    +    assert env_text.count("GITHUB_TOKEN") == 1, (
    +        "save must not append a duplicate GITHUB_TOKEN line alongside the "
    +        "export-prefixed one"
    +    )
    +
    +    from hermes_cli.config import load_env
    +
    +    assert load_env()["GITHUB_TOKEN"] == NEW_PAT
    +
    +
    +def test_plain_line_save_and_remove_still_work(hermes_home):
    +    """Sanity: the ordinary KEY= path is unchanged."""
    +    from hermes_cli.config import load_env, remove_env_value, save_env_value
    +
    +    save_env_value("GITHUB_TOKEN", OLD_PAT)
    +    assert load_env()["GITHUB_TOKEN"] == OLD_PAT
    +    save_env_value("GITHUB_TOKEN", NEW_PAT)
    +    env_text = hermes_home.joinpath(".env").read_text(encoding="utf-8")
    +    assert env_text.count("GITHUB_TOKEN") == 1
    +    assert remove_env_value("GITHUB_TOKEN") is True
    +    assert "GITHUB_TOKEN" not in load_env()
    +
    +
    +def test_export_line_with_comment_untouched(hermes_home):
    +    """Commented-out export lines are not live assignments — leave them."""
    +    _write_env_raw(
    +        hermes_home,
    +        f"# export GITHUB_TOKEN=[redacted:secret-assignment]",
    +    )
    +
    +    resp = client.request(
    +        "DELETE", "/api/env", json={"key": "GITHUB_TOKEN"}, headers=HEADERS
    +    )
    +    assert resp.status_code == 404
    +    env_text = hermes_home.joinpath(".env").read_text(encoding="utf-8")
    +    assert "# export GITHUB_TOKEN=" in env_text
    +    assert "OTHER_KEY=value" in env_text

  • tui_gateway/server.py

    @@ -13984,7 +13984,7 @@ def _(rid, params: dict) -> dict:
         """
         try:
             from hermes_cli.auth import PROVIDER_REGISTRY
    -        from hermes_cli.config import is_managed, save_env_value
    +        from hermes_cli.config import is_managed
             from hermes_cli.inventory import build_models_payload, load_picker_context
     
             slug = (params.get("slug") or "").strip()
    @@ -14008,9 +14008,13 @@ def _(rid, params: dict) -> dict:
             if not pconfig.api_key_env_vars:
                 return _err(rid, 4004, f"no env var defined for {pconfig.name}")
     
    -        # Save the key to ~/.hermes/.env
    +        # Save the key to ~/.hermes/.env via the unified credential lifecycle
    +        # so any stale config.yaml mirror of the previous key (model.api_key,
    +        # custom_providers[*].api_key) is rotated in the same action (#62269).
             env_var = pconfig.api_key_env_vars[0]
    -        save_env_value(env_var, api_key)
    +        from hermes_cli.credential_lifecycle import save_provider_env_credential
    +
    +        save_provider_env_credential(env_var, api_key)
             # Also set in current process so the refreshed inventory sees it.
             import os
     
    @@ -14064,7 +14068,7 @@ def _(rid, params: dict) -> dict:
         """
         try:
             from hermes_cli.auth import PROVIDER_REGISTRY, clear_provider_auth
    -        from hermes_cli.config import remove_env_value
    +        from hermes_cli.credential_lifecycle import remove_provider_env_credential
     
             slug = (params.get("slug") or "").strip()
             if not slug:
    @@ -14074,13 +14078,19 @@ def _(rid, params: dict) -> dict:
             cleared_env = False
             cleared_auth = False
     
    -        # Remove API key env vars from .env and process
    +        # Remove API key env vars from .env and process, plus every mirror
    +        # (env-seeded credential_pool entries, provider model cache rows,
    +        # value-matched config.yaml api_key copies) via the unified helper —
    +        # otherwise the provider resurrects in the picker after restart
    +        # (#51071 / #59761).
             if pconfig and pconfig.api_key_env_vars:
                 for ev in pconfig.api_key_env_vars:
    -                if remove_env_value(ev):
    +                if remove_provider_env_credential(ev).get("found"):
                         cleared_env = True
     
    -        # Clear OAuth / credential pool state
    +        # Clear OAuth / credential pool state. This is a full provider
    +        # disconnect (TUI "disconnect" action), so removing OAuth grants
    +        # here is the documented intent — unlike the key-only delete paths.
             cleared_auth = clear_provider_auth(slug)
     
             if not cleared_env and not cleared_auth:

• #71141 fix: custom endpoint keys go to .env, and Save keeps the whole model list

  merged to main

  files: apps/desktop/src/app/settings/custom-endpoints-settings.tsx, apps/desktop/src/types/hermes.ts, hermes_cli/config.py, hermes_cli/main.py, hermes_cli/model_setup_flows.py, hermes_cli/web_server.py, tests/cli/test_cli_provider_resolution.py, tests/hermes_cli/test_web_server.py