From f99fb5f27f84257aa23da0afd737f85977d974be Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Wed, 20 May 2026 17:47:33 -0700 Subject: [PATCH] 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 30b5d36e14..0cca9414b2 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 71abdfa5e9..fecfc1b471 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 210347aaf9..958b028c54 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 b4343f6b2e..3d7cb1e35f 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) == {}