diff --git a/litellm/proxy/auth/auth_utils.py b/litellm/proxy/auth/auth_utils.py index 87db8ed3b5e..b4e634b5eb1 100644 --- a/litellm/proxy/auth/auth_utils.py +++ b/litellm/proxy/auth/auth_utils.py @@ -20,6 +20,7 @@ validate_url, ) from litellm.proxy._types import * +from litellm.proxy.common_utils.http_parsing_utils import extract_nested_form_metadata from litellm.types.passthrough_endpoints.pass_through_endpoints import ( LITELLM_PASS_THROUGH_ENDPOINT_MARKER, ) @@ -440,6 +441,13 @@ def is_request_body_safe(request_body: dict, general_settings: dict, llm_router: metadata = _coerce_metadata_to_dict(request_body.get(metadata_key)) if metadata is not None: _check_banned_params(metadata, general_settings, llm_router, model) + if any(isinstance(key, str) and key.startswith(f"{metadata_key}[") for key in request_body): + _check_banned_params( + extract_nested_form_metadata(form_data=request_body, prefix=f"{metadata_key}["), + general_settings, + llm_router, + model, + ) for target in iter_request_fallback_targets(request_body): if isinstance(target, dict): _check_banned_params(target, general_settings, llm_router, model) diff --git a/litellm/proxy/common_request_processing.py b/litellm/proxy/common_request_processing.py index 0b9a2d5e4c0..0ea2109166e 100644 --- a/litellm/proxy/common_request_processing.py +++ b/litellm/proxy/common_request_processing.py @@ -67,7 +67,10 @@ ProxyConfig = _ProxyConfig else: ProxyConfig = Any -from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request +from litellm.proxy.litellm_pre_call_utils import ( + add_litellm_data_to_request, + reject_url_valued_destination, +) from litellm.types.utils import ( ModelResponse, ModelResponseStream, @@ -1286,6 +1289,9 @@ async def common_processing_pre_call_logic( self.data[_metadata_variable_name] = {} self.data[_metadata_variable_name]["queue_time_seconds"] = queue_time_seconds + if isinstance(model, str): + reject_url_valued_destination("model", model) + self.data["model"] = ( general_settings.get("completion_model", None) # server default or user_model # model name passed via cli args diff --git a/litellm/proxy/health_endpoints/_health_endpoints.py b/litellm/proxy/health_endpoints/_health_endpoints.py index 521feb26ad4..52b7faeac07 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -5,7 +5,7 @@ import secrets import time import traceback -from collections.abc import Iterable +from collections.abc import Iterable, Mapping from datetime import datetime, timedelta from typing import Any, Final, Literal, TypedDict, cast @@ -29,6 +29,9 @@ UserAPIKeyAuth, WebhookEvent, ) +from litellm.proxy.auth.auth_utils import ( + _BANNED_REQUEST_BODY_PARAMS, # pyright: ignore[reportPrivateUsage] # one canonical list, shared with the request-body check +) from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.db.exception_handler import PrismaDBExceptionHandler from litellm.proxy.health_check import ( @@ -43,6 +46,10 @@ get_in_flight_requests, ) from litellm.proxy.shutdown.graceful_shutdown_manager import GracefulShutdownManager +from litellm.router_utils.clientside_credential_handler import ( + _ADMIN_CONFIG_FIELDS_TO_CLEAR_ON_BASE_OVERRIDE, # pyright: ignore[reportPrivateUsage] # one canonical list, shared with the router path + clientside_credential_keys, +) #### Health ENDPOINTS #### @@ -80,6 +87,45 @@ def _reject_os_environ_references(params: dict) -> None: stack.append(value) +_CONFIG_CONNECTION_FIELDS: Final[frozenset[str]] = frozenset( + ( + *_ADMIN_CONFIG_FIELDS_TO_CLEAR_ON_BASE_OVERRIDE, + *clientside_credential_keys, + "litellm_credential_name", + ) +) + + +def _config_base_for_health_check( + config_params: Mapping[str, object], + request_params: Mapping[str, object], + allow_client_side_credentials: bool = False, +) -> dict[str, object]: + """Return the configured parameters to merge under a connection-test request. + + A request that sets its own connection fields describes a connection of its + own, so the configuration's credentials are not carried into it: they belong + to the endpoint the configuration names. Anything the request does not set + still comes from the configuration, which is what lets a request name a + configured model and test it as configured. + + ``litellm_credential_name`` is dropped alongside the literal credential + fields: it names a stored credential that ``load_credentials_from_list`` + resolves into the same secrets further down the call, so leaving it in place + would reintroduce them by reference. + + ``general_settings.allow_client_side_credentials`` is the existing proxy-wide + opt-in for callers supplying their own connection parameters. Where an admin + has enabled it, a request may pair its own endpoint with the configured + credentials, as it could before. + """ + if allow_client_side_credentials: + return dict(config_params) + if not any(param in request_params for param in _BANNED_REQUEST_BODY_PARAMS): + return dict(config_params) + return {key: value for key, value in config_params.items() if key not in _CONFIG_CONNECTION_FIELDS} + + def get_callback_identifier(callback): """ Get the callback identifier string, handling both strings and objects. @@ -1785,7 +1831,12 @@ async def test_model_connection( from litellm.proxy.management_endpoints.model_management_endpoints import ( ModelManagementAuthChecks, ) - from litellm.proxy.proxy_server import llm_router, premium_user, prisma_client + from litellm.proxy.proxy_server import ( + general_settings, + llm_router, + premium_user, + prisma_client, + ) from litellm.types.router import Deployment, LiteLLM_Params try: @@ -1854,8 +1905,14 @@ async def test_model_connection( ) # Merge: config params (from proxy config) as base, request params override - # This allows users to override specific params while using config for credentials - litellm_params = {**config_litellm_params, **request_litellm_params} + litellm_params = { + **_config_base_for_health_check( + config_litellm_params, + request_litellm_params, + allow_client_side_credentials=general_settings.get("allow_client_side_credentials") is True, + ), + **request_litellm_params, + } ## Auth check auth_model_info: Final = loaded_model_info if loaded_model_info is not None else model_info diff --git a/litellm/proxy/image_endpoints/endpoints.py b/litellm/proxy/image_endpoints/endpoints.py index 587e1aad8f9..24ee1d96a0d 100644 --- a/litellm/proxy/image_endpoints/endpoints.py +++ b/litellm/proxy/image_endpoints/endpoints.py @@ -70,6 +70,7 @@ async def image_generation( user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), model: str | None = None, ): + from litellm.proxy.litellm_pre_call_utils import reject_url_valued_destination from litellm.proxy.proxy_server import ( add_litellm_data_to_request, general_settings, @@ -96,6 +97,9 @@ async def image_generation( proxy_config=proxy_config, ) + if isinstance(model, str): + reject_url_valued_destination("model", model) + data["model"] = ( model or general_settings.get("image_generation_model", None) # server default diff --git a/litellm/proxy/litellm_pre_call_utils.py b/litellm/proxy/litellm_pre_call_utils.py index 5b0916b54c4..b00ba35b14e 100644 --- a/litellm/proxy/litellm_pre_call_utils.py +++ b/litellm/proxy/litellm_pre_call_utils.py @@ -262,29 +262,37 @@ def _reject_url_valued_destinations(data: dict[str, Any]) -> None: are unaffected, while admins can opt specific hosts back in via ``litellm.provider_url_destination_allowed_hosts``. """ - allowed_hosts: Final = getattr(litellm, "provider_url_destination_allowed_hosts", []) or [] for field in _URL_DESTINATION_REQUEST_FIELDS: value = data.get(field) - if not isinstance(value, str): + if isinstance(value, str): + reject_url_valued_destination(field, value) + + +def reject_url_valued_destination(field: str, value: str) -> None: + """Reject a URL-valued destination identifier unless admin-allowlisted. + + Operates on one field/value pair. ``_reject_url_valued_destinations`` applies + it across ``_URL_DESTINATION_REQUEST_FIELDS`` for a request body. + """ + allowed_hosts: Final = getattr(litellm, "provider_url_destination_allowed_hosts", []) or [] + for candidate in provider_url_destination_candidates(value): + if not candidate.lower().startswith(("http://", "https://")): continue - for candidate in provider_url_destination_candidates(value): - if not candidate.lower().startswith(("http://", "https://")): - continue - if is_url_destination_allowed_by_host(candidate, allowed_hosts): - continue - raise HTTPException( - status_code=400, - detail={ - "error": "invalid_request", - "param": field, - "message": ( - f"URL-valued '{field}' is not allowed. Configure custom " - "endpoints with api_base instead, or add the destination " - "host to `provider_url_destination_allowed_hosts` in " - "litellm_settings." - ), - }, - ) + if is_url_destination_allowed_by_host(candidate, allowed_hosts): + continue + raise HTTPException( + status_code=400, + detail={ + "error": "invalid_request", + "param": field, + "message": ( + f"URL-valued '{field}' is not allowed. Configure custom " + "endpoints with api_base instead, or add the destination " + "host to `provider_url_destination_allowed_hosts` in " + "litellm_settings." + ), + }, + ) def _strip_untrusted_request_header_controls( diff --git a/tests/test_litellm/proxy/auth/test_auth_utils.py b/tests/test_litellm/proxy/auth/test_auth_utils.py index 1610d76efb7..78b7e771239 100644 --- a/tests/test_litellm/proxy/auth/test_auth_utils.py +++ b/tests/test_litellm/proxy/auth/test_auth_utils.py @@ -3010,3 +3010,80 @@ def test_reads_tag_rpm_limit_from_metadata(self): def test_returns_none_when_unset(self): key = UserAPIKeyAuth(api_key="sk-123") assert get_key_tag_rpm_limit(key) is None + + +class TestIsRequestBodySafeChecksBracketNotationMetadata: + """Bracket notation is how multipart callers express nested metadata; it is + validated the same way the dict form is.""" + + @pytest.mark.parametrize("metadata_key", ["metadata", "litellm_metadata"]) + def test_bracket_notation_banned_param_is_rejected(self, metadata_key): + with pytest.raises(ValueError, match="langfuse_host"): + is_request_body_safe( + request_body={ + "purpose": "assistants", + f"{metadata_key}[langfuse_host]": "https://example.invalid", + }, + general_settings={}, + llm_router=None, + model="gpt-4", + ) + + def test_bracket_notation_api_base_is_rejected(self): + with pytest.raises(ValueError, match="api_base"): + is_request_body_safe( + request_body={"litellm_metadata[api_base]": "https://example.invalid"}, + general_settings={}, + llm_router=None, + model="gpt-4", + ) + + def test_bracket_notation_allowed_under_proxy_wide_opt_in(self): + assert ( + is_request_body_safe( + request_body={"litellm_metadata[langfuse_host]": "https://byok.example"}, + general_settings={"allow_client_side_credentials": True}, + llm_router=None, + model="gpt-4", + ) + is True + ) + + def test_benign_bracket_notation_metadata_is_allowed(self): + assert ( + is_request_body_safe( + request_body={ + "purpose": "assistants", + "litellm_metadata[spend_logs_metadata][owner]": "john", + "litellm_metadata[tags]": "production", + }, + general_settings={}, + llm_router=None, + model="gpt-4", + ) + is True + ) + + def test_bracket_notation_matches_json_encoding_for_deeper_nesting(self): + """A value nested below the first level is treated the same either way: + the check descends one level into metadata, for both encodings.""" + deep_bracket = { + "litellm_metadata[spend_logs_metadata][langfuse_host]": "https://example.invalid" + } + deep_json = { + "litellm_metadata": {"spend_logs_metadata": {"langfuse_host": "https://example.invalid"}} + } + kwargs = dict(general_settings={}, llm_router=None, model="gpt-4") + assert is_request_body_safe(request_body=deep_bracket, **kwargs) is True + assert is_request_body_safe(request_body=deep_json, **kwargs) is True + + def test_body_without_bracket_keys_is_unaffected(self): + assert ( + is_request_body_safe( + request_body={"model": "gpt-4", "messages": [{"role": "user", "content": "hi"}]}, + general_settings={}, + llm_router=None, + model="gpt-4", + ) + is True + ) 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 917bedcb93f..f74aafd9df1 100644 --- a/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py +++ b/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py @@ -2364,3 +2364,96 @@ def test_clean_endpoint_data_strips_credentials_keeps_routing_fields(): assert "aws_access_key_id" not in cleaned assert cleaned.get("api_base") == "https://example.test/v1" assert cleaned.get("api_version") == "2024-10-21" + + +class TestConfigBaseForHealthCheck: + """A request that sets its own connection fields gets a base without the + configuration's credentials; anything it leaves unset still comes from + the configuration.""" + + CONFIG = { + "model": "openai/gpt-4o", + "api_key": "sk-configured", + "api_base": "https://configured.example/v1", + "vertex_credentials": "configured-creds", + "rpm": 100, + } + + def _base(self, config, request, allow_client_side_credentials=False): + from litellm.proxy.health_endpoints._health_endpoints import ( + _config_base_for_health_check, + ) + + return _config_base_for_health_check( + config, request, allow_client_side_credentials=allow_client_side_credentials + ) + + def test_request_without_connection_fields_inherits_config(self): + base = self._base(self.CONFIG, {"model": "openai/gpt-4o"}) + assert base["api_key"] == "sk-configured" + assert base["api_base"] == "https://configured.example/v1" + + def test_request_setting_api_base_does_not_inherit_config_credentials(self): + base = self._base(self.CONFIG, {"api_base": "https://caller.example/v1"}) + assert "api_key" not in base + assert "api_base" not in base + assert "vertex_credentials" not in base + assert base["rpm"] == 100 + + def test_add_model_flow_keeps_its_own_credentials(self): + """Adding a second deployment for an already-configured name sends a + complete connection; it is tested as sent, not as configured.""" + request = { + "model": "openai/gpt-4o", + "api_base": "https://new-deployment.example/v1", + "api_key": "sk-new-deployment", + } + merged = {**self._base(self.CONFIG, request), **request} + assert merged["api_base"] == "https://new-deployment.example/v1" + assert merged["api_key"] == "sk-new-deployment" + assert "sk-configured" not in str(merged) + + def test_destination_override_without_own_key_inherits_no_credential(self): + """A request that redirects the destination but supplies no credential + of its own gets none from the configuration.""" + request = {"api_base": "https://elsewhere.example"} + merged = {**self._base(self.CONFIG, request), **request} + assert "api_key" not in merged + assert "sk-configured" not in str(merged) + + def test_non_api_base_destination_field_also_drops_credentials(self): + base = self._base( + {**self.CONFIG, "aws_secret_access_key": "configured-secret"}, + {"aws_bedrock_runtime_endpoint": "https://caller.example"}, + ) + assert "api_key" not in base + assert "aws_secret_access_key" not in base + + def test_opt_in_restores_configured_credentials_under_a_request_endpoint(self): + """With general_settings.allow_client_side_credentials enabled, a request + may pair its own endpoint with the configured credentials, as before.""" + base = self._base( + self.CONFIG, + {"api_base": "https://caller.example/v1"}, + allow_client_side_credentials=True, + ) + assert base["api_key"] == "sk-configured" + + def test_stored_credential_reference_is_dropped_with_the_credentials(self): + """A stored-credential name resolves to the same secrets downstream, so a + request that redirects the destination must not keep it either.""" + config = {**self.CONFIG, "litellm_credential_name": "OpenAI-prod"} + base = self._base(config, {"api_base": "https://caller.example/v1"}) + assert "litellm_credential_name" not in base + assert "api_key" not in base + + def test_stored_credential_reference_kept_when_request_sets_no_connection(self): + """The Admin UI tests a configured model by naming it plus its stored + credential and nothing else; that keeps working.""" + config = {**self.CONFIG, "litellm_credential_name": "OpenAI-prod"} + base = self._base( + config, + {"model": "openai/gpt-4o", "litellm_credential_name": "OpenAI-prod", "custom_llm_provider": "openai"}, + ) + assert base["litellm_credential_name"] == "OpenAI-prod" + assert base["api_key"] == "sk-configured" diff --git a/tests/test_litellm/proxy/image_endpoints/test_azure_routes.py b/tests/test_litellm/proxy/image_endpoints/test_azure_routes.py index 16fc6c19505..f5410ef0d70 100644 --- a/tests/test_litellm/proxy/image_endpoints/test_azure_routes.py +++ b/tests/test_litellm/proxy/image_endpoints/test_azure_routes.py @@ -120,3 +120,28 @@ def test_azure_image_edit_route(client_no_auth): assert called_kwargs["prompt"] == "A cute baby sea otter" assert response.status_code == 200 assert response.json()["data"] + + +def test_azure_image_generation_route_rejects_url_valued_path_model(client_no_auth): + """A URL-valued deployment segment is refused before any provider call.""" + client, mock_aimage_generation, _ = client_no_auth + response = client.post( + "/openai/deployments/oobabooga/https://example.invalid/images/generations", + json={"prompt": "A cute baby sea otter", "n": 1, "size": "1024x1024"}, + ) + + assert response.status_code == 400 + assert "URL-valued" in response.text + mock_aimage_generation.assert_not_called() + + +def test_azure_image_generation_route_allows_ordinary_path_model(client_no_auth): + """A deployment name that merely contains a provider prefix still routes.""" + client, mock_aimage_generation, _ = client_no_auth + response = client.post( + "/openai/deployments/dall-e-3/images/generations", + json={"prompt": "A cute baby sea otter", "n": 1, "size": "1024x1024"}, + ) + + assert response.status_code == 200 + mock_aimage_generation.assert_called_once() diff --git a/tests/test_litellm/proxy/test_provider_url_destination_guard.py b/tests/test_litellm/proxy/test_provider_url_destination_guard.py index c8771abbc8e..cd993a076e8 100644 --- a/tests/test_litellm/proxy/test_provider_url_destination_guard.py +++ b/tests/test_litellm/proxy/test_provider_url_destination_guard.py @@ -177,3 +177,16 @@ async def test_add_litellm_data_to_request_rejects_url_valued_model(): ) assert exc_info.value.status_code == 400 assert exc_info.value.detail["param"] == "model" + + +class TestNonStringDestinationValues: + """Only string identifiers are inspected. Anything else is left alone for the + request's normal validation to handle.""" + + @pytest.mark.parametrize("value", [123, None, True, {"a": 1}, ["x"], 1.5]) + def test_non_string_model_is_ignored(self, value): + _reject_url_valued_destinations({"model": value}) + + @pytest.mark.parametrize("value", [123, None, True, {"a": 1}, ["x"]]) + def test_non_string_file_id_is_ignored(self, value): + _reject_url_valued_destinations({"file_id": value})