Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions agent/credential_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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":
Expand All @@ -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
Expand All @@ -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")
Expand All @@ -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)

Expand Down
58 changes: 58 additions & 0 deletions hermes_cli/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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],
*,
Expand Down
1 change: 1 addition & 0 deletions scripts/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
219 changes: 219 additions & 0 deletions tests/agent/test_codex_oauth_writethrough.py
Original file line number Diff line number Diff line change
@@ -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"