Skip to content

fix(auth): preserve independent Codex OAuth profiles - #42434

Closed
DeadlySilent wants to merge 1 commit into
NousResearch:mainfrom
DeadlySilent:fix/codex-profile-auth-isolation
Closed

fix(auth): preserve independent Codex OAuth profiles#42434
DeadlySilent wants to merge 1 commit into
NousResearch:mainfrom
DeadlySilent:fix/codex-profile-auth-isolation

Conversation

@DeadlySilent

@DeadlySilent DeadlySilent commented Jun 8, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes Codex OAuth credential-pool isolation for multi-profile/multi-account setups.

A labelled Codex OAuth re-auth now updates only the intended profile entry and does not rewrite, relabel, or clear status on non-target Codex profiles. Unlabelled singleton refreshes keep the existing legacy-alias behavior, but when the provider singleton has a label, the refresh is scoped to that matching labelled pool entry instead of blindly updating the first device_code row.

This also prevents provider-singleton sync/seed paths from overwriting a device_code pool entry when the singleton label belongs to a different profile.

Related Issue

Refs #42102
Refs #39236
Complements #42110

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • hermes_cli/auth.py: labelled Codex token saves now target only the matching pool label; duplicate labels fail closed; labelled writes guard non-target entries against accidental mutation.
  • agent/credential_pool.py: Codex provider singleton sync/seed paths now respect label mismatches before adopting token material.
  • tests/hermes_cli/test_auth_codex_provider.py: added regressions for generic profile isolation, duplicate-label fail-closed behavior, and unlabelled refresh targeting.
  • tests/agent/test_credential_pool.py: added regression for provider singleton label mismatch.

How to Test

Focused pytest:

python -m pytest -q -o addopts='' tests/hermes_cli/test_auth_codex_provider.py tests/agent/test_credential_pool.py tests/hermes_cli/test_auth_commands.py

Canonical per-file runner for touched suites:

scripts/run_tests.sh tests/hermes_cli/test_auth_codex_provider.py tests/agent/test_credential_pool.py tests/hermes_cli/test_auth_commands.py

Static checks:

python -m py_compile hermes_cli/auth.py agent/credential_pool.py
git diff --check
python -m ruff check hermes_cli/auth.py agent/credential_pool.py tests/hermes_cli/test_auth_codex_provider.py tests/agent/test_credential_pool.py

Checklist

Code

  • I've read the Contributing Guide
  • My code follows the project's style guidelines
  • I've performed a self-review of my code
  • Code comments N/A: the changed logic is covered by names, structure, and regression tests without adding explanatory comments
  • My changes generate no new warnings
  • I've added tests that prove my fix is effective
  • New and existing focused tests pass locally

Documentation

  • Documentation N/A: behavior is covered by regression tests; no user-facing config or CLI docs changed
  • cli-config.yaml.example N/A: no config keys changed
  • CONTRIBUTING.md / AGENTS.md N/A: no contributor workflow changes

Testing

  • Focused pytest suite passed locally: 161 passed
  • Canonical per-file runner passed locally for touched suites: 161 passed
  • Ruff check passed for changed Python files
  • git diff --check passed

Security / Privacy

  • Tests use synthetic generic profile labels only (profile-one, profile-two, profile-three)
  • No real tokens, refresh tokens, credential fingerprints, account names, or user-specific paths are included
  • No auth store, backup file, or runtime log output is included

Screenshots / Logs

Not applicable. This is a credential-store behavior fix covered by regression tests with synthetic generic credential data only.

@liuhao1024

Copy link
Copy Markdown
Contributor

Verified — clean auth isolation logic

Reviewed the Codex OAuth profile isolation changes across credential_pool.py and auth.py. A few things that stand out as well-done:

  1. Snapshot guard pattern_codex_non_target_snapshot + _assert_codex_non_targets_unchanged is a strong safety net. Taking a tuple snapshot of all non-target entries before mutation and asserting equality after prevents silent cross-profile corruption. This is the right primitive for auth state where a bad write can invalidate independent accounts.

  2. Duplicate label detection — Failing closed with AuthError on ambiguous labels before writing auth.json prevents a nasty edge case where two entries with the same label get different token material.

  3. Label-aware legacy fallback — The unlabelled path correctly checks for a provider singleton label and routes to the matching pool entry instead of blindly updating the first device_code row. This preserves backward compat while closing the cross-profile overwrite gap.

  4. Test coverage — The three new test cases (label-scoped update, duplicate refusal, singleton label fallback) cover the critical state transitions. The assertion that non-target entries remain byte-identical to original is the right level of strictness.

No issues found. The safety-guard + snapshot pattern here is a good template for any future multi-profile auth state mutations.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools codex labels Jun 8, 2026
@DeadlySilent
DeadlySilent marked this pull request as ready for review June 9, 2026 06:24
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the careful credential-isolation regression work. Current main already provides this behavior, so this is superseded.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 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 codex comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants