Skip to content

fix(auth): openai-codex multi-profile rotation write-through (#48415) - #49032

Closed
ochsec wants to merge 3 commits into
NousResearch:mainfrom
ochsec:fix/issue-48415-codex-multi-profile-rotation
Closed

fix(auth): openai-codex multi-profile rotation write-through (#48415)#49032
ochsec wants to merge 3 commits into
NousResearch:mainfrom
ochsec:fix/issue-48415-codex-multi-profile-rotation

Conversation

@ochsec

@ochsec ochsec commented Jun 19, 2026

Copy link
Copy Markdown

Problem

When a profile session reads the openai-codex grant from the root fallback and refreshes it, the rotated refresh token was saved only to the profile store. The root kept the now-revoked refresh token, so every sibling profile reading root's stale grant died with refresh_token_reused once their access token expired.

This is the Codex analog of #43589 (which fixed the same hazard for xai-oauth).

Fix

Mirrors the existing xAI OAuth write-through pattern for the openai-codex provider:

  • _profile_has_own_codex_state() — new helper, mirrors _profile_has_own_xai_oauth_state(). Returns True when the profile has its own providers.openai-codex block (meaning it genuinely shadows root and must not clobber it).
  • _write_through_codex_to_global_root() — new helper, mirrors _write_through_xai_oauth_to_global_root(). Best-effort write of the rotated state to the global root auth.json. 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.
  • _save_codex_tokens() — check _profile_has_own_codex_state() before modifying auth_store, then conditionally call _write_through_codex_to_global_root() after the profile save.
  • CredentialPool._sync_device_code_entry_to_auth_store() — wire the same write-through check for the openai-codex branch. Also adds the missing write-through for the xai-oauth branch (this was a pre-existing gap — the pool-sync path was not covered by Bug: profile auth shadowing + rotating refresh tokens guarantees grant revocation on multi-profile installs #43589).

Key design details

  • The _profile_has_own_codex_state check happens before _save_provider_state modifies auth_store, because after the save the profile would always appear to have its own block.
  • _store_provider_state(global_store, ..., set_active=False) ensures we never flip root's active_provider.
  • The pytest seat belt in _write_through_codex_to_global_root refuses to write the real user's ~/.hermes/auth.json under test.

Tests

5 new regression tests in tests/hermes_cli/test_codex_oauth_writethrough.py:

  1. Write-through fires when profile has no own codex block → rotated tokens land in root
  2. Write-through skipped when profile has its own codex block → root untouched
  3. Noop in classic mode when _global_auth_file_path() returns None
  4. Failure isolation — a root write-through failure does not break the profile save
  5. active_provider preserved — write-through does not flip root's active provider

All existing xAI write-through tests continue to pass.

Files changed

File Change
hermes_cli/auth.py +_profile_has_own_codex_state, +_write_through_codex_to_global_root, wire into _save_codex_tokens
agent/credential_pool.py Wire write-through for openai-codex and xai-oauth branches in _sync_device_code_entry_to_auth_store
tests/hermes_cli/test_codex_oauth_writethrough.py New: 5 regression tests

Hermes Agent added 3 commits June 18, 2026 13:22
When a profile session reads the openai-codex grant from the root fallback
and refreshes it, the rotated refresh token was saved only to the profile
store. The root kept the now-revoked refresh token, so every sibling profile
reading root's stale grant died with refresh_token_reused.

This mirrors the xAI OAuth write-through pattern (#43589) for Codex:

- Add _profile_has_own_codex_state() helper (mirrors _profile_has_own_xai_oauth_state)
- Add _write_through_codex_to_global_root() helper (mirrors _write_through_xai_oauth_to_global_root)
- Wire write-through into _save_codex_tokens() in auth.py
- Wire write-through into CredentialPool._sync_device_code_entry_to_auth_store() in credential_pool.py
- Check BEFORE modifying auth_store so the profile doesn't look like it has its own block
- Use set_active=False to avoid flipping root's active_provider

Also fixes a pre-existing gap: the xAI branch in
_sync_device_code_entry_to_auth_store() was missing the write-through check
that auth.py's _save_xai_oauth_tokens() already had. Added it for parity.

Tests: 5 new regression tests covering write-through, no-write-when-shadow,
classic-mode noop, failure isolation, and active_provider preservation.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround comp/cli CLI entry point, hermes_cli/, setup wizard comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/auth Authentication, OAuth, credential pools provider/openai OpenAI / Codex Responses API labels Jun 19, 2026

@egilewski egilewski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requesting changes

Blocking checks:

  • The PR is not focused on a single reviewable change. Its title/body and auth commit cover openai-codex OAuth token write-through, but the same PR head also includes unrelated kanban blocked-task behavior from commit 7d7801347 and changes hermes_cli/kanban_db.py plus tests/hermes_cli/test_kanban_blocked_sticky.py.
  • Reproduce with gh pr diff 49032 --repo NousResearch/hermes-agent --name-only and gh pr view 49032 --repo NousResearch/hermes-agent --json commits --jq '.commits[].messageHeadline'.

Security evidence:

  • trust boundary: profile/root OAuth credential state for openai-codex; review stopped before validating it because unrelated kanban behavior is bundled.
  • source/sink/invariant: security PRs need a focused diff so auth-token rotation changes can be reviewed and tested without unrelated task-dispatch behavior.
  • current-main reproduction: current main was refreshed to 1b04e4ede5199102f54393abec8e128ddd994645; git merge-tree --write-tree refs/remotes/security-review-49032/main refs/remotes/security-review-49032/pr succeeded, so this is not a merge-conflict stop.
  • PR-head or patch-replay validation: PR head 2491a21781bcc79e573858e9e74684c08b3d9c2b includes both auth files/tests and kanban files/tests; the no-merge commit list includes 2491a2178 for auth and 7d7801347 for kanban.
  • positive/negative cases: auth regression tests are present, but the unrelated kanban code/test pair makes focused security validation premature.
  • residual bypass search: not run; review stopped at the first serious PR-shape blocker, so there may be additional issues.

Please split the kanban change out or retarget this PR to only the auth write-through fix, then request another review.

Signed: GPT-5.5-xhigh in Codex

@ochsec

ochsec commented Jun 19, 2026

Copy link
Copy Markdown
Author

Closing in favor of #49127 — this branch contained unrelated kanban commits that were included in the PR diff. The new PR has a clean branch based on upstream/main with only the auth write-through changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P1 High — major feature broken, no workaround provider/openai OpenAI / Codex Responses API type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants