From 856efae1e52bf1fa9c6c145e17a4fcb5a140cb7e Mon Sep 17 00:00:00 2001 From: fwa-wup Date: Wed, 16 Sep 2026 11:47:05 +0200 Subject: [PATCH] fix(proxy): stop null key/team router_settings from shadowing router defaults The admin UI persists the whole team/key router-settings form on save, including blank fields as explicit nulls. route_request forwarded those nulls as per-request kwargs, and Router treats a present-but-None kwarg as an override (kwargs.get('fallbacks', self.fallbacks)), so a saved-but- empty team form silently disables the globally configured fallback chain (and num_retries/timeout) for every request with that key/team - errors surface as e.g. 401 with 'Available Model Group Fallbacks=None' even though fallbacks are configured. Strip null entries where stored key/team router_settings are parsed, and guard the per-request merge so a null means 'not configured' instead of 'override with nothing'. --- litellm/proxy/proxy_server.py | 14 ++++++-- litellm/proxy/route_llm_request.py | 7 ++-- .../proxy/proxy_server/test_proxy_config.py | 33 +++++++++++++++++++ .../proxy/test_route_llm_request.py | 31 +++++++++++++++++ 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index d7964556531..87dd2a67e54 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -6775,6 +6775,12 @@ class ProxyConfig: """ Parse a router_settings value that may be a dict or a JSON/YAML string. + Null entries are stripped: the admin UI persists the whole router-settings + form on save, including fields left blank as explicit nulls. A null means + "no opinion - keep the router default", never "override the default with + nothing" - forwarding it would shadow global settings such as `fallbacks` + or `num_retries` for every request made with that key/team. + Returns a non-empty dict if valid, otherwise None. """ if value is None: @@ -6794,9 +6800,11 @@ class ProxyConfig: except json.JSONDecodeError: pass - if isinstance(parsed, dict) and parsed: - return parsed - return None + if not isinstance(parsed, dict) or not parsed: + return None + + stripped = {k: v for k, v in parsed.items() if v is not None} + return stripped or None async def get_hierarchical_router_settings( self, diff --git a/litellm/proxy/route_llm_request.py b/litellm/proxy/route_llm_request.py index 20b4708c193..0e27ac4b2e1 100644 --- a/litellm/proxy/route_llm_request.py +++ b/litellm/proxy/route_llm_request.py @@ -526,9 +526,12 @@ async def _route_request_single_attempt( # noqa: ANN202 # returns unawaited pr "enable_tag_filtering", ] - # Merge override settings into data (only if not already set in request) + # Merge override settings into data (only if not already set in request). + # A null override means "not configured" and must not be forwarded: it + # would shadow the Router's own default (e.g. `fallbacks=None` in kwargs + # disables the globally configured fallback chain for the request). for key in per_request_settings: - if key in override_settings and key not in data: + if key in override_settings and override_settings[key] is not None and key not in data: data[key] = override_settings[key] # Use main router with overridden kwargs diff --git a/tests/test_litellm/proxy/proxy_server/test_proxy_config.py b/tests/test_litellm/proxy/proxy_server/test_proxy_config.py index 9a9b47ce3bb..8541e0c2c11 100644 --- a/tests/test_litellm/proxy/proxy_server/test_proxy_config.py +++ b/tests/test_litellm/proxy/proxy_server/test_proxy_config.py @@ -3036,6 +3036,39 @@ def test_ProxyConfig__parse_router_settings_value_invalid_returns_none(): assert ProxyConfig._parse_router_settings_value({}) is None +def test_ProxyConfig__parse_router_settings_value_strips_null_entries(): + # The admin UI persists the whole router-settings form, blank fields included. + # Nulls must not survive parsing: as per-request kwargs they would shadow the + # Router's own defaults (e.g. fallbacks=None disables the global fallback chain). + ui_form_dump = { + "routing_strategy": "simple-shuffle", + "enable_tag_filtering": False, + "fallbacks": None, + "num_retries": None, + "timeout": None, + "retry_after": None, + "retry_policy": None, + "allowed_fails": None, + "cooldown_time": None, + "model_group_alias": None, + "routing_strategy_args": None, + "context_window_fallbacks": None, + } + assert ProxyConfig._parse_router_settings_value(ui_form_dump) == { + "routing_strategy": "simple-shuffle", + "enable_tag_filtering": False, + } + # Falsy-but-set values are NOT nulls and must be kept. + assert ProxyConfig._parse_router_settings_value({"fallbacks": [], "timeout": 0}) == { + "fallbacks": [], + "timeout": 0, + } + # A settings dict that is *only* nulls carries no opinion -> None. + assert ProxyConfig._parse_router_settings_value({"fallbacks": None, "timeout": None}) is None + # Also for the JSON-string storage shape. + assert ProxyConfig._parse_router_settings_value('{"num_retries": null, "timeout": 30}') == {"timeout": 30} + + # --------------------------------------------------------------------------- # ProxyConfig._get_hierarchical_router_settings # --------------------------------------------------------------------------- diff --git a/tests/test_litellm/proxy/test_route_llm_request.py b/tests/test_litellm/proxy/test_route_llm_request.py index f7021763a4d..39a7b01ab9c 100644 --- a/tests/test_litellm/proxy/test_route_llm_request.py +++ b/tests/test_litellm/proxy/test_route_llm_request.py @@ -474,6 +474,37 @@ async def test_route_request_with_router_settings_override_preserves_existing(): assert call_kwargs["timeout"] == 30 +@pytest.mark.asyncio +async def test_route_request_with_router_settings_override_ignores_null_values(): + """A key/team router_settings entry of null means "not configured" and must not + be forwarded to the Router: `fallbacks=None` in kwargs shadows Router.fallbacks + and silently disables the globally configured fallback chain for the request.""" + data = { + "model": "gpt-3.5-turbo", + "messages": [{"role": "user", "content": "Hello"}], + "router_settings_override": { + "fallbacks": None, # UI form-save of a blank field + "num_retries": None, + "timeout": None, + "routing_strategy": "simple-shuffle", # actually configured + }, + } + + llm_router = MagicMock() + llm_router.acompletion.return_value = "success" + + response = await route_request(data, llm_router, None, "acompletion") + + assert response == "success" + call_kwargs = llm_router.acompletion.call_args[1] + # Null overrides must not appear at all -> Router keeps its own defaults. + assert "fallbacks" not in call_kwargs + assert "num_retries" not in call_kwargs + assert "timeout" not in call_kwargs + # Configured values still merge. + assert call_kwargs["routing_strategy"] == "simple-shuffle" + + def test_gated_mock_params_cover_mock_router_testing_params(): """``GATED_MOCK_PARAM_NAMES`` is hardcoded to avoid a cyclic import against ``litellm.types.router``. This test guards against drift — if a