From 35520adb4f217472675ff6517bb3a618c857efc6 Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Wed, 20 May 2026 17:34:36 -0700 Subject: [PATCH 1/3] fix: serialize guardrail_response to JSON in OTEL traces (#28362) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: serialize guardrail_response to JSON in OTEL traces Guardrail spans previously set the `guardrail_response` attribute via `safe_set_attribute`, which let dict payloads reach the OTEL exporter as Python repr strings. Downstream log pipelines could not parse those as JSON, breaking metric creation from guardrail traces. Serialize `guardrail_response` with `safe_dumps` before setting the attribute, matching how `masked_entity_count` is already handled. Co-Authored-By: Claude Opus 4.7 (1M context) * test: cover dict-serialization and None-skip for guardrail_response Address Greptile feedback on #28362 — add explicit coverage for the two behavioral guarantees of this fix: - Dict payloads (the OpenAI moderation case in the report) reach the span as a JSON string, not a Python repr. - ``None`` guardrail_response skips the attribute entirely, so no ``"null"`` leaks into traces. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Yassin Kortam Co-authored-by: Claude Opus 4.7 (1M context) --- litellm/integrations/opentelemetry.py | 10 +-- .../integrations/test_opentelemetry.py | 63 ++++++++++++++++++- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/litellm/integrations/opentelemetry.py b/litellm/integrations/opentelemetry.py index a70574952b8..e1a3cecfce5 100644 --- a/litellm/integrations/opentelemetry.py +++ b/litellm/integrations/opentelemetry.py @@ -1611,11 +1611,11 @@ class OpenTelemetry(OTELGenAISemconvMixin, CustomLogger): "masked_entity_count", safe_dumps(masked_entity_count) ) - self.safe_set_attribute( - span=guardrail_span, - key="guardrail_response", - value=guardrail_information.get("guardrail_response"), - ) + guardrail_response = guardrail_information.get("guardrail_response") + if guardrail_response is not None: + guardrail_span.set_attribute( + "guardrail_response", safe_dumps(guardrail_response) + ) self._set_team_attributes_from_kwargs(guardrail_span, kwargs) diff --git a/tests/test_litellm/integrations/test_opentelemetry.py b/tests/test_litellm/integrations/test_opentelemetry.py index 6de855262bd..b65e629c890 100644 --- a/tests/test_litellm/integrations/test_opentelemetry.py +++ b/tests/test_litellm/integrations/test_opentelemetry.py @@ -66,7 +66,7 @@ class TestOpenTelemetryGuardrails(unittest.TestCase): mock_span.set_attribute.assert_any_call("guardrail_name", "test_guardrail") mock_span.set_attribute.assert_any_call("guardrail_mode", "input") mock_span.set_attribute.assert_any_call( - "guardrail_response", "filtered_content" + "guardrail_response", safe_dumps("filtered_content") ) mock_span.set_attribute.assert_any_call( "masked_entity_count", safe_dumps({"CREDIT_CARD": 2}) @@ -87,6 +87,65 @@ class TestOpenTelemetryGuardrails(unittest.TestCase): # Verify that start_span was never called otel.tracer.start_span.assert_not_called() + @patch("litellm.integrations.opentelemetry.datetime") + def test_guardrail_response_dict_is_json_serialized(self, mock_datetime): + """Dict guardrail_response (e.g. OpenAI moderation result) must reach + the span as a JSON string so downstream pipelines can parse it for + metric extraction — this is the bug the PR fixes.""" + otel = OpenTelemetry() + otel.tracer = MagicMock() + mock_span = MagicMock() + otel.tracer.start_span.return_value = mock_span + + moderation_payload = { + "id": "modr-7740", + "model": "omni-moderation-latest", + "results": [{"categories": {"harassment": False}}], + } + guardrail_info = { + "guardrail_name": "test_guardrail", + "guardrail_mode": "input", + "guardrail_response": moderation_payload, + "start_time": 1609459200.0, + "end_time": 1609459201.0, + } + kwargs = { + "standard_logging_object": {"guardrail_information": [guardrail_info]} + } + + otel._create_guardrail_span(kwargs=kwargs, context=None) + + mock_span.set_attribute.assert_any_call( + "guardrail_response", safe_dumps(moderation_payload) + ) + + @patch("litellm.integrations.opentelemetry.datetime") + def test_guardrail_response_none_is_skipped(self, mock_datetime): + """When guardrail_response is None, the attribute must not be set — + guards against round-tripping ``"null"`` into traces.""" + otel = OpenTelemetry() + otel.tracer = MagicMock() + mock_span = MagicMock() + otel.tracer.start_span.return_value = mock_span + + guardrail_info = { + "guardrail_name": "test_guardrail", + "guardrail_mode": "input", + "guardrail_response": None, + "start_time": 1609459200.0, + "end_time": 1609459201.0, + } + kwargs = { + "standard_logging_object": {"guardrail_information": [guardrail_info]} + } + + otel._create_guardrail_span(kwargs=kwargs, context=None) + + attribute_keys = [ + call.args[0] for call in mock_span.set_attribute.call_args_list + ] + self.assertNotIn("guardrail_response", attribute_keys) + class TestOpenTelemetryTeamAttributesOnChildSpans(unittest.TestCase): """team_id / team_alias must land on every child span of a @@ -1169,7 +1228,7 @@ class TestOpenTelemetry(unittest.TestCase): mock_span.set_attribute.assert_any_call("guardrail_name", "test_guardrail") mock_span.set_attribute.assert_any_call("guardrail_mode", "input") mock_span.set_attribute.assert_any_call( - "guardrail_response", "filtered_content" + "guardrail_response", safe_dumps("filtered_content") ) mock_span.set_attribute.assert_any_call( "masked_entity_count", safe_dumps({"CREDIT_CARD": 2}) From f99fb5f27f84257aa23da0afd737f85977d974be Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Wed, 20 May 2026 17:47:33 -0700 Subject: [PATCH 2/3] chore(ci): merge dev branch (#28314) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore(proxy): strict media-type match for form bodies (#27939) * chore(proxy): strict media-type match for form bodies ``_read_request_body`` and ``get_request_body`` routed on ``"form" in content_type`` / ``"multipart/form-data" in content_type``, which match any header containing the literal — ``application/form-json``, ``multiform/anything``, ``application/json; xform=1``. Starlette's ``request.form()`` returns an empty ``FormData`` for any non-canonical type without consuming the body, so the auth-time pre-read saw ``{}`` and skipped the banned-param check while the handler's later ``request.body()`` saw the original JSON payload. Parse the media type per RFC 7231 (substring before ``;``, trimmed, lowercased) and accept only ``application/x-www-form-urlencoded`` and ``multipart/form-data``. Replace both substring sites with the shared ``_is_form_content_type`` helper. Tests pin: case/whitespace/charset variants of the two real types match; ``application/form-json`` and similar substring-match traps fall through to the JSON parse path; real form POSTs continue to route through ``request.form()``. * chore(proxy): extract _is_json_content_type symmetric helper Mirror ``_is_form_content_type`` for the JSON branch of ``get_request_body`` so both classifications share the same media-type normalisation (strip params, trim, lowercase) and any future change to the parsing rules has one place to update. Adds tests for ``_is_json_content_type`` and for ``get_request_body`` covering the canonical JSON / form / unsupported / non-POST paths. * chore(proxy): surface form-parse failures instead of caching empty body Starlette's ``request.form()`` raises ``MultiPartException`` / ``ValueError`` / ``AssertionError`` on malformed multipart input (missing boundary, malformed chunk encoding, etc.). The outer ``except Exception: return {}`` swallowed every form-parse failure and cached an empty parsed body — auth-time pre-reads saw ``{}`` and skipped every banned-param check while a later raw-body re-read in the handler still saw the original payload. Same TOCTOU shape as the substring-match bypass: the auth gate and the handler don't agree on what the body is. Wrap ``request.form()`` in a narrow ``try`` that converts any parse failure to a 400 ``ProxyException``. The outer broad ``except`` is retained for unrelated unexpected errors but no longer covers form-parse-side bypass shapes. Adds a regression test parametrised over the exception classes Starlette can raise from ``request.form()``. * chore(proxy): drop redundant _is_json_content_type test class ``_is_json_content_type`` is a 3-line wrapper around the shared ``_normalize_media_type`` helper. Positive coverage lives in ``TestGetRequestBody.test_json_with_charset_param_parses_as_json``; negative coverage is covered transitively by ``TestIsFormContentType``'s non-form parametrize matrix (anything that isn't a form type falls through to the JSON branch). * chore(proxy): carry ASGI path into WebSocket auth synthetic Request (#27940) ``user_api_key_auth_websocket`` built a synthetic ``Request`` with a two-key scope (``type`` + ``headers``) and set ``request._url = websocket.url``. ``get_request_route`` reads ``scope.get("path", ...)`` and falls back to ``request.url.path`` only when ``path`` is absent. For the WebSocket flow that fallback fires and resolves to the Host-header-derived value (Starlette reconstructs ``websocket.url`` from the Host header), so a malformed Host collapses the resolved route and lets the auth gate compare against the wrong value. Carry the ASGI scope's ``path``, ``root_path``, and ``app_root_path`` into the synthetic scope so the lookup never reaches the fallback on the legitimate path. Regression test pins that the request handed to ``user_api_key_auth`` has ``scope["path"]`` equal to the ASGI scope's path. --------- Co-authored-by: stuxf <70670632+stuxf@users.noreply.github.com> --- litellm/proxy/auth/user_api_key_auth.py | 20 ++- .../proxy/common_utils/http_parsing_utils.py | 61 ++++++-- .../test_user_api_key_auth.py | 30 ++++ .../common_utils/test_http_parsing_utils.py | 143 ++++++++++++++++++ 4 files changed, 240 insertions(+), 14 deletions(-) diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index 30b5d36e14a..0cca9414b2a 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -12,7 +12,7 @@ import fnmatch import re import secrets from datetime import datetime, timezone -from typing import Any, Iterator, List, Optional, Tuple, Union, cast +from typing import Any, Dict, Iterator, List, Optional, Tuple, Union, cast import fastapi from fastapi import HTTPException, Request, WebSocket, status @@ -333,8 +333,22 @@ def _apply_budget_limits_to_end_user_params( async def user_api_key_auth_websocket(websocket: WebSocket): # Accept the WebSocket connection - scope_headers = list(websocket.scope.get("headers") or []) - request = Request(scope={"type": "http", "headers": scope_headers}) + ws_scope = websocket.scope or {} + scope_headers = list(ws_scope.get("headers") or []) + # ``get_request_route`` falls back to ``request.url.path`` when + # ``scope["path"]`` is absent. On WebSockets that fallback reads + # ``websocket.url``, which Starlette reconstructs from the (poisonable) + # Host header. Carry the ASGI scope's path / root_path so the lookup + # never reaches the fallback. + synthetic_scope: Dict[str, Any] = { + "type": "http", + "headers": scope_headers, + "path": ws_scope.get("path", ""), + } + for key in ("root_path", "app_root_path"): + if key in ws_scope: + synthetic_scope[key] = ws_scope[key] + request = Request(scope=synthetic_scope) request._url = websocket.url diff --git a/litellm/proxy/common_utils/http_parsing_utils.py b/litellm/proxy/common_utils/http_parsing_utils.py index 71abdfa5e9e..fecfc1b4714 100644 --- a/litellm/proxy/common_utils/http_parsing_utils.py +++ b/litellm/proxy/common_utils/http_parsing_utils.py @@ -13,6 +13,34 @@ from litellm.proxy.common_utils.callback_utils import ( from litellm.types.router import Deployment +_FORM_CONTENT_TYPES: frozenset[str] = frozenset( + {"application/x-www-form-urlencoded", "multipart/form-data"} +) + + +def _normalize_media_type(content_type: str) -> str: + """Return the bare media type per RFC 7231: strip params, trim, lowercase.""" + if not content_type: + return "" + return content_type.split(";", 1)[0].strip().lower() + + +def _is_form_content_type(content_type: str) -> bool: + """ + True iff Starlette's ``request.form()`` will actually parse this body. + + Substring matching ``"form"`` is unsafe: ``request.form()`` returns empty + ``FormData`` for non-canonical types without consuming the body, leaving + the auth-time pre-read and the handler's read seeing different payloads. + """ + return _normalize_media_type(content_type) in _FORM_CONTENT_TYPES + + +def _is_json_content_type(content_type: str) -> bool: + """True iff the body should be parsed as JSON.""" + return _normalize_media_type(content_type) == "application/json" + + async def _read_request_body(request: Optional[Request]) -> Dict: """ Safely read the request body and parse it as JSON. @@ -37,8 +65,24 @@ async def _read_request_body(request: Optional[Request]) -> Dict: _request_headers: dict = _safe_get_request_headers(request=request) content_type = _request_headers.get("content-type", "") - if "form" in content_type: - parsed_body = dict(await request.form()) + if _is_form_content_type(content_type): + try: + form_data = await request.form() + except Exception as e: + # ``request.form()`` raises on malformed multipart (missing + # boundary, malformed chunk encoding, …). Surface as 400 so + # the auth-time pre-read does not silently cache ``{}`` while + # a later raw-body re-read sees the original payload — + # banned-param checks must see the same body the handler + # acts on. + verbose_proxy_logger.error(f"Invalid form payload: {e}") + raise ProxyException( + message=f"Invalid form payload: {e}", + type="invalid_request_error", + param="request_body", + code=status.HTTP_400_BAD_REQUEST, + ) + parsed_body = dict(form_data) if "metadata" in parsed_body and isinstance(parsed_body["metadata"], str): parsed_body["metadata"] = json.loads(parsed_body["metadata"]) else: @@ -306,18 +350,13 @@ async def get_request_body(request: Request) -> Dict[str, Any]: Read the request body and parse it as JSON. """ if request.method == "POST": - if request.headers.get("content-type", "") == "application/json": + content_type = request.headers.get("content-type", "") + if _is_json_content_type(content_type): return await _read_request_body(request) - elif "multipart/form-data" in request.headers.get( - "content-type", "" - ) or "application/x-www-form-urlencoded" in request.headers.get( - "content-type", "" - ): + elif _is_form_content_type(content_type): return await get_form_data(request) else: - raise ValueError( - f"Unsupported content type: {request.headers.get('content-type')}" - ) + raise ValueError(f"Unsupported content type: {content_type}") return {} diff --git a/tests/proxy_unit_tests/test_user_api_key_auth.py b/tests/proxy_unit_tests/test_user_api_key_auth.py index 210347aaf94..958b028c542 100644 --- a/tests/proxy_unit_tests/test_user_api_key_auth.py +++ b/tests/proxy_unit_tests/test_user_api_key_auth.py @@ -915,6 +915,36 @@ async def test_user_api_key_auth_websocket(): ) +@pytest.mark.asyncio +async def test_user_api_key_auth_websocket_carries_asgi_path(): + """ + The synthetic Request must carry the ASGI scope's ``path`` so + ``get_request_route`` returns the real WebSocket path, not a value + reconstructed from the (Host-poisonable) ``websocket.url``. + """ + from litellm.proxy.auth.user_api_key_auth import user_api_key_auth_websocket + + mock_websocket = MagicMock(spec=WebSocket) + mock_websocket.query_params = {"model": "some_model"} + mock_websocket.headers = {"authorization": "Bearer some_api_key"} + mock_websocket.scope = { + "type": "websocket", + "path": "/v1/realtime", + "root_path": "", + "headers": [(b"authorization", b"Bearer some_api_key")], + } + mock_websocket.url = URL(url="/v1/realtime") + + with patch( + "litellm.proxy.auth.user_api_key_auth.user_api_key_auth", autospec=True + ) as mock_user_api_key_auth: + await user_api_key_auth_websocket(mock_websocket) + + request_arg = mock_user_api_key_auth.call_args.kwargs["request"] + assert request_arg.scope.get("path") == "/v1/realtime" + assert request_arg.scope.get("root_path") == "" + + @pytest.mark.parametrize("enforce_rbac", [True, False]) @pytest.mark.asyncio async def test_jwt_user_api_key_auth_builder_enforce_rbac(enforce_rbac, monkeypatch): diff --git a/tests/test_litellm/proxy/common_utils/test_http_parsing_utils.py b/tests/test_litellm/proxy/common_utils/test_http_parsing_utils.py index b4343f6b2e1..3d7cb1e35f3 100644 --- a/tests/test_litellm/proxy/common_utils/test_http_parsing_utils.py +++ b/tests/test_litellm/proxy/common_utils/test_http_parsing_utils.py @@ -16,6 +16,7 @@ sys.path.insert( import litellm from litellm.proxy._types import ProxyException from litellm.proxy.common_utils.http_parsing_utils import ( + _is_form_content_type, _read_request_body, _safe_get_request_headers, _safe_get_request_parsed_body, @@ -853,3 +854,145 @@ class TestGetTagsFromRequestBodyStringCoerce: tags = get_tags_from_request_body({"metadata": {"tags": ["x"]}}) assert tags == ["x"] + + +class TestIsFormContentType: + @pytest.mark.parametrize( + "content_type", + [ + "application/x-www-form-urlencoded", + "multipart/form-data", + "multipart/form-data; boundary=----WebKitFormBoundary", + "Application/X-WWW-Form-Urlencoded", + " multipart/form-data ", + "application/x-www-form-urlencoded; charset=utf-8", + ], + ) + def test_form_types_match(self, content_type): + assert _is_form_content_type(content_type) is True + + @pytest.mark.parametrize( + "content_type", + [ + "", + "application/json", + "application/json; charset=utf-8", + "application/form-json", + "multiform/anything", + "application/json; xform=1", + "application/xml-with-form-data-but-not-actually", + "text/plain", + "form", + ], + ) + def test_non_form_types_rejected(self, content_type): + assert _is_form_content_type(content_type) is False + + +class TestReadRequestBodyNonCanonicalContentType: + """A JSON body with a ``"form"``-substring Content-Type must parse as JSON.""" + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "content_type", + [ + "application/form-json", + "application/json; xform=1", + "multiform/anything", + ], + ) + async def test_json_body_with_formlike_content_type_parses_as_json( + self, content_type + ): + payload = {"user_config": {"model_list": []}, "model": "x"} + + mock_request = MagicMock() + mock_request.body = AsyncMock(return_value=orjson.dumps(payload)) + mock_request.form = AsyncMock(return_value={}) + mock_request.headers = {"content-type": content_type} + mock_request.scope = {} + + result = await _read_request_body(mock_request) + assert result == payload + mock_request.form.assert_not_called() + + @pytest.mark.asyncio + async def test_real_form_post_still_parsed_as_form(self): + mock_request = MagicMock() + mock_request.form = AsyncMock(return_value={"k": "v"}) + mock_request.body = AsyncMock(return_value=b"") + mock_request.headers = {"content-type": "application/x-www-form-urlencoded"} + mock_request.scope = {} + + result = await _read_request_body(mock_request) + assert result == {"k": "v"} + mock_request.form.assert_awaited_once() + + +class TestReadRequestBodyFormParseFailure: + """ + A failed ``request.form()`` parse (e.g. multipart with missing boundary) + must surface as a 400, not silently return ``{}`` — otherwise the + auth-time pre-read sees an empty body while a later raw-body re-read + sees the original payload, defeating every banned-param check. + """ + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "raised_exception", + [ + ValueError("Missing boundary in multipart."), + AssertionError("malformed chunk"), + RuntimeError("form parser exploded"), + ], + ) + async def test_form_parse_failure_raises_400(self, raised_exception): + mock_request = MagicMock() + mock_request.form = AsyncMock(side_effect=raised_exception) + mock_request.headers = {"content-type": "multipart/form-data"} + mock_request.scope = {} + + with pytest.raises(ProxyException) as exc_info: + await _read_request_body(mock_request) + assert str(exc_info.value.code) == "400" + + +class TestGetRequestBody: + @pytest.mark.asyncio + async def test_json_with_charset_param_parses_as_json(self): + payload = {"k": "v"} + mock_request = MagicMock() + mock_request.method = "POST" + mock_request.body = AsyncMock(return_value=orjson.dumps(payload)) + mock_request.headers = {"content-type": "application/json; charset=utf-8"} + mock_request.scope = {} + + result = await get_request_body(mock_request) + assert result == payload + + @pytest.mark.asyncio + async def test_form_post_routes_to_form_data(self): + mock_request = MagicMock() + mock_request.method = "POST" + mock_request.headers = {"content-type": "multipart/form-data; boundary=x"} + mock_request.form = AsyncMock(return_value={"k": "v"}) + mock_request.scope = {} + + result = await get_request_body(mock_request) + assert result == {"k": "v"} + + @pytest.mark.asyncio + async def test_substring_match_no_longer_accepted(self): + mock_request = MagicMock() + mock_request.method = "POST" + mock_request.headers = {"content-type": "application/form-json"} + mock_request.scope = {} + + with pytest.raises(ValueError, match="Unsupported content type"): + await get_request_body(mock_request) + + @pytest.mark.asyncio + async def test_non_post_returns_empty(self): + mock_request = MagicMock() + mock_request.method = "GET" + assert await get_request_body(mock_request) == {} From e23d06dda4f4ef22a046da3a034f58091a31c40e Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Wed, 20 May 2026 19:01:31 -0700 Subject: [PATCH 3/3] test(realtime): expect session.created as xAI realtime initial event (#28424) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit xAI's Grok Voice Agent API now sends session.created as its first realtime event (matching OpenAI), followed by conversation.created. The E2E canary pinned the old conversation.created value and failed. LiteLLM's xAI realtime path is a verbatim passthrough (provider_config is None, raw forwarding), so the event ordering is xAI's own — no transformation on our side. Update the pinned expected value and the now-stale comments to match the current API behavior. --- tests/llm_translation/realtime/base_realtime_tests.py | 2 +- tests/llm_translation/realtime/test_xai_realtime.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/llm_translation/realtime/base_realtime_tests.py b/tests/llm_translation/realtime/base_realtime_tests.py index 1d55f13b00d..f1c42659007 100644 --- a/tests/llm_translation/realtime/base_realtime_tests.py +++ b/tests/llm_translation/realtime/base_realtime_tests.py @@ -79,7 +79,7 @@ class RealTimeWebSocketClient: def _is_initial_event(self, msg_type: str) -> bool: """Check if message type is an initial connection event""" - # OpenAI sends "session.created", xAI sends "conversation.created" + # OpenAI and xAI send "session.created"; some providers send "conversation.created" return msg_type in ["session.created", "conversation.created"] async def receive_text(self): diff --git a/tests/llm_translation/realtime/test_xai_realtime.py b/tests/llm_translation/realtime/test_xai_realtime.py index 0bb7a59bb1a..86d0ebe3a3c 100644 --- a/tests/llm_translation/realtime/test_xai_realtime.py +++ b/tests/llm_translation/realtime/test_xai_realtime.py @@ -19,8 +19,8 @@ class TestXAIRealtime(BaseRealtimeTest): """ E2E tests for xAI Realtime API. - xAI's Grok Voice Agent API is OpenAI-compatible but uses: - - Different initial event: "conversation.created" instead of "session.created" + xAI's Grok Voice Agent API is OpenAI-compatible: + - Initial event: "session.created" (matches OpenAI) - Different endpoint: wss://api.x.ai/v1/realtime - Model: grok-4-1-fast-non-reasoning """ @@ -32,4 +32,4 @@ class TestXAIRealtime(BaseRealtimeTest): return "XAI_API_KEY" def get_initial_event_type(self) -> str: - return "conversation.created" + return "session.created"