diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 04b22c76a684..49389c6bab32 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -26,13 +26,16 @@ _auth_store_lock, _codex_access_token_is_expiring, _decode_jwt_claims, + _global_auth_file_path, _load_auth_store, _load_provider_state, + _profile_has_own_codex_oauth_state, _resolve_kimi_base_url, _resolve_zai_base_url, _save_auth_store, _save_provider_state, _store_provider_state, + _write_through_codex_oauth_to_global_root, read_credential_pool, write_credential_pool, ) @@ -797,6 +800,14 @@ def _sync_device_code_entry_to_auth_store(self, entry: PooledCredential) -> None if entry.source not in {"device_code", "loopback_pkce"}: return try: + # When an openai-codex grant was resolved from the global-root + # fallback (this profile has no own providers.openai-codex block) + # and the runtime rotated it, the rotated chain must be written + # back to root too — otherwise root keeps a revoked refresh token + # and every other profile reading root's stale grant dies with + # ``refresh_token_reused``. Mirrors the xAI write-through (#43589). + # Captured here so we can fire it after the profile save below. + codex_write_through_state: Optional[Dict[str, Any]] = None with _auth_store_lock(): auth_store = _load_auth_store() if self.provider == "nous": @@ -823,6 +834,13 @@ def _sync_device_code_entry_to_auth_store(self, entry: PooledCredential) -> None _store_provider_state(auth_store, "nous", state, set_active=False) elif self.provider == "openai-codex": + # Profile-mode + no own openai-codex block ⇒ this grant was + # resolved from the root fallback; the rotated chain must be + # written through to root after the profile save (#43589). + write_through_to_root = ( + _global_auth_file_path() is not None + and not _profile_has_own_codex_oauth_state(auth_store) + ) state = _load_provider_state(auth_store, "openai-codex") if not isinstance(state, dict): return @@ -835,6 +853,8 @@ def _sync_device_code_entry_to_auth_store(self, entry: PooledCredential) -> None if entry.last_refresh: state["last_refresh"] = entry.last_refresh _store_provider_state(auth_store, "openai-codex", state, set_active=False) + if write_through_to_root: + codex_write_through_state = state elif self.provider == "xai-oauth": state = _load_provider_state(auth_store, "xai-oauth") @@ -854,6 +874,19 @@ def _sync_device_code_entry_to_auth_store(self, entry: PooledCredential) -> None return _save_auth_store(auth_store) + # Best-effort write-through of the rotated openai-codex grant to + # the global root (#43589). Swallow all errors: a failed root + # write must never break the profile's own successful save. + if codex_write_through_state is not None: + try: + _write_through_codex_oauth_to_global_root( + codex_write_through_state + ) + except Exception as wexc: # pragma: no cover - best effort + logger.debug( + "openai-codex OAuth: write-through to global root failed: %s", + wexc, + ) except Exception as exc: logger.debug("Failed to sync %s pool entry back to auth store: %s", self.provider, exc) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 61c2bbed7865..a2905c7ef472 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -4142,6 +4142,64 @@ def _write_through_xai_oauth_to_global_root(state: Dict[str, Any]) -> None: logger.debug("xAI OAuth: write-through to global root failed: %s", exc) +def _profile_has_own_codex_oauth_state(auth_store: Dict[str, Any]) -> bool: + """True when this store has its OWN ``providers.openai-codex`` block. + + Distinguishes a profile that genuinely shadows the root Codex grant from + one that only *reads* root via ``_load_provider_state``'s fallback. Only + the latter needs the refresh write-through below. Mirrors + ``_profile_has_own_xai_oauth_state`` (#43589) for openai-codex. + """ + providers = auth_store.get("providers") + return isinstance(providers, dict) and isinstance(providers.get("openai-codex"), dict) + + +def _write_through_codex_oauth_to_global_root(state: Dict[str, Any]) -> None: + """Persist a rotated openai-codex OAuth ``state`` into the global-root auth.json. + + Best-effort write-through for the multi-profile rotation hazard (mirrors + the xAI fix #43589): openai-codex rotates the refresh_token on every + refresh, so when a profile session refreshes a grant it resolved from the + root fallback, the rotated chain must land back in root. Otherwise root + keeps a now-revoked refresh token and every other profile reading the + stale root grant dies with ``refresh_token_reused`` once its access token + expires. + + Only updates ``providers.openai-codex`` in the root store; never touches + the profile store (the caller already saved that). Swallows all errors — a + failed write-through degrades to the pre-existing behavior (root stale), + it must never break the profile's own successful save. + """ + global_path = _global_auth_file_path() + if global_path is None: + # Classic mode (profile == root); the profile save already hit root. + return + # Seat belt: under pytest, refuse to write the real user's + # ~/.hermes/auth.json even when HERMES_HOME points at a profile path + # (mirrors the read-side guard in _load_global_auth_store). Uses the + # unmodified HOME env, not Path.home() which fixtures may monkeypatch. + if os.environ.get("PYTEST_CURRENT_TEST"): + real_home_env = os.environ.get("HOME", "") + if real_home_env: + real_root = Path(real_home_env) / ".hermes" / "auth.json" + try: + if global_path.resolve(strict=False) == real_root.resolve(strict=False): + return + except Exception: + return + try: + if global_path.exists(): + global_store = _load_auth_store(global_path) + else: + global_store = {} + if not isinstance(global_store, dict): + return + _store_provider_state(global_store, "openai-codex", dict(state), set_active=False) + _save_auth_store(global_store, global_path) + except Exception as exc: # pragma: no cover - best effort + logger.debug("openai-codex OAuth: write-through to global root failed: %s", exc) + + def _save_xai_oauth_tokens( tokens: Dict[str, Any], *, diff --git a/scripts/release.py b/scripts/release.py index 36cc15008a0b..25aefb28468e 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -45,6 +45,7 @@ # Auto-extracted from noreply emails + manual overrides AUTHOR_MAP = { + "spiky02plateau@users.noreply.github.com": "spiky02plateau", "59806492+sitkarev@users.noreply.github.com": "sitkarev", "zheng@omegasys.eu": "omegazheng", "220877172+james47kjv@users.noreply.github.com": "james47kjv", diff --git a/tests/agent/test_codex_oauth_writethrough.py b/tests/agent/test_codex_oauth_writethrough.py new file mode 100644 index 000000000000..08e191922320 --- /dev/null +++ b/tests/agent/test_codex_oauth_writethrough.py @@ -0,0 +1,219 @@ +"""Regression tests for openai-codex OAuth refresh write-through to global root. + +Companion to ``tests/hermes_cli/test_xai_oauth_writethrough.py``. That file +covers the xAI WRITE side; these cover the equivalent for openai-codex. + +The hazard (mirrors xAI #43589): openai-codex rotates the refresh_token on +every refresh. When a profile that has no own ``providers.openai-codex`` block +resolves the grant from the global-root fallback and the credential pool +rotates it, the rotated chain is written only to the PROFILE auth store — +leaving root holding a now-revoked refresh token. Every other profile reading +root's stale grant then dies with ``refresh_token_reused`` once its access +token expires. + +These tests drive the real +``CredentialPool._sync_device_code_entry_to_auth_store`` against real on-disk +auth stores (profile + root under ``tmp_path``) rather than mocking the save +boundary, so they exercise the actual atomic write + write-through path. +""" + +import json + +import pytest + +from hermes_cli import auth +from agent import credential_pool as cp +from agent.credential_pool import CredentialPool, PooledCredential + + +def _write_store(path, store): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(store), encoding="utf-8") + + +def _read_store(path): + return json.loads(path.read_text(encoding="utf-8")) + + +def _codex_entry() -> PooledCredential: + """A pooled openai-codex entry carrying freshly-rotated tokens.""" + return PooledCredential( + provider="openai-codex", + id="openai-codex", + label="codex", + auth_type="oauth", + priority=0, + source="device_code", + access_token="new-access", + refresh_token="new-refresh", + last_refresh="2026-06-18T00:00:00Z", + ) + + +def _sync(entry: PooledCredential) -> None: + pool = CredentialPool("openai-codex", [entry]) + pool._sync_device_code_entry_to_auth_store(entry) + + +@pytest.fixture +def profile_and_root(tmp_path, monkeypatch): + """Wire a profile auth store + a distinct global-root auth store on disk. + + Returns (profile_path, root_path). The pytest seat belt in + ``_write_through_codex_oauth_to_global_root`` only refuses the *real* + user's ``$HOME/.hermes/auth.json``; a tmp_path root is allowed, so we + point HOME away from the tmp root to keep the guard from tripping. + """ + profile_path = tmp_path / "profiles" / "work" / "auth.json" + root_path = tmp_path / "root" / "auth.json" + + # Patch both namespaces: auth.py owns the write path + the write-through + # helper's own resolution, while credential_pool.py binds its own imported + # references for the gate check (`from hermes_cli.auth import ...`). + monkeypatch.setattr(auth, "_auth_file_path", lambda: profile_path) + monkeypatch.setattr(auth, "_global_auth_file_path", lambda: root_path) + monkeypatch.setattr(cp, "_global_auth_file_path", lambda: root_path) + # Keep the pytest write seat belt from matching our tmp root. + monkeypatch.setenv("HOME", str(tmp_path / "not-the-root")) + return profile_path, root_path + + +def test_refresh_writes_through_to_root_when_profile_has_no_own_state(profile_and_root): + """Profile reading root's grant must push rotated tokens back to root.""" + profile_path, root_path = profile_and_root + # Profile has NO own openai-codex block (reads root via fallback). The + # codex sync path requires an existing tokens dict to mutate, which + # _load_provider_state resolves from the root fallback. + _write_store(profile_path, {"version": 1, "providers": {}}) + _write_store( + root_path, + { + "version": 1, + "providers": { + "openai-codex": { + "tokens": { + "access_token": "old-access", + "refresh_token": "old-refresh", + } + } + }, + }, + ) + + _sync(_codex_entry()) + + # Profile got the rotated chain (existing behavior). + profile = _read_store(profile_path) + assert profile["providers"]["openai-codex"]["tokens"]["refresh_token"] == "new-refresh" + + # AND the global root no longer holds the revoked refresh token (#43589). + root = _read_store(root_path) + assert root["providers"]["openai-codex"]["tokens"]["access_token"] == "new-access" + assert root["providers"]["openai-codex"]["tokens"]["refresh_token"] == "new-refresh" + + +def test_refresh_does_not_touch_root_when_profile_has_own_state(profile_and_root): + """A profile that genuinely shadows root must NOT clobber the root grant.""" + profile_path, root_path = profile_and_root + # Profile has its OWN openai-codex block: it shadows root legitimately. + _write_store( + profile_path, + { + "version": 1, + "providers": { + "openai-codex": { + "tokens": { + "access_token": "profile-old", + "refresh_token": "profile-old-refresh", + } + } + }, + }, + ) + _write_store( + root_path, + { + "version": 1, + "providers": { + "openai-codex": { + "tokens": { + "access_token": "root-untouched", + "refresh_token": "root-untouched-refresh", + } + } + }, + }, + ) + + _sync(_codex_entry()) + + profile = _read_store(profile_path) + assert profile["providers"]["openai-codex"]["tokens"]["refresh_token"] == "new-refresh" + + # Root is a separate grant chain — must be left exactly as-is. + root = _read_store(root_path) + assert root["providers"]["openai-codex"]["tokens"]["access_token"] == "root-untouched" + assert root["providers"]["openai-codex"]["tokens"]["refresh_token"] == "root-untouched-refresh" + + +def test_write_through_is_noop_in_classic_mode(tmp_path, monkeypatch): + """Classic mode (profile == root) already saves to root; no double write.""" + profile_path = tmp_path / "auth.json" + monkeypatch.setattr(auth, "_auth_file_path", lambda: profile_path) + # Classic mode: _global_auth_file_path returns None in both namespaces. + monkeypatch.setattr(auth, "_global_auth_file_path", lambda: None) + monkeypatch.setattr(cp, "_global_auth_file_path", lambda: None) + _write_store( + profile_path, + { + "version": 1, + "providers": { + "openai-codex": { + "tokens": { + "access_token": "old-access", + "refresh_token": "old-refresh", + } + } + }, + }, + ) + + # Should not raise and should persist to the single store. + _sync(_codex_entry()) + store = _read_store(profile_path) + assert store["providers"]["openai-codex"]["tokens"]["refresh_token"] == "new-refresh" + + +def test_write_through_failure_does_not_break_profile_save(profile_and_root, monkeypatch): + """A failed root write-through must not break the profile's own save.""" + profile_path, root_path = profile_and_root + _write_store(profile_path, {"version": 1, "providers": {}}) + _write_store( + root_path, + { + "version": 1, + "providers": { + "openai-codex": { + "tokens": { + "access_token": "old-access", + "refresh_token": "old-refresh", + } + } + }, + }, + ) + + # Make the root write blow up; the profile save must still succeed. + real_save = auth._save_auth_store + + def _exploding_save(store, target_path=None): + if target_path is not None and target_path == root_path: + raise OSError("simulated root write failure") + return real_save(store, target_path) + + monkeypatch.setattr(auth, "_save_auth_store", _exploding_save) + + _sync(_codex_entry()) + + profile = _read_store(profile_path) + assert profile["providers"]["openai-codex"]["tokens"]["refresh_token"] == "new-refresh"