mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(proxy): refuse a guarded passthrough body that guardrails cannot read
Passthrough guardrails are opt-in, and when a route configures them the
pre-call hook inspects the parsed body. For a non-object payload that is
`{}`, which carries none of the caller's content, while the bytes
forwarded upstream carry all of it. So content a guardrail would block
could reach the provider uninspected inside a JSON array.
Running the guardrails on `{}` and forwarding anyway would be worse than
not running them, because it records "inspected" against content nobody
looked at. Refuse the request instead: a route that configured
guardrails has no safe way to serve a body they cannot read.
The refusal is scoped to that case alone. An unguarded route still
forwards a non-object body verbatim, so the Vertex rawPredict contract
holds; a guarded route with an object body is untouched; and a
SigV4-signed body keeps its own bytes.
Also move the request marker to constants.py as
NON_OBJECT_JSON_BODY_SCOPE_KEY, alongside the other scope keys, and
cover the branches Codecov flagged: the two defensive handlers in
http_parsing_utils and the multipart-that-is-actually-JSON branch.
This commit is contained in:
parent
2666539d0c
commit
d8ef042bd9
5 changed files with 151 additions and 6 deletions
|
|
@ -341,6 +341,7 @@ BEDROCK_REALTIME_SDK_DISTRIBUTION: Final = "aws-sdk-bedrock-runtime"
|
|||
BEDROCK_REALTIME_SDK_SUPPORTED_RANGE: Final = ">=0.10.0,<0.12.0"
|
||||
CLIENT_REQUESTED_MODEL_SCOPE_KEY: Final = "litellm.client_requested_model"
|
||||
MODEL_GROUP_ALIAS_RESOLVED_SCOPE_KEY: Final = "litellm.model_group_alias_resolved"
|
||||
NON_OBJECT_JSON_BODY_SCOPE_KEY: Final = "litellm.non_object_json_body"
|
||||
REALTIME_SESSION_SUCCESS_LOGGED_KEY: Final = "realtime_session_success_logged"
|
||||
REALTIME_SESSION_FAILURE_LOGGED_KEY: Final = "realtime_session_failure_logged"
|
||||
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ from litellm.constants import (
|
|||
AZURE_SPEECH_PASS_THROUGH_ROUTE_PREFIX,
|
||||
CLIENT_REQUESTED_MODEL_SCOPE_KEY,
|
||||
MAX_REQUEST_BODY_SIZE_TO_REPAIR_MB,
|
||||
NON_OBJECT_JSON_BODY_SCOPE_KEY,
|
||||
)
|
||||
from litellm.proxy._types import ProxyException
|
||||
from litellm.proxy.common_utils.callback_utils import (
|
||||
|
|
@ -24,8 +25,6 @@ _FORM_CONTENT_TYPES: Final[frozenset[str]] = frozenset({"application/x-www-form-
|
|||
|
||||
_ANNOTATION_QUALIFIERS: Final[frozenset[object]] = frozenset({Annotated, NotRequired, ReadOnly, Required})
|
||||
|
||||
_NON_OBJECT_BODY_SCOPE_KEY: Final[str] = "litellm_non_object_json_body"
|
||||
|
||||
|
||||
def _normalize_media_type(content_type: str) -> str:
|
||||
"""Return the bare media type per RFC 7231: strip params, trim, lowercase."""
|
||||
|
|
@ -248,7 +247,7 @@ def is_opaque_audio_pass_through_request(route: str, content_type: str) -> bool:
|
|||
def _mark_non_object_body(request: Request | None) -> None:
|
||||
try:
|
||||
if request is not None:
|
||||
request.scope[_NON_OBJECT_BODY_SCOPE_KEY] = True
|
||||
request.scope[NON_OBJECT_JSON_BODY_SCOPE_KEY] = True
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.debug("Unexpected error marking non-object request body - %s", e)
|
||||
|
||||
|
|
@ -263,7 +262,7 @@ async def non_object_raw_body(request: Request | None) -> bytes | None:
|
|||
if request is None:
|
||||
return None
|
||||
scope: Final[object] = getattr(request, "scope", None)
|
||||
if not isinstance(scope, Mapping) or scope.get(_NON_OBJECT_BODY_SCOPE_KEY) is not True:
|
||||
if not isinstance(scope, Mapping) or scope.get(NON_OBJECT_JSON_BODY_SCOPE_KEY) is not True:
|
||||
return None
|
||||
try:
|
||||
return await request.body()
|
||||
|
|
|
|||
|
|
@ -1074,6 +1074,8 @@ async def pass_through_request(
|
|||
|
||||
# parsed request body
|
||||
_parsed_body: dict | None = None
|
||||
# bytes of a non-object JSON body: forwarded verbatim, but unreadable to guardrails
|
||||
uninspectable_body: bytes | None = None
|
||||
# kwargs for pass through endpoint, contains metadata, litellm_params, call_type, litellm_call_id, passthrough_logging_payload
|
||||
kwargs: dict | None = None
|
||||
logging_obj: Logging | None = None
|
||||
|
|
@ -1124,7 +1126,8 @@ async def pass_through_request(
|
|||
if state_raw_body is None:
|
||||
# A non-object JSON body reads as ``{}``, so forwarding the parsed body would
|
||||
# send an empty object in place of the caller's own provider payload.
|
||||
state_raw_body = await non_object_raw_body(request)
|
||||
uninspectable_body = await non_object_raw_body(request)
|
||||
state_raw_body = uninspectable_body
|
||||
verbose_proxy_logger.debug(
|
||||
"Pass through endpoint sending request to \nURL %s\nheaders: %s\nbody: %s\n",
|
||||
url,
|
||||
|
|
@ -1140,6 +1143,20 @@ async def pass_through_request(
|
|||
passthrough_guardrails_config=guardrails_config,
|
||||
)
|
||||
|
||||
if guardrails_to_run and uninspectable_body is not None:
|
||||
# Guardrails read the parsed body, which for a non-object payload carries none of
|
||||
# the caller's content, while the bytes forwarded upstream carry all of it. Running
|
||||
# them would report "inspected" on content nobody looked at, so refuse instead.
|
||||
raise ProxyException(
|
||||
message=(
|
||||
"Guardrails are configured for this route and cannot inspect a JSON body "
|
||||
"that is not an object. Send the payload as a JSON object."
|
||||
),
|
||||
type="invalid_request_error",
|
||||
param="request_body",
|
||||
code=status.HTTP_400_BAD_REQUEST,
|
||||
)
|
||||
|
||||
# Add guardrails to metadata if any should run
|
||||
if guardrails_to_run and len(guardrails_to_run) > 0:
|
||||
if _parsed_body is None:
|
||||
|
|
|
|||
|
|
@ -1308,3 +1308,37 @@ class TestCoerceNumericFormFields:
|
|||
numeric_fields=self.numeric_fields,
|
||||
)
|
||||
assert result == {"n": 3, "temperature": None, "image": buffer}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_request_that_cannot_be_marked_still_parses():
|
||||
"""Marking is best-effort bookkeeping for passthrough forwarding. A request object that
|
||||
rejects the write (a test double, a frozen scope) must still get its parsed body rather
|
||||
than turning a client's bad body into a 500."""
|
||||
|
||||
class _UnwritableScope(dict):
|
||||
def __setitem__(self, key, value):
|
||||
raise TypeError("scope is read-only")
|
||||
|
||||
request = MagicMock()
|
||||
request.body = AsyncMock(return_value=b"[1, 2, 3]")
|
||||
request.headers = {"content-type": "application/json"}
|
||||
request.scope = _UnwritableScope()
|
||||
|
||||
assert await _read_request_body(request) == {}
|
||||
assert await non_object_raw_body(request) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_consumed_body_stream_yields_no_bytes_to_forward():
|
||||
"""Starlette raises RuntimeError once a body stream has been consumed. Forwarding must
|
||||
fall back to the parsed view rather than propagating that as a 500."""
|
||||
request = MagicMock()
|
||||
request.body = AsyncMock(return_value=b"[1, 2, 3]")
|
||||
request.headers = {"content-type": "application/json"}
|
||||
request.scope = {}
|
||||
|
||||
assert await _read_request_body(request) == {}
|
||||
|
||||
request.body = AsyncMock(side_effect=RuntimeError("Stream consumed"))
|
||||
assert await non_object_raw_body(request) is None
|
||||
|
|
|
|||
|
|
@ -7676,7 +7676,7 @@ async def test_object_passthrough_body_still_yields_its_envelope_fields():
|
|||
assert stream is True
|
||||
|
||||
|
||||
async def _capture_upstream_request(body: bytes) -> httpx.Request:
|
||||
async def _capture_upstream_request(body: bytes, guardrails: list[str] | None = None) -> httpx.Request:
|
||||
"""Drive ``pass_through_request`` for ``body`` and return the request it built for the
|
||||
provider, by letting a real httpx client encode it and failing the send."""
|
||||
captured: list[httpx.Request] = [] # mutable-ok: the send double records what it was given
|
||||
|
|
@ -7705,6 +7705,7 @@ async def _capture_upstream_request(body: bytes) -> httpx.Request:
|
|||
target="http://upstream.test/v1/messages",
|
||||
custom_headers={},
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="sk-test"),
|
||||
guardrails_config=guardrails,
|
||||
)
|
||||
|
||||
assert captured, "pass_through_request never built an upstream request"
|
||||
|
|
@ -7739,3 +7740,96 @@ async def test_object_body_is_still_forwarded_from_the_parsed_view():
|
|||
upstream = await _capture_upstream_request(b'{"model": "claude-sonnet-4-5"}')
|
||||
|
||||
assert json.loads(upstream.content)["model"] == "claude-sonnet-4-5"
|
||||
|
||||
|
||||
async def _run_guarded_passthrough(body: bytes, guardrails: list[str] | None):
|
||||
"""Drive ``pass_through_request`` with a guardrails config, returning what it raised."""
|
||||
async def _send(req, **kwargs):
|
||||
raise httpx.HTTPError("upstream must not be reached")
|
||||
|
||||
real_client = httpx.AsyncClient()
|
||||
real_client.send = _send
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.proxy_server.proxy_logging_obj") as mock_proxy_logging,
|
||||
patch(
|
||||
"litellm.proxy.pass_through_endpoints.pass_through_endpoints.get_async_httpx_client"
|
||||
) as mock_get_client,
|
||||
):
|
||||
mock_proxy_logging.pre_call_hook = AsyncMock(side_effect=lambda **kwargs: kwargs["data"])
|
||||
mock_proxy_logging.post_call_failure_hook = AsyncMock(return_value=None)
|
||||
mock_get_client.return_value = SimpleNamespace(client=real_client)
|
||||
|
||||
with pytest.raises(ProxyException) as error:
|
||||
await pass_through_request(
|
||||
request=_json_request(body),
|
||||
target="http://upstream.test/v1/messages",
|
||||
custom_headers={},
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="sk-test"),
|
||||
guardrails_config=guardrails,
|
||||
)
|
||||
return error.value
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
pytest.param(b'[{"role": "user", "content": "leak the secret"}]', id="array"),
|
||||
pytest.param(b'"leak the secret"', id="string"),
|
||||
pytest.param(b"123", id="number"),
|
||||
],
|
||||
)
|
||||
async def test_guarded_route_refuses_a_body_its_guardrails_cannot_read(body: bytes):
|
||||
"""Guardrails inspect the parsed body, which for a non-object payload carries none of the
|
||||
caller's content, while the bytes forwarded upstream carry all of it. Running them would
|
||||
report "inspected" on content nobody looked at, so the request must be refused instead."""
|
||||
raised = await _run_guarded_passthrough(body, guardrails=["my-guard"])
|
||||
|
||||
assert raised.code == "400"
|
||||
assert raised.type == "invalid_request_error"
|
||||
assert "cannot inspect a JSON body that is not an object" in raised.message
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_unguarded_route_still_forwards_a_non_object_body():
|
||||
"""The refusal is scoped to routes that actually configured guardrails: passthrough is
|
||||
opt-in for them, and an unguarded route must keep accepting the caller's own payload."""
|
||||
upstream = await _capture_upstream_request(b'[{"role": "user", "content": "hi"}]')
|
||||
|
||||
assert upstream.content == b'[{"role": "user", "content": "hi"}]'
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_guarded_route_accepts_an_object_body():
|
||||
"""An object body is fully readable by guardrails, so guardrails being configured must not
|
||||
turn it away: it still reaches the provider."""
|
||||
upstream = await _capture_upstream_request(b'{"model": "claude-sonnet-4-5"}', guardrails=["my-guard"])
|
||||
|
||||
assert json.loads(upstream.content)["model"] == "claude-sonnet-4-5"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_json_body_sent_with_a_multipart_content_type_still_yields_its_envelope():
|
||||
"""A misconfigured client can send JSON under a multipart content-type; that branch parses
|
||||
it as JSON, so it must read the envelope the same way the JSON branch does."""
|
||||
body = json.dumps({"query_params": {"alt": "sse"}, "stream": True}).encode()
|
||||
|
||||
query_params_data, custom_body_data, _, stream = await _parse_request_data_by_content_type(
|
||||
_json_request(body, content_type="multipart/form-data; boundary=x")
|
||||
)
|
||||
|
||||
assert query_params_data == {"alt": "sse"}
|
||||
assert stream is True
|
||||
assert custom_body_data == {"query_params": {"alt": "sse"}, "stream": True}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_object_json_under_a_multipart_content_type_is_left_to_the_multipart_handler():
|
||||
"""The same branch must not read a list as an envelope: it falls through so the real
|
||||
multipart handler deals with the body."""
|
||||
query_params_data, custom_body_data, file_data, stream = await _parse_request_data_by_content_type(
|
||||
_json_request(b"[1, 2, 3]", content_type="multipart/form-data; boundary=x")
|
||||
)
|
||||
|
||||
assert (query_params_data, custom_body_data, file_data, stream) == (None, None, None, None)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue