diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index fb269e846fc..194ef5d756c 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -313,6 +313,10 @@ _CLIENT_PRICING_CONTROL_FIELDS: Final = frozenset(CustomPricingLiteLLMParams.mod # into response_cost and spend; a client seeding it forges (even negative) # guardrail cost. _CLIENT_PRICING_METADATA_FIELDS: Final = frozenset({"model_info", "standard_logging_guardrail_information"}) +# ``attempted_fallbacks`` and ``original_model_group`` are written by the router +# and read by spend logs as fact; a client value has no legitimate meaning and no +# key or team setting keeps it, so the strip is never gated. +_ROUTER_RESERVED_METADATA_FIELDS: Final = frozenset({"attempted_fallbacks", "original_model_group"}) _ALLOW_CLIENT_PRICING_OVERRIDE_METADATA_KEY: Final = "allow_client_pricing_override" # Request fields whose value, when URL-valued, becomes the outbound destination @@ -538,6 +542,20 @@ def _strip_client_pricing_overrides(data: dict[str, Any]) -> None: ) +def _strip_router_reserved_metadata( + data: dict[str, Any], # mutable-ok: strips in place on the request body the pre-call pipeline threads through +) -> None: + """Drop the router-owned fallback stamps from any client-supplied metadata bucket.""" + for metadata_key in ("metadata", "litellm_metadata"): + if not isinstance(metadata := data.get(metadata_key), dict): + continue + for field in _ROUTER_RESERVED_METADATA_FIELDS & metadata.keys(): + metadata.pop(field) + verbose_proxy_logger.debug( + "Stripped router-reserved metadata field from request body: %s.%s", metadata_key, field + ) + + def _get_metadata_variable_name(request: Request) -> str: """ Helper to return what the "metadata" field should be called in the request data @@ -1882,6 +1900,7 @@ async def add_litellm_data_to_request( # would silently skip the field. if not _key_or_team_allows_client_pricing_override(user_api_key_dict): _strip_client_pricing_overrides(data) + _strip_router_reserved_metadata(data) # Same reason as the strips above: runs after the metadata string-to-dict parse # so JSON-string metadata cannot smuggle callback credentials past the dict guard. diff --git a/litellm/router.py b/litellm/router.py index f3fb59f9ecc..5ed92d845c8 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -7051,13 +7051,11 @@ class Router: _sibling_metadata_key: Final = ( "metadata" if _fallback_metadata_key == "litellm_metadata" else "litellm_metadata" ) - if isinstance(_sibling_metadata := kwargs.get(_sibling_metadata_key), dict) and ( - "attempted_fallbacks" in _sibling_metadata or "original_model_group" in _sibling_metadata - ): - _scrubbed_sibling_metadata: Final = _sibling_metadata.copy() - _scrubbed_sibling_metadata.pop("attempted_fallbacks", None) - _scrubbed_sibling_metadata.pop("original_model_group", None) - kwargs[_sibling_metadata_key] = _scrubbed_sibling_metadata + if isinstance(_sibling_metadata := kwargs.get(_sibling_metadata_key), dict): + # In place, like every other router bucket write: downstream resolves the bucket by + # key presence, so rebinding kwargs to a copy detaches the proxy's request_data write-backs + _sibling_metadata.pop("attempted_fallbacks", None) + _sibling_metadata.pop("original_model_group", None) if isinstance(_fallback_metadata := kwargs.get(_fallback_metadata_key), dict): _fallback_metadata["attempted_fallbacks"] = 0 if model_group is not None: 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 a10f3672dff..c51c1c3f73b 100644 --- a/tests/test_litellm/proxy/test_litellm_pre_call_utils.py +++ b/tests/test_litellm/proxy/test_litellm_pre_call_utils.py @@ -7456,3 +7456,190 @@ def test_newrelic_vars_scoped_to_newrelic_callback_entry(): None, ) assert legit.callback_vars == {"newrelic_api_key": "REAL", "newrelic_region": "us"} + + +def _reserved_stamp_request(path: str) -> MagicMock: + request_mock = MagicMock(spec=Request) + request_mock.url = MagicMock() + request_mock.url.path = path + request_mock.url.__str__.return_value = f"http://localhost{path}" + 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 + + +def _reserved_stamp_key(key_metadata: dict | None = None) -> UserAPIKeyAuth: + return UserAPIKeyAuth( + api_key="hashed-key", + metadata=key_metadata or {}, + team_metadata={}, + spend=0.0, + max_budget=100.0, + model_max_budget={}, + team_spend=0.0, + team_max_budget=200.0, + ) + + +_PLANTED_STAMPS = {"attempted_fallbacks": 99, "original_model_group": "spoofed-group", "client_key": "client_value"} + + +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_strips_router_reserved_stamps_from_both_buckets(): + """attempted_fallbacks and original_model_group are router-written facts the spend row + reads back; a client planting them in either bucket is dropped at the boundary so the + router never sees a reserved key it did not write.""" + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + data = { + "model": "gpt-3.5-turbo", + "messages": [{"role": "user", "content": "hi"}], + "metadata": dict(_PLANTED_STAMPS), + "litellm_metadata": dict(_PLANTED_STAMPS), + } + + updated = await add_litellm_data_to_request( + data=data, + request=_reserved_stamp_request("/v1/chat/completions"), + user_api_key_dict=_reserved_stamp_key(), + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + for bucket in ("metadata", "litellm_metadata"): + assert "attempted_fallbacks" not in updated[bucket] + assert "original_model_group" not in updated[bucket] + assert updated[bucket]["client_key"] == "client_value" + + +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_strips_router_reserved_stamps_from_json_string_litellm_metadata(): + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + data = { + "model": "gpt-3.5-turbo", + "messages": [{"role": "user", "content": "hi"}], + "litellm_metadata": json.dumps(_PLANTED_STAMPS), + } + + updated = await add_litellm_data_to_request( + data=data, + request=_reserved_stamp_request("/v1/chat/completions"), + user_api_key_dict=_reserved_stamp_key(), + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + assert isinstance(updated["litellm_metadata"], dict) + assert "attempted_fallbacks" not in updated["litellm_metadata"] + assert "original_model_group" not in updated["litellm_metadata"] + assert updated["litellm_metadata"]["client_key"] == "client_value" + + +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_strips_router_reserved_stamps_despite_pricing_override_opt_in(): + """The pricing strip is gated on allow_client_pricing_override; the reserved-stamp strip + is not, because no key or team setting makes a client-written fallback count valid.""" + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + data = { + "model": "gpt-3.5-turbo", + "messages": [{"role": "user", "content": "hi"}], + "litellm_metadata": {**_PLANTED_STAMPS, "model_info": {"input_cost_per_token": 0.0}}, + } + + updated = await add_litellm_data_to_request( + data=data, + request=_reserved_stamp_request("/v1/chat/completions"), + user_api_key_dict=_reserved_stamp_key({"allow_client_pricing_override": True}), + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + assert updated["litellm_metadata"]["model_info"] == {"input_cost_per_token": 0.0} + assert "attempted_fallbacks" not in updated["litellm_metadata"] + assert "original_model_group" not in updated["litellm_metadata"] + + +@pytest.mark.asyncio +async def test_add_litellm_data_to_request_strips_router_reserved_stamps_on_responses_route(): + """On the Responses family the proxy-owned bucket is litellm_metadata and the client's + OpenAI metadata param is the sibling; both lose the reserved keys.""" + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + data = { + "model": "gpt-3.5-turbo", + "input": "hi", + "metadata": dict(_PLANTED_STAMPS), + "litellm_metadata": dict(_PLANTED_STAMPS), + } + + updated = await add_litellm_data_to_request( + data=data, + request=_reserved_stamp_request("/v1/responses"), + user_api_key_dict=_reserved_stamp_key(), + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + + for bucket in ("metadata", "litellm_metadata"): + assert "attempted_fallbacks" not in updated[bucket] + assert "original_model_group" not in updated[bucket] + assert updated[bucket]["client_key"] == "client_value" + + +@pytest.mark.asyncio +async def test_router_keeps_proxy_metadata_bucket_identity_after_reserved_stamp_strip(): + """Regression for the #38586 break: a client that planted a reserved key in + litellm_metadata made the router hand downstream a scrubbed copy, so the proxy's + post_call write-backs (guardrail telemetry, applied guardrails) landed in a dict the + spend row never read. After the boundary strip plus the in-place scrub, the object the + router forwards is the proxy's own request_data bucket.""" + from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request + + data = { + "model": "gpt-3.5-turbo", + "messages": [{"role": "user", "content": "hi"}], + "litellm_metadata": dict(_PLANTED_STAMPS), + } + request_data = await add_litellm_data_to_request( + data=data, + request=_reserved_stamp_request("/v1/chat/completions"), + user_api_key_dict=_reserved_stamp_key(), + proxy_config=MagicMock(), + general_settings={}, + version="test-version", + ) + proxy_bucket = request_data["litellm_metadata"] + router = litellm.Router( + model_list=[ + { + "model_name": "gpt-3.5-turbo", + "litellm_params": {"model": "gpt-3.5-turbo", "mock_response": "hi"}, + } + ] + ) + forwarded_buckets = [] + original_acompletion = router._acompletion + + async def _spy(*args, **spy_kwargs): + forwarded_buckets.append(spy_kwargs["litellm_metadata"]) + return await original_acompletion(*args, **spy_kwargs) + + router._acompletion = _spy + + await router.acompletion(**request_data) + + assert forwarded_buckets == [proxy_bucket] + assert forwarded_buckets[0] is proxy_bucket + assert "attempted_fallbacks" not in proxy_bucket + assert "original_model_group" not in proxy_bucket + proxy_bucket["standard_logging_guardrail_information"] = [{"guardrail_name": "postcall-guard"}] + assert forwarded_buckets[0]["standard_logging_guardrail_information"] == [{"guardrail_name": "postcall-guard"}] diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index 42b15326e76..ff8e568a935 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -10845,9 +10845,8 @@ def _record_router_acompletion_kwargs(router: litellm.Router) -> list: @pytest.mark.asyncio async def test_async_function_with_fallbacks_scrubs_spoofed_values_from_sibling_bucket(): """Spend logs read a truthy litellm_metadata dict in preference to metadata, so spoofed - stamp keys planted in the bucket the route does not own are removed from the request's - downstream view on entry instead of flowing into the spend log row. The caller's own - dict object is never mutated: the scrub replaces the kwargs entry with a cleaned copy.""" + stamp keys planted in the bucket the route does not own are removed on entry, in place, + before they can flow into the spend log row.""" router = litellm.Router( model_list=[ { @@ -10876,20 +10875,19 @@ async def test_async_function_with_fallbacks_scrubs_spoofed_values_from_sibling_ assert "attempted_fallbacks" not in downstream_sibling assert "original_model_group" not in downstream_sibling assert downstream_sibling["client_key"] == "client_value" - assert litellm_metadata == { - "attempted_fallbacks": 99, - "original_model_group": "spoofed-group", - "client_key": "client_value", - } + assert "attempted_fallbacks" not in litellm_metadata + assert "original_model_group" not in litellm_metadata + assert litellm_metadata["client_key"] == "client_value" assert metadata["attempted_fallbacks"] == 0 assert metadata["original_model_group"] == "gpt-3.5-turbo" @pytest.mark.asyncio -async def test_async_function_with_fallbacks_leaves_caller_sibling_dict_object_untouched(): - """The sibling-bucket scrub hands downstream a cleaned copy and never edits the dict - object the caller passed in: callers reuse metadata dicts across requests, and logging - callbacks observe the caller's object.""" +async def test_async_function_with_fallbacks_scrubs_sibling_bucket_in_place(): + """Everything below the router resolves the bucket by key presence, so the scrub edits + the caller's dict object like every other router bucket write. Rebinding kwargs to a + scrubbed copy detaches the proxy's request_data write-backs (guardrail telemetry, retry + accounting) from the object the spend row is built from.""" router = litellm.Router( model_list=[ { @@ -10914,8 +10912,43 @@ async def test_async_function_with_fallbacks_leaves_caller_sibling_dict_object_u ) assert len(downstream_calls) == 1 - assert downstream_calls[0]["litellm_metadata"] is not litellm_metadata - assert litellm_metadata == caller_snapshot + assert downstream_calls[0]["litellm_metadata"] is litellm_metadata + assert "attempted_fallbacks" not in litellm_metadata + assert "original_model_group" not in litellm_metadata + assert litellm_metadata["client_key"] == caller_snapshot["client_key"] + + +@pytest.mark.asyncio +async def test_async_function_with_fallbacks_stamps_aliased_buckets_on_every_call(): + """One dict object passed as both metadata and litellm_metadata: the first call's own + stamp puts the reserved keys into the shared object, so the second call enters the + scrub with them present. Scrubbing in place keeps the stamp and the bucket on the same + object; a scrubbed copy would leave the spend reader's preferred bucket unstamped.""" + router = litellm.Router( + model_list=[ + { + "model_name": "chat-group", + "litellm_params": {"model": "gpt-3.5-turbo", "mock_response": "hi"}, + } + ] + ) + shared_metadata = {"team": "alpha"} + downstream_calls = _record_router_acompletion_kwargs(router) + + for _ in range(3): + await router.acompletion( + model="chat-group", + messages=[{"role": "user", "content": "hey"}], + metadata=shared_metadata, + litellm_metadata=shared_metadata, + ) + + assert len(downstream_calls) == 3 + for call_kwargs in downstream_calls: + assert call_kwargs["litellm_metadata"] is shared_metadata + assert call_kwargs["metadata"] is shared_metadata + assert call_kwargs["litellm_metadata"]["attempted_fallbacks"] == 0 + assert call_kwargs["litellm_metadata"]["original_model_group"] == "chat-group" @pytest.mark.asyncio