mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
Merge pull request #37203 from BerriAI/litellm_batches_metadata_type_400
fix(proxy): return 400 for non-object metadata and litellm_metadata instead of silent drop or 500
This commit is contained in:
commit
cde5465e6f
9 changed files with 210 additions and 27 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"reportAny": {
|
||||
"limit": 22344
|
||||
"limit": 22343
|
||||
},
|
||||
"reportArgumentType": {
|
||||
"limit": 2578
|
||||
|
|
|
|||
|
|
@ -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 {}
|
||||
|
|
|
|||
|
|
@ -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,13 @@ async def add_litellm_data_to_request(
|
|||
continue
|
||||
data.pop(_internal_key, None)
|
||||
_reject_url_valued_destinations(data)
|
||||
_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):
|
||||
|
|
@ -1711,29 +1750,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
|
||||
|
|
|
|||
|
|
@ -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)),
|
||||
|
|
|
|||
|
|
@ -201,7 +201,7 @@
|
|||
"limit": 58
|
||||
},
|
||||
"SIM102": {
|
||||
"limit": 319
|
||||
"limit": 317
|
||||
},
|
||||
"SIM103": {
|
||||
"limit": 119
|
||||
|
|
|
|||
|
|
@ -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"""
|
||||
|
||||
|
|
|
|||
|
|
@ -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,91 @@ 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_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"})}
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@
|
|||
"limit": 16713
|
||||
},
|
||||
"LIT011": {
|
||||
"limit": 5591
|
||||
"limit": 5590
|
||||
},
|
||||
"LIT012": {
|
||||
"limit": 4519
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue