From ed6bba8d91027459d0323dd33f4b51d058502cb2 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Sun, 31 May 2026 08:35:28 +0000 Subject: [PATCH] chore(proxy): require own credential and SSRF-validate destination overrides on connection test for non-admins The connection-test guard only blocked credentials inherited from a resolved deployment, but an ad-hoc OpenAI-compatible model with an api_base override and no api_key still triggers the provider's OPENAI_API_KEY environment fallback, sending the proxy's key to the caller-chosen address. A non-admin overriding the connection target (api_base/base_url/provider endpoint) must now supply their own non-empty credential, and the override URL is SSRF-validated under the user_url_validation toggle so it cannot point at an internal or cloud-metadata host. Proxy admins are unaffected. Replaces the inherited-only redirect guard; adds tests for the missing-credential rejection (helper and end-to-end ad-hoc), admin exemption, and the SSRF block on a metadata IP. --- .../health_endpoints/_health_endpoints.py | 68 +++++++++----- .../health_endpoints/test_health_endpoints.py | 89 ++++++++++++++----- 2 files changed, 114 insertions(+), 43 deletions(-) diff --git a/litellm/proxy/health_endpoints/_health_endpoints.py b/litellm/proxy/health_endpoints/_health_endpoints.py index d29b9acea5f..b94bee8d312 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -95,33 +95,60 @@ _HEALTH_DESTINATION_FIELDS = ( "vertex_project", "aws_region_name", ) +# Caller-controlled endpoint URLs: overriding one re-points the request (and any +# credential) at a new host, so it is SSRF-validated. +_HEALTH_URL_FIELDS = ( + "api_base", + "base_url", + "aws_bedrock_runtime_endpoint", + "aws_sts_endpoint", +) -def _reject_inherited_credential_redirect( - config_litellm_params: dict, request_litellm_params: dict +def _assert_non_admin_destination_override_is_safe( + request_litellm_params: dict, user_api_key_dict: UserAPIKeyAuth ) -> None: - """Confused-deputy guard for /health/test_connection. + """Confused-deputy / SSRF guard for /health/test_connection. - If a credential is inherited from the resolved deployment (present in the - config params, not supplied by the request) while the request overrides the - connection target, the inherited secret would be sent to a caller-chosen - endpoint. Refuse it; the caller must supply their own credential or omit the - destination override. + When a non-admin overrides the connection target (api_base/base_url/provider + endpoint), the request must carry its own non-empty credential. Otherwise the + downstream provider falls back to a stored or environment secret (e.g. the + resolved deployment's key, or OPENAI_API_KEY) and sends it to the + caller-chosen address. The overridden URL is also SSRF-validated so it cannot + point at an internal host or cloud-metadata endpoint. Proxy admins are + unaffected. """ - inherited_credential = any( - config_litellm_params.get(field) and not request_litellm_params.get(field) - for field in _HEALTH_CREDENTIAL_FIELDS - ) - overrides_destination = any( - request_litellm_params.get(field) for field in _HEALTH_DESTINATION_FIELDS - ) - if inherited_credential and overrides_destination: + from litellm.litellm_core_utils.url_utils import SSRFError, validate_url + from litellm.proxy.common_utils.resource_ownership import is_proxy_admin + + if is_proxy_admin(user_api_key_dict): + return + if not any(request_litellm_params.get(field) for field in _HEALTH_URL_FIELDS): + return + if not any( + request_litellm_params.get(field) for field in _HEALTH_CREDENTIAL_FIELDS + ): raise HTTPException( status_code=400, detail={ - "error": "Cannot override the connection target (e.g. api_base) while inheriting stored credentials. Supply your own api_key, or omit the destination override." + "error": "Supply your own api_key when overriding the connection target (e.g. api_base); otherwise a stored or environment credential could be sent to it." }, ) + if not getattr(litellm, "user_url_validation", False): + return + for field in _HEALTH_URL_FIELDS: + url = request_litellm_params.get(field) + if not url or not isinstance(url, str): + continue + try: + validate_url(url) + except SSRFError as e: + raise HTTPException( + status_code=400, + detail={ + "error": f"{field} is rejected by the SSRF guard ({e}). Add the host to general_settings.user_url_allowed_hosts to allow it." + }, + ) def get_callback_identifier(callback): @@ -1896,10 +1923,11 @@ async def test_model_connection( # noqa: PLR0915 # This allows users to override specific params while using config for credentials litellm_params = {**config_litellm_params, **request_litellm_params} - # Refuse sending an inherited credential to a caller-overridden destination. - _reject_inherited_credential_redirect( - config_litellm_params=config_litellm_params, + # A non-admin overriding the destination must bring their own credential + # and a non-internal URL, so no stored/environment secret is sent out. + _assert_non_admin_destination_override_is_safe( request_litellm_params=request_litellm_params, + user_api_key_dict=user_api_key_dict, ) ## Auth check — when the deployment was resolved by id, authorize against 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 2c9aae51b69..915719f82fe 100644 --- a/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py +++ b/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py @@ -1955,38 +1955,81 @@ async def test_test_connection_rejects_inherited_key_with_api_base_override(): mock_ahealth.assert_not_called() # the inherited key never went out -def test_reject_inherited_credential_redirect_helper(): +def test_non_admin_destination_override_guard(): + import litellm from fastapi import HTTPException + from litellm.proxy._types import LitellmUserRoles from litellm.proxy.health_endpoints._health_endpoints import ( - _reject_inherited_credential_redirect, + _assert_non_admin_destination_override_is_safe, ) - # inherited credential + overridden destination -> rejected + non_admin = MagicMock(user_role="internal_user") + admin = MagicMock(user_role=LitellmUserRoles.PROXY_ADMIN) + + # Non-admin overrides api_base without supplying a credential -> rejected + # (a stored or environment key would otherwise be sent there). with pytest.raises(HTTPException): - _reject_inherited_credential_redirect( - config_litellm_params={"api_key": "sk-x", "api_base": "https://real"}, - request_litellm_params={"api_base": "https://attacker"}, + _assert_non_admin_destination_override_is_safe( + {"api_base": "https://attacker.example"}, non_admin ) - # inherited stored-credential-NAME (no inline api_key) + override -> also rejected - with pytest.raises(HTTPException): - _reject_inherited_credential_redirect( - config_litellm_params={ - "litellm_credential_name": "openai-cred", - "api_base": "https://real", - }, - request_litellm_params={"api_base": "https://attacker"}, + with patch.object(litellm, "user_url_validation", False): + # Non-admin overrides WITH their own credential -> allowed. + _assert_non_admin_destination_override_is_safe( + {"api_base": "https://attacker.example", "api_key": "sk-own"}, non_admin ) - # request supplies its own credential -> allowed - _reject_inherited_credential_redirect( - config_litellm_params={"api_key": "sk-x"}, - request_litellm_params={"api_key": "sk-own", "api_base": "https://attacker"}, - ) - # no destination override -> allowed - _reject_inherited_credential_redirect( - config_litellm_params={"api_key": "sk-x"}, - request_litellm_params={"model": "gpt-4o"}, + # No destination override -> allowed. + _assert_non_admin_destination_override_is_safe({"model": "gpt-4o"}, non_admin) + # Proxy admin is exempt even when overriding without a credential. + _assert_non_admin_destination_override_is_safe( + {"api_base": "https://attacker.example"}, admin ) + # Non-admin override to an internal/metadata IP -> SSRF-rejected. + with patch.object(litellm, "user_url_validation", True): + with pytest.raises(HTTPException): + _assert_non_admin_destination_override_is_safe( + { + "api_base": "http://169.254.169.254/latest/meta-data/", + "api_key": "sk-own", + }, + non_admin, + ) + + +@pytest.mark.asyncio +async def test_test_connection_rejects_adhoc_override_without_credential(): + """An ad-hoc OpenAI-compatible model (no resolved deployment) with an api_base + override and no api_key must be refused: the provider would otherwise fall + back to the proxy's OPENAI_API_KEY and send it to the override.""" + from fastapi import HTTPException + + mock_router = MagicMock() + mock_router.get_deployment.return_value = None + mock_router.get_model_list.return_value = [] + mock_auth = AsyncMock(return_value=True) + 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) + with pytest.raises(HTTPException) as exc_info: + await health_test_model_connection( + request=MagicMock(), + mode="chat", + litellm_params={ + "model": "openai/gpt-4o", + "api_base": "https://attacker.example/v1", + }, + model_info={"team_id": "team-ATTACKER"}, + user_api_key_dict=MagicMock( + user_id="attacker", token="t", user_role="internal_user" + ), + ) + assert exc_info.value.status_code == 400 + + mock_ahealth.assert_not_called() # OPENAI_API_KEY never went out @pytest.mark.asyncio