Skip to content

fix(auth): share Codex OAuth across profiles - #28277

Open
netgridorg wants to merge 1 commit into
NousResearch:mainfrom
netgridorg:fix/shared-codex-auth-profiles
Open

fix(auth): share Codex OAuth across profiles#28277
netgridorg wants to merge 1 commit into
NousResearch:mainfrom
netgridorg:fix/shared-codex-auth-profiles

Conversation

@netgridorg

Copy link
Copy Markdown

Summary

  • Make OpenAI Codex OAuth use the global-root Hermes auth store from named profiles
  • Add a dedicated shared Codex auth lock/path so refresh-token rotation is coordinated across profile workers
  • Keep generic credential pool/profile fallback behavior unchanged
  • Add regression tests for stale profile-local Codex state and profile-triggered shared writes

Test Plan

  • python -m pytest tests/hermes_cli/test_auth_profile_fallback.py tests/hermes_cli/test_auth_codex_provider.py -q -o 'addopts='
  • python -m pytest tests/hermes_cli/test_auth_profile_fallback.py tests/hermes_cli/test_auth_codex_provider.py tests/cron/test_codex_execution_paths.py -q -o 'addopts='
  • hermes -p codereviewer chat -t safe -Q --ignore-rules -q 'Reply exactly: SHARED_CODEX_AUTH_OK'

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools provider/openai OpenAI / Codex Responses API labels May 19, 2026

@teknium1 teknium1 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.

Thanks for chasing this. I can verify the premise still exists on current main, but this PR needs a current-main salvage rather than a direct apply.

Problems

  • hermes_cli/auth.py:3365 and hermes_cli/auth.py:3509 on current main still use the active profile auth store for Codex singleton reads/writes, so the profile/root rotation hazard is real.
  • The PR's _save_codex_tokens edit is based on an older body. Current main has label support plus credential-pool synchronization at hermes_cli/auth.py:3528; applying this version would drop that newer behavior.
  • The PR only covers providers.openai-codex. Current main also has Codex pool fallback at hermes_cli/auth.py:3855 and pool refresh/sync behavior in agent/credential_pool.py; those sibling paths would still read profile-local pool state. The later related #34121 diff touches those paths, which is a useful signal for the missing scope.

Suggested changes

  • Port the shared-root Codex lock/store idea onto current main while preserving _save_codex_tokens(tokens, last_refresh=None, label=None), _sync_codex_pool_entries, and the independent manual:device_code safeguards.
  • Add regression coverage for both singleton state and credential_pool.openai-codex in named profiles, including profile-triggered writes back to the shared/root pool.

Automated hermes-sweeper review.

Comment thread hermes_cli/auth.py
@@ -3010,17 +3044,17 @@ def _read_codex_tokens(*, _lock: bool = True) -> Dict[str, Any]:


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.

This replacement is based on an older _save_codex_tokens body. Current main also accepts label and syncs provider updates into credential_pool.openai-codex; the shared-root write needs to preserve those newer behaviors or Codex re-auth will regress pool users.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing the cross-profile Codex refresh-token problem. The singleton premise remains valid on current main: hermes_cli/auth.py:3238-3243 reads the active profile store, and _load_provider_state() gives a local provider block priority over root fallback (hermes_cli/auth.py:1207-1219).

Problems

  • The diff only shares providers.openai-codex. Current Codex runtime also selects credential_pool.openai-codex: profile pool entries shadow root entries in hermes_cli/auth.py:1322-1366, and the pool refresh path uses the active-store lock in agent/credential_pool.py:1006-1028. That leaves the same stale rotating-token hazard on the pool path.
  • This branch predates current _save_codex_tokens(tokens, last_refresh=None, label=None) and its _sync_codex_pool_entries() call (hermes_cli/auth.py:3382-3407). Its older function body must not replace those behaviors.

Suggested changes

  • Port the shared-root approach while retaining current singleton label and pool-sync behavior.
  • Make Codex pool reads, writes, and refresh synchronization use the same canonical store, with regression coverage for stale profile-local pool entries and root write-back.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
@teknium1 teknium1 added the area/profiles Multi-profile isolation, HERMES_HOME scoping label Jul 19, 2026
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 area/profiles Multi-profile isolation, HERMES_HOME scoping comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists provider/openai OpenAI / Codex Responses API sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants