Files
hermes-webui/tests/test_issue1362_codex_oauth_onboarding.py
T
Nathan Esquenazi b34ce63c97 fix(oauth): honor cancel during Codex device-token exchange (follow-up to #1652)
The Codex OAuth onboarding worker introduced in #1652 had a cancel-vs-worker
race: a `cancel_onboarding_oauth_flow` request that arrived while the worker
was mid-network-call (between the `live = dict(...)` snapshot and the next
status check) would be silently overridden:

  1. User clicks Cancel → server sets flow.status = "cancelled" and drops
     sensitive lifecycle fields under the lock.
  2. Worker is mid-`_poll_codex_authorization` / `_exchange_codex_authorization`
     using the local `live` snapshot it captured before the cancel.
  3. Worker calls `_persist_codex_credentials(...)` — auth.json gets written.
  4. Worker calls `_set_flow_status(flow_id, "success")` — overrides the
     cancelled status.

Net effect: the user's explicit cancel is ignored, credentials are persisted,
and the UI reports success. Reproduced with a behavioural harness that drove
a real worker thread against patched network helpers and confirmed:

  pre-fix : flow status `success`, auth.json written despite cancel
  post-fix: flow status `cancelled`, auth.json NOT written

The fix re-checks the flow status under `_OAUTH_FLOWS_LOCK` after the token
exchange completes and before persisting. If the status is no longer
`pending`, the worker exits without persisting credentials and without
overwriting the terminal status.

Regression test `test_cancel_during_token_exchange_does_not_persist_credentials`
drives the worker against threading.Event-gated network stubs to reproduce
the race deterministically and lock the new invariant.

Trace verified against fresh hermes-agent tarball — credential_pool entry
shape (`auth_type=oauth`, `source=manual:device_code`, `priority=0`, base_url)
remains compatible with `agent.credential_pool.load_pool("openai-codex")` and
the agent CLI's `_save_codex_tokens` legacy fallback path.

Tests:
- 10/10 in tests/test_issue1362_codex_oauth_onboarding.py
- Full suite: 4230 passed, 57 skipped, 3 xpassed, 0 failed in 33.82s

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 14:49:38 -07:00

237 lines
8.1 KiB
Python

"""Regression tests for issue #1362 — Codex OAuth from onboarding."""
from __future__ import annotations
import json
import stat
import time
from pathlib import Path
import pytest
REPO = Path(__file__).resolve().parents[1]
def test_onboarding_codex_oauth_routes_use_post_start_cancel_and_get_poll():
routes = (REPO / "api" / "routes.py").read_text(encoding="utf-8")
get_idx = routes.find("def handle_get(")
post_idx = routes.find("def handle_post(")
assert get_idx != -1 and post_idx != -1
get_body = routes[get_idx:post_idx]
post_body = routes[post_idx:]
assert '"/api/onboarding/oauth/poll"' in get_body
assert '"/api/onboarding/oauth/start"' not in get_body
assert '"/api/oauth/codex/start"' not in routes
assert '"/api/oauth/codex/poll"' not in routes
assert '"/api/onboarding/oauth/start"' in post_body
assert '"/api/onboarding/oauth/cancel"' in post_body
def test_onboarding_oauth_rejects_non_codex_providers(monkeypatch):
import api.oauth as oauth
for provider in ("anthropic", "claude", "claude-code", "nous", "qwen-oauth", "copilot", "bogus"):
with pytest.raises(ValueError):
oauth.start_onboarding_oauth_flow({"provider": provider})
def test_start_payload_does_not_leak_provider_device_secrets(monkeypatch, tmp_path):
import api.oauth as oauth
oauth._OAUTH_FLOWS.clear()
monkeypatch.setattr(oauth, "_get_active_hermes_home", lambda: tmp_path)
monkeypatch.setattr(oauth, "_request_codex_user_code", lambda: {
"device_auth_id": "device-secret",
"user_code": "ABCD-EFGH",
"interval": 3,
})
monkeypatch.setattr(oauth, "_spawn_codex_oauth_worker", lambda flow_id: None)
payload = oauth.start_onboarding_oauth_flow({"provider": "openai-codex"})
assert payload["ok"] is True
assert payload["provider"] == "openai-codex"
assert payload["status"] == "pending"
assert payload["verification_uri"] == "https://auth.openai.com/codex/device"
assert payload["user_code"] == "ABCD-EFGH"
serialized = json.dumps(payload)
for forbidden in (
"device_auth_id",
"device-secret",
"authorization_code",
"code_verifier",
"access_token",
"refresh_token",
):
assert forbidden not in serialized
def test_poll_returns_high_level_status_only(monkeypatch, tmp_path):
import api.oauth as oauth
oauth._OAUTH_FLOWS.clear()
flow_id = "flow-test"
oauth._OAUTH_FLOWS[flow_id] = {
"provider": "openai-codex",
"status": "pending",
"device_auth_id": "device-secret",
"user_code": "ABCD-EFGH",
"code_verifier": "verifier-secret",
"authorization_code": "auth-secret",
"expires_at": time.time() + 60,
"poll_interval_seconds": 3,
"hermes_home": tmp_path,
}
payload = oauth.poll_onboarding_oauth_flow(flow_id)
assert payload == {"ok": True, "provider": "openai-codex", "flow_id": flow_id, "status": "pending"}
serialized = json.dumps(payload)
for forbidden in ("device_auth_id", "device-secret", "code_verifier", "authorization_code"):
assert forbidden not in serialized
def test_cancel_marks_flow_cancelled_and_poll_stops(tmp_path):
import api.oauth as oauth
oauth._OAUTH_FLOWS.clear()
flow_id = "flow-cancel"
oauth._OAUTH_FLOWS[flow_id] = {
"provider": "openai-codex",
"status": "pending",
"expires_at": time.time() + 60,
"hermes_home": tmp_path,
}
cancelled = oauth.cancel_onboarding_oauth_flow({"flow_id": flow_id})
polled = oauth.poll_onboarding_oauth_flow(flow_id)
assert cancelled["status"] == "cancelled"
assert polled["status"] == "cancelled"
def test_cancel_during_token_exchange_does_not_persist_credentials(monkeypatch, tmp_path):
"""Cancel arriving while the worker is mid-network-call must win.
Without the post-exchange status re-check, the worker would proceed to
persist credentials to auth.json AND override the cancelled status with
"success" — silently storing tokens the user explicitly aborted.
"""
import threading
import api.oauth as oauth
oauth._OAUTH_FLOWS.clear()
poll_started = threading.Event()
poll_continue = threading.Event()
def _slow_poll(device_auth_id, user_code):
poll_started.set()
assert poll_continue.wait(timeout=5)
return {"authorization_code": "auth-code", "code_verifier": "verifier"}
def _exchange(authorization_code, code_verifier):
return {"access_token": "ACCESS", "refresh_token": "REFRESH"}
monkeypatch.setattr(oauth, "_poll_codex_authorization", _slow_poll)
monkeypatch.setattr(oauth, "_exchange_codex_authorization", _exchange)
flow_id = "race-flow"
oauth._OAUTH_FLOWS[flow_id] = {
"provider": "openai-codex",
"status": "pending",
"device_auth_id": "device-secret",
"user_code": "ABCD-EFGH",
"expires_at": time.time() + 600,
"poll_interval_seconds": 1,
"hermes_home": str(tmp_path),
"created_at": time.time(),
"updated_at": time.time(),
}
worker = threading.Thread(target=oauth._run_codex_oauth_worker, args=(flow_id,), daemon=True)
worker.start()
assert poll_started.wait(timeout=5)
oauth.cancel_onboarding_oauth_flow({"flow_id": flow_id})
assert oauth._OAUTH_FLOWS[flow_id]["status"] == "cancelled"
poll_continue.set()
worker.join(timeout=5)
assert not worker.is_alive()
assert oauth._OAUTH_FLOWS[flow_id]["status"] == "cancelled"
assert not (tmp_path / "auth.json").exists()
def test_expired_flow_reports_expired_and_drops_sensitive_lifecycle(tmp_path):
import api.oauth as oauth
oauth._OAUTH_FLOWS.clear()
flow_id = "flow-expired"
oauth._OAUTH_FLOWS[flow_id] = {
"provider": "openai-codex",
"status": "pending",
"device_auth_id": "device-secret",
"expires_at": time.time() - 1,
"hermes_home": tmp_path,
}
payload = oauth.poll_onboarding_oauth_flow(flow_id)
assert payload["status"] == "expired"
assert oauth._OAUTH_FLOWS[flow_id]["status"] == "expired"
assert "device_auth_id" not in oauth._OAUTH_FLOWS[flow_id]
def test_codex_credentials_written_to_active_profile_auth_json(monkeypatch, tmp_path):
import api.oauth as oauth
from api.onboarding import _provider_oauth_authenticated
active_home = tmp_path / "active-profile"
realish_home = tmp_path / "process-home"
active_home.mkdir()
realish_home.mkdir()
monkeypatch.setattr(Path, "home", lambda: realish_home)
auth_path = oauth._persist_codex_credentials(
active_home,
{"access_token": "access-secret", "refresh_token": "refresh-secret"},
)
assert auth_path == active_home / "auth.json"
assert auth_path.exists()
assert not (realish_home / ".hermes" / "auth.json").exists()
mode = stat.S_IMODE(auth_path.stat().st_mode)
assert mode == 0o600
store = json.loads(auth_path.read_text(encoding="utf-8"))
entry = store["credential_pool"]["openai-codex"][0]
assert entry["auth_type"] == "oauth"
assert entry["source"] == "manual:device_code"
assert entry["base_url"] == "https://chatgpt.com/backend-api/codex"
assert _provider_oauth_authenticated("openai-codex", active_home) is True
def test_frontend_uses_onboarding_oauth_endpoints_and_no_secret_poll_url():
js = (REPO / "static" / "onboarding.js").read_text(encoding="utf-8")
assert "/api/onboarding/oauth/start" in js
assert "/api/onboarding/oauth/poll" in js
assert "/api/onboarding/oauth/cancel" in js
assert "window.open(verification_uri" not in js
assert "device_code=" not in js
assert "device_code" not in js
assert "flow_id" in js
assert "copyCodexOAuthCode" in js
assert "cancelCodexOAuth" in js
def test_unsupported_note_no_longer_calls_openai_codex_terminal_first():
src = (REPO / "api" / "onboarding.py").read_text(encoding="utf-8")
start = src.find("_UNSUPPORTED_PROVIDER_NOTE")
body = src[start:start + 400]
assert "OpenAI Codex, and GitHub" not in body
assert "OpenAI Codex" in body and "authenticated in this onboarding flow" in body
assert "Anthropic" not in body
assert "Claude" not in body