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.
This commit is contained in:
yucheng-berri 2026-08-28 14:18:56 -07:00 committed by GitHub
parent 1e4d358f3f
commit 3e280b1be9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 258 additions and 21 deletions

View file

@ -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.

View file

@ -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:

View file

@ -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"}]

View file

@ -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