diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 10c508fccaa..85e9bdd05e6 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -14621,13 +14621,6 @@ def _apply_webhook_role_gate(webhook_map, is_full_admin: bool): def _apply_router_settings_role_gate(router_settings: dict, is_full_admin: bool) -> dict: - """ - ``default_litellm_params`` is an open-ended kwargs dict (Router merges it into - every completion call), so an admin can put a shared ``api_key`` or an - ``extra_headers`` Authorization token there. Unlike the other fields on - this page, nothing else masks it before it reaches non-full-admin callers - of /get/config/callbacks. - """ default_litellm_params = router_settings.get("default_litellm_params") if is_full_admin or not isinstance(default_litellm_params, dict): return router_settings diff --git a/litellm/router.py b/litellm/router.py index 58427a1797f..18f7f79f965 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -9706,23 +9706,11 @@ class Router: _settings_to_return["routing_groups"] = [group.model_dump() for group in self._routing_groups.values()] return _settings_to_return - def _merge_default_litellm_params_setting(self, value: dict | None) -> None: + def _replace_default_litellm_params_setting(self, value: dict | None) -> None: if value is not None: - self.default_litellm_params = {**self.default_litellm_params, **value} + self.default_litellm_params = value def _remove_optional_pre_call_checks(self, removed_checks: OptionalPreCallChecks) -> list[str]: - """ - Reverse of `add_optional_pre_call_checks` for the subset of checks that can be - safely turned off at runtime: clears the corresponding flag(s) on shared - affinity callbacks, or unregisters the dedicated callback instance entirely. - - Returns the subset of `removed_checks` that were kept active anyway because - they're still required by config (currently only `router_budget_limiting`, - which `Router.__init__` auto-enables whenever budgets are configured on the - deployments/provider, independent of `optional_pre_call_checks`) - callers - should fold these back into the tracked `optional_pre_call_checks` list so it - doesn't claim a check is off when it's actually still enforced. - """ from litellm.router_utils.pre_call_checks.encrypted_content_affinity_check import ( EncryptedContentAffinityCheck, ) @@ -9762,9 +9750,6 @@ class Router: if check == "router_budget_limiting" and RouterBudgetLimiting.should_init_router_budget_limiter( model_list=self.model_list, provider_budget_config=self.provider_budget_config ): - # Budgets are still configured on the deployments/provider - Router.__init__ - # would auto-enable this regardless of optional_pre_call_checks, so a save - # that omits it must not silently disable budget enforcement. retained_checks.append(check) continue litellm.logging_callback_manager.remove_callbacks_by_type(optional_callbacks, callback_type) @@ -9783,12 +9768,8 @@ class Router: retained_checks = self._remove_optional_pre_call_checks(removed_checks) if removed_checks else [] self.optional_pre_call_checks = list(dict.fromkeys([*value, *retained_checks])) - # Settings whose update logic doesn't fit `setattr(self, var, value)` (e.g. - # merge-not-replace, or side effects beyond storing the value). Dispatched via - # a single `var in ...` branch in update_settings so adding an entry here - # doesn't grow that function's branch count per field. _CUSTOM_UPDATE_SETTINGS_HANDLERS: dict = { - "default_litellm_params": _merge_default_litellm_params_setting, + "default_litellm_params": _replace_default_litellm_params_setting, "optional_pre_call_checks": _apply_optional_pre_call_checks_setting, } diff --git a/litellm/types/management_endpoints/router_settings_endpoints.py b/litellm/types/management_endpoints/router_settings_endpoints.py index 72d1cd2f30d..fb3d6e10fcf 100644 --- a/litellm/types/management_endpoints/router_settings_endpoints.py +++ b/litellm/types/management_endpoints/router_settings_endpoints.py @@ -243,9 +243,6 @@ ROUTER_SETTINGS_FIELDS: List[RouterSettingsField] = [ "that cached the prompt." ), field_default=[], - # "forward_client_headers_by_model_group" is excluded: it's a literal in the - # OptionalPreCallChecks type union, but add_optional_pre_call_checks has no - # handler for it, so selecting it from the UI would silently do nothing. options=[ "prompt_caching", "router_budget_limiting", diff --git a/tests/code_coverage_tests/router_code_coverage.py b/tests/code_coverage_tests/router_code_coverage.py index 9ee210c0bb2..fcd1ca2f392 100644 --- a/tests/code_coverage_tests/router_code_coverage.py +++ b/tests/code_coverage_tests/router_code_coverage.py @@ -80,7 +80,7 @@ ignored_function_names = [ "_override_vector_store_methods_for_router", # No-op placeholder, called during Router init "_merge_tools_from_deployment", # Tested indirectly via _update_kwargs_with_deployment (test files lack "router" in name) "_invalidate_access_groups_cache", # Tested indirectly via set_model_list, upsert_model etc. (test files lack "router" in name) - "_merge_default_litellm_params_setting", # Tested indirectly via update_settings(default_litellm_params=...) + "_replace_default_litellm_params_setting", # Tested indirectly via update_settings(default_litellm_params=...) "_apply_optional_pre_call_checks_setting", # Tested indirectly via update_settings(optional_pre_call_checks=...) "_remove_optional_pre_call_checks", # Tested indirectly via update_settings(optional_pre_call_checks=...) ] diff --git a/tests/test_litellm/test_router.py b/tests/test_litellm/test_router.py index fcd2725c185..57a4bca46fe 100644 --- a/tests/test_litellm/test_router.py +++ b/tests/test_litellm/test_router.py @@ -5065,19 +5065,15 @@ def _make_router_for_settings_tests(**kwargs): ) -def test_update_settings_merges_default_litellm_params_without_dropping_existing_keys(): - """ - Regression test: `update_settings(default_litellm_params=...)` must merge into - the existing dict, not replace it wholesale. A naive `setattr` replace would - silently drop keys the Router set at init (e.g. `timeout`, `max_retries`, - `metadata`) whenever an admin edits `default_litellm_params` from the UI to - add something like `cache_control_injection_points`. - """ +def test_update_settings_replaces_default_litellm_params_wholesale(): router = _make_router_for_settings_tests(timeout=42) assert router.default_litellm_params.get("timeout") == 42 router.update_settings( - default_litellm_params={"cache_control_injection_points": [{"location": "message", "role": "system"}]} + default_litellm_params={ + "timeout": 42, + "cache_control_injection_points": [{"location": "message", "role": "system"}], + } ) assert router.default_litellm_params["timeout"] == 42 @@ -5086,6 +5082,25 @@ def test_update_settings_merges_default_litellm_params_without_dropping_existing ] +def test_update_settings_can_clear_default_litellm_params_keys(): + """ + Regression test: a prior merge-not-replace implementation could never remove a + key - `{**old, **new}` keeps any key present in `old` but absent from `new`. + Saving `default_litellm_params` after removing `cache_control_injection_points` + from the Admin UI's textarea must actually clear it, not silently keep the old + callback-affecting value active. + """ + router = _make_router_for_settings_tests() + router.update_settings( + default_litellm_params={"cache_control_injection_points": [{"location": "message", "role": "system"}]} + ) + assert "cache_control_injection_points" in router.default_litellm_params + + router.update_settings(default_litellm_params={}) + + assert "cache_control_injection_points" not in router.default_litellm_params + + def test_update_settings_optional_pre_call_checks_is_idempotent(): """ Regression test: `add_optional_pre_call_checks` has no built-in guard against diff --git a/ui/litellm-dashboard/src/components/router_settings/index.test.tsx b/ui/litellm-dashboard/src/components/router_settings/index.test.tsx index 536548a95b9..e155fbddf84 100644 --- a/ui/litellm-dashboard/src/components/router_settings/index.test.tsx +++ b/ui/litellm-dashboard/src/components/router_settings/index.test.tsx @@ -207,11 +207,6 @@ describe("RouterSettings", () => { }); it("should round-trip default_litellm_params and optional_pre_call_checks unmodified on save", async () => { - // Regression test: default_litellm_params holds a dict (e.g. cache_control_injection_points), - // not a plain string - without listing it in the save handler's jsonKeys set, it'd be persisted - // as raw stringified text instead of parsed JSON. optional_pre_call_checks is a list owned by - // its own multi-select (no DOM input); without the fallback-to-state path in parseInputValue, - // an untouched value would be silently dropped instead of round-tripping through Save. vi.mocked(getCallbacksCall).mockResolvedValue({ router_settings: { ...mockCallbacksResponse.router_settings,