mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-27 12:10:40 +00:00
b34ce63c97
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>
237 lines
8.1 KiB
Python
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
|