From eaad229a4d611a4441265f319f73ce8f28aeb62d Mon Sep 17 00:00:00 2001 From: Cooper Gamble Date: Fri, 29 May 2026 16:50:04 +0000 Subject: [PATCH 1/4] fix(auth): protect shared Codex store consumers --- gateway/platforms/base.py | 17 ++++++---- hermes_cli/models.py | 14 +++++++-- tests/gateway/test_platform_base.py | 17 ++++++++++ .../hermes_cli/test_codex_cli_model_picker.py | 31 +++++++++++++++++++ 4 files changed, 70 insertions(+), 9 deletions(-) diff --git a/gateway/platforms/base.py b/gateway/platforms/base.py index 0d141d0fcf1d..ef0beabea7b0 100644 --- a/gateway/platforms/base.py +++ b/gateway/platforms/base.py @@ -484,7 +484,7 @@ def is_host_excluded_by_no_proxy(hostname: str, no_proxy_value: str | None = Non from gateway.config import Platform, PlatformConfig from gateway.session import SessionSource, build_session_key -from hermes_constants import get_hermes_dir, get_hermes_home +from hermes_constants import get_default_hermes_root, get_hermes_dir, get_hermes_home GATEWAY_SECRET_CAPTURE_UNSUPPORTED_MESSAGE = ( @@ -954,11 +954,16 @@ def _media_delivery_denied_paths() -> List[Path]: home = Path(os.path.expanduser("~")) for sub in _MEDIA_DELIVERY_DENIED_HOME_SUBPATHS: denied.append(home / sub) - # The Hermes home itself contains credentials (auth.json, .env) — only the - # cache subdirectories under it are explicitly allowlisted above. - denied.append(_HERMES_HOME / ".env") - denied.append(_HERMES_HOME / "auth.json") - denied.append(_HERMES_HOME / "credentials") + # In profile mode, both the active profile and the canonical root contain + # credentials. Only cache subdirectories are explicitly allowlisted above. + hermes_homes = [_HERMES_HOME] + root = get_default_hermes_root() + if root not in hermes_homes: + hermes_homes.append(root) + for hermes_home in hermes_homes: + denied.append(hermes_home / ".env") + denied.append(hermes_home / "auth.json") + denied.append(hermes_home / "credentials") return denied diff --git a/hermes_cli/models.py b/hermes_cli/models.py index 42eadfd76290..16d717d0c7a7 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -2121,9 +2121,17 @@ def _credential_fingerprint(provider: str) -> str: # OAuth / external-file mtimes that change on re-auth try: - from hermes_constants import get_hermes_home - for rel in ("auth.json", "credentials.json"): - p = get_hermes_home() / rel + from hermes_constants import get_default_hermes_root, get_hermes_home + hermes_home = get_hermes_home() + credential_files = [ + ("auth.json", hermes_home / "auth.json"), + ("credentials.json", hermes_home / "credentials.json"), + ] + if provider == "openai-codex": + codex_auth_file = get_default_hermes_root() / "auth.json" + if codex_auth_file != hermes_home / "auth.json": + credential_files.append(("codex-root-auth.json", codex_auth_file)) + for rel, p in credential_files: try: parts.append(f"{rel}@{p.stat().st_mtime_ns}") except FileNotFoundError: diff --git a/tests/gateway/test_platform_base.py b/tests/gateway/test_platform_base.py index 6a5b8c15c14b..f561910bfd78 100644 --- a/tests/gateway/test_platform_base.py +++ b/tests/gateway/test_platform_base.py @@ -641,6 +641,23 @@ def test_denylist_blocks_hermes_credentials(self, tmp_path, monkeypatch): assert BasePlatformAdapter.validate_media_delivery_path(str(env_file)) is None + def test_denylist_blocks_profile_root_auth_store(self, tmp_path, monkeypatch): + """Named profiles must not deliver the shared root Codex auth store.""" + self._patch_roots(monkeypatch) + + root = tmp_path / "hermes" + profile = root / "profiles" / "worker" + profile.mkdir(parents=True) + root_auth = root / "auth.json" + root_auth.write_text('{"refresh_token": "shared-secret"}') + monkeypatch.setenv("HERMES_HOME", str(profile)) + monkeypatch.setattr( + "gateway.platforms.base._HERMES_HOME", + profile, + ) + + assert BasePlatformAdapter.validate_media_delivery_path(str(root_auth)) is None + def test_strict_mode_envvar_restores_legacy_behavior(self, tmp_path, monkeypatch): """Setting HERMES_MEDIA_DELIVERY_STRICT=1 reactivates the older allowlist+recency logic. A stale file outside the allowlist is diff --git a/tests/hermes_cli/test_codex_cli_model_picker.py b/tests/hermes_cli/test_codex_cli_model_picker.py index ca8c4cb388da..f6ead6e2096b 100644 --- a/tests/hermes_cli/test_codex_cli_model_picker.py +++ b/tests/hermes_cli/test_codex_cli_model_picker.py @@ -12,6 +12,7 @@ import base64 import json +import os import time from pathlib import Path @@ -102,6 +103,36 @@ def test_codex_picker_uses_live_codex_catalog(hermes_auth_only_env, tmp_path, mo assert codex["total_models"] == len(codex["models"]) +def test_codex_picker_cache_invalidates_when_shared_root_auth_changes(tmp_path, monkeypatch): + """Named profiles must observe canonical Codex token rotations.""" + root = tmp_path / "hermes" + profile = root / "profiles" / "worker" + profile.mkdir(parents=True) + root_auth = root / "auth.json" + root_auth.write_text('{"tokens": "before"}') + (profile / "auth.json").write_text("{}") + monkeypatch.setenv("HERMES_HOME", str(profile)) + + import hermes_cli.models as models + + catalogs = iter([["old-model"], ["new-model"]]) + calls = [] + + def _provider_model_ids(provider, *, force_refresh=False): + calls.append((provider, force_refresh)) + return next(catalogs) + + monkeypatch.setattr(models, "provider_model_ids", _provider_model_ids) + + assert models.cached_provider_model_ids("openai-codex") == ["old-model"] + previous_mtime = root_auth.stat().st_mtime_ns + root_auth.write_text('{"tokens": "after"}') + os.utime(root_auth, ns=(previous_mtime + 1_000_000_000,) * 2) + + assert models.cached_provider_model_ids("openai-codex") == ["new-model"] + assert calls == [("openai-codex", False), ("openai-codex", False)] + + @pytest.fixture() def claude_code_only_env(tmp_path, monkeypatch): """Set up an environment where Anthropic credentials only exist in From f8d1c8423268ba31780b0be4f1fc4fd2fea91012 Mon Sep 17 00:00:00 2001 From: Cooper Gamble Date: Fri, 29 May 2026 17:04:38 +0000 Subject: [PATCH 2/4] fix(auth): reject unclaimed Codex refresh families --- hermes_cli/auth.py | 24 ++++----- tests/agent/test_credential_pool.py | 54 ++++++++++++++++++- tests/hermes_cli/test_auth_codex_provider.py | 57 ++++++++++++++++---- 3 files changed, 109 insertions(+), 26 deletions(-) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index dcffaeecf1b8..e4a6bf62e31e 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -3832,28 +3832,21 @@ def _read_codex_tokens(*, _lock: bool = True) -> Dict[str, Any]: } -def _codex_profiles_exist() -> bool: - """Return whether this Hermes root contains named profiles.""" - return (_codex_auth_file_path().parent / "profiles").is_dir() - - def _require_codex_refresh_owner(state: Optional[Dict[str, Any]] = None) -> None: - """Refuse ambiguous pre-upgrade profile refreshes. + """Refuse ambiguous pre-upgrade refreshes. - Older Hermes versions could copy one Codex refresh-token family into - profile-local stores. The canonical root store cannot know which copy won - the last rotation, so spending its token could replay an already-consumed - value. A fresh Hermes device-code login claims the canonical family. + Older Hermes versions could import Codex CLI credentials or copy one + refresh-token family into profile-local stores. Hermes cannot know which + client or copy won the last rotation, so spending its token could replay an + already-consumed value. A fresh Hermes device-code login claims the family. """ - if not _codex_profiles_exist(): - return if state is None: auth_store = _load_auth_store(_codex_auth_file_path()) state = _load_provider_state(auth_store, "openai-codex") if isinstance(state, dict) and state.get("refresh_owner") == CODEX_REFRESH_OWNER: return raise AuthError( - "Codex credentials predate profile-safe refresh ownership. " + "Codex credentials predate Hermes-safe refresh ownership. " "Run `hermes model`, choose OpenAI Codex, and reauthenticate to create " "a fresh Hermes-owned Codex session.", provider="openai-codex", @@ -5209,7 +5202,9 @@ def _is_terminal_codex_oauth_refresh_error(exc: Exception) -> bool: (invalid_grant, token revoked, refresh_token_reused). ``codex_auth_missing_refresh_token`` means the pool entry has no refresh token at all — retrying will never work. - Both carry ``relogin_required=True``; transient failures (429, 5xx) do not. + ``codex_auth_refresh_owner_unclaimed`` means Hermes cannot safely spend a + legacy token family. These carry ``relogin_required=True``; transient + failures (429, 5xx) do not. """ return ( isinstance(exc, AuthError) @@ -5217,6 +5212,7 @@ def _is_terminal_codex_oauth_refresh_error(exc: Exception) -> bool: and exc.code in { "codex_refresh_failed", "codex_auth_missing_refresh_token", + "codex_auth_refresh_owner_unclaimed", "invalid_grant", "invalid_token", "refresh_token_reused", diff --git a/tests/agent/test_credential_pool.py b/tests/agent/test_credential_pool.py index 6fae4bf3f68b..70250d9c2bee 100644 --- a/tests/agent/test_credential_pool.py +++ b/tests/agent/test_credential_pool.py @@ -2583,12 +2583,15 @@ def test_nous_exhausted_entry_recovers_via_auth_store_sync(tmp_path, monkeypatch # ── OpenAI Codex OAuth cross-process sync tests ──────────────────────────── def _codex_auth_store(access: str, refresh: str) -> dict: + from hermes_cli.auth import CODEX_REFRESH_OWNER + return { "version": 1, "active_provider": "openai-codex", "providers": { "openai-codex": { "auth_mode": "chatgpt", + "refresh_owner": CODEX_REFRESH_OWNER, "tokens": { "access_token": access, "refresh_token": refresh, @@ -2987,10 +2990,17 @@ def test_codex_profile_pool_flush_does_not_restore_stale_manual_entry(tmp_path, def test_codex_pool_only_device_refresh_persists_canonical_state(tmp_path, monkeypatch): - """A restored device-code pool row can refresh even before singleton seeding.""" + """A claimed device-code pool row can refresh before singleton seeding.""" monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + import hermes_cli.auth as auth_mod + _write_auth_store(tmp_path, { "version": 1, + "providers": { + "openai-codex": { + "refresh_owner": auth_mod.CODEX_REFRESH_OWNER, + }, + }, "credential_pool": { "openai-codex": [{ "id": "shared-codex", @@ -3002,7 +3012,6 @@ def test_codex_pool_only_device_refresh_persists_canonical_state(tmp_path, monke }, }) - import hermes_cli.auth as auth_mod from agent.credential_pool import load_pool monkeypatch.setattr( @@ -3029,6 +3038,41 @@ def test_codex_pool_only_device_refresh_persists_canonical_state(tmp_path, monke assert shared["refresh_token"] == "refresh-NEW" +def test_codex_pool_only_unowned_device_refresh_fails_closed(tmp_path, monkeypatch): + """An ambiguous restored row must never submit its refresh token.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + _write_auth_store(tmp_path, { + "version": 1, + "credential_pool": { + "openai-codex": [{ + "id": "shared-codex", + "source": "device_code", + "auth_type": "oauth", + "access_token": "access-OLD", + "refresh_token": "refresh-OLD", + }], + }, + }) + + import hermes_cli.auth as auth_mod + from agent.credential_pool import load_pool + + monkeypatch.setattr( + auth_mod, + "refresh_codex_oauth_pure", + lambda *_args, **_kwargs: pytest.fail("unowned token must not be spent"), + ) + pool = load_pool("openai-codex") + + assert pool._refresh_entry(pool.entries()[0], force=True) is None + + auth_payload = json.loads((tmp_path / "hermes" / "auth.json").read_text()) + state = auth_payload["providers"]["openai-codex"] + assert state["last_auth_error"]["code"] == "codex_auth_refresh_owner_unclaimed" + assert state["tokens"] == {} + assert auth_payload["credential_pool"]["openai-codex"] == [] + + def test_codex_round_robin_priority_updates_survive_reload(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) _write_auth_store(tmp_path, { @@ -3525,11 +3569,14 @@ def _transient_failure(*_args, **_kwargs): def _codex_auth_store(access_token: str, refresh_token: str) -> dict: + from hermes_cli.auth import CODEX_REFRESH_OWNER + return { "version": 1, "active_provider": "openai-codex", "providers": { "openai-codex": { + "refresh_owner": CODEX_REFRESH_OWNER, "tokens": { "access_token": access_token, "refresh_token": refresh_token, @@ -3548,6 +3595,9 @@ def test_is_terminal_codex_oauth_refresh_error(): assert _is_terminal_codex_oauth_refresh_error( AuthError("No token", provider="openai-codex", code="codex_auth_missing_refresh_token", relogin_required=True) ) + assert _is_terminal_codex_oauth_refresh_error( + AuthError("Unclaimed", provider="openai-codex", code="codex_auth_refresh_owner_unclaimed", relogin_required=True) + ) assert _is_terminal_codex_oauth_refresh_error( AuthError("Revoked", provider="openai-codex", code="invalid_grant", relogin_required=True) ) diff --git a/tests/hermes_cli/test_auth_codex_provider.py b/tests/hermes_cli/test_auth_codex_provider.py index dfc68a29797e..7f05e2b02a55 100644 --- a/tests/hermes_cli/test_auth_codex_provider.py +++ b/tests/hermes_cli/test_auth_codex_provider.py @@ -23,21 +23,30 @@ ) -def _setup_hermes_auth(hermes_home: Path, *, access_token: str = "access", refresh_token: str = "refresh"): +def _setup_hermes_auth( + hermes_home: Path, + *, + access_token: str = "access", + refresh_token: str = "refresh", + owned: bool = True, +): """Write Codex tokens into the Hermes auth store.""" hermes_home.mkdir(parents=True, exist_ok=True) + state = { + "tokens": { + "access_token": access_token, + "refresh_token": refresh_token, + }, + "last_refresh": "2026-02-26T00:00:00Z", + "auth_mode": "chatgpt", + } + if owned: + state["refresh_owner"] = CODEX_REFRESH_OWNER auth_store = { "version": 1, "active_provider": "openai-codex", "providers": { - "openai-codex": { - "tokens": { - "access_token": access_token, - "refresh_token": refresh_token, - }, - "last_refresh": "2026-02-26T00:00:00Z", - "auth_mode": "chatgpt", - }, + "openai-codex": state, }, } auth_file = hermes_home / "auth.json" @@ -379,11 +388,39 @@ def _fail_profile_save(auth_store, auth_file=None): assert "Failed to sync Codex tokens to profile auth store" not in caplog.text +def test_classic_mode_refuses_to_refresh_unclaimed_legacy_tokens(tmp_path, monkeypatch): + hermes_home = tmp_path / "hermes" + _setup_hermes_auth( + hermes_home, + access_token="legacy-at", + refresh_token="legacy-rt", + owned=False, + ) + monkeypatch.setenv("HERMES_HOME", str(hermes_home)) + monkeypatch.setattr( + "hermes_cli.auth._refresh_codex_auth_tokens", + lambda *_args, **_kwargs: pytest.fail("legacy classic token must not be spent"), + ) + + with pytest.raises(AuthError) as exc: + resolve_codex_runtime_credentials(force_refresh=True, refresh_if_expiring=False) + + assert exc.value.code == "codex_auth_refresh_owner_unclaimed" + assert exc.value.relogin_required is True + assert "`hermes model`" in str(exc.value) + assert "reauthenticate" in str(exc.value) + + def test_profile_mode_refuses_to_refresh_unclaimed_legacy_tokens(tmp_path, monkeypatch): root_home = tmp_path / "hermes" profile_home = root_home / "profiles" / "worker" profile_home.mkdir(parents=True) - _setup_hermes_auth(root_home, access_token="legacy-at", refresh_token="legacy-rt") + _setup_hermes_auth( + root_home, + access_token="legacy-at", + refresh_token="legacy-rt", + owned=False, + ) monkeypatch.setenv("HERMES_HOME", str(profile_home)) monkeypatch.setattr( "hermes_cli.auth._refresh_codex_auth_tokens", From 61974ba3533f3e1c2b45dd7b278d3d855c3012f3 Mon Sep 17 00:00:00 2001 From: Cooper Gamble Date: Fri, 29 May 2026 17:22:09 +0000 Subject: [PATCH 3/4] fix(auth): remove linked Codex aliases with family --- agent/credential_pool.py | 40 ++++++++---- hermes_cli/auth.py | 48 ++++++++++++++ tests/hermes_cli/test_auth_commands.py | 90 ++++++++++++++++++++++++++ 3 files changed, 167 insertions(+), 11 deletions(-) diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 9b6455932be2..18a387cf67e2 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -1672,21 +1672,39 @@ def remove_index(self, index: int) -> Optional[PooledCredential]: if index < 1 or index > len(self._entries): return None removed = self._entries.pop(index - 1) + remove_codex_family = ( + self.provider == "openai-codex" + and removed.source == "device_code" + ) + removed_entry_ids = {removed.id} + if remove_codex_family and removed.refresh_token: + retained_entries = [] + for entry in self._entries: + if ( + entry.source == "manual:device_code" + and entry.refresh_token == removed.refresh_token + ): + removed_entry_ids.add(entry.id) + continue + retained_entries.append(entry) + self._entries = retained_entries self._entries = [ replace(entry, priority=new_priority) for new_priority, entry in enumerate(self._entries) ] - self._persist( - replace_shared_entries=( - self.provider == "openai-codex" and removed.source == "device_code" - ), - remove_entry_ids={removed.id}, - update_order_entry_ids={entry.id for entry in self._entries}, - clear_shared_provider_state=( - self.provider == "openai-codex" and removed.source == "device_code" - ), - ) - if self._current_id == removed.id: + persist_kwargs = { + "replace_shared_entries": remove_codex_family, + "remove_entry_ids": removed_entry_ids, + "update_order_entry_ids": {entry.id for entry in self._entries}, + "clear_shared_provider_state": remove_codex_family, + } + if remove_codex_family: + with auth_mod._codex_auth_store_lock(): + auth_mod._remove_codex_linked_legacy_aliases(removed.refresh_token) + self._persist(**persist_kwargs) + else: + self._persist(**persist_kwargs) + if self._current_id in removed_entry_ids: self._current_id = None return removed diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index e4a6bf62e31e..c5934e21279f 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -4043,6 +4043,54 @@ def _sync_codex_profile_legacy_aliases( ) +def _remove_codex_linked_legacy_aliases(refresh_token: Optional[str]) -> None: + """Remove manual aliases that still reference a canonical token family.""" + if not isinstance(refresh_token, str) or not refresh_token: + return + + def _remove_from_store(auth_store: Dict[str, Any]) -> bool: + pool = auth_store.get("credential_pool") + if not isinstance(pool, dict): + return False + entries = pool.get("openai-codex") + if not isinstance(entries, list): + return False + filtered = [ + entry for entry in entries + if not ( + isinstance(entry, dict) + and entry.get("source") == "manual:device_code" + and entry.get("refresh_token") == refresh_token + ) + ] + if len(filtered) == len(entries): + return False + pool["openai-codex"] = filtered + return True + + with _codex_auth_store_lock(): + auth_file = _codex_auth_file_path() + profiles_dir = auth_file.parent / "profiles" + if profiles_dir.is_dir(): + for profile_dir in sorted(profiles_dir.iterdir()): + profile_auth_file = profile_dir / "auth.json" + if not profile_dir.is_dir() or not profile_auth_file.exists(): + continue + with _file_lock( + profile_auth_file.with_suffix(".lock"), + threading.local(), + AUTH_LOCK_TIMEOUT_SECONDS, + f"Timed out waiting for Codex profile auth lock: {profile_auth_file}", + ): + auth_store = _load_auth_store(profile_auth_file) + if _remove_from_store(auth_store): + _save_auth_store(auth_store, auth_file=profile_auth_file) + + auth_store = _load_auth_store(auth_file) + if _remove_from_store(auth_store): + _save_auth_store(auth_store, auth_file=auth_file) + + def _save_codex_tokens(tokens: Dict[str, str], last_refresh: str = None) -> None: """Save Codex OAuth tokens to Hermes's canonical auth store.""" if last_refresh is None: diff --git a/tests/hermes_cli/test_auth_commands.py b/tests/hermes_cli/test_auth_commands.py index 6d5392aaf615..8dacc95b60f5 100644 --- a/tests/hermes_cli/test_auth_commands.py +++ b/tests/hermes_cli/test_auth_commands.py @@ -1179,6 +1179,96 @@ def test_auth_remove_codex_device_code_clears_canonical_root_in_profile_mode(tmp assert exc.value.code == "codex_auth_missing" +def test_auth_remove_codex_device_code_clears_linked_aliases_in_all_profiles( + tmp_path, monkeypatch, +): + """Shared removal deletes legacy aliases but preserves independent rows.""" + root_home = tmp_path / "hermes" + profile_home = root_home / "profiles" / "worker" + sibling_home = root_home / "profiles" / "sibling" + profile_home.mkdir(parents=True) + sibling_home.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(profile_home)) + monkeypatch.setattr( + "agent.credential_pool._seed_from_singletons", + lambda provider, entries: (False, {"device_code"}), + ) + (root_home / "auth.json").write_text(json.dumps({ + "version": 1, + "providers": { + "openai-codex": { + "tokens": { + "access_token": "shared-at", + "refresh_token": "shared-rt", + }, + }, + }, + "credential_pool": { + "openai-codex": [{ + "id": "shared-codex", + "source": "device_code", + "auth_type": "oauth", + "access_token": "shared-at", + "refresh_token": "shared-rt", + }, { + "id": "root-linked", + "source": "manual:device_code", + "auth_type": "oauth", + "access_token": "root-stale-at", + "refresh_token": "shared-rt", + }, { + "id": "root-independent", + "source": "manual:device_code", + "auth_type": "oauth", + "access_token": "root-independent-at", + "refresh_token": "root-independent-rt", + }], + }, + })) + for profile, prefix in ((profile_home, "profile"), (sibling_home, "sibling")): + (profile / "auth.json").write_text(json.dumps({ + "version": 1, + "credential_pool": { + "openai-codex": [{ + "id": f"{prefix}-linked", + "source": "manual:device_code", + "auth_type": "oauth", + "access_token": f"{prefix}-stale-at", + "refresh_token": "shared-rt", + }, { + "id": f"{prefix}-independent", + "source": "manual:device_code", + "auth_type": "oauth", + "access_token": f"{prefix}-independent-at", + "refresh_token": f"{prefix}-independent-rt", + }], + }, + })) + + from types import SimpleNamespace + from agent.credential_pool import load_pool + from hermes_cli.auth_commands import auth_remove_command + + auth_remove_command(SimpleNamespace(provider="openai-codex", target="shared-codex")) + + root_payload = json.loads((root_home / "auth.json").read_text()) + assert "openai-codex" not in root_payload.get("providers", {}) + assert [ + entry["id"] for entry in root_payload["credential_pool"]["openai-codex"] + ] == ["root-independent"] + profile_payload = json.loads((profile_home / "auth.json").read_text()) + assert [ + entry["id"] for entry in profile_payload["credential_pool"]["openai-codex"] + ] == ["profile-independent"] + sibling_payload = json.loads((sibling_home / "auth.json").read_text()) + assert [ + entry["id"] for entry in sibling_payload["credential_pool"]["openai-codex"] + ] == ["sibling-independent"] + assert [entry.id for entry in load_pool("openai-codex").entries()] == [ + "profile-independent", + ] + + def test_auth_remove_codex_device_code_clears_legacy_profile_provider_state( tmp_path, monkeypatch, ): From 0515a0c51f27e835b74bba3b00fe6ddcb4d9156c Mon Sep 17 00:00:00 2001 From: Cooper Gamble Date: Fri, 29 May 2026 17:33:00 +0000 Subject: [PATCH 4/4] fix(auth): sanitize merged Codex pool rows --- agent/credential_pool.py | 29 +++++++++- hermes_cli/auth.py | 17 +++--- tests/agent/test_credential_pool.py | 86 +++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 8 deletions(-) diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 18a387cf67e2..8fb25cb715a4 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -2349,6 +2349,11 @@ def _is_suppressed(_p, _s): # type: ignore[misc] def load_pool(provider: str) -> CredentialPool: provider = (provider or "").strip().lower() raw_entries = read_credential_pool(provider) + raw_entries_by_id = { + payload["id"]: payload + for payload in raw_entries + if isinstance(payload, dict) and isinstance(payload.get("id"), str) + } raw_needs_sanitization = any( isinstance(payload, dict) and sanitize_borrowed_credential_payload(payload, provider) != payload @@ -2369,10 +2374,32 @@ def load_pool(provider: str) -> CredentialPool: changed |= _normalize_pool_priorities(provider, entries) if changed: + serialized_entries = [ + entry.to_dict() + for entry in sorted(entries, key=lambda item: item.priority) + ] + serialized_entries_by_id = { + payload["id"]: payload + for payload in serialized_entries + if isinstance(payload.get("id"), str) + } + entry_ids = set(serialized_entries_by_id) + raw_entry_ids = set(raw_entries_by_id) write_credential_pool( provider, - [entry.to_dict() for entry in sorted(entries, key=lambda item: item.priority)], + serialized_entries, preserve_shared_entries=True, preserve_profile_entries=True, + add_entry_ids=frozenset(entry_ids - raw_entry_ids), + replace_entry_ids=frozenset( + entry_id + for entry_id in entry_ids & raw_entry_ids + if is_borrowed_credential_source( + serialized_entries_by_id[entry_id].get("source"), + provider, + ) + and serialized_entries_by_id[entry_id] != raw_entries_by_id[entry_id] + ), + remove_entry_ids=frozenset(raw_entry_ids - entry_ids), ) return CredentialPool(provider, entries) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index c5934e21279f..c82b5844fbad 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -1601,11 +1601,14 @@ def write_credential_pool( credentials. Callers may pass raw dictionaries, so sanitize here even when ``PooledCredential.to_dict()`` already did the same work upstream. """ - sanitized_entries = [ - sanitize_borrowed_credential_payload(entry, provider_id) - if isinstance(entry, dict) else entry - for entry in entries - ] + def _sanitize_entries(payloads: List[Any]) -> List[Any]: + return [ + sanitize_borrowed_credential_payload(entry, provider_id) + if isinstance(entry, dict) else entry + for entry in payloads + ] + + sanitized_entries = _sanitize_entries(entries) if provider_id in SHARED_CREDENTIAL_POOL_PROVIDERS: shared_auth_file = _codex_auth_file_path() profile_auth_file = _auth_file_path() @@ -1670,7 +1673,7 @@ def write_credential_pool( update_order_entry_ids=set(update_order_entry_ids), update_status_entry_ids=set(update_status_entry_ids), ) - shared_pool[provider_id] = ( + shared_pool[provider_id] = _sanitize_entries( root_profile_entries + shared_entries if split_shared_store else profile_entries + shared_entries @@ -1720,7 +1723,7 @@ def write_credential_pool( update_order_entry_ids=set(update_order_entry_ids), clear=not shared_entries, ) - profile_pool[provider_id] = profile_entries + profile_pool[provider_id] = _sanitize_entries(profile_entries) return _save_auth_store(profile_auth_store, auth_file=profile_auth_file) return shared_auth_file diff --git a/tests/agent/test_credential_pool.py b/tests/agent/test_credential_pool.py index 70250d9c2bee..032d6405c784 100644 --- a/tests/agent/test_credential_pool.py +++ b/tests/agent/test_credential_pool.py @@ -901,6 +901,45 @@ def test_load_pool_sanitizes_legacy_raw_borrowed_entry_when_value_unchanged(tmp_ +@pytest.mark.parametrize("profile_mode", [False, True]) +def test_load_pool_prunes_legacy_raw_codex_borrowed_entry( + tmp_path, monkeypatch, profile_mode, +): + """Shared Codex persistence must not restore a stale borrowed row.""" + sentinel = "S3NTINEL_DO_NOT_PERSIST_STALE_CODEX" + auth_home = tmp_path / "hermes" + if profile_mode: + auth_home = auth_home / "profiles" / "worker" + auth_home.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(auth_home)) + (auth_home / "auth.json").write_text(json.dumps({ + "version": 1, + "credential_pool": { + "openai-codex": [ + { + "id": "legacy-vault-codex", + "label": "vault-ref", + "auth_type": "oauth", + "priority": 0, + "source": "vault:codex", + "access_token": sentinel, + "refresh_token": f"refresh-{sentinel}", + } + ] + }, + })) + + from agent.credential_pool import load_pool + + pool = load_pool("openai-codex") + + assert pool.entries() == [] + auth_text = (auth_home / "auth.json").read_text() + assert sentinel not in auth_text + assert json.loads(auth_text)["credential_pool"]["openai-codex"] == [] + + + def test_pooled_credential_to_dict_strips_borrowed_secret_fields(): from agent.credential_pool import PooledCredential @@ -1095,6 +1134,53 @@ def test_write_credential_pool_treats_unowned_oauth_source_as_borrowed(tmp_path, +@pytest.mark.parametrize("profile_mode", [False, True]) +def test_write_codex_pool_sanitizes_preserved_borrowed_payload( + tmp_path, monkeypatch, profile_mode, +): + """Merged shared-store rows are sanitized at the final disk boundary.""" + sentinel = "S3NTINEL_DO_NOT_PERSIST_PRESERVED_CODEX" + auth_home = tmp_path / "hermes" + if profile_mode: + auth_home = auth_home / "profiles" / "worker" + auth_home.mkdir(parents=True) + monkeypatch.setenv("HERMES_HOME", str(auth_home)) + (auth_home / "auth.json").write_text(json.dumps({ + "version": 1, + "credential_pool": { + "openai-codex": [ + { + "id": "legacy-vault-codex", + "label": "vault-ref", + "auth_type": "oauth", + "priority": 0, + "source": "vault:codex", + "access_token": sentinel, + "refresh_token": f"refresh-{sentinel}", + } + ] + }, + })) + + from hermes_cli.auth import write_credential_pool + + write_credential_pool( + "openai-codex", + [], + preserve_shared_entries=True, + preserve_profile_entries=True, + ) + + auth_text = (auth_home / "auth.json").read_text() + assert sentinel not in auth_text + persisted = json.loads(auth_text)["credential_pool"]["openai-codex"][0] + assert persisted["source"] == "vault:codex" + assert "access_token" not in persisted + assert "refresh_token" not in persisted + assert persisted["secret_fingerprint"].startswith("sha256:") + + + def test_write_credential_pool_preserves_known_provider_owned_oauth_state(tmp_path, monkeypatch): sentinel = "PROVIDER_OWNED_DEVICE_CODE_STAYS_PERSISTABLE" monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))