From 3e280b1be9e87cb6fa860ebdf2d8eba25ec268f4 Mon Sep 17 00:00:00 2001 From: yucheng-berri Date: Fri, 28 Aug 2026 14:18:56 -0700 Subject: [PATCH] fix(router): scrub fallback stamp keys in place and strip them at the proxy boundary (#38690) PR #38586 changed the fallback-stamp scrub in async_function_with_fallbacks to rebind kwargs[sibling] to a scrubbed copy instead of popping in place. Every other router bucket write mutates the caller's dict in place, and everything below the router resolves the metadata bucket by key presence, so on a proxy request that carries litellm_metadata the copy becomes a detached object: the proxy's post_call guardrail write-backs land in request_data while the spend row is built from the router's copy. Result: guardrail_information and the guardrail cost silently drop from the spend row on any request that planted a reserved key, and an SDK caller aliasing one dict as both buckets loses the router stamps entirely. Scrub in place again, and move the anti-spoof to the proxy boundary: strip attempted_fallbacks and original_model_group from client-supplied metadata and litellm_metadata in add_litellm_data_to_request, next to the pricing-field strip, so proxy traffic never carries a reserved key and the in-place pop only ever fires for an SDK caller that planted one. Keep #38586's hop-stamp ordering fix (caller keys first, stamps appended) untouched. --- litellm/proxy/litellm_pre_call_utils.py | 19 ++ litellm/router.py | 12 +- .../proxy/test_litellm_pre_call_utils.py | 187 ++++++++++++++++++ tests/test_litellm/test_router.py | 61 ++++-- 4 files changed, 258 insertions(+), 21 deletions(-) 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