diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 2a5e7a213fe..78c21252b3c 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -2405,15 +2405,37 @@ def _make_xai_callback_handler(expected_path: str) -> tuple[type[BaseHTTPRequest "error": params.get("error", [None])[0], "error_description": params.get("error_description", [None])[0], } + + # Treat a hit on the callback path with neither `code` nor `error` + # as a missing OAuth callback (e.g. xAI's auth backend failed to + # redirect and the user navigated to the bare loopback URL by hand). + # Show an explicit "not received" page rather than the success page — + # otherwise the browser claims authorization succeeded while the CLI + # is still waiting for a real callback and eventually times out. + if incoming["code"] is None and incoming["error"] is None: + self.send_response(400) + self._maybe_write_cors_headers() + self.send_header("Content-Type", "text/html; charset=utf-8") + self.end_headers() + body = ( + "" + "

xAI authorization not received.

" + "

No authorization code was present in this callback URL. " + "Return to the terminal and re-run " + "hermes auth add xai-oauth to retry.

" + "" + ) + self.wfile.write(body.encode("utf-8")) + return + # ThreadingHTTPServer allows a fallback/manual callback to complete # while a browser connection is stuck. Once we have a terminal # OAuth result (code or error), keep the first one so a later # concurrent/invalid callback cannot overwrite state before # validation in _xai_oauth_loopback_login(). - if incoming["code"] or incoming["error"]: - with result_lock: - if not (result["code"] or result["error"]): - result.update(incoming) + with result_lock: + if not (result["code"] or result["error"]): + result.update(incoming) self.send_response(200) self._maybe_write_cors_headers() diff --git a/tests/hermes_cli/test_auth_xai_oauth_provider.py b/tests/hermes_cli/test_auth_xai_oauth_provider.py index 344f8d6f153..b2795cf23aa 100644 --- a/tests/hermes_cli/test_auth_xai_oauth_provider.py +++ b/tests/hermes_cli/test_auth_xai_oauth_provider.py @@ -326,6 +326,84 @@ def test_xai_callback_server_latches_first_terminal_callback_result(): thread.join(timeout=1.0) +# --------------------------------------------------------------------------- +# Loopback callback handler GET responses +# --------------------------------------------------------------------------- + + +def _get_callback(redirect_uri: str, query: str = "") -> tuple[int, str]: + """GET the loopback callback URL with an optional query string.""" + from urllib.request import Request, urlopen + from urllib.error import HTTPError + + target = redirect_uri + (("?" + query) if query else "") + req = Request(target, method="GET") + try: + with urlopen(req, timeout=5.0) as resp: + return resp.getcode(), resp.read().decode("utf-8", "replace") + except HTTPError as exc: + return exc.code, exc.read().decode("utf-8", "replace") + + +def test_xai_callback_handler_returns_400_when_callback_url_lacks_code_and_error(): + """Bare loopback URL (no code, no error) must not claim authorization received. + + Regression for #27385: when xAI's auth backend fails to redirect and the user + manually navigates to http://127.0.0.1:/callback, the handler used to + return 200 "xAI authorization received" while the CLI's wait loop still timed + out — leaving the user with a contradictory success page and a CLI error. + """ + server, thread, result, redirect_uri = _xai_start_callback_server(preferred_port=0) + try: + status, body = _get_callback(redirect_uri) + assert status == 400 + assert "not received" in body.lower() + assert "hermes auth add xai-oauth" in body + # Wait loop must still see no code/error so it raises a real timeout, + # rather than treating this empty hit as a successful callback. + assert result["code"] is None + assert result["error"] is None + finally: + server.shutdown() + server.server_close() + thread.join(timeout=1.0) + + +def test_xai_callback_handler_accepts_callback_with_code(): + """A real OAuth redirect (code + state) still records both and shows success.""" + server, thread, result, redirect_uri = _xai_start_callback_server(preferred_port=0) + try: + status, body = _get_callback(redirect_uri, query="code=abc&state=xyz") + assert status == 200 + assert "xAI authorization received" in body + assert result["code"] == "abc" + assert result["state"] == "xyz" + assert result["error"] is None + finally: + server.shutdown() + server.server_close() + thread.join(timeout=1.0) + + +def test_xai_callback_handler_records_error_callback(): + """A redirect carrying an `error` param must surface the failure page and capture detail.""" + server, thread, result, redirect_uri = _xai_start_callback_server(preferred_port=0) + try: + status, body = _get_callback( + redirect_uri, + query="error=access_denied&error_description=user%20cancelled", + ) + assert status == 200 + assert "xAI authorization failed" in body + assert result["error"] == "access_denied" + assert result["error_description"] == "user cancelled" + assert result["code"] is None + finally: + server.shutdown() + server.server_close() + thread.join(timeout=1.0) + + # --------------------------------------------------------------------------- # Token roundtrip + reads # ---------------------------------------------------------------------------