feat(auth): add secure loopback OAuth broker for Codex profiles - #64406
feat(auth): add secure loopback OAuth broker for Codex profiles#64406aslxiaomu wants to merge 5 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the substantial security-focused implementation. The underlying Codex refresh race is already addressed on current main, so this needs review as a new optional broker architecture rather than as a still-unfixed concurrency repair.
Problems
- Current main already locks the Codex refresh read→POST→write-back sequence in
agent/credential_pool.py:1022-1044; commitda6d5fcd13af2adb6ce7961e06f85cf714fde7f5documents that exact single-use-token race. - The guide is added under
docs/user-guide/features/oauth-broker.md:1, whereas published docs are sourced fromwebsite/docs;website/sidebars.ts:53-123also has no new entry. Itsdocs/design/oauth-broker.mdreference at line 7 is not added by this PR. tests/hermes_cli/test_oauth_broker_cli.py:136readshermes_cli/main.pyand asserts strings, contrary to the source-reading test prohibition inAGENTS.md:1370.
Suggested changes
- Reframe the feature’s independent value and move the guide into the published docs surface.
- Replace the source-shape assertion with a real parser/dispatch behavior test.
Automated hermes-sweeper review.
| chain per account alias (`A`, `B`, `C`) and transparently forwards Codex | ||
| Responses/usage traffic for local Hermes profiles. Profiles stop holding | ||
| OpenAI tokens entirely — they persist only a `keychain://` reference to the | ||
| broker's local client key. |
There was a problem hiding this comment.
This guide is outside the Docusaurus source tree (website/docs), and this referenced docs/design/oauth-broker.md file is not part of the PR. Please move the published guide into website/docs/user-guide/features/, add the sidebar entry, and either add this design document or remove the link.
|
|
||
|
|
||
| def test_main_registers_oauth_broker_parser(): | ||
| main_source = Path("hermes_cli/main.py").read_text(encoding="utf-8") |
There was a problem hiding this comment.
Please replace this source-text assertion with a behavior test. AGENTS.md explicitly forbids tests that read source code; parse/dispatch the real CLI and assert the registered command behavior instead.
What does this PR do?
Adds an opt-in, per-machine loopback OAuth broker for OpenAI Codex profiles. The broker owns one rotating OAuth grant per account alias (
A,B,C), keeps grants and the local client key in macOS Keychain, and transparently proxies Codex Responses and usage traffic.The goal is to prevent multiple Hermes runtimes from cloning and concurrently consuming the same single-use refresh token. The broker is fail-closed: profiles do not retain upstream access/refresh tokens and there is no automatic fallback to a cloned legacy pool.
This is related to #23944 and the refresh-token failure described in #22903.
Related open PRs were reviewed before submission:
Related Issue
Related to #23944 and #22903.
Type of Change
Changes Made
agent/oauth_broker/with per-account singleflight refresh, cross-process locks, recovery markers, strict grant schemas, generation-aware 401 replay, and transparent 429/SSE forwarding.keychain://secret resolution without shelling out to thesecurityCLI.hermes oauth-brokercommands for auth, run/status/doctor, launchd lifecycle, migration, and rollback.How to Test
741 passed, 1 deselected.python -m pytest -q -o 'addopts=' \ tests/hermes_cli/test_startup_plugin_gating.py \ tests/hermes_cli/test_oauth_broker_cli.py \ tests/hermes_cli/test_oauth_broker_help.py60 passed.gitwrapper intercepts temp-HERMES_HOMErepository tests. An exact-parent baseline run at226e8de82reported 33 failing files / 180 failed tests; the post-fix branch run reported 31 failing files / 177 failed tests, and every branch failure file is contained in the baseline set (zero branch-only failing files). The changed-area suites above pass.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — the exact parent baseline and this branch both hit the same machine-level Git-wrapper/temp-HERMES_HOMEfailure family; focused changed-area tests passDocumentation & Housekeeping
cli-config.yaml.example— N/A; no persistent config key is addedCONTRIBUTING.md/AGENTS.md— N/A; contributor workflow is unchangedScreenshots / Logs
741 passed, 1 deselected60 passed