diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 60cd375d358..e62ed59b9b6 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -1527,6 +1527,48 @@ def _seed_from_singletons(provider: str, entries: List[PooledCredential]) -> Tup except ImportError: pass + # API-key vs OAuth is a user-visible choice at `hermes setup` ("Claude + # Pro/Max subscription" vs "Anthropic API key"). The signal that the + # user picked the API-key path is: ANTHROPIC_API_KEY set in the env, + # AND no OAuth env vars set — `save_anthropic_api_key()` writes the + # API key and zeros ANTHROPIC_TOKEN; `save_anthropic_oauth_token()` + # does the inverse. When that signal is present we MUST NOT seed + # autodiscovered OAuth tokens (~/.claude/.credentials.json from the + # Claude Code CLI, hermes_pkce creds from a previous OAuth login) + # into the anthropic pool — otherwise rotation on a 401/429 silently + # flips the session onto an OAuth credential, which forces the Claude + # Code identity injection, `mcp_` tool-name rewrite, and claude-cli + # User-Agent header (`agent/anthropic_adapter.py:2128`). Users who + # explicitly opted into the API-key path are explicitly opting OUT of + # that masquerade. Prefer ~/.hermes/.env over os.environ for the + # same reason `_seed_from_env` does — that's the authoritative file + # that `hermes setup` writes. + _env_file = load_env() + + def _env_val(key: str) -> str: + return (_env_file.get(key) or os.environ.get(key) or "").strip() + + anthropic_api_key = _env_val("ANTHROPIC_API_KEY") + anthropic_oauth_env = ( + _env_val("ANTHROPIC_TOKEN") or _env_val("CLAUDE_CODE_OAUTH_TOKEN") + ) + api_key_path_explicit = bool(anthropic_api_key and not anthropic_oauth_env) + + if api_key_path_explicit: + # Prune any stale autodiscovered OAuth entries that may have been + # seeded into the on-disk pool during a previous OAuth session. + # Without this, switching OAuth -> API key at setup leaves the + # OAuth entries dormant in auth.json forever and rotation on a + # transient 401 could revive them. + retained = [ + entry for entry in entries + if entry.source not in {"hermes_pkce", "claude_code"} + ] + if len(retained) != len(entries): + entries[:] = retained + changed = True + return changed, active_sources + from agent.anthropic_adapter import read_claude_code_credentials, read_hermes_oauth_credentials for source_name, creds in ( diff --git a/tests/agent/test_credential_pool.py b/tests/agent/test_credential_pool.py index d7fec49aaac..69b30730e57 100644 --- a/tests/agent/test_credential_pool.py +++ b/tests/agent/test_credential_pool.py @@ -1182,6 +1182,150 @@ def test_load_pool_prefers_anthropic_env_token_over_file_backed_oauth(tmp_path, assert entry.access_token == "env-override-token" +def test_load_pool_api_key_path_skips_oauth_autodiscovery(tmp_path, monkeypatch): + """API-key auth path: autodiscovered OAuth creds must NOT be seeded. + + When the user picks "Anthropic API key" at `hermes setup`, + `save_anthropic_api_key()` writes ANTHROPIC_API_KEY and zeros + ANTHROPIC_TOKEN. That env-var pattern is the explicit signal that the + user opted into the API-key path and explicitly OUT of the OAuth + masquerade (Claude Code identity injection + `mcp_` tool-name rewrite + + claude-cli user-agent). Autodiscovered Claude Code / Hermes PKCE + tokens from other tools' credential files must NOT be silently mixed + into the anthropic pool — otherwise rotation on a 401/429 could flip + the session onto OAuth credentials mid-conversation. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-api03-explicit-user-key") + monkeypatch.delenv("ANTHROPIC_TOKEN", raising=False) + monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False) + _write_auth_store(tmp_path, {"version": 1, "providers": {}}) + monkeypatch.setattr("hermes_cli.auth.is_provider_explicitly_configured", lambda pid: True) + + pkce_called = {"n": 0} + cc_called = {"n": 0} + + def _fake_pkce(): + pkce_called["n"] += 1 + return { + "accessToken": "sk-ant-oat01-pkce-token", + "refreshToken": "pkce-refresh", + "expiresAt": int(time.time() * 1000) + 3_600_000, + } + + def _fake_cc(): + cc_called["n"] += 1 + return { + "accessToken": "sk-ant-oat01-claude-code-token", + "refreshToken": "cc-refresh", + "expiresAt": int(time.time() * 1000) + 3_600_000, + } + + monkeypatch.setattr("agent.anthropic_adapter.read_hermes_oauth_credentials", _fake_pkce) + monkeypatch.setattr("agent.anthropic_adapter.read_claude_code_credentials", _fake_cc) + + from agent.credential_pool import load_pool + + pool = load_pool("anthropic") + sources = {entry.source for entry in pool.entries()} + + # Only the explicit API-key entry should be in the pool. + assert sources == {"env:ANTHROPIC_API_KEY"}, f"got {sources}" + # And we should not have even called the autodiscovery readers. + assert pkce_called["n"] == 0 + assert cc_called["n"] == 0 + + +def test_load_pool_api_key_path_prunes_stale_oauth_entries(tmp_path, monkeypatch): + """Switching OAuth -> API key must prune stale OAuth entries from auth.json. + + Without this, a user who logs into OAuth (seeding `claude_code` or + `hermes_pkce` into auth.json) and later switches to the API key at + `hermes setup` would still have those OAuth entries dormant on disk. + Pool rotation on a transient 401 could revive them and flip the + session onto the OAuth masquerade. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-api03-explicit-user-key") + monkeypatch.delenv("ANTHROPIC_TOKEN", raising=False) + monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False) + + # Plant a stale claude_code entry in the on-disk pool (as if a previous + # OAuth session seeded it). + _write_auth_store( + tmp_path, + { + "version": 1, + "providers": {}, + "credential_pool": { + "anthropic": [ + { + "id": "stale1", + "source": "claude_code", + "auth_type": "oauth", + "access_token": "sk-ant-oat01-stale-claude-code", + "refresh_token": "stale-refresh", + "expires_at_ms": int(time.time() * 1000) + 3_600_000, + "priority": 0, + "label": "stale-claude-code", + "request_count": 0, + }, + ], + }, + }, + ) + monkeypatch.setattr("hermes_cli.auth.is_provider_explicitly_configured", lambda pid: True) + monkeypatch.setattr("agent.anthropic_adapter.read_hermes_oauth_credentials", lambda: None) + monkeypatch.setattr("agent.anthropic_adapter.read_claude_code_credentials", lambda: None) + + from agent.credential_pool import load_pool + + pool = load_pool("anthropic") + sources = {entry.source for entry in pool.entries()} + + # Stale claude_code entry must be gone, API key must be present. + assert "claude_code" not in sources + assert "env:ANTHROPIC_API_KEY" in sources + + +def test_load_pool_oauth_path_still_autodiscovers(tmp_path, monkeypatch): + """OAuth path: ANTHROPIC_TOKEN set, autodiscovery still fires. + + Regression guard: the API-key gate must not affect users who chose the + OAuth path at `hermes setup`. When ANTHROPIC_TOKEN is set (and + ANTHROPIC_API_KEY is empty), autodiscovered Claude Code creds should + still be seeded into the pool as before. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.setenv("ANTHROPIC_TOKEN", "sk-ant-oat01-explicit-oauth-token") + monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False) + _write_auth_store(tmp_path, {"version": 1, "providers": {}}) + monkeypatch.setattr("hermes_cli.auth.is_provider_explicitly_configured", lambda pid: True) + + monkeypatch.setattr( + "agent.anthropic_adapter.read_hermes_oauth_credentials", + lambda: None, + ) + monkeypatch.setattr( + "agent.anthropic_adapter.read_claude_code_credentials", + lambda: { + "accessToken": "sk-ant-oat01-autodiscovered-cc", + "refreshToken": "cc-refresh", + "expiresAt": int(time.time() * 1000) + 3_600_000, + }, + ) + + from agent.credential_pool import load_pool + + pool = load_pool("anthropic") + sources = {entry.source for entry in pool.entries()} + + # Both env OAuth token and autodiscovered Claude Code creds should be there. + assert "env:ANTHROPIC_TOKEN" in sources + assert "claude_code" in sources + + def test_least_used_strategy_selects_lowest_count(tmp_path, monkeypatch): """least_used strategy should select the credential with the lowest request_count.""" monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))