chore(ci): merge dev branch (#28314)
* 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>
This commit is contained in:
parent
35520adb4f
commit
f99fb5f27f
@ -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
|
||||
|
||||
|
||||
@ -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 {}
|
||||
|
||||
|
||||
|
||||
@ -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):
|
||||
|
||||
@ -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) == {}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user