From 57e1bf41c6e546440681bfe7e423876b9945decb Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:08:56 -0700 Subject: [PATCH 1/4] fix(proxy): return 400 for non-object metadata and litellm_metadata instead of silent drop or 500 --- basedpyright-code-budget.json | 2 +- litellm/proxy/litellm_pre_call_utils.py | 63 +++++++++++------- ruff-strict-budget.json | 2 +- .../proxy/test_litellm_pre_call_utils.py | 65 ++++++++++++++++++- type-discipline-budget.json | 2 +- 5 files changed, 107 insertions(+), 27 deletions(-) diff --git a/basedpyright-code-budget.json b/basedpyright-code-budget.json index c09369b9b2c..ba131ecac4c 100644 --- a/basedpyright-code-budget.json +++ b/basedpyright-code-budget.json @@ -1,6 +1,6 @@ { "reportAny": { - "limit": 22344 + "limit": 22343 }, "reportArgumentType": { "limit": 2578 diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index c4a350fb285..1b1e3c70110 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -38,6 +38,8 @@ from litellm.proxy._types import ( CommonProxyErrors, LitellmDataForBackendLLMCall, LitellmUserRoles, + ProxyErrorTypes, + ProxyException, SpecialHeaders, TeamCallbackMetadata, UserAPIKeyAuth, @@ -348,6 +350,36 @@ def reject_url_valued_destination(field: str, value: str) -> None: ) +_METADATA_JSON_TYPE_NAMES: Final[Mapping[type, str]] = MappingProxyType( + {bool: "a boolean", int: "an integer", float: "a number", str: "a string", list: "an array"} +) + + +def _invalid_metadata_type_error(field: str, value: object) -> ProxyException: + received_type: Final = _METADATA_JSON_TYPE_NAMES.get(type(value), f"a {type(value).__name__}") + return ProxyException( + message=f"Invalid type for '{field}': expected an object, but got {received_type} instead.", + type=ProxyErrorTypes.bad_request_error, + param=field, + code=400, + ) + + +def _normalized_metadata_object(field: str, value: object) -> Mapping[str, Any]: + """Return ``value`` as a metadata object or raise a 400 like OpenAI does. + + A JSON string that parses to an object is accepted because multipart/form-data + and ``extra_body`` callers can only send metadata as a string. The caller pops + the raw value from the request body before validating so the failure-logging + hooks that inspect the body afterwards don't crash on it and mask the 400 as a 500. + """ + if isinstance(value, dict): + return value + if isinstance(value, str) and isinstance((parsed := safe_json_loads(value)), dict): + return parsed + raise _invalid_metadata_type_error(field=field, value=value) + + def _strip_untrusted_request_header_controls( headers: Any, *, @@ -1572,6 +1604,10 @@ async def add_litellm_data_to_request( continue data.pop(_internal_key, None) _reject_url_valued_destinations(data) + for _metadata_field in ("metadata", "litellm_metadata"): + if (_raw_metadata := data.get(_metadata_field)) is not None: + data.pop(_metadata_field) + data[_metadata_field] = _normalized_metadata_object(_metadata_field, _raw_metadata) # Strip spoofable auth metadata from user-supplied metadata dict _user_metadata = data.get("metadata") if isinstance(_user_metadata, dict): @@ -1711,29 +1747,10 @@ async def add_litellm_data_to_request( verbose_proxy_logger.debug("receiving data: %s", data) - # Parse metadata if it's a string (e.g., from multipart/form-data) - if "metadata" in data and data["metadata"] is not None: - if isinstance(data["metadata"], str): - data["metadata"] = safe_json_loads(data["metadata"]) - if not isinstance(data["metadata"], dict): - verbose_proxy_logger.warning( - "Failed to parse 'metadata' as JSON dict. Received value: %s", data["metadata"] - ) - # requester_metadata is snapshotted AFTER the strip below so - # downstream consumers (e.g. PANW guardrail reading user_ip / - # profile_id) don't see attacker-injected admin slots preserved in - # the deepcopy. - - # Parse litellm_metadata if it's a string (e.g., from multipart/form-data or extra_body) - if "litellm_metadata" in data and data["litellm_metadata"] is not None: - if isinstance(data["litellm_metadata"], str): - parsed_litellm_metadata: Final = safe_json_loads(data["litellm_metadata"]) - if not isinstance(parsed_litellm_metadata, dict): - verbose_proxy_logger.warning( - "Failed to parse 'litellm_metadata' as JSON dict. Received value: %s", data["litellm_metadata"] - ) - else: - data["litellm_metadata"] = parsed_litellm_metadata + # requester_metadata is snapshotted AFTER the strip below so + # downstream consumers (e.g. PANW guardrail reading user_ip / + # profile_id) don't see attacker-injected admin slots preserved in + # the deepcopy. # Strip internal pipeline state and admin-injection slots from user input. # Runs AFTER the string-to-dict parse above so JSON-string metadata (sent diff --git a/ruff-strict-budget.json b/ruff-strict-budget.json index 22d3119d457..15396a95632 100644 --- a/ruff-strict-budget.json +++ b/ruff-strict-budget.json @@ -201,7 +201,7 @@ "limit": 58 }, "SIM102": { - "limit": 319 + "limit": 317 }, "SIM103": { "limit": 119 diff --git a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py index e31058f402e..f6b801d3122 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -11,7 +11,7 @@ from pydantic import ValidationError as PydanticValidationError from starlette.datastructures import Headers import litellm -from litellm.proxy._types import AddTeamCallback, TeamCallbackMetadata, UserAPIKeyAuth +from litellm.proxy._types import AddTeamCallback, ProxyException, TeamCallbackMetadata, UserAPIKeyAuth from litellm.proxy.litellm_pre_call_utils import ( KeyAndTeamLoggingSettings, LiteLLMProxyRequestSetup, @@ -417,6 +417,69 @@ async def test_add_litellm_data_to_request_string_metadata_does_not_crash(): assert updated["metadata"].get("generation_name") == "test" +def _batches_request_mock() -> MagicMock: + request_mock = MagicMock(spec=Request) + request_mock.url = MagicMock() + request_mock.url.__str__.return_value = "http://localhost/v1/batches" + request_mock.url.path = "/v1/batches" + request_mock.method = "POST" + request_mock.query_params = {} + request_mock.headers = {"Content-Type": "application/json"} + request_mock.client = MagicMock() + request_mock.client.host = "127.0.0.1" + return request_mock + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "field,value,received_type", + [ + ("metadata", "abc", "a string"), + ("litellm_metadata", "abc", "a string"), + ("metadata", 42, "an integer"), + ("litellm_metadata", [1, 2], "an array"), + ("metadata", True, "a boolean"), + ], +) +async def test_add_litellm_data_to_request_rejects_non_object_metadata(field, value, received_type): + """Regression for https://github.com/BerriAI/litellm/issues/37147: a + non-object metadata was silently dropped with a 200, and a non-object + litellm_metadata crashed later with a 500 ('str' object has no attribute + 'update'). Both must be a 400 naming the field, like OpenAI returns.""" + data = {"input_file_id": "file-abc", "endpoint": "/v1/chat/completions", field: value} + + with pytest.raises(ProxyException) as exc_info: + await add_litellm_data_to_request( + data=data, + request=_batches_request_mock(), + user_api_key_dict=UserAPIKeyAuth(api_key="hashed-key"), + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + assert exc_info.value.code == "400" + assert exc_info.value.param == field + assert exc_info.value.message == f"Invalid type for '{field}': expected an object, but got {received_type} instead." + assert field not in data + + +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_parses_json_object_string_litellm_metadata(): + data = {"input_file_id": "file-abc", "litellm_metadata": json.dumps({"cost_centre": "research"})} + + updated = await add_litellm_data_to_request( + data=data, + request=_batches_request_mock(), + user_api_key_dict=UserAPIKeyAuth(api_key="hashed-key"), + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + assert updated["litellm_metadata"]["cost_centre"] == "research" + + @pytest.mark.asyncio async def test_add_litellm_data_to_request_proxy_server_request_body_is_post_strip(): """Regression: proxy_server_request['body'] used to be snapshotted before diff --git a/type-discipline-budget.json b/type-discipline-budget.json index ca848190a32..6d70a6aa5f4 100644 --- a/type-discipline-budget.json +++ b/type-discipline-budget.json @@ -30,7 +30,7 @@ "limit": 16713 }, "LIT011": { - "limit": 5591 + "limit": 5590 }, "LIT012": { "limit": 4519 From c894697a2a88dd85c874c36f8710f384ff7d93a4 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:21:12 -0700 Subject: [PATCH 2/4] fix(proxy): keep ProxyException status codes on /v1/messages instead of wrapping into 500 --- .../proxy/anthropic_endpoints/endpoints.py | 3 ++ .../anthropic_endpoints/test_endpoints.py | 39 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/litellm/proxy/anthropic_endpoints/endpoints.py b/litellm/proxy/anthropic_endpoints/endpoints.py index f75899b91dc..a48ef0f08bb 100644 --- a/litellm/proxy/anthropic_endpoints/endpoints.py +++ b/litellm/proxy/anthropic_endpoints/endpoints.py @@ -193,6 +193,9 @@ async def anthropic_response( ) verbose_proxy_logger.exception("litellm.proxy.proxy_server.anthropic_response(): Exception occured - %s", e) + if isinstance(e, ProxyException): + raise + # Extract model_id from request metadata (same as success path) litellm_metadata: Final = data.get("litellm_metadata", {}) or {} model_info: Final = litellm_metadata.get("model_info", {}) or {} diff --git a/tests/test_litellm/proxy/anthropic_endpoints/test_endpoints.py b/tests/test_litellm/proxy/anthropic_endpoints/test_endpoints.py index 69d90a8b59b..0a427df0cb7 100644 --- a/tests/test_litellm/proxy/anthropic_endpoints/test_endpoints.py +++ b/tests/test_litellm/proxy/anthropic_endpoints/test_endpoints.py @@ -125,6 +125,45 @@ class TestBlockedResponseUsage: mock_logging.post_call_failure_hook.assert_awaited_once() +class TestProxyExceptionPassthrough: + @pytest.mark.asyncio + async def test_anthropic_response_reraises_proxy_exception_unwrapped(self): + """A 400 ProxyException from request validation must surface as-is, + not be re-wrapped into a code-500 ProxyException.""" + import litellm.proxy.anthropic_endpoints.endpoints as ep + import litellm.proxy.proxy_server as proxy_server + from litellm.proxy._types import ProxyErrorTypes, ProxyException + + exc = ProxyException( + message="Invalid type for 'metadata': expected an object, but got a string instead.", + type=ProxyErrorTypes.bad_request_error, + param="metadata", + code=400, + ) + + with ( + patch.object(ep, "_read_request_body", new=AsyncMock(return_value={})), + patch.object( + ep.ProxyBaseLLMRequestProcessing, + "base_process_llm_request", + new=AsyncMock(side_effect=exc), + ), + patch.object(proxy_server, "proxy_logging_obj") as mock_logging, + ): + mock_logging.post_call_failure_hook = AsyncMock() + with pytest.raises(ProxyException) as exc_info: + await ep.anthropic_response( + fastapi_response=MagicMock(), + request=MagicMock(), + user_api_key_dict=MagicMock(), + ) + + assert exc_info.value is exc + assert exc_info.value.code == "400" + assert exc_info.value.param == "metadata" + mock_logging.post_call_failure_hook.assert_awaited_once() + + class TestEventLoggingBatchEndpoint: """Test the stubbed event logging batch endpoint""" From f3b0cdca433e8683d4af9944fb008fa9666c90c5 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:32:30 -0700 Subject: [PATCH 3/4] fix(proxy): keep ProxyException status codes on /v1/moderations instead of wrapping into 500 --- litellm/proxy/proxy_server.py | 2 ++ tests/test_litellm/proxy/test_proxy_server.py | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index d76128a76e5..9692d4449d4 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -10373,6 +10373,8 @@ async def moderations( user_api_key_dict=user_api_key_dict, original_exception=e, request_data=data ) verbose_proxy_logger.exception("litellm.proxy.proxy_server.moderations(): Exception occured - %s", e) + if isinstance(e, ProxyException): + raise if isinstance(e, HTTPException): raise ProxyException( message=getattr(e, "message", str(e)), diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index ba207242e29..5545ee92e84 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -11064,3 +11064,37 @@ async def test_ptu_rollup_job_not_registered_without_opt_in(monkeypatch): assert scheduler.get_job(PTU_ROLLUP_JOB_ID) is None assert len(scheduler.get_jobs()) > 0 + + +@pytest.mark.asyncio +async def test_moderations_reraises_proxy_exception_unwrapped(): + """A 400 ProxyException from request validation must surface as-is, + not be re-wrapped into a code-500 ProxyException.""" + from litellm.proxy._types import ProxyErrorTypes, ProxyException + + exc = ProxyException( + message="Invalid type for 'metadata': expected an object, but got a string instead.", + type=ProxyErrorTypes.bad_request_error, + param="metadata", + code=400, + ) + + request = MagicMock() + request.body = AsyncMock(return_value=b'{"input": "hi", "metadata": "abc"}') + + with ( + patch.object(proxy_server_module, "add_litellm_data_to_request", new=AsyncMock(side_effect=exc)), + patch.object(proxy_server_module, "proxy_logging_obj") as mock_logging, + ): + mock_logging.post_call_failure_hook = AsyncMock() + with pytest.raises(ProxyException) as exc_info: + await proxy_server_module.moderations( + request=request, + fastapi_response=MagicMock(), + user_api_key_dict=MagicMock(), + ) + + assert exc_info.value is exc + assert exc_info.value.code == "400" + assert exc_info.value.param == "metadata" + mock_logging.post_call_failure_hook.assert_awaited_once() From a21de07b3faea70032d9f14fc59348fd322b1426 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:44:46 -0700 Subject: [PATCH 4/4] fix(proxy): drop every invalid metadata field before raising so failure hooks never see them --- litellm/proxy/litellm_pre_call_utils.py | 11 ++++++---- .../proxy/test_litellm_pre_call_utils.py | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 1b1e3c70110..6172f3a9158 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -1604,10 +1604,13 @@ async def add_litellm_data_to_request( continue data.pop(_internal_key, None) _reject_url_valued_destinations(data) - for _metadata_field in ("metadata", "litellm_metadata"): - if (_raw_metadata := data.get(_metadata_field)) is not None: - data.pop(_metadata_field) - data[_metadata_field] = _normalized_metadata_object(_metadata_field, _raw_metadata) + _raw_metadata_by_field: Final = { + _metadata_field: data.pop(_metadata_field) + for _metadata_field in ("metadata", "litellm_metadata") + if data.get(_metadata_field) is not None + } + for _metadata_field, _raw_metadata in _raw_metadata_by_field.items(): + data[_metadata_field] = _normalized_metadata_object(_metadata_field, _raw_metadata) # Strip spoofable auth metadata from user-supplied metadata dict _user_metadata = data.get("metadata") if isinstance(_user_metadata, dict): diff --git a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py index f6b801d3122..d3b1e089489 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -464,6 +464,28 @@ async def test_add_litellm_data_to_request_rejects_non_object_metadata(field, va assert field not in data +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_removes_every_invalid_metadata_field_before_raising(): + """When both fields are invalid, the raise for the first must not leave the + second invalid value in data, or failure-logging hooks that inspect the body + can crash on it and mask the 400 as a 500.""" + data = {"input_file_id": "file-abc", "metadata": "abc", "litellm_metadata": "xyz"} + + with pytest.raises(ProxyException) as exc_info: + await add_litellm_data_to_request( + data=data, + request=_batches_request_mock(), + user_api_key_dict=UserAPIKeyAuth(api_key="hashed-key"), + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + assert exc_info.value.param == "metadata" + assert "metadata" not in data + assert "litellm_metadata" not in data + + @pytest.mark.asyncio async def test_add_litellm_data_to_request_parses_json_object_string_litellm_metadata(): data = {"input_file_id": "file-abc", "litellm_metadata": json.dumps({"cost_centre": "research"})}