From 6bd29f26f6631be5b02db7e7d23c75800fd3934a Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sun, 16 Aug 2026 15:15:23 +0800 Subject: [PATCH] fix(auth): write profile-refreshed Codex tokens through to the global store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex refresh tokens are single-use with rotation-family reuse detection. _save_codex_tokens resolved the state via the profile's root fallback but always persisted into the ACTIVE (profile) store, so a profile-scoped refresh left the global store holding the consumed refresh token — the next process to read it replayed it and OpenAI revoked the whole rotation family, forcing a manual device-code re-auth (#87503; observed four times on one multi-profile deployment). Mirror the xAI source-aware save (#43589/#74339): resolve the state with _load_provider_state_with_source; when the grant came from the global root, write the rotated chain back to root only — singleton AND credential_pool entries, under the root store's own lock, without creating a shadowing profile key. Best-effort, with the same pytest seat belt as the xAI path. Fixes #87503 --- hermes_cli/auth_codex.py | 29 +++- .../test_codex_token_writethrough.py | 153 ++++++++++++++++++ 2 files changed, 176 insertions(+), 6 deletions(-) create mode 100644 tests/hermes_cli/test_codex_token_writethrough.py diff --git a/hermes_cli/auth_codex.py b/hermes_cli/auth_codex.py index 584ef738fb..33afc236e1 100644 --- a/hermes_cli/auth_codex.py +++ b/hermes_cli/auth_codex.py @@ -143,15 +143,22 @@ def _sync_codex_pool_entries( def _save_codex_tokens(tokens: Dict[str, str], last_refresh: str = None, label: str = None) -> None: - """Save Codex OAuth tokens to Hermes auth store (~/.hermes/auth.json).""" + """Save Codex OAuth tokens to the auth store the grant was resolved FROM. + + Codex refresh tokens are single-use with rotation-family reuse detection. A profile without its + own ``providers.openai-codex`` block reads root's grant via the fallback, so a refresh under that + profile must rotate ROOT's chain — singleton AND ``credential_pool`` entries — or root keeps the + consumed refresh token, the next process replays it and OpenAI revokes the whole family + (#87503). Root-only write-back: a profile copy would shadow root and disable the write-through + on the next refresh (#74339). Mirrors the xAI source-aware save. + """ from hermes_cli.auth import ( - _auth_store_lock, _load_auth_store, _load_provider_state, _save_auth_store, - _save_provider_state, _utc_now_z) + _auth_file_path, _load_auth_store, _provider_state_transaction, _same_path, + _save_auth_store, _save_provider_state, _store_provider_state, _utc_now_z) if last_refresh is None: last_refresh = _utc_now_z() - with _auth_store_lock(): - auth_store = _load_auth_store() - state = _load_provider_state(auth_store, "openai-codex") or {} + with _provider_state_transaction("openai-codex") as (auth_store, state, source_path): + state = dict(state) if state else {} # Capture the previous singleton tokens BEFORE overwriting: the pool sync uses them to # tell legacy singleton-aliases (refresh) from independent ``auth add`` accounts (keep). previous_singleton_tokens = ( @@ -159,6 +166,16 @@ def _save_codex_tokens(tokens: Dict[str, str], last_refresh: str = None, label: state.update(tokens=tokens, last_refresh=last_refresh, auth_mode="chatgpt") if label and str(label).strip(): state["label"] = str(label).strip() + if source_path is not None and not _same_path(source_path, _auth_file_path()): + # Root-borrowed grant: the transaction already holds root's lock, so write the rotated + # chain into ROOT's store (never set_active — a refresh is not a provider choice). + root_store = _load_auth_store(source_path) + _store_provider_state(root_store, "openai-codex", state, set_active=False) + _sync_codex_pool_entries( + root_store, tokens, last_refresh, + previous_singleton_tokens=previous_singleton_tokens) + _save_auth_store(root_store, target_path=source_path) + return _save_provider_state(auth_store, "openai-codex", state) _sync_codex_pool_entries( auth_store, tokens, last_refresh, previous_singleton_tokens=previous_singleton_tokens) diff --git a/tests/hermes_cli/test_codex_token_writethrough.py b/tests/hermes_cli/test_codex_token_writethrough.py new file mode 100644 index 0000000000..b6d85f039e --- /dev/null +++ b/tests/hermes_cli/test_codex_token_writethrough.py @@ -0,0 +1,153 @@ +"""Regression tests for Codex OAuth refresh write-through to the global root. + +Mirrors ``test_xai_oauth_writethrough.py`` for the Codex family (#87503): +Codex refresh tokens are single-use with rotation-family reuse detection, +so when a profile that has no own ``providers.openai-codex`` block refreshes +the grant it resolved from the root fallback, the rotated chain must land +back in root — including the ``credential_pool`` entries the runtime selects +credentials from. Otherwise root keeps the consumed refresh token, the next +process to read it replays it, and OpenAI revokes the whole rotation family. + +The tests drive the real ``_save_codex_tokens`` against real on-disk auth +stores (profile + root under ``tmp_path``). All token values are synthetic +placeholders assembled by ``_pair`` — no real credentials are involved. +""" + +import json + +import pytest + +from hermes_cli import auth + + +def _pair(prefix: str) -> dict: + """Synthetic OAuth pair for fixtures/assertions (not credentials).""" + return { + "access_token": f"{prefix}-at", + "refresh_token": f"{prefix}-rt", + } + + +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")) + + +@pytest.fixture +def profile_and_root(tmp_path, monkeypatch): + """Wire a profile auth store + a distinct global-root auth store on disk.""" + profile_path = tmp_path / "profiles" / "work" / "auth.json" + root_path = tmp_path / "root" / "auth.json" + + monkeypatch.setattr(auth, "_auth_file_path", lambda: profile_path) + monkeypatch.setattr(auth, "_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_profile_refresh_of_root_grant_writes_through(profile_and_root): + """#87503: rotating the root-resolved grant must reach the root store — + singleton AND credential-pool entries — without forking a shadowing + profile key.""" + profile_path, root_path = profile_and_root + _write_store( + root_path, + { + "version": 1, + "providers": { + "openai-codex": { + "auth_mode": "chatgpt", + "tokens": _pair("old"), + } + }, + "credential_pool": { + "openai-codex": [ + { + "provider": "openai-codex", + "source": "device_code", + **_pair("old"), + } + ] + }, + }, + ) + _write_store(profile_path, {"version": 1, "providers": {}}) + + rotated = _pair("new") + auth._save_codex_tokens( + rotated, + last_refresh="2026-08-16T00:00:00Z", + ) + + root = _read_store(root_path) + assert ( + root["providers"]["openai-codex"]["tokens"]["refresh_token"] + == rotated["refresh_token"] + ), "root singleton must hold the rotated refresh token" + pool_entry = root["credential_pool"]["openai-codex"][0] + assert pool_entry["refresh_token"] == rotated["refresh_token"] + assert pool_entry["access_token"] == rotated["access_token"] + + profile = _read_store(profile_path) + assert "openai-codex" not in profile.get("providers", {}), ( + "profile must not gain a shadowing providers.openai-codex key — " + "it would disable the write-through on the next refresh (#74339)" + ) + + +def test_profile_owned_state_saves_to_profile_only(profile_and_root): + """A profile with its own openai-codex block keeps the existing + profile-local save; root is untouched.""" + profile_path, root_path = profile_and_root + _write_store( + profile_path, + { + "version": 1, + "providers": { + "openai-codex": { + "auth_mode": "chatgpt", + "tokens": _pair("prof"), + } + }, + }, + ) + _write_store(root_path, {"version": 1, "providers": {}}) + + rotated = _pair("next") + auth._save_codex_tokens( + rotated, + last_refresh="2026-08-16T00:00:00Z", + ) + + profile = _read_store(profile_path) + assert ( + profile["providers"]["openai-codex"]["tokens"]["refresh_token"] + == rotated["refresh_token"] + ) + root = _read_store(root_path) + assert "openai-codex" not in root.get("providers", {}) + + +def test_classic_mode_still_saves_single_store(tmp_path, monkeypatch): + """Classic mode (profile == root): unchanged single-store save.""" + profile_path = tmp_path / "auth.json" + monkeypatch.setattr(auth, "_auth_file_path", lambda: profile_path) + monkeypatch.setattr(auth, "_global_auth_file_path", lambda: None) + _write_store(profile_path, {"version": 1, "providers": {}}) + + rotated = _pair("classic") + auth._save_codex_tokens( + rotated, + last_refresh="2026-08-16T00:00:00Z", + ) + + store = _read_store(profile_path) + assert ( + store["providers"]["openai-codex"]["tokens"]["refresh_token"] + == rotated["refresh_token"] + )