fix: avoid persisting borrowed credential secrets (#31416)

agent/credential_persistence.py

@@ -0,0 +1,174 @@
+"""Credential-pool disk-boundary sanitization helpers.
+
+These helpers define which credential-pool entries are references to borrowed
+runtime secrets and strip raw values before those entries are written to
+``auth.json``.  They intentionally have no dependency on ``hermes_cli.auth`` so
+both the pool model and the final auth-store write boundary can share the same
+policy without import cycles.
+"""
+
+from __future__ import annotations
+
+import hashlib
+import re
+from typing import Any, Dict, Mapping
+
+
+# Sources Hermes owns and can intentionally persist in auth.json.  Everything
+# else with a non-empty source is treated as borrowed/reference-only by default
+# so future external secret providers fail closed at the disk boundary.
+_PERSISTABLE_PROVIDER_SOURCES = frozenset({
+    ("anthropic", "hermes_pkce"),
+    ("minimax-oauth", "oauth"),
+    ("nous", "device_code"),
+    ("openai-codex", "device_code"),
+    ("xai-oauth", "loopback_pkce"),
+})
+
+_SAFE_SECRETISH_METADATA_KEYS = frozenset({
+    "secret_fingerprint",
+    "secret_source",
+    "token_type",
+    "scope",
+    "client_id",
+    "agent_key_id",
+    "agent_key_expires_at",
+    "agent_key_expires_in",
+    "agent_key_reused",
+    "agent_key_obtained_at",
+    "expires_at",
+    "expires_at_ms",
+    "expires_in",
+    "last_refresh",
+    "last_status",
+    "last_status_at",
+    "last_error_code",
+    "last_error_reason",
+    "last_error_message",
+    "last_error_reset_at",
+})
+
+_SECRET_VALUE_KEYS = frozenset({
+    "access_token",
+    "refresh_token",
+    "agent_key",
+    "api_key",
+    "apikey",
+    "api_token",
+    "auth_token",
+    "authorization",
+    "bearer_token",
+    "client_secret",
+    "credential",
+    "credentials",
+    "id_token",
+    "oauth_token",
+    "private_key",
+    "secret_key",
+    "session_token",
+    "password",
+    "secret",
+    "token",
+    "tokens",
+})
+
+_SECRET_VALUE_SUFFIXES = (
+    "_api_key",
+    "_api_token",
+    "_access_token",
+    "_auth_token",
+    "_refresh_token",
+    "_bearer_token",
+    "_client_secret",
+    "_id_token",
+    "_oauth_token",
+    "_private_key",
+    "_session_token",
+    "_secret_key",
+    "_password",
+    "_secret",
+    "_token",
+    "_key",
+)
+
+_CAMEL_CASE_BOUNDARY = re.compile(r"(?<=[a-z0-9])(?=[A-Z])")
+
+
+def _normalize_key(key: Any) -> str:
+    raw = str(key or "").strip()
+    raw = _CAMEL_CASE_BOUNDARY.sub("_", raw)
+    return raw.lower().replace("-", "_").replace(".", "_")
+
+
+def is_borrowed_credential_source(source: Any, provider_id: Any = None) -> bool:
+    """Return True when ``source`` points at a borrowed/reference-only secret."""
+    normalized_source = str(source or "").strip().lower()
+    if not normalized_source:
+        return False
+    if normalized_source == "manual" or normalized_source.startswith("manual:"):
+        return False
+    normalized_provider = str(provider_id or "").strip().lower()
+    return (normalized_provider, normalized_source) not in _PERSISTABLE_PROVIDER_SOURCES
+
+
+def _is_secret_payload_key(key: Any) -> bool:
+    normalized = _normalize_key(key)
+    if not normalized or normalized in _SAFE_SECRETISH_METADATA_KEYS:
+        return False
+    if normalized in _SECRET_VALUE_KEYS:
+        return True
+    return normalized.endswith(_SECRET_VALUE_SUFFIXES)
+
+
+def _fingerprint_value(value: Any) -> str | None:
+    if value is None:
+        return None
+    text = str(value)
+    if not text:
+        return None
+    digest = hashlib.sha256(text.encode("utf-8", errors="surrogatepass")).hexdigest()
+    return f"sha256:{digest[:16]}"
+
+
+def _credential_secret_fingerprint(payload: Mapping[str, Any]) -> str | None:
+    for key in ("agent_key", "access_token", "refresh_token", "api_key", "token", "secret"):
+        fingerprint = _fingerprint_value(payload.get(key))
+        if fingerprint:
+            return fingerprint
+
+    for key, value in payload.items():
+        if _is_secret_payload_key(key):
+            fingerprint = _fingerprint_value(value)
+            if fingerprint:
+                return fingerprint
+
+    existing = payload.get("secret_fingerprint")
+    if isinstance(existing, str) and existing.startswith("sha256:"):
+        return existing
+    return None
+
+
+def sanitize_borrowed_credential_payload(
+    payload: Mapping[str, Any],
+    provider_id: Any = None,
+) -> Dict[str, Any]:
+    """Return a disk-safe credential-pool payload.
+
+    Owned sources (manual entries and Hermes-owned OAuth/device-code state)
+    pass through unchanged.  Borrowed/reference-only sources keep labels,
+    source refs, status/cooldown metadata, counters, and a non-reversible
+    fingerprint, but raw secret value fields are removed.
+    """
+    result = dict(payload)
+    if not is_borrowed_credential_source(result.get("source"), provider_id):
+        return result
+
+    fingerprint = _credential_secret_fingerprint(result)
+    sanitized = {
+        key: value
+        for key, value in result.items()
+        if not _is_secret_payload_key(key)
+    }
+    if fingerprint:
+        sanitized["secret_fingerprint"] = fingerprint
+    return sanitized

agent/credential_pool.py

@@ -15,6 +15,10 @@ from typing import Any, Dict, List, Optional, Set, Tuple
 
 from hermes_constants import OPENROUTER_BASE_URL
 from hermes_cli.config import get_env_value, load_env
+from agent.credential_persistence import (
+    is_borrowed_credential_source,
+    sanitize_borrowed_credential_payload,
+)
 import hermes_cli.auth as auth_mod
 from hermes_cli.auth import (
     CODEX_ACCESS_TOKEN_REFRESH_SKEW_SECONDS,
@@ -86,7 +90,7 @@ CUSTOM_POOL_PREFIX = "custom:"
 _EXTRA_KEYS = frozenset({
     "token_type", "scope", "client_id", "portal_base_url", "obtained_at",
     "expires_in", "agent_key_id", "agent_key_expires_in", "agent_key_reused",
-    "agent_key_obtained_at", "tls",
+    "agent_key_obtained_at", "tls", "secret_source", "secret_fingerprint",
 })
 
 
@@ -161,7 +165,7 @@ class PooledCredential:
         for k, v in self.extra.items():
             if v is not None:
                 result[k] = v
-        return result
+        return sanitize_borrowed_credential_payload(result, self.provider)
 
     @property
     def runtime_api_key(self) -> str:
@@ -1433,8 +1437,12 @@ def _upsert_entry(entries: List[PooledCredential], provider: str, source: str, p
     if field_updates or extra_updates:
         if extra_updates:
             field_updates["extra"] = {**existing.extra, **extra_updates}
-        entries[existing_idx] = replace(existing, **field_updates)
-        return True
+        updated = replace(existing, **field_updates)
+        entries[existing_idx] = updated
+        # Runtime-only borrowed secret updates should refresh the in-memory
+        # entry without forcing auth.json churn when the disk-safe payload is
+        # unchanged (for example env keys with the same fingerprint).
+        return existing.to_dict() != updated.to_dict()
     return False
 
 
@@ -1772,6 +1780,35 @@ def _seed_from_env(provider: str, entries: List[PooledCredential]) -> Tuple[bool
     except ImportError:
         def _is_source_suppressed(_p, _s):  # type: ignore[misc]
             return False
+
+    def _secret_source_for_env(env_var: str) -> Optional[str]:
+        try:
+            from hermes_cli.env_loader import get_secret_source
+            source_label = get_secret_source(env_var)
+        except Exception:
+            source_label = None
+        return str(source_label).strip() if source_label else None
+
+    def _env_payload(
+        *,
+        source: str,
+        env_var: str,
+        token: str,
+        base_url: str,
+        auth_type: str = AUTH_TYPE_API_KEY,
+    ) -> Dict[str, Any]:
+        payload: Dict[str, Any] = {
+            "source": source,
+            "auth_type": auth_type,
+            "access_token": token,
+            "base_url": base_url,
+            "label": env_var,
+        }
+        secret_source = _secret_source_for_env(env_var)
+        if secret_source:
+            payload["secret_source"] = secret_source
+        return payload
+
     if provider == "openrouter":
         # Prefer ~/.hermes/.env over os.environ
         token = _get_env_prefer_dotenv("OPENROUTER_API_KEY")
@@ -1784,13 +1821,12 @@ def _seed_from_env(provider: str, entries: List[PooledCredential]) -> Tuple[bool
                 entries,
                 provider,
                 source,
-                {
-                    "source": source,
-                    "auth_type": AUTH_TYPE_API_KEY,
-                    "access_token": token,
-                    "base_url": OPENROUTER_BASE_URL,
-                    "label": "OPENROUTER_API_KEY",
-                },
+                _env_payload(
+                    source=source,
+                    env_var="OPENROUTER_API_KEY",
+                    token=token,
+                    base_url=OPENROUTER_BASE_URL,
+                ),
             )
         return changed, active_sources
 
@@ -1829,13 +1865,13 @@ def _seed_from_env(provider: str, entries: List[PooledCredential]) -> Tuple[bool
             entries,
             provider,
             source,
-            {
-                "source": source,
-                "auth_type": auth_type,
-                "access_token": token,
-                "base_url": base_url,
-                "label": env_var,
-            },
+            _env_payload(
+                source=source,
+                env_var=env_var,
+                token=token,
+                base_url=base_url,
+                auth_type=auth_type,
+            ),
         )
     return changed, active_sources
 
@@ -1847,8 +1883,11 @@ def _prune_stale_seeded_entries(entries: List[PooledCredential], active_sources:
         if _is_manual_source(entry.source)
         or entry.source in active_sources
         or not (
-            entry.source.startswith("env:")
-            or entry.source in {"claude_code", "hermes_pkce"}
+            is_borrowed_credential_source(entry.source, entry.provider)
+            # Hermes PKCE is Hermes-owned/persistable while present, but it is
+            # still a file-backed singleton and should disappear from the pool
+            # when the backing OAuth file is gone.
+            or entry.source == "hermes_pkce"
         )
     ]
     if len(retained) == len(entries):
@@ -1933,17 +1972,22 @@ def _seed_custom_pool(pool_key: str, entries: List[PooledCredential]) -> Tuple[b
 def load_pool(provider: str) -> CredentialPool:
     provider = (provider or "").strip().lower()
     raw_entries = read_credential_pool(provider)
+    raw_needs_sanitization = any(
+        isinstance(payload, dict)
+        and sanitize_borrowed_credential_payload(payload, provider) != payload
+        for payload in raw_entries
+    )
     entries = [PooledCredential.from_dict(provider, payload) for payload in raw_entries]
 
     if provider.startswith(CUSTOM_POOL_PREFIX):
         # Custom endpoint pool — seed from custom_providers config and model config
         custom_changed, custom_sources = _seed_custom_pool(provider, entries)
-        changed = custom_changed
+        changed = raw_needs_sanitization or custom_changed
         changed |= _prune_stale_seeded_entries(entries, custom_sources)
     else:
         singleton_changed, singleton_sources = _seed_from_singletons(provider, entries)
         env_changed, env_sources = _seed_from_env(provider, entries)
-        changed = singleton_changed or env_changed
+        changed = raw_needs_sanitization or singleton_changed or env_changed
         changed |= _prune_stale_seeded_entries(entries, singleton_sources | env_sources)
         changed |= _normalize_pool_priorities(provider, entries)