From a82c88bac082c3942f830e7760111140baf35fc7 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 28 May 2026 05:47:30 -0700 Subject: [PATCH] fix(xai-oauth): accept bare-code manual paste (state=None) (#26923) (#33880) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xAI's consent page renders the authorization code in-page rather than redirecting through the 127.0.0.1 callback, so on remote/headless setups (GCP Cloud Shell, Codespaces, container consoles, headless VPS) the only value the user can paste is the opaque code with no `code=`/`state=` query parameters. `_parse_pasted_callback` correctly returns `state=None` for that input, but `_xai_oauth_loopback_login` then validated state unconditionally and raised `xai_state_mismatch`, making the documented bare-code paste path unreachable. PKCE (code_verifier) still binds the token exchange to this client, so the local state-equality check is redundant when there is no state to compare. On the manual-paste path only, substitute the locally generated state when the callback returned none — the rest of the validation chain (code presence, error field, token exchange) is unchanged. The loopback HTTP-server path still requires a matching state (a real browser redirect always carries one). Also: clarify the manual-paste prompt to mention xAI's in-page code rendering so users know pasting the bare code on its own is expected. Root-cause analysis from #26923 comment by @AccursedGalaxy (2026-05-20). Tests ----- * test_xai_loopback_login_manual_paste_bare_code_succeeds — positive end-to-end through the token exchange with state=None. * test_xai_loopback_login_loopback_path_rejects_missing_state — the HTTP-server path still rejects state=None as a regression guard (the bare-code relaxation must NOT widen the loopback path). * Existing test_xai_loopback_login_manual_paste_state_mismatch_raises continues to verify wrong (non-None) state is rejected on manual-paste. Closes #26923. --- hermes_cli/auth.py | 19 +++- tests/hermes_cli/test_auth_manual_paste.py | 101 +++++++++++++++++++++ 2 files changed, 119 insertions(+), 1 deletion(-) diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 365abd33375..5f0c44f7ed5 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -3181,6 +3181,9 @@ def _prompt_manual_callback_paste(redirect_uri: str) -> dict: print("not on your laptop) — that is expected. Copy the FULL URL") print("from your browser's address bar of that failed page and paste") print("it below. A bare '?code=...&state=...' fragment also works.") + print("If the consent page shows the authorization code in-page") + print("(xAI's current behavior) rather than redirecting, paste the") + print("bare code value on its own.") print("───────────────────────────────────────────────────────────────") try: raw = input("Callback URL: ") @@ -6965,7 +6968,21 @@ def _xai_oauth_loopback_login( provider="xai-oauth", code="xai_authorization_failed", ) - if callback.get("state") != state: + callback_state = callback.get("state") + # Manual-paste bare-code path: when a user pastes only the opaque + # authorization code (no ``code=``/``state=`` query parameters), + # ``_parse_pasted_callback`` returns ``state=None``. xAI's consent + # page renders the code in-page rather than redirecting through the + # 127.0.0.1 callback, so on many remote setups (Cloud Shell, headless + # VPS, container consoles) the bare code is the only thing the user + # can obtain. PKCE (code_verifier) still binds the exchange to this + # client, so the local state-equality check is redundant on the + # bare-code path — we substitute the locally generated state to keep + # the rest of the validation chain (and the token exchange) unchanged. + # See #26923 (AccursedGalaxy comment, 2026-05-20). + if callback_state is None and manual_paste: + callback_state = state + if callback_state != state: raise AuthError( "xAI authorization failed: state mismatch.", provider="xai-oauth", diff --git a/tests/hermes_cli/test_auth_manual_paste.py b/tests/hermes_cli/test_auth_manual_paste.py index 7230b2a365c..2c567ff6ee5 100644 --- a/tests/hermes_cli/test_auth_manual_paste.py +++ b/tests/hermes_cli/test_auth_manual_paste.py @@ -330,6 +330,107 @@ def test_xai_loopback_login_manual_paste_state_mismatch_raises(monkeypatch): assert exc.value.code == "xai_state_mismatch" +def test_xai_loopback_login_manual_paste_bare_code_succeeds(monkeypatch): + """Bare-code paste (state=None) must complete login under manual_paste. + + xAI's consent page renders the authorization code in-page rather than + redirecting through 127.0.0.1, so on remote/headless setups the only + value the user can obtain is the opaque code with no ``state=`` + parameter. ``_parse_pasted_callback`` correctly returns + ``state=None`` for that input. The login flow must accept this case + (PKCE still protects the exchange); historically it raised + ``xai_state_mismatch``. Regression for the bare-code branch of #26923. + """ + monkeypatch.setattr( + auth_mod, "_xai_oauth_discovery", + lambda *_a, **_k: { + "authorization_endpoint": "https://auth.x.ai/oauth2/authorize", + "token_endpoint": "https://auth.x.ai/oauth2/token", + }, + ) + monkeypatch.setattr( + auth_mod, "_prompt_manual_callback_paste", + lambda _ru: { + "code": "bare-opaque-code", + "state": None, + "error": None, + "error_description": None, + }, + ) + + def _fake_token_post(*_a, **_k): + return _StubTokenResponse( + { + "access_token": "at", + "refresh_token": "rt", + "id_token": "", + "expires_in": 3600, + "token_type": "Bearer", + } + ) + + monkeypatch.setattr(auth_mod.httpx, "post", _fake_token_post) + + with contextlib.redirect_stdout(io.StringIO()): + creds = auth_mod._xai_oauth_loopback_login(manual_paste=True) + + assert creds["tokens"]["access_token"] == "at" + assert creds["tokens"]["refresh_token"] == "rt" + + +def test_xai_loopback_login_loopback_path_rejects_missing_state(monkeypatch): + """Loopback (manual_paste=False) must NOT accept ``state=None``. + + The bare-code relaxation only applies to the manual-paste path, + where the user demonstrably has no way to supply ``state``. The + HTTP-server path always sees ``state`` populated from the real + callback query string, so missing state there means something is + wrong (a malformed callback, an attacker-supplied request) and + must still raise ``xai_state_mismatch``. + """ + monkeypatch.setattr( + auth_mod, "_xai_oauth_discovery", + lambda *_a, **_k: { + "authorization_endpoint": "https://auth.x.ai/oauth2/authorize", + "token_endpoint": "https://auth.x.ai/oauth2/token", + }, + ) + + class _StubServer: + def shutdown(self): + return None + + def server_close(self): + return None + + monkeypatch.setattr( + auth_mod, "_xai_start_callback_server", + lambda *_a, **_k: ( + _StubServer(), + None, + {"code": "fake", "state": None, "error": None, + "error_description": None}, + "http://127.0.0.1:56121/callback", + ), + ) + monkeypatch.setattr( + auth_mod, "_xai_wait_for_callback", + lambda *_a, **_k: { + "code": "fake", + "state": None, + "error": None, + "error_description": None, + }, + ) + monkeypatch.setattr(auth_mod, "_xai_validate_loopback_redirect_uri", lambda _u: None) + monkeypatch.setattr(auth_mod, "_print_loopback_ssh_hint", lambda *_a, **_k: None) + + with contextlib.redirect_stdout(io.StringIO()): + with pytest.raises(auth_mod.AuthError) as exc: + auth_mod._xai_oauth_loopback_login(manual_paste=False, open_browser=False) + assert exc.value.code == "xai_state_mismatch" + + def test_xai_loopback_login_manual_paste_missing_code_raises(monkeypatch): """Empty paste must surface as ``xai_code_missing``, not crash.""" monkeypatch.setattr(