From 6866eac96feed14e47e051896c45263fee53086a Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Sat, 5 Sep 2026 23:53:36 -0700 Subject: [PATCH] fix(router): ignore a retry skip list the caller sent itself The retry skip travels as a request kwarg, and the router forwards keys it does not recognize, so a client can put _retry_skipped_deployment_ids in its own request body. The value went straight into a pydantic TypeAdapter and then into a set(), so an int or an object raised TypeError and a string, a list, or a dict raised a ValidationError, each of them replacing the 400 the provider had actually returned. Every read now goes through one narrowing function that keeps a tuple of strings and skips nothing otherwise, so a forged value costs the caller nothing beyond the retry landing on the same deployment again. --- litellm/router.py | 11 +++++---- tests/test_litellm/test_router.py | 39 +++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index 93178dc8e26..e42bc2af398 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -401,7 +401,10 @@ def _stream_chunks_have_generated_content(chunks: Sequence[ModelResponseStream]) _NO_SESSION_KWARGS: Final[Mapping[str, Mapping[str, object]]] = MappingProxyType({}) _SESSION_ADAPTER: Final = TypeAdapter(Mapping[str, object]) -_SKIPPED_DEPLOYMENT_IDS_ADAPTER: Final = TypeAdapter(tuple[str, ...]) + + +def _as_retry_skipped_deployment_ids(value: object) -> tuple[str, ...]: + return tuple(item for item in value if isinstance(item, str)) if isinstance(value, tuple) else () def _with_router_resolved_session_model(session: object, model_name: str) -> Mapping[str, Mapping[str, object]]: @@ -7467,7 +7470,7 @@ class Router: return () if litellm._should_retry(status_code): # pyright: ignore[reportPrivateUsage] # as in should_retry_this_error return () - already_skipped_ids: Final = _SKIPPED_DEPLOYMENT_IDS_ADAPTER.validate_python(already_skipped or ()) + already_skipped_ids: Final = _as_retry_skipped_deployment_ids(already_skipped) skipped: Final = tuple(sorted(frozenset((*already_skipped_ids, failed_deployment_id)))) verbose_router_logger.debug( "Retry skips deployments that already answered %s to this request: %s", status_code, skipped @@ -12496,7 +12499,7 @@ class Router: ## RETRY SKIP ## -> drop deployments that already refused this request with a ## non-retryable status, unless that leaves nothing, so the caller still gets ## the provider's own error instead of a no-deployments error. - _retry_skipped_deployment_ids: Final = ( + _retry_skipped_deployment_ids: Final = _as_retry_skipped_deployment_ids( request_kwargs.pop("_retry_skipped_deployment_ids", None) if request_kwargs else None ) healthy_deployments = ( @@ -13412,7 +13415,7 @@ class Router: ) ## RETRY SKIP ## -> see async counterpart in async_get_healthy_deployments. - _retry_skipped_deployment_ids: Final = ( + _retry_skipped_deployment_ids: Final = _as_retry_skipped_deployment_ids( request_kwargs.pop("_retry_skipped_deployment_ids", None) if request_kwargs else None ) healthy_deployments = ( diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index 34c435af706..696abcd65a5 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -13263,6 +13263,10 @@ def _retry_skip_deployment(deployment_id, host, litellm_params=None, model_info= (400, None, None, ()), (None, "rejecting", None, ()), ("400", "rejecting", None, ()), + (400, "second", 7, ("second",)), + (400, "second", "first", ("second",)), + (400, "second", ["first"], ("second",)), + (400, "second", ("first", 7), ("first", "second")), ], ) def test_router_deployment_ids_to_skip_on_retry(status_code, failed_deployment_id, already_skipped, expected): @@ -13282,6 +13286,11 @@ def test_router_deployment_ids_to_skip_on_retry(status_code, failed_deployment_i (["rejecting", "sibling"], (), ["rejecting", "sibling"]), (["rejecting", "sibling"], None, ["rejecting", "sibling"]), (["rejecting", "sibling"], ("absent",), ["rejecting", "sibling"]), + (["rejecting", "sibling"], 7, ["rejecting", "sibling"]), + (["rejecting", "sibling"], "rejecting", ["rejecting", "sibling"]), + (["rejecting", "sibling"], ["rejecting"], ["rejecting", "sibling"]), + (["rejecting", "sibling"], {"rejecting": True}, ["rejecting", "sibling"]), + (["rejecting", "sibling"], ("rejecting", 7), ["sibling"]), ], ) @pytest.mark.asyncio @@ -13298,6 +13307,36 @@ async def test_router_healthy_deployments_keep_the_last_candidate_a_retry_skippe assert "_retry_skipped_deployment_ids" not in request_kwargs +@pytest.mark.parametrize("client_supplied", [7, "rejecting", ["rejecting"], {"rejecting": True}, object()]) +@pytest.mark.asyncio +async def test_router_retry_policy_400_keeps_upstream_error_when_a_client_forges_the_skip_list( + monkeypatch: pytest.MonkeyPatch, client_supplied +): + monkeypatch.setattr(litellm, "disable_aiohttp_transport", True) + router = litellm.Router( + model_list=[_retry_skip_deployment("rejecting", "rejecting"), _retry_skip_deployment("sibling", "sibling")], + num_retries=2, + retry_policy={"BadRequestErrorRetries": 2}, + disable_cooldowns=True, + ) + + with respx.mock as respx_mock: + respx_mock.post("https://rejecting.local/v1/chat/completions").mock( + return_value=httpx.Response(400, json={"error": _UPSTREAM_400}) + ) + respx_mock.post("https://sibling.local/v1/chat/completions").mock( + return_value=httpx.Response(400, json={"error": _UPSTREAM_400}) + ) + with pytest.raises(litellm.BadRequestError) as raised: + await router.acompletion( + model="gpt-5.6", + messages=[{"role": "user", "content": "hi"}], + _retry_skipped_deployment_ids=client_supplied, + ) + + assert "upstream refused this request" in str(raised.value) + + @pytest.mark.asyncio async def test_router_retry_policy_400_keeps_upstream_error_on_order_fallback_hop(monkeypatch: pytest.MonkeyPatch): monkeypatch.setattr(litellm, "disable_aiohttp_transport", True)