fix(security): harden Codex OAuth refresh ownership - #34644
Closed
cooper-oai wants to merge 27 commits into
Closed
Conversation
cooper-oai
marked this pull request as ready for review
May 29, 2026 13:40
Author
|
@codex review |
Author
|
Local PR readiness review is clean on Validation:
Review ledger:
|
Author
|
Superseded by a priority-ordered stack so each review stays bounded:
Part 1 contains the highest-priority refresh-token reuse prevention and targets upstream |
Author
|
Superseded by a priority-ordered stack so each review stays bounded:
Part 1 contains the highest-priority refresh-token reuse prevention and targets upstream |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
auth.json, while leaving other providers and independent manual Codex entries profile-localDEADCodex rows when resolving the access-token fallback(id, source)and reject ambiguous string-ID removal targetshermes auth listby(id, source)so colliding shared and profile-local IDs do not both appear activeUser-Agent: hermes-cli/<version>on refresh requestsWhy
OpenAI Codex OAuth refresh tokens rotate after use. Copying a Codex CLI refresh token into Hermes or refreshing the same Hermes token concurrently can submit an already-consumed token to the token endpoint.
The refresh transaction must hold one lock across:
The existing pool path synchronized before and after refresh but released the auth-store lock during the network request. Named profiles also used separate auth locks while reading fallback Codex state from the global root. Both left reuse windows. Later snapshot writes could also replay stale shared or independent manual rows after another process refreshed or removed them. Canonical reauth and terminal quarantine could also overwrite or remove independent families.
This is security-sensitive auth hardening. No live credentials or OpenAI auth endpoints were used during validation.
Profile migration
Named profiles now share one canonical Hermes-owned Codex token family at the Hermes root. Existing credentials without the new ownership marker are intentionally not refreshed in either classic or profile mode: users must run
hermes model, choose OpenAI Codex, and complete one fresh login so Hermes can claim an unambiguous token family.Relationship to #34121
This includes the profile-sharing intent from #34121 and extends it:
DEADpruning cannot delete an independent row with a colliding IDDEADaccess tokenhermes auth listuses the same composite identity as runtime routing when displaying the active rowValidation
python3 -m pytest -q tests/hermes_cli/test_auth_profile_fallback.py tests/hermes_cli/test_auth_codex_provider.py tests/agent/test_credential_pool.py tests/hermes_cli/test_auth_commands.py tests/hermes_cli/test_codex_cli_model_picker.py tests/hermes_cli/test_doctor.py tests/hermes_cli/test_codex_models.py tests/hermes_cli/test_status.py tests/hermes_cli/test_status_model_provider.py tests/gateway/test_platform_base.py448 passed, 2 skippedpython3 -m pytest -q tests/hermes_cli/test_auth*.py tests/hermes_cli/test_web_oauth_dispatch.py369 passedpython3 -m pytest -q tests/run_agent/test_codex_xai_oauth_recovery.py tests/agent/test_auxiliary_client.py tests/hermes_cli/test_proxy.py263 passed555 passedpython3 -m pytest -q tests/tools/test_delegate.py135 passedDEADrowsDEADlocal-row pruning preserving a colliding healthy shared row330/330scenarios passed with zero duplicate initial refresh-token submissions, stale-row resurrection, cross-profile ordering leaks, superseded alias POSTs, unclaimed canonical POSTs, refreshable aliases left behind after canonical quarantine, linked aliases surviving canonical removal, logout, or sibling-profile terminal quarantine, original auth failures attributed to surviving clean fallbacks, quarantinedDEADaccess-token fallback, stale borrowed rows restored by shared-store merges, cross-row deletion or pruning through colliding IDs, suppressed canonical rows exposed after removal, identity-less removed rows restored after reload, sibling cleanup failure blocking canonical logout, busy sibling migration locks delaying canonical persistence, shared-store readers timing out before valid slow refreshes commit, colliding-row lease aliasing, ambiguous string-ID removal, or profile-local refreshes routed through colliding shared rowsruff check hermes_cli/auth.py hermes_cli/models.py gateway/platforms/base.py agent/credential_pool.py agent/credential_sources.py tests/hermes_cli/test_auth_codex_provider.py tests/hermes_cli/test_auth_profile_fallback.py tests/hermes_cli/test_auth_commands.py tests/hermes_cli/test_codex_cli_model_picker.py tests/agent/test_credential_pool.py tests/gateway/test_platform_base.pypython3 scripts/check-windows-footguns.py hermes_cli/auth.py hermes_cli/models.py gateway/platforms/base.py agent/credential_pool.py agent/credential_sources.py tests/hermes_cli/test_auth_codex_provider.py tests/hermes_cli/test_auth_profile_fallback.py tests/hermes_cli/test_auth_commands.py tests/hermes_cli/test_codex_cli_model_picker.py tests/agent/test_credential_pool.py tests/gateway/test_platform_base.pypython3 -m py_compile hermes_cli/auth.py hermes_cli/models.py gateway/platforms/base.py agent/credential_pool.py agent/credential_sources.pygit diff --checkValidated locally on Linux.
scripts/run_tests.shcould not start in this checkout because.venv,venv, and$HOME/.hermes/hermes-agent/venvare absent. The direct pytest commands above used the available environment.