Skip to content

fix(auth): stop Codex OAuth worker on cancel, pin resolved profile - #74341

Closed
JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/ia-01-oauth-cancel-worker
Closed

fix(auth): stop Codex OAuth worker on cancel, pin resolved profile#74341
JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/ia-01-oauth-cancel-worker

Conversation

@JoaoMarcos44

Copy link
Copy Markdown
Contributor
%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#00f0ff', 'mainBkg': '#0a0a16', 'primaryTextColor': '#ffffff', 'primaryBorderColor': '#ff007f', 'lineColor': '#00f0ff'}}}%%
graph TD
    A[🔒 Cancel Request] --> B{Old Behavior}
    A --> C{Fixed Behavior}

    B --> D[⚠️ Session Removed]
    D --> E[🐛 Worker Keeps Polling]
    E --> F[🐛 Code Approved Late]
    F --> G[💀 Profile Lookup Returns None]
    G --> H[💀 Tokens Written To Wrong Profile]

    C --> I[⚡ Cancel Event Set]
    I --> J[🚀 Worker Checks Event]
    J --> K[✅ Safe Abort - No Write]

    style B fill:#3a0a12,stroke:#ff007f
    style D fill:#3a0a12,stroke:#ff007f
    style E fill:#3a0a12,stroke:#ff007f
    style F fill:#3a0a12,stroke:#ff007f
    style G fill:#3a0a12,stroke:#ff007f
    style H fill:#3a0a12,stroke:#ff007f
    style C fill:#0a2a2f,stroke:#00f0ff
    style I fill:#0a2a2f,stroke:#00f0ff
    style J fill:#0a2a2f,stroke:#00f0ff
    style K fill:#0a2a2f,stroke:#00f0ff
Loading

Summary

  • Cancelling an OpenAI Codex device-code OAuth flow from the dashboard did not stop the background polling worker, and the worker could still write credentials afterward — potentially to the wrong profile.
  • Adds a per-session threading.Event that the cancel endpoint sets, and wires it into every checkpoint of the Codex device-code worker (poll loop, code exchange, token write).
  • Pins the worker's target profile once at session start instead of re-resolving it lazily at write time, so a cancelled/removed session can never fall back to whatever profile happens to be "current".

Root Cause

In hermes_cli/web_server.py:

  1. _codex_full_login_worker's poll loop (while time.monotonic() < deadline: time.sleep(poll_interval); poll = client.post(...)) never checked any cancellation signal — it ran to completion regardless of whether the dashboard had cancelled the session.
  2. DELETE /api/providers/oauth/sessions/{session_id} only did _oauth_sessions.pop(session_id, None) — it removed the session bookkeeping but never signalled the worker thread to stop.
  3. The token-write path resolved the destination profile lazily, at write time, via _oauth_session_profile(session_id). Once the session had been popped by cancellation, that lookup returned None, and _profile_scope(None) silently fell back to whatever profile was currently active — not the profile the session was originally started for.

Net effect: cancel a Codex login, then approve the device code late (or race the cancel against an in-flight approval) → the worker kept running, "succeeded", and wrote OAuth tokens into the wrong profile with no user-visible error.

Fix

  • _new_oauth_session now stores a threading.Event() (cancel_event) alongside each session.
  • DELETE /api/providers/oauth/sessions/{session_id} sets that event before popping the session from _oauth_sessions.
  • _codex_full_login_worker:
    • Captures cancel_event and the resolved target_profile once, at the very start, while the session entry is still guaranteed to exist.
    • Checks cancel_event.is_set() before and after each poll-loop sleep, before exchanging the device code for tokens, and immediately before calling _save_codex_tokens.
    • Returns immediately (no write, thread exits) the moment cancellation is observed.
    • Uses the pinned target_profile for the write instead of re-resolving _oauth_session_profile(session_id), so even a raced write always lands on the originally-intended profile — and only happens if the session was never cancelled.

The other device-code pollers (Nous, MiniMax, xAI) are unchanged — this fix is scoped to the Codex flow reported in the issue, since it's the only one that inlines its own poll loop directly in web_server.py.

Test Plan

  • Added test_codex_dashboard_worker_aborts_after_cancel in tests/hermes_cli/test_web_oauth_dispatch.py: starts a Codex session, cancels it via the real DELETE /api/providers/oauth/sessions/{id} endpoint from inside a monkeypatched time.sleep (simulating "user clicks Cancel while the worker is paused between polls"), then lets the poll "succeed" and code-exchange "succeed" — asserts _save_codex_tokens was never called and the session stays removed.
  • pytest tests/hermes_cli/test_web_oauth_dispatch.py -v → 25 passed (new test + all pre-existing OAuth dispatch tests, including the two pre-existing Codex worker tests, confirming no regression).
  • python -m py_compile hermes_cli/web_server.py → compiles cleanly.
  • Confirmed test_dashboard_oauth_write_uses_owner_only_permissions failure in test_web_server_oauth_write.py is pre-existing (fails identically on main before this change) — Windows file-mode semantics, unrelated to this fix.

Closes #74308

…rofile

Cancelling an OpenAI Codex device-code login left the background poller
running, and the token-write path re-resolved the target profile lazily
via _oauth_session_profile(), which returned None once cancel popped the
session and silently fell back to whatever profile was "current".

Adds a per-session threading.Event set by the cancel endpoint. The codex
worker now pins its target profile once at start (before the session can
be popped), checks the cancel event before each poll iteration and again
before exchanging the code and before writing tokens, and aborts without
writing if cancelled.
@alt-glitch alt-glitch added type/security Security vulnerability or hardening comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data duplicate This issue or pull request already exists labels Jul 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #73914: both patches add cancellation checks through the Codex device-code worker and pin its session profile to prevent post-cancel credential persistence.

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/cli CLI entry point, hermes_cli/, setup wizard duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

auth: Codex OAuth cancel doesn't stop device-code worker; can still write token to wrong profile

2 participants