diff --git a/agent/credential_pool.py b/agent/credential_pool.py index e1307e51f8..43a67a923a 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -1208,6 +1208,19 @@ def _seed_from_singletons(provider: str, entries: List[PooledCredential]) -> Tup logger.debug("Qwen OAuth token seed failed: %s", exc) elif provider == "openai-codex": + # Respect user suppression — `hermes auth remove openai-codex` marks + # the device_code source as suppressed so it won't be re-seeded from + # either the Hermes auth store or ~/.codex/auth.json. Without this + # gate the removal is instantly undone on the next load_pool() call. + codex_suppressed = False + try: + from hermes_cli.auth import is_source_suppressed + codex_suppressed = is_source_suppressed(provider, "device_code") + except ImportError: + pass + if codex_suppressed: + return changed, active_sources + state = _load_provider_state(auth_store, "openai-codex") tokens = state.get("tokens") if isinstance(state, dict) else None # Fallback: import from Codex CLI (~/.codex/auth.json) if Hermes auth diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 1763a8e6b4..e79a6dca6d 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -773,6 +773,28 @@ def is_source_suppressed(provider_id: str, source: str) -> bool: return False +def unsuppress_credential_source(provider_id: str, source: str) -> bool: + """Clear a suppression marker so the source will be re-seeded on the next load. + + Returns True if a marker was cleared, False if no marker existed. + """ + with _auth_store_lock(): + auth_store = _load_auth_store() + suppressed = auth_store.get("suppressed_sources") + if not isinstance(suppressed, dict): + return False + provider_list = suppressed.get(provider_id) + if not isinstance(provider_list, list) or source not in provider_list: + return False + provider_list.remove(source) + if not provider_list: + suppressed.pop(provider_id, None) + if not suppressed: + auth_store.pop("suppressed_sources", None) + _save_auth_store(auth_store) + return True + + def get_provider_auth_state(provider_id: str) -> Optional[Dict[str, Any]]: """Return persisted auth state for a provider, or None.""" auth_store = _load_auth_store() diff --git a/hermes_cli/auth_commands.py b/hermes_cli/auth_commands.py index d58a6a3872..baca5c90c5 100644 --- a/hermes_cli/auth_commands.py +++ b/hermes_cli/auth_commands.py @@ -233,6 +233,9 @@ def auth_add_command(args) -> None: return if provider == "openai-codex": + # Clear any existing suppression marker so a re-link after `hermes auth + # remove openai-codex` works without the new tokens being skipped. + auth_mod.unsuppress_credential_source(provider, "device_code") creds = auth_mod._codex_device_code_login() label = (getattr(args, "label", None) or "").strip() or label_from_token( creds["tokens"]["access_token"], @@ -352,7 +355,34 @@ def auth_remove_command(args) -> None: # If this was a singleton-seeded credential (OAuth device_code, hermes_pkce), # clear the underlying auth store / credential file so it doesn't get # re-seeded on the next load_pool() call. - elif removed.source == "device_code" and provider in ("openai-codex", "nous"): + elif provider == "openai-codex" and ( + removed.source == "device_code" or removed.source.endswith(":device_code") + ): + # Codex tokens live in TWO places: the Hermes auth store and + # ~/.codex/auth.json (the Codex CLI shared file). On every refresh, + # refresh_codex_oauth_pure() writes to both. So clearing only the + # Hermes auth store is not enough — _seed_from_singletons() will + # auto-import from ~/.codex/auth.json on the next load_pool() and + # the removal is instantly undone. Mark the source as suppressed + # so auto-import is skipped; leave ~/.codex/auth.json untouched so + # the Codex CLI itself keeps working. + from hermes_cli.auth import ( + _load_auth_store, _save_auth_store, _auth_store_lock, + suppress_credential_source, + ) + with _auth_store_lock(): + auth_store = _load_auth_store() + providers_dict = auth_store.get("providers") + if isinstance(providers_dict, dict) and provider in providers_dict: + del providers_dict[provider] + _save_auth_store(auth_store) + print(f"Cleared {provider} OAuth tokens from auth store") + suppress_credential_source(provider, "device_code") + print("Suppressed openai-codex device_code source — it will not be re-seeded.") + print("Note: Codex CLI credentials still live in ~/.codex/auth.json") + print("Run `hermes auth add openai-codex` to re-enable if needed.") + + elif removed.source == "device_code" and provider == "nous": from hermes_cli.auth import ( _load_auth_store, _save_auth_store, _auth_store_lock, ) diff --git a/tests/hermes_cli/test_auth_commands.py b/tests/hermes_cli/test_auth_commands.py index b26757a227..a9db905926 100644 --- a/tests/hermes_cli/test_auth_commands.py +++ b/tests/hermes_cli/test_auth_commands.py @@ -703,3 +703,231 @@ def test_auth_remove_claude_code_suppresses_reseed(tmp_path, monkeypatch): suppressed = updated.get("suppressed_sources", {}) assert "anthropic" in suppressed assert "claude_code" in suppressed["anthropic"] + + +def test_unsuppress_credential_source_clears_marker(tmp_path, monkeypatch): + """unsuppress_credential_source() removes a previously-set marker.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + _write_auth_store(tmp_path, {"version": 1}) + + from hermes_cli.auth import suppress_credential_source, unsuppress_credential_source, is_source_suppressed + + suppress_credential_source("openai-codex", "device_code") + assert is_source_suppressed("openai-codex", "device_code") is True + + cleared = unsuppress_credential_source("openai-codex", "device_code") + assert cleared is True + assert is_source_suppressed("openai-codex", "device_code") is False + + payload = json.loads((tmp_path / "hermes" / "auth.json").read_text()) + # Empty suppressed_sources dict should be cleaned up entirely + assert "suppressed_sources" not in payload + + +def test_unsuppress_credential_source_returns_false_when_absent(tmp_path, monkeypatch): + """unsuppress_credential_source() returns False if no marker exists.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + _write_auth_store(tmp_path, {"version": 1}) + + from hermes_cli.auth import unsuppress_credential_source + + assert unsuppress_credential_source("openai-codex", "device_code") is False + assert unsuppress_credential_source("nonexistent", "whatever") is False + + +def test_unsuppress_credential_source_preserves_other_markers(tmp_path, monkeypatch): + """Clearing one marker must not affect unrelated markers.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + _write_auth_store(tmp_path, {"version": 1}) + + from hermes_cli.auth import ( + suppress_credential_source, + unsuppress_credential_source, + is_source_suppressed, + ) + + suppress_credential_source("openai-codex", "device_code") + suppress_credential_source("anthropic", "claude_code") + + assert unsuppress_credential_source("openai-codex", "device_code") is True + assert is_source_suppressed("anthropic", "claude_code") is True + + +def test_auth_remove_codex_device_code_suppresses_reseed(tmp_path, monkeypatch): + """Removing an auto-seeded openai-codex credential must mark the source as suppressed.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + monkeypatch.setattr( + "agent.credential_pool._seed_from_singletons", + lambda provider, entries: (False, {"device_code"}), + ) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir(parents=True, exist_ok=True) + + auth_store = { + "version": 1, + "providers": { + "openai-codex": { + "tokens": { + "access_token": "acc-1", + "refresh_token": "ref-1", + }, + }, + }, + "credential_pool": { + "openai-codex": [{ + "id": "cx1", + "label": "codex-auto", + "auth_type": "oauth", + "priority": 0, + "source": "device_code", + "access_token": "acc-1", + "refresh_token": "ref-1", + }] + }, + } + (hermes_home / "auth.json").write_text(json.dumps(auth_store)) + + from types import SimpleNamespace + from hermes_cli.auth_commands import auth_remove_command + + auth_remove_command(SimpleNamespace(provider="openai-codex", target="1")) + + updated = json.loads((hermes_home / "auth.json").read_text()) + suppressed = updated.get("suppressed_sources", {}) + assert "openai-codex" in suppressed + assert "device_code" in suppressed["openai-codex"] + # Tokens in providers state should also be cleared + assert "openai-codex" not in updated.get("providers", {}) + + +def test_auth_remove_codex_manual_source_suppresses_reseed(tmp_path, monkeypatch): + """Removing a manually-added (`manual:device_code`) openai-codex credential must also suppress.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + monkeypatch.setattr( + "agent.credential_pool._seed_from_singletons", + lambda provider, entries: (False, set()), + ) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir(parents=True, exist_ok=True) + + auth_store = { + "version": 1, + "providers": { + "openai-codex": { + "tokens": { + "access_token": "acc-2", + "refresh_token": "ref-2", + }, + }, + }, + "credential_pool": { + "openai-codex": [{ + "id": "cx2", + "label": "manual-codex", + "auth_type": "oauth", + "priority": 0, + "source": "manual:device_code", + "access_token": "acc-2", + "refresh_token": "ref-2", + }] + }, + } + (hermes_home / "auth.json").write_text(json.dumps(auth_store)) + + from types import SimpleNamespace + from hermes_cli.auth_commands import auth_remove_command + + auth_remove_command(SimpleNamespace(provider="openai-codex", target="1")) + + updated = json.loads((hermes_home / "auth.json").read_text()) + suppressed = updated.get("suppressed_sources", {}) + # Critical: manual:device_code source must also trigger the suppression path + assert "openai-codex" in suppressed + assert "device_code" in suppressed["openai-codex"] + assert "openai-codex" not in updated.get("providers", {}) + + +def test_auth_add_codex_clears_suppression_marker(tmp_path, monkeypatch): + """Re-linking codex via `hermes auth add openai-codex` must clear any suppression marker.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + monkeypatch.setattr( + "agent.credential_pool._seed_from_singletons", + lambda provider, entries: (False, set()), + ) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir(parents=True, exist_ok=True) + + # Pre-existing suppression (simulating a prior `hermes auth remove`) + (hermes_home / "auth.json").write_text(json.dumps({ + "version": 1, + "providers": {}, + "suppressed_sources": {"openai-codex": ["device_code"]}, + })) + + token = _jwt_with_email("codex@example.com") + monkeypatch.setattr( + "hermes_cli.auth._codex_device_code_login", + lambda: { + "tokens": { + "access_token": token, + "refresh_token": "refreshed", + }, + "base_url": "https://chatgpt.com/backend-api/codex", + "last_refresh": "2026-01-01T00:00:00Z", + }, + ) + + from hermes_cli.auth_commands import auth_add_command + + class _Args: + provider = "openai-codex" + auth_type = "oauth" + api_key = None + label = None + + auth_add_command(_Args()) + + payload = json.loads((hermes_home / "auth.json").read_text()) + # Suppression marker must be cleared + assert "openai-codex" not in payload.get("suppressed_sources", {}) + # New pool entry must be present + entries = payload["credential_pool"]["openai-codex"] + assert any(e["source"] == "manual:device_code" for e in entries) + + +def test_seed_from_singletons_respects_codex_suppression(tmp_path, monkeypatch): + """_seed_from_singletons() for openai-codex must skip auto-import when suppressed.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + hermes_home = tmp_path / "hermes" + hermes_home.mkdir(parents=True, exist_ok=True) + + # Suppression marker in place + (hermes_home / "auth.json").write_text(json.dumps({ + "version": 1, + "providers": {}, + "suppressed_sources": {"openai-codex": ["device_code"]}, + })) + + # Make _import_codex_cli_tokens return tokens — these would normally trigger + # a re-seed, but suppression must skip it. + def _fake_import(): + return { + "access_token": "would-be-reimported", + "refresh_token": "would-be-reimported", + } + + monkeypatch.setattr("hermes_cli.auth._import_codex_cli_tokens", _fake_import) + + from agent.credential_pool import _seed_from_singletons + + entries = [] + changed, active_sources = _seed_from_singletons("openai-codex", entries) + + # With suppression in place: nothing changes, no entries added, no sources + assert changed is False + assert entries == [] + assert active_sources == set() + + # Verify the auth store was NOT modified (no auto-import happened) + after = json.loads((hermes_home / "auth.json").read_text()) + assert "openai-codex" not in after.get("providers", {})