diff --git a/hermes_cli/credential_lifecycle.py b/hermes_cli/credential_lifecycle.py index 7dbfc3fee7..3448add360 100644 --- a/hermes_cli/credential_lifecycle.py +++ b/hermes_cli/credential_lifecycle.py @@ -218,6 +218,15 @@ def save_provider_env_credential(env_var: str, value: str) -> Dict[str, Any]: a stale higher-precedence copy cannot shadow the rotation (#62269). Suppressed ``env:`` pool sources are re-enabled so a deliberate re-add through the UI behaves like ``hermes auth add``. + + The save also forces an immediate ``load_pool()`` for every provider + registered against this env var so the env-seeded ``credential_pool`` + entry is materialized to ``auth.json`` right now — the live runtime reads + from the pool, and before #96058 the Desktop "Save" action only touched + ``.env`` while ``auth.json``'s mtime stayed unchanged, so an OpenCode Go + (or any other env-backed provider) request kept 401'ing until the user + ran ``hermes auth add --type api-key`` separately. This makes + the Desktop save's effect on disk match what ``hermes auth add`` does. """ from hermes_cli.config import load_env, save_env_value @@ -239,6 +248,19 @@ def save_provider_env_credential(env_var: str, value: str) -> Dict[str, Any]: except Exception: pass + # Materialize the env-seeded credential_pool entry to auth.json NOW so the + # next request authenticates against the just-saved key. ``load_pool`` is + # idempotent and additive-only for env sources (#9331), so re-running it + # is safe even when the pool already had this entry. Best-effort: a + # failure here must not mask the successful .env write above. + try: + from agent.credential_pool import load_pool + + for provider in _providers_for_env_var(env_var): + load_pool(provider) + except Exception: + pass + return {"ok": True, "key": env_var, "config_updates": config_updates} diff --git a/tests/hermes_cli/test_credential_lifecycle.py b/tests/hermes_cli/test_credential_lifecycle.py index 88fc7ec0a3..7b2f507881 100644 --- a/tests/hermes_cli/test_credential_lifecycle.py +++ b/tests/hermes_cli/test_credential_lifecycle.py @@ -145,6 +145,118 @@ def test_update_rotates_config_yaml_model_mirror(hermes_home): +# --------------------------------------------------------------------------- +# Desktop PUT /api/env — #96058: credential_pool must be materialized so the +# live runtime picks up the new key without waiting for its next background +# load_pool() or a separate `hermes auth add`. +# --------------------------------------------------------------------------- + + +# OpenCode Go is a registered api_key provider with api_key_env_vars containing +# the single env var OPENCODE_GO_API_KEY — exercising the exact reproducer +# from issue #96058 (Ubuntu 24.04, openai_sdk 2.24.0, provider=opencode-go). +OPENCODE_KEY_NEW = "ocg-" + "e" * 28 + + +def test_put_api_env_materializes_credential_pool_entry(hermes_home): + """Desktop Providers → API keys → Save must write a credential_pool entry. + + Pre-fix: save_provider_env_credential only mutated .env. The live pool + kept authenticating with a stale higher-precedence config.yaml mirror or + the old cached credential until a separate ``hermes auth add opencode-go`` + ran. auth.json mtime was unchanged before/after Save (#96058). + + Post-fix: the same PUT /api/env call must also materialize an entry under + ``credential_pool.`` in auth.json so the next request + authenticates immediately, matching ``hermes auth add --type + api-key`` behavior. + """ + # Start clean: empty auth.json so the only way a pool entry shows up is + # via the PUT /api/env handler we're testing. + _write_auth(hermes_home, {}) + + resp = client.put( + "/api/env", + json={"key": "OPENCODE_GO_API_KEY", "value": OPENCODE_KEY_NEW}, + headers=HEADERS, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body.get("ok") is True + assert body.get("key") == "OPENCODE_GO_API_KEY" + + # auth.json must now have a credential_pool entry for opencode-go. The + # exact source string lives in source="env:OPENCODE_GO_API_KEY" — env + # sources are sanitized on disk (the raw token is replaced with a + # fingerprint; the canonical secret lives in .env and gets re-hydrated by + # load_pool() on each read). The critical observable is: load_pool() on + # the next call returns an in-memory entry carrying the just-saved token. + auth = _read_auth(hermes_home) + pool = auth.get("credential_pool", {}) + assert "opencode-go" in pool, ( + "PUT /api/env did not materialize a credential_pool entry for " + "opencode-go (#96058)" + ) + entries = pool["opencode-go"] + assert isinstance(entries, list) and entries, pool + matched_disk = [ + e for e in entries + if isinstance(e.get("source"), str) + and e["source"] == "env:OPENCODE_GO_API_KEY" + ] + assert matched_disk, ( + f"credential_pool.opencode-go env-seeded reference missing on disk; " + f"got {entries!r}" + ) + # And: a fresh load_pool() must surface the just-saved token to the + # runtime. This is the actual end-to-end contract — anything weaker + # means the OpenAI client will 401 because it never receives the new key. + from agent.credential_pool import load_pool + pool_obj = load_pool("opencode-go") + runtime_entries = pool_obj.entries() + matched_runtime = [ + e for e in runtime_entries + if e.access_token == OPENCODE_KEY_NEW + and isinstance(e.source, str) + and e.source == "env:OPENCODE_GO_API_KEY" + ] + assert matched_runtime, ( + "load_pool('opencode-go') did not surface the just-saved token; " + "the live runtime will keep 401'ing (#96058). " + f"Got sources: {[e.source for e in runtime_entries]!r}" + ) + assert matched_runtime[0].auth_type == "api_key" + + +def test_put_api_env_writes_auth_json_for_provider(hermes_home): + """Sentinel for #96058: PUT /api/env must modify auth.json on disk. + + The reported symptom was ``stat -c '%y' ~/.hermes/auth.json`` returning + the same value before and after the Desktop Save. After the fix the file's + mtime advances because the save materializes the env-seeded pool entry. + """ + _write_auth(hermes_home, {}) + auth_path = hermes_home / "auth.json" + assert auth_path.exists() + mtime_before = auth_path.stat().st_mtime_ns + + # Tiny delay so a write is observable even on filesystems with 1s mtime + # resolution. Use ns precision so this is reliable on every FS. + import time + time.sleep(0.05) + + resp = client.put( + "/api/env", + json={"key": "OPENCODE_GO_API_KEY", "value": OPENCODE_KEY_NEW}, + headers=HEADERS, + ) + assert resp.status_code == 200, resp.text + + assert auth_path.stat().st_mtime_ns > mtime_before, ( + "auth.json was not modified by the Desktop Save — bug #96058" + ) + + # --------------------------------------------------------------------------- # Suppression round-trip: delete sticks, re-add lifts it # ---------------------------------------------------------------------------