diff --git a/litellm/proxy/health_endpoints/_health_endpoints.py b/litellm/proxy/health_endpoints/_health_endpoints.py index 6b35f41deb5..6282d3894f7 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -1881,6 +1881,11 @@ async def test_model_connection( # noqa: PLR0915 config_litellm_params = dict( deployments[0].get("litellm_params", {}) ) + # Authorize against the matched deployment's owner, not + # the caller-supplied model_info (same as the id path). + resolved_model_info = dict( + deployments[0].get("model_info", {}) or {} + ) except Exception as e: verbose_proxy_logger.debug( f"Could not find model {model_name} in router: {e}. " diff --git a/litellm/proxy/management_endpoints/model_management_endpoints.py b/litellm/proxy/management_endpoints/model_management_endpoints.py index ccb49bf7fcd..713d386de5b 100644 --- a/litellm/proxy/management_endpoints/model_management_endpoints.py +++ b/litellm/proxy/management_endpoints/model_management_endpoints.py @@ -135,19 +135,19 @@ def _strip_credentials_on_destination_change( patch_plaintext: dict, db_plaintext: "Callable[[str], object]", ) -> None: - """If the patch changes a destination field (api_base/base_url/custom_llm_provider) - without supplying a fresh credential, drop the inherited secret(s) from the - merged params so a stored credential is never silently re-pointed at a new - (possibly attacker-controlled) endpoint.""" + """When a destination field (api_base/base_url/provider endpoint) changes, + drop every INHERITED credential — one the patch did not itself supply — so a + stored secret is never silently re-pointed at a new (possibly attacker- + controlled) endpoint. Cleared per field: supplying one credential must not + preserve the others.""" destination_changed = any( field in patch_plaintext and patch_plaintext[field] != db_plaintext(field) for field in _DESTINATION_LITELLM_PARAMS ) - supplied_new_credential = any( - field in patch_plaintext for field in _CREDENTIAL_LITELLM_PARAMS - ) - if destination_changed and not supplied_new_credential: - for field in _CREDENTIAL_LITELLM_PARAMS: + if not destination_changed: + return + for field in _CREDENTIAL_LITELLM_PARAMS: + if field not in patch_plaintext: merged_litellm_params.pop(field, None) diff --git a/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py b/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py index 69189522f68..2c9aae51b69 100644 --- a/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py +++ b/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py @@ -1987,3 +1987,45 @@ def test_reject_inherited_credential_redirect_helper(): config_litellm_params={"api_key": "sk-x"}, request_litellm_params={"model": "gpt-4o"}, ) + + +@pytest.mark.asyncio +async def test_test_connection_model_name_path_authorizes_against_resolved_owner(): + """When the deployment is resolved by model_name (no id), auth must still run + against the resolved deployment's owner, not the caller-supplied model_info.""" + victim_deployment = { + "model_name": "victim-model", + "litellm_params": { + "model": "azure/gpt-4o", + "api_key": "sk-victim", + "api_base": "https://victim.example/v1", + }, + "model_info": {"id": "victim-id", "team_id": "team-OWNER"}, + } + mock_router = MagicMock() + mock_router.get_deployment.return_value = None # no id resolution + mock_router.get_model_list.return_value = [victim_deployment] + + captured = {} + + async def _capture(*, model_params, **kwargs): + captured["team_id"] = model_params.model_info.team_id + return True + + mock_auth = AsyncMock(side_effect=_capture) + mock_ahealth = AsyncMock(return_value={"status": "healthy"}) + + with contextlib.ExitStack() as stack: + for p in _health_test_connection_patches( + MagicMock(), mock_router, mock_auth, mock_ahealth + ): + stack.enter_context(p) + await health_test_model_connection( + request=MagicMock(), + mode="chat", + litellm_params={"model": "victim-model"}, # resolved by name, no id + model_info={"team_id": "team-ATTACKER"}, + user_api_key_dict=MagicMock(user_id="attacker", token="t"), + ) + + assert captured.get("team_id") == "team-OWNER" diff --git a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py index f3d3a8a8b9b..867002d6192 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_model_management_endpoints.py @@ -2480,3 +2480,26 @@ class TestModelMgmtAuthzHardening: model_info=None, user_api_key_dict=self._admin(), ) + + def test_destination_change_clears_inherited_credential_per_field(self): + from litellm.proxy.management_endpoints.model_management_endpoints import ( + _strip_credentials_on_destination_change, + ) + + # Patch redirects api_base AND supplies an UNRELATED credential + # (aws_secret_access_key); the inherited api_key must still be dropped. + merged = { + "api_key": "sk-inherited", + "aws_secret_access_key": "supplied", + "api_base": "https://attacker.example", + } + patch_plaintext = { + "api_base": "https://attacker.example", + "aws_secret_access_key": "supplied", + } + db = {"api_base": "https://real.example", "api_key": "sk-inherited"} + _strip_credentials_on_destination_change( + merged, patch_plaintext, lambda f: db.get(f) + ) + assert "api_key" not in merged # inherited, not re-supplied -> dropped + assert merged.get("aws_secret_access_key") == "supplied" # supplied -> kept