diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 521b135867766..bf7b00f203138 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -8536,16 +8536,20 @@ def _refresh_minimax_oauth_state( "Accept": "application/json", }, ) - if response.status_code != 200: - body = _minimax_response_error_text(response) - body_lower = body.lower() - relogin = any(m in body_lower for m in - ("invalid_grant", "refresh_token_reused", "invalid_refresh_token")) - raise AuthError( - f"MiniMax OAuth refresh failed: {body or response.reason_phrase}", - provider="minimax-oauth", code="refresh_failed", - relogin_required=relogin, - ) + # The non-200 branch reads a STREAMED body, so it must run while + # the client is still open — iter_bytes() after the client context + # closes raises (StreamClosed). The 200 path was already read by + # _minimax_post_form, so response.json() below is safe outside. + if response.status_code != 200: + body = _minimax_response_error_text(response) + body_lower = body.lower() + relogin = any(m in body_lower for m in + ("invalid_grant", "refresh_token_reused", "invalid_refresh_token")) + raise AuthError( + f"MiniMax OAuth refresh failed: {body or response.reason_phrase}", + provider="minimax-oauth", code="refresh_failed", + relogin_required=relogin, + ) payload = response.json() if payload.get("status") != "success": raise AuthError( diff --git a/tests/test_minimax_oauth.py b/tests/test_minimax_oauth.py index a7a5b66b8b394..0f807a6dc7209 100644 --- a/tests/test_minimax_oauth.py +++ b/tests/test_minimax_oauth.py @@ -42,7 +42,13 @@ from hermes_cli.auth import ( # --------------------------------------------------------------------------- def _make_httpx_response(status_code: int, body: dict | None = None, text: str = ""): - """Return a minimal mock that quacks like httpx.Response.""" + """Return a minimal mock that quacks like httpx.Response. + + Includes the streamed-read surface used by ``_minimax_post_form`` / + ``_minimax_response_error_text``: ``is_stream_consumed`` is False and + ``iter_bytes()`` yields the body/text bytes, so non-200 paths exercise + the real bounded-read code instead of a truthy MagicMock attribute. + """ resp = MagicMock() resp.status_code = status_code if body is not None: @@ -52,6 +58,9 @@ def _make_httpx_response(status_code: int, body: dict | None = None, text: str = resp.json.side_effect = Exception("No body") resp.text = text resp.reason_phrase = "OK" if status_code == 200 else "Error" + resp.is_stream_consumed = False + resp.encoding = "utf-8" + resp.iter_bytes.return_value = iter([resp.text.encode("utf-8")] if resp.text else []) return resp @@ -128,6 +137,7 @@ def test_request_user_code_state_mismatch_raises(): client = MagicMock() client.post.return_value = mock_response + client.send.return_value = mock_response with pytest.raises(AuthError) as exc_info: _minimax_request_user_code( @@ -387,6 +397,7 @@ def test_token_provider_refreshes_when_near_expiry(): mock_instance.__enter__ = MagicMock(return_value=mock_instance) mock_instance.__exit__ = MagicMock(return_value=False) mock_instance.post.return_value = mock_resp + mock_instance.send.return_value = mock_resp mock_client_class.return_value = mock_instance token = provider() @@ -441,6 +452,7 @@ def test_token_provider_quarantines_state_on_terminal_refresh(): mock_instance.__enter__ = MagicMock(return_value=mock_instance) mock_instance.__exit__ = MagicMock(return_value=False) mock_instance.post.return_value = bad_resp + mock_instance.send.return_value = bad_resp mock_client_class.return_value = mock_instance with pytest.raises(AuthError) as exc_info: @@ -475,3 +487,84 @@ def test_resolve_returns_callable_when_as_token_provider_true(): assert creds["base_url"] == MINIMAX_OAUTH_GLOBAL_INFERENCE.rstrip("/") +# --------------------------------------------------------------------------- +# Bounded error-body reads (#56548 / PR #56549) +# --------------------------------------------------------------------------- + +def test_refresh_error_body_bounded_and_readable_with_real_client(): + """Refresh non-200 path over a REAL socket transport. + + The error body is obtained via a streamed response; the bounded read + must happen while the client context is still open. A real socket is + required to bind this contract: closing the client tears the connection + down, so a read after the ``with httpx.Client(...)`` block raises + ReadError/StreamClosed. (MockTransport buffers in memory and would NOT + catch the regression.) + """ + import http.server + import socketserver + import threading + + import httpx + + from hermes_cli.auth import _refresh_minimax_oauth_state + + big_body = b"invalid_grant " + b"x" * (64 * 1024) # 64KB error body + + class Handler(http.server.BaseHTTPRequestHandler): + def do_POST(self): + self.send_response(400) + self.send_header("Content-Length", str(len(big_body))) + self.end_headers() + self.wfile.write(big_body) + + def log_message(self, *args): + pass + + with socketserver.TCPServer(("127.0.0.1", 0), Handler) as server: + port = server.server_address[1] + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + state = { + "access_token": "expired", + "refresh_token": "burned-rt", + "portal_base_url": f"http://127.0.0.1:{port}", + "client_id": MINIMAX_OAUTH_CLIENT_ID, + "inference_base_url": MINIMAX_OAUTH_GLOBAL_INFERENCE, + "expires_at": _past_iso(100), + } + with pytest.raises(AuthError) as exc_info: + _refresh_minimax_oauth_state(state, force=True) + finally: + server.shutdown() + + msg = str(exc_info.value) + assert "invalid_grant" in msg + assert exc_info.value.relogin_required is True + # Bounded: 16KB limit + truncation marker, never the full 64KB body. + assert len(msg) < 20 * 1024 + assert "...[truncated]" in msg + + +def test_minimax_response_error_text_truncates_above_limit(): + """Bodies above the 16KB bound are cut and marked truncated.""" + import httpx + + from hermes_cli.auth import ( + _MINIMAX_OAUTH_ERROR_BODY_LIMIT, + _minimax_response_error_text, + ) + + big = "e" * (_MINIMAX_OAUTH_ERROR_BODY_LIMIT * 4) + + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(500, text=big) + + with httpx.Client(transport=httpx.MockTransport(handler)) as client: + request = client.build_request("POST", "https://api.minimax.io/oauth/token") + response = client.send(request, stream=True) + text = _minimax_response_error_text(response) + + assert text.endswith("...[truncated]") + assert len(text) <= _MINIMAX_OAUTH_ERROR_BODY_LIMIT + len("...[truncated]")