diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 4c2d76a27598..b8f0e09d3e33 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -10588,6 +10588,9 @@ def _new_oauth_session( "created_at": time.time(), "status": "pending", # pending | approved | denied | expired | error "error_message": None, + # Set by DELETE /api/providers/oauth/sessions/{id}; background + # pollers check this to abort without writing credentials. + "cancel_event": threading.Event(), } with _oauth_sessions_lock: _oauth_sessions[sid] = sess @@ -11263,6 +11266,17 @@ def _codex_full_login_worker(session_id: str) -> None: single function — we need to surface the user_code to the dashboard the moment we receive it, well before polling completes. """ + with _oauth_sessions_lock: + sess = _oauth_sessions.get(session_id) + if sess is None: + return + cancel_event: threading.Event = sess["cancel_event"] + # Pin the target profile now, while the session entry still exists. + # Cancellation pops the session from _oauth_sessions, so resolving + # the profile lazily at write time would silently fall back to + # whatever profile happens to be "current" (see _oauth_session_profile). + target_profile = sess.get("profile") + try: import httpx from hermes_cli.auth import ( @@ -11303,7 +11317,11 @@ def _codex_full_login_worker(session_id: str) -> None: code_resp = None with httpx.Client(timeout=httpx.Timeout(15.0)) as client: while time.monotonic() < deadline: + if cancel_event.is_set(): + return time.sleep(poll_interval) + if cancel_event.is_set(): + return poll = client.post( f"{issuer}/api/accounts/deviceauth/token", json={"device_auth_id": device_auth_id, "user_code": user_code}, @@ -11316,6 +11334,9 @@ def _codex_full_login_worker(session_id: str) -> None: continue # user hasn't authorized yet raise RuntimeError(f"deviceauth/token poll returned {poll.status_code}") + if cancel_event.is_set(): + return + if code_resp is None: with _oauth_sessions_lock: sess["status"] = "expired" @@ -11349,7 +11370,14 @@ def _codex_full_login_worker(session_id: str) -> None: from hermes_cli.auth import _save_codex_tokens - with _profile_scope(_oauth_session_profile(session_id)): + # Refuse to write if cancellation raced past the checks above. + if cancel_event.is_set(): + _log.info( + "oauth/device: openai-codex session %s cancelled before token write", session_id, + ) + return + + with _profile_scope(target_profile): _save_codex_tokens({ "access_token": access_token, "refresh_token": refresh_token, @@ -11456,6 +11484,10 @@ async def cancel_oauth_session( _require_token(request) with _oauth_sessions_lock: sess = _oauth_sessions.pop(session_id, None) + if sess is not None: + cancel_event = sess.get("cancel_event") + if cancel_event is not None: + cancel_event.set() if sess is None: return {"ok": False, "message": "session not found"} return {"ok": True, "session_id": session_id} diff --git a/tests/hermes_cli/test_web_oauth_dispatch.py b/tests/hermes_cli/test_web_oauth_dispatch.py index f4725f069228..d6af812f8624 100644 --- a/tests/hermes_cli/test_web_oauth_dispatch.py +++ b/tests/hermes_cli/test_web_oauth_dispatch.py @@ -341,6 +341,92 @@ def post(self, url, **kwargs): ws._oauth_sessions.pop(sid, None) +def test_codex_dashboard_worker_aborts_after_cancel(tmp_path, monkeypatch): + """DELETE /api/providers/oauth/sessions/{id} must stop the codex poll + worker before it exchanges the device code and writes tokens. + + Regression for issue #74308: cancelling mid-poll used to leave the + background worker running to completion, and — since the session dict + entry had already been popped — the eventual token write resolved its + target profile as ``None`` and fell back to whatever profile happened + to be "current" instead of the profile the session actually targeted. + """ + from hermes_cli import auth as auth_mod + from hermes_cli import web_server as ws + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + + save_calls = [] + monkeypatch.setattr( + auth_mod, + "_save_codex_tokens", + lambda tokens: save_calls.append(tokens), + ) + + class _Resp: + def __init__(self, status_code, payload): + self.status_code = status_code + self._payload = payload + + def json(self): + return self._payload + + class _Client: + def __init__(self, *args, **kwargs): + pass + + def __enter__(self): + return self + + def __exit__(self, *args): + return False + + def post(self, url, **kwargs): + if url.endswith("/deviceauth/usercode"): + return _Resp(200, { + "device_auth_id": "device-auth-id", + "interval": 3, + "user_code": "CODEX-1234", + }) + if url.endswith("/deviceauth/token"): + # The user "approves" on the very first poll — if the worker + # doesn't check cancellation, this looks like a success. + return _Resp(200, { + "authorization_code": "authorization-code", + "code_verifier": "code-verifier", + }) + return _Resp(200, { + "access_token": "codex-access-should-not-be-saved", + "refresh_token": "codex-refresh-should-not-be-saved", + }) + + monkeypatch.setattr(httpx, "Client", _Client) + + sid, _ = ws._new_oauth_session("openai-codex", "device_code", profile="coder") + + # Cancel the session the moment the worker calls time.sleep() inside the + # poll loop — simulates the user clicking "Cancel" while the worker is + # paused between poll attempts, published device code already in hand. + def fake_sleep(_seconds): + resp = client.delete( + f"/api/providers/oauth/sessions/{sid}", headers=HEADERS, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["ok"] is True + + monkeypatch.setattr(ws.time, "sleep", fake_sleep) + + try: + ws._codex_full_login_worker(sid) + finally: + ws._oauth_sessions.pop(sid, None) + + # Cancellation must have removed the session before the worker exits. + assert sid not in ws._oauth_sessions + # And the worker must have aborted BEFORE persisting any credentials. + assert save_calls == [] + + def test_codex_dashboard_start_rewords_device_authorization_error(monkeypatch): from hermes_cli import web_server as ws