fix(xai-oauth): echo code_challenge in token POST so PKCE exchange succeeds (#26990)#27560
Merged
Conversation
…cceeds xAI's OAuth implementation at ``auth.x.ai`` validates the PKCE ``code_challenge`` at the **token** endpoint, not just at the authorize step. When Hermes sends the standards-compliant token POST with ``code_verifier`` alone — exactly what RFC 7636 §4.5 prescribes — xAI rejects the exchange with ``code_challenge is required`` and the user is stuck with no working OAuth login. The fix: * Extract the token POST into ``_xai_oauth_exchange_code_for_tokens`` so the wire format is unit-testable in isolation. * Send the original ``code_challenge`` and ``code_challenge_method`` in the form body alongside ``code_verifier``. Strict RFC-compliant servers ignore the extras at the token endpoint, and xAI's permissive implementation accepts the exchange. This is the standard "defensive echo" workaround used by every OAuth client that targets a server with this quirk. * Refuse to fire the POST when ``code_verifier`` is empty — leaking the authorization code to a server that can't redeem it is worse than failing locally with an actionable error. The new error code is ``xai_pkce_verifier_missing`` and the message points at this issue for context. * Surface the HTTP status code prominently in the 4xx error message (``xAI token exchange failed (HTTP 400). Response: …``) so users and maintainers can tell a 400 (bad request / PKCE problem) from a 403 (tier denied, see #26847) at a glance instead of parsing the JSON body by eye. Closes #26990
14 focused tests on the extracted helper ``_xai_oauth_exchange_code_for_tokens`` cover: Core contract: * ``code_verifier`` is on the wire (RFC 7636 §4.5). * ``code_challenge`` + ``code_challenge_method=S256`` are echoed (the #26990 defense-in-depth that makes xAI's token endpoint stop rejecting valid exchanges). * ``grant_type=authorization_code``, ``code``, ``redirect_uri``, and ``client_id`` are all locked. * Content-Type is ``application/x-www-form-urlencoded`` (xAI rejects ``application/json`` on this endpoint). * The supplied ``token_endpoint`` URL is used verbatim — no hard-coded constant sneaks in via a future refactor. * ``timeout_seconds`` is forwarded; floored at 20s. Sanity guard: * Empty ``code_verifier`` raises ``xai_pkce_verifier_missing`` with a link to #26990 — and NOTHING is sent. Leaking the auth code to a server that can't redeem it is the wrong failure mode. * Empty ``code_challenge`` omits only the defensive echo; the standards-compliant ``code_verifier`` request still goes out so RFC-compliant servers keep working. Error surfacing: * Non-200 responses include both ``HTTP <status>`` and the body verbatim — disambiguates 400 (PKCE / bad request) from 403 (tier denied, see #26847). * Transport errors are wrapped as ``AuthError`` with the ``xai_token_exchange_failed`` code, so the surrounding ``format_auth_error`` UI mapping still fires. * Non-dict JSON payloads raise ``xai_token_exchange_invalid``. * 200 happy path returns the parsed payload dict verbatim. End-to-end wire-format guard: * A real ``httpx.Client`` with a stub transport captures the bytes on the wire and asserts every PKCE field round-trips through ``urlencode``. Catches a future refactor that swaps ``data=`` for ``json=`` (which xAI would silently reject).
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-import |
2 |
invalid-argument-type |
1 |
First entries
tests/hermes_cli/test_xai_oauth_pkce_token_exchange.py:32: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/hermes_cli/test_xai_oauth_pkce_token_exchange.py:31: [unresolved-import] unresolved-import: Cannot resolve imported module `httpx`
tests/hermes_cli/test_xai_oauth_pkce_token_exchange.py:286: [invalid-argument-type] invalid-argument-type: Argument to function `_ok_response` is incorrect: Expected `dict[Unknown, Unknown]`, found `list[int]`
✅ Fixed issues: none
Unchanged: 4587 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Salvage of #26999 by @xxxigm, cherry-picked onto current main with authorship preserved.
Summary
xAI's OAuth token endpoint rejects some users' code-for-token exchanges with
code_challenge is requiredeven though Hermes was sending a validcode_verifier(RFC 7636 §4.5 compliant). Fix is to echo the originalcode_challenge+code_challenge_method=S256in the token POST as defense-in-depth — harmless for strict RFC-compliant servers (RFC 6749 §3.2 says they MUST ignore unknown parameters), necessary for xAI's stricter code paths.Most users don't hit this. The minority who do are likely routed to a stricter xAI backend variant, account-tier-gated path, or transient state where xAI's authorize-step session storage didn't retain the PKCE state. The defensive echo unblocks them without affecting anyone else.
Changes
hermes_cli/auth.py— extracts the token POST into_xai_oauth_exchange_code_for_tokenshelper. Sendscode_verifieras before, also echoescode_challenge+code_challenge_method=S256. Refuses to POST locally ifcode_verifieris empty (new error codexai_pkce_verifier_missing). Embeds HTTP status code in 4xx errors so callers can tell 400 (bad request) from 403 (tier-denied) at a glance.tests/hermes_cli/test_xai_oauth_pkce_token_exchange.py— 14 new regression tests pinning wire format, including a real-httpx.Client+ stub-transport test that catches futuredata=→json=refactors.Validation
E2E verified the actual wire bytes go out form-urlencoded with all PKCE fields, the empty-verifier guard refuses to POST, and the missing-challenge case still sends the standards-compliant request.
Closes #26990
Co-authored-by: xxxigm tuancanhnguyen706@gmail.com