diff --git a/litellm/proxy/_experimental/mcp_server/tool_registry.py b/litellm/proxy/_experimental/mcp_server/tool_registry.py index 58570aafadf..ec227a482bc 100644 --- a/litellm/proxy/_experimental/mcp_server/tool_registry.py +++ b/litellm/proxy/_experimental/mcp_server/tool_registry.py @@ -76,13 +76,20 @@ class MCPToolRegistry: ] def load_tools_from_config( - self, mcp_tools_config: Optional[Dict[str, Any]] = None + self, + mcp_tools_config: Optional[Dict[str, Any]] = None, + config_file_path: Optional[str] = None, ) -> None: """ Load and register tools from the proxy config Args: mcp_tools_config: The mcp_tools config from the proxy config + config_file_path: Path to the operator's config.yaml. Threaded + through to ``get_instance_fn`` so an ``s3://``/``gcs://`` + ``handler`` declared in the YAML resolves; callers from a + non-YAML path must leave this ``None`` so the runtime gate + fires. """ if mcp_tools_config is None: raise ValueError( @@ -105,7 +112,7 @@ class MCPToolRegistry: # First check if it's a module path (e.g., "module.submodule.function") if handler_name is None: raise ValueError(f"handler is required for tool {name}") - handler = get_instance_fn(handler_name) + handler = get_instance_fn(handler_name, config_file_path) if handler is None: verbose_logger.warning( diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index c6653a722d6..b95dbac95dd 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -4440,6 +4440,14 @@ class LiteLLM_JWTAuth(LiteLLMPydanticObjectBase): ######################################################### def __init__(self, **kwargs: Any) -> None: + # ``config_file_path`` is a non-field kwarg threaded by the + # startup-load path so an operator-configured + # ``custom_validate: s3://bucket/module.fn`` resolves through + # the documented config-file flow. Pop before the invalid-keys + # check; the runtime gate in ``get_instance_fn`` refuses + # ``s3://`` / ``gcs://`` when this is None. + config_file_path = kwargs.pop("config_file_path", None) + # get the attribute names for this Pydantic model allowed_keys = LiteLLM_JWTAuth.__annotations__.keys() @@ -4453,7 +4461,7 @@ class LiteLLM_JWTAuth(LiteLLMPydanticObjectBase): custom_validate = kwargs.get("custom_validate") if custom_validate is not None: - fn = get_instance_fn(custom_validate) + fn = get_instance_fn(custom_validate, config_file_path=config_file_path) validate_custom_validate_return_type(fn) kwargs["custom_validate"] = fn diff --git a/litellm/proxy/auth/auth_utils.py b/litellm/proxy/auth/auth_utils.py index 567b8307af2..8012a9be768 100644 --- a/litellm/proxy/auth/auth_utils.py +++ b/litellm/proxy/auth/auth_utils.py @@ -169,9 +169,13 @@ def _allow_model_level_clientside_configurable_parameters( # Config dicts whose entries are spread as ``**dict`` into outbound LLM # API calls. ``litellm_embedding_config`` is consumed by the Milvus -# vector store transformer; future nested-config keys with the same -# threat shape should be added here. -_NESTED_CONFIG_KEYS: Tuple[str, ...] = ("litellm_embedding_config",) +# vector store transformer. ``extra_body`` is the OpenAI-SDK passthrough +# container: provider modules pull provider-auth fields out of it +# (e.g. Azure's ``extra_body.azure_ad_token``, Bedrock's +# ``extra_body.aws_web_identity_token``) without re-validating, so the +# banned-key check has to descend into it the same way it descends into +# ``litellm_embedding_config``. +_NESTED_CONFIG_KEYS: Tuple[str, ...] = ("litellm_embedding_config", "extra_body") # Metadata containers that carry per-request configuration consumed by the # observability callbacks. The same banned-param list applies — a value @@ -246,6 +250,13 @@ _BANNED_REQUEST_BODY_PARAMS: Tuple[str, ...] = ( "aws_web_identity_token", "aws_role_name", "vertex_credentials", + # Azure managed-identity / federated-auth token. The Azure provider + # transformer reads ``azure_ad_token`` (top-level or via + # ``extra_body``) and resolves it through ``get_secret`` before + # passing it as the bearer token to the Azure endpoint, so a + # caller-supplied value is the same exfil shape as + # ``aws_web_identity_token`` on the Bedrock path. + "azure_ad_token", # Endpoint-targeting fields that retarget the outbound request or # an observability callback. An attacker-controlled value either # exfiltrates the request payload (incl. messages + admin-set @@ -341,8 +352,8 @@ def is_request_body_safe( """ _check_banned_params(request_body, general_settings, llm_router, model) for nested_key in _NESTED_CONFIG_KEYS: - nested = request_body.get(nested_key) - if isinstance(nested, dict): + nested = _coerce_metadata_to_dict(request_body.get(nested_key)) + if nested is not None: _check_banned_params(nested, general_settings, llm_router, model) for metadata_key in _NESTED_METADATA_KEYS: metadata = _coerce_metadata_to_dict(request_body.get(metadata_key)) diff --git a/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py b/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py index cc6c26fdf90..df52c0fe204 100644 --- a/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py +++ b/litellm/proxy/pass_through_endpoints/pass_through_endpoints.py @@ -1236,6 +1236,7 @@ def create_pass_through_route( query_params: Optional[dict] = None, default_query_params: Optional[dict] = None, guardrails: Optional[Dict[str, Any]] = None, + config_file_path: Optional[str] = None, ): # check if target is an adapter.py or a url from litellm._uuid import uuid @@ -1245,7 +1246,7 @@ def create_pass_through_route( if isinstance(target, CustomLogger): adapter = target else: - adapter = get_instance_fn(value=target) + adapter = get_instance_fn(value=target, config_file_path=config_file_path) adapter_id = str(uuid.uuid4()) litellm.adapters = [{"id": adapter_id, "adapter": adapter}] @@ -2019,6 +2020,7 @@ class InitPassThroughEndpointHelpers: guardrails: Optional[dict] = None, methods: Optional[List[str]] = None, default_query_params: Optional[dict] = None, + config_file_path: Optional[str] = None, ): """Add exact path route for pass-through endpoint""" # Default to all methods if none specified (backward compatibility) @@ -2058,6 +2060,7 @@ class InitPassThroughEndpointHelpers: cost_per_request=cost_per_request, default_query_params=default_query_params, guardrails=guardrails, + config_file_path=config_file_path, ), methods=methods, dependencies=dependencies, @@ -2095,6 +2098,7 @@ class InitPassThroughEndpointHelpers: guardrails: Optional[dict] = None, methods: Optional[List[str]] = None, default_query_params: Optional[dict] = None, + config_file_path: Optional[str] = None, ): """Add wildcard route for sub-paths""" # Default to all methods if none specified (backward compatibility) @@ -2135,6 +2139,7 @@ class InitPassThroughEndpointHelpers: cost_per_request=cost_per_request, default_query_params=default_query_params, guardrails=guardrails, + config_file_path=config_file_path, ), methods=methods, dependencies=dependencies, @@ -2298,6 +2303,7 @@ async def _register_pass_through_endpoint( app: FastAPI, premium_user: bool, visited_endpoints: set[str], + config_file_path: Optional[str] = None, ) -> None: endpoint_data: Dict[str, Any] if isinstance(endpoint, PassThroughGenericEndpoint): @@ -2324,10 +2330,10 @@ async def _register_pass_through_endpoint( dependencies = None if auth is not None and str(auth).lower() == "true": - # Authentication on a pass-through endpoint used to be enterprise-only. - # That left OSS with no safe configuration: auth=True raised at startup - # unless the operator had a license. The safe option must always be free, - # and unauthenticated forwarding should require explicit opt-in. + # Authentication on a pass-through endpoint used to be enterprise-only. + # That left OSS with no safe configuration: auth=True raised at startup + # unless the operator had a license. The safe option must always be free, + # and unauthenticated forwarding should require explicit opt-in. dependencies = [Depends(user_api_key_auth)] if path not in LiteLLMRoutes.openai_routes.value: LiteLLMRoutes.openai_routes.value.append(path) @@ -2355,6 +2361,7 @@ async def _register_pass_through_endpoint( guardrails=guardrails, methods=methods, default_query_params=default_query_params, + config_file_path=config_file_path, ) methods_for_key = methods if methods else ["GET", "POST", "PUT", "DELETE", "PATCH"] @@ -2379,6 +2386,7 @@ async def _register_pass_through_endpoint( guardrails=guardrails, methods=methods, default_query_params=default_query_params, + config_file_path=config_file_path, ) visited_endpoints.add(f"{endpoint_id}:subpath:{path}:{methods_str}") @@ -2389,6 +2397,7 @@ async def _register_pass_through_endpoint( async def initialize_pass_through_endpoints( pass_through_endpoints: Union[List[Dict], List[PassThroughGenericEndpoint]], + config_file_path: Optional[str] = None, ): """ 1. Create a global list of pass-through endpoints (db + config) @@ -2399,6 +2408,12 @@ async def initialize_pass_through_endpoints( Args: pass_through_endpoints: List of pass-through endpoints to initialize + config_file_path: Path to the operator's config.yaml when this call + originates from a YAML-load. Threaded through to + ``create_pass_through_route`` so an operator using + ``s3://``/``gcs://`` ``custom_handler`` in their config still + loads. Callers from the DB-overlay / runtime API path must leave + this ``None`` so the runtime gate in ``get_instance_fn`` fires. Returns: None @@ -2438,6 +2453,7 @@ async def initialize_pass_through_endpoints( app=app, premium_user=premium_user, visited_endpoints=visited_endpoints, + config_file_path=config_file_path, ) # remove the ones that are not visited from the list diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index a5905765c6e..39d4f34f086 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -3087,6 +3087,168 @@ class StreamingCallbackError(Exception): pass +# Fields in ``litellm_settings`` / ``general_settings`` whose values flow +# into ``get_instance_fn`` during config load. Remote-URL values +# (``s3://`` / ``gcs://``) are scrubbed from these when the value +# originates from a DB-overlay merge: at the point ``get_instance_fn`` +# is invoked, ``config_file_path`` is non-None (the YAML load chain is +# active), so the runtime gate cannot distinguish a YAML-sourced value +# from a DB-sourced value. Scrubbing at the merge boundary closes that +# gap without tracking source on every config dict entry. +_DB_OVERLAY_REMOTE_MODULE_STR_FIELDS: Dict[str, Tuple[str, ...]] = { + "litellm_settings": ("post_call_rules",), + "general_settings": ( + "custom_auth", + "custom_key_generate", + "custom_key_update", + "custom_sso", + "custom_ui_sso_sign_in_handler", + ), +} +_DB_OVERLAY_REMOTE_MODULE_LIST_FIELDS: Dict[str, Tuple[str, ...]] = { + "litellm_settings": ( + "callbacks", + "success_callback", + "failure_callback", + "audit_log_callbacks", + ), +} + + +def _is_remote_module_url(value: Any) -> bool: + return isinstance(value, str) and ( + value.startswith("s3://") or value.startswith("gcs://") + ) + + +def _scrub_guardrail_inner(inner: Dict[str, Any]) -> None: + """Strip remote-URL entries from a guardrail's ``callbacks`` list + and ``guardrail`` (v2 module-path) field. Mutates in place.""" + cbs = inner.get("callbacks") + if isinstance(cbs, list): + cleaned = [c for c in cbs if not _is_remote_module_url(c)] + if len(cleaned) != len(cbs): + verbose_proxy_logger.warning( + "Refused %d remote-URL entries from DB-overlay " + "litellm_settings.guardrails[...].callbacks", + len(cbs) - len(cleaned), + ) + inner["callbacks"] = cleaned + if _is_remote_module_url(inner.get("guardrail")): + verbose_proxy_logger.warning( + "Refused remote-URL guardrail module from DB-overlay " + "litellm_settings.guardrails[...].guardrail: %r", + inner.get("guardrail"), + ) + inner["guardrail"] = None + + +def _scrub_db_overlay_remote_module_loads(section: str, db_value: Any) -> Any: + """Strip ``s3://`` / ``gcs://`` entries from the DB-overlay value for + fields whose contents reach ``get_instance_fn``. The same scheme is + allowed from a YAML config (the documented operator flow) but a + DB-overlay write would otherwise smuggle the same payload through + the YAML-load chain and reach ``_load_instance_from_remote_storage``.""" + if not isinstance(db_value, dict): + return db_value + str_fields = _DB_OVERLAY_REMOTE_MODULE_STR_FIELDS.get(section, ()) + list_fields = _DB_OVERLAY_REMOTE_MODULE_LIST_FIELDS.get(section, ()) + if not str_fields and not list_fields and section != "general_settings": + return db_value + sanitized = copy.deepcopy(db_value) + for field in str_fields: + v = sanitized.get(field) + if _is_remote_module_url(v): + verbose_proxy_logger.warning( + "Refused remote-URL value for DB-overlay %s.%s=%r; only " + "config.yaml entries may reference s3:// / gcs:// modules.", + section, + field, + v, + ) + sanitized[field] = None + for field in list_fields: + v = sanitized.get(field) + if isinstance(v, list): + cleaned = [item for item in v if not _is_remote_module_url(item)] + if len(cleaned) != len(v): + verbose_proxy_logger.warning( + "Refused %d remote-URL entries from DB-overlay %s.%s; " + "only config.yaml entries may reference s3:// / gcs:// " + "modules.", + len(v) - len(cleaned), + section, + field, + ) + sanitized[field] = cleaned + # ``custom_provider_map`` is a list of dicts with ``custom_handler`` — + # walk it explicitly. + if section == "litellm_settings": + cpm = sanitized.get("custom_provider_map") + if isinstance(cpm, list): + for item in cpm: + if isinstance(item, dict) and _is_remote_module_url( + item.get("custom_handler") + ): + verbose_proxy_logger.warning( + "Refused remote-URL custom_handler from DB-overlay " + "litellm_settings.custom_provider_map: %r", + item.get("custom_handler"), + ) + item["custom_handler"] = None + # ``litellm_settings.guardrails`` is a list of single-key dicts in + # v1 ({guardrail_name: {callbacks: [...], default_on: bool}}) or a + # list of v2 entries ({guardrail_name, litellm_params: {guardrail: + # "module.path", callbacks: [...]}}). Both shapes terminate in + # ``callbacks`` (a list) or ``guardrail`` (a single dotted name) + # that flow into ``get_instance_fn`` during config load. + if section == "litellm_settings": + guardrails = sanitized.get("guardrails") + if isinstance(guardrails, list): + for entry in guardrails: + if not isinstance(entry, dict): + continue + for inner in entry.values(): + if not isinstance(inner, dict): + continue + _scrub_guardrail_inner(inner) + lp = entry.get("litellm_params") + if isinstance(lp, dict): + _scrub_guardrail_inner(lp) + + # ``general_settings.litellm_jwtauth.custom_validate`` is a nested + # string field. + if section == "general_settings": + jwt = sanitized.get("litellm_jwtauth") + if isinstance(jwt, dict) and _is_remote_module_url(jwt.get("custom_validate")): + verbose_proxy_logger.warning( + "Refused remote-URL custom_validate from DB-overlay " + "general_settings.litellm_jwtauth: %r", + jwt.get("custom_validate"), + ) + jwt["custom_validate"] = None + # ``pass_through_endpoints`` is a list of dicts whose ``target`` + # is passed through ``create_pass_through_route`` → + # ``get_instance_fn``. A DB-overlay ``target: "s3://attacker/m.i"`` + # would otherwise reach the loader because the YAML-load chain + # has ``config_file_path`` set. + pte = sanitized.get("pass_through_endpoints") + if isinstance(pte, list): + for entry in pte: + if isinstance(entry, dict) and _is_remote_module_url( + entry.get("target") + ): + verbose_proxy_logger.warning( + "Refused remote-URL target from DB-overlay " + "general_settings.pass_through_endpoints " + "(path=%r): %r", + entry.get("path"), + entry.get("target"), + ) + entry["target"] = None + return sanitized + + class ProxyConfig: """ Abstraction class on top of config loading/updating logic. Gives us one place to control all config updating logic. @@ -3781,7 +3943,10 @@ class ProxyConfig: # user passed custom_callbacks.async_on_succes_logger. They need us to import a function if "." in callback: litellm.logging_callback_manager.add_litellm_success_callback( - get_instance_fn(value=callback) + get_instance_fn( + value=callback, + config_file_path=config_file_path, + ) ) # these are litellm callbacks - "langfuse", "sentry", "wandb" else: @@ -3809,7 +3974,10 @@ class ProxyConfig: # user passed custom_callbacks.async_on_succes_logger. They need us to import a function if "." in callback: litellm.logging_callback_manager.add_litellm_failure_callback( - get_instance_fn(value=callback) + get_instance_fn( + value=callback, + config_file_path=config_file_path, + ) ) # these are litellm callbacks - "langfuse", "sentry", "wandb" else: @@ -3825,7 +3993,10 @@ class ProxyConfig: for callback in value: if "." in callback: litellm.audit_log_callbacks.append( - get_instance_fn(value=callback) + get_instance_fn( + value=callback, + config_file_path=config_file_path, + ) ) else: litellm.audit_log_callbacks.append(callback) @@ -4048,7 +4219,8 @@ class ProxyConfig: "pass_through_endpoints" ] await initialize_pass_through_endpoints( - pass_through_endpoints=general_settings["pass_through_endpoints"] + pass_through_endpoints=general_settings["pass_through_endpoints"], + config_file_path=config_file_path, ) ## ADMIN UI ACCESS ## @@ -4269,11 +4441,15 @@ class ProxyConfig: litellm.credential_list = credential_list_dict ## NON-LLM CONFIGS eg. MCP tools, vector stores, etc. - await self._init_non_llm_configs(config=config) + await self._init_non_llm_configs( + config=config, config_file_path=config_file_path + ) return router, router.get_model_list(), general_settings - async def _init_non_llm_configs(self, config: dict): + async def _init_non_llm_configs( + self, config: dict, config_file_path: Optional[str] = None + ): """ Initialize non-LLM configs eg. MCP tools, vector stores, etc. """ @@ -4284,7 +4460,9 @@ class ProxyConfig: global_mcp_tool_registry, ) - global_mcp_tool_registry.load_tools_from_config(mcp_tools_config) + global_mcp_tool_registry.load_tools_from_config( + mcp_tools_config, config_file_path=config_file_path + ) ## AGENTS agent_config = config.get("agent_list", None) @@ -5228,6 +5406,15 @@ class ProxyConfig: else: d[k] = v + # Strip remote-URL module loads from the DB-overlay before merge — + # the YAML-load callsites have ``config_file_path`` set, so a + # DB-sourced ``s3://`` value would otherwise reach + # ``_load_instance_from_remote_storage`` without going through + # the runtime gate. + db_param_value = _scrub_db_overlay_remote_module_loads( + section=param_name, db_value=db_param_value + ) + if param_name == "environment_variables": decrypted_env_vars = self._decrypt_and_set_db_env_variables( db_param_value, return_original_value=True @@ -6710,7 +6897,15 @@ class ProxyStartupEvent: for k, v in general_settings["litellm_jwtauth"].items(): if isinstance(v, str) and v.startswith("os.environ/"): general_settings["litellm_jwtauth"][k] = get_secret(v) - litellm_jwtauth = LiteLLM_JWTAuth(**general_settings["litellm_jwtauth"]) + # ``user_config_file_path`` is set by ``ProxyConfig._get_config_from_file`` + # during startup. Threading it through lets an operator- + # configured ``custom_validate: s3://...`` resolve through + # the runtime gate; admin-API JWT config writes (no config + # file context) hit the gate and refuse remote loads. + litellm_jwtauth = LiteLLM_JWTAuth( + config_file_path=user_config_file_path, + **general_settings["litellm_jwtauth"], + ) else: litellm_jwtauth = LiteLLM_JWTAuth() jwt_handler.update_environment( diff --git a/litellm/proxy/types_utils/utils.py b/litellm/proxy/types_utils/utils.py index 676d7fb51b2..8bdaf3fd9e8 100644 --- a/litellm/proxy/types_utils/utils.py +++ b/litellm/proxy/types_utils/utils.py @@ -11,6 +11,20 @@ def get_instance_fn(value: str, config_file_path: Optional[str] = None) -> Any: try: # Check if value starts with s3:// or gcs:// if value.startswith("s3://") or value.startswith("gcs://"): + # Remote module loading is a documented operator feature when + # invoked from config-file load (``config_file_path`` carries + # the YAML path). Without that signal the URL is request-body + # data on an admin endpoint — a one-step admin-to-RCE primitive + # via ``_load_instance_from_remote_storage``'s ``exec_module``. + # Register the module under ``litellm_settings`` in the + # config.yaml instead. + if config_file_path is None: + raise ValueError( + "Remote module loading (s3://, gcs://) is only " + "permitted from the config-file load path. Register " + "the module under ``litellm_settings`` in your " + "config.yaml instead." + ) return _load_instance_from_remote_storage(value, config_file_path) # Split the path by dots to separate module from instance diff --git a/tests/proxy_unit_tests/test_custom_logger_s3_gcs.py b/tests/proxy_unit_tests/test_custom_logger_s3_gcs.py index 38d28f87e7e..01c2a7562f0 100644 --- a/tests/proxy_unit_tests/test_custom_logger_s3_gcs.py +++ b/tests/proxy_unit_tests/test_custom_logger_s3_gcs.py @@ -113,9 +113,11 @@ test_logger_instance = TestCustomLogger() mock_s3_download.side_effect = mock_download - # Test loading with S3 URL + # Test loading with S3 URL — pass config_file_path to indicate + # this is a startup config-file load (the documented operator + # flow that the runtime gate preserves). test_url = "s3://test-bucket/test_custom_logger.test_logger_instance" - instance = get_instance_fn(test_url) + instance = get_instance_fn(test_url, config_file_path="/any/path") assert instance is not None assert hasattr(instance, "initialized") @@ -141,9 +143,9 @@ test_logger_instance = TestCustomLogger() mock_gcs_download.side_effect = mock_download - # Test loading with GCS URL + # Test loading with GCS URL (startup config-file load path). test_url = "gcs://test-bucket/test_custom_logger.test_logger_instance" - instance = get_instance_fn(test_url) + instance = get_instance_fn(test_url, config_file_path="/any/path") assert instance is not None assert hasattr(instance, "initialized") @@ -179,25 +181,27 @@ test_logger_instance = TestCustomLogger() get_instance_fn("ftp://bucket/module.instance") def test_invalid_url_format(self): - """Test error handling for invalid URL formats""" + """Test error handling for invalid URL formats (config-file load path).""" # Missing bucket with pytest.raises(ImportError, match="Invalid URL format"): - get_instance_fn("s3://") + get_instance_fn("s3://", config_file_path="/any/path") # Missing path with pytest.raises(ImportError, match="Invalid URL format"): - get_instance_fn("s3://bucket-only") + get_instance_fn("s3://bucket-only", config_file_path="/any/path") # Missing instance name with pytest.raises(ImportError, match="Invalid module specification"): - get_instance_fn("s3://bucket/module-only") + get_instance_fn("s3://bucket/module-only", config_file_path="/any/path") # Including .py extension (common mistake) with pytest.raises( ImportError, match="Don't include '\\.py' extension and you must specify the instance name", ): - get_instance_fn("s3://bucket/custom_guardrail.py") + get_instance_fn( + "s3://bucket/custom_guardrail.py", config_file_path="/any/path" + ) @patch("litellm.proxy.common_utils.load_config_utils.download_python_file_from_s3") def test_download_failure_handling(self, mock_s3_download): @@ -207,7 +211,7 @@ test_logger_instance = TestCustomLogger() test_url = "s3://test-bucket/failing_logger.instance" with pytest.raises(ImportError, match="Failed to download"): - get_instance_fn(test_url) + get_instance_fn(test_url, config_file_path="/any/path") @patch("litellm.proxy.common_utils.load_config_utils.download_python_file_from_s3") def test_file_cleanup(self, mock_s3_download, sample_custom_logger_content): @@ -223,7 +227,7 @@ test_logger_instance = TestCustomLogger() mock_s3_download.side_effect = mock_download test_url = "s3://test-bucket/test_custom_logger.test_logger_instance" - instance = get_instance_fn(test_url) + instance = get_instance_fn(test_url, config_file_path="/any/path") assert instance is not None diff --git a/tests/test_litellm/proxy/auth/test_banned_params_extra_body.py b/tests/test_litellm/proxy/auth/test_banned_params_extra_body.py new file mode 100644 index 00000000000..2ccee386281 --- /dev/null +++ b/tests/test_litellm/proxy/auth/test_banned_params_extra_body.py @@ -0,0 +1,96 @@ +""" +``extra_body`` is the OpenAI-SDK passthrough container — provider modules +pull provider-auth fields out of it without re-validating. Without +descending into it, the banned-param boundary check is bypassed by +nesting the same fields under ``extra_body``. +""" + +import os +import sys + +import pytest + +sys.path.insert( + 0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../..")) +) + +from litellm.proxy.auth.auth_utils import is_request_body_safe # noqa: E402 + + +@pytest.mark.parametrize( + "banned_param", + [ + "aws_web_identity_token", + "aws_sts_endpoint", + "aws_role_name", + "api_base", + "base_url", + "vertex_credentials", + "azure_ad_token", + ], +) +def test_banned_param_under_extra_body_is_rejected(banned_param): + body = { + "model": "bedrock/anthropic.claude-v2", + "messages": [{"role": "user", "content": "x"}], + "extra_body": {banned_param: "anything-attacker-chose"}, + } + with pytest.raises(ValueError, match="not allowed in request body"): + is_request_body_safe( + request_body=body, + general_settings={}, + llm_router=None, + model="bedrock/anthropic.claude-v2", + ) + + +def test_extra_body_with_safe_fields_is_allowed(): + body = { + "model": "openai/gpt-4", + "messages": [{"role": "user", "content": "x"}], + "extra_body": {"reasoning_effort": "low", "seed": 42}, + } + assert is_request_body_safe( + request_body=body, + general_settings={}, + llm_router=None, + model="openai/gpt-4", + ) + + +def test_admin_opt_in_still_permits_extra_body_credentials(): + # ``allow_client_side_credentials`` is the admin escape; descending + # into ``extra_body`` must preserve it. + body = { + "model": "openai/gpt-4", + "messages": [{"role": "user", "content": "x"}], + "extra_body": {"api_base": "https://my-private-openai.internal"}, + } + assert is_request_body_safe( + request_body=body, + general_settings={"allow_client_side_credentials": True}, + llm_router=None, + model="openai/gpt-4", + ) + + +def test_banned_param_under_stringified_extra_body_is_rejected(): + # Raw-HTTP and multipart/form-data clients can send ``extra_body`` as + # a JSON-encoded string rather than an object. An ``isinstance(..., + # dict)`` guard on the nested descent would skip such payloads, + # leaving the banned-key check bypassed. Coercion via + # ``_coerce_metadata_to_dict`` closes that variant. + import json + + body = { + "model": "bedrock/anthropic.claude-v2", + "messages": [{"role": "user", "content": "x"}], + "extra_body": json.dumps({"aws_web_identity_token": "anything"}), + } + with pytest.raises(ValueError, match="not allowed in request body"): + is_request_body_safe( + request_body=body, + general_settings={}, + llm_router=None, + model="bedrock/anthropic.claude-v2", + ) diff --git a/tests/test_litellm/proxy/types_utils/__init__.py b/tests/test_litellm/proxy/types_utils/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/test_litellm/proxy/types_utils/test_db_overlay_remote_module_scrub.py b/tests/test_litellm/proxy/types_utils/test_db_overlay_remote_module_scrub.py new file mode 100644 index 00000000000..100ba653f3a --- /dev/null +++ b/tests/test_litellm/proxy/types_utils/test_db_overlay_remote_module_scrub.py @@ -0,0 +1,219 @@ +""" +Regression tests: ``s3://`` / ``gcs://`` values in DB-overlay config +must be stripped at the merge boundary so they never reach +``get_instance_fn`` with ``config_file_path`` set. + +Without this scrub, a PROXY_ADMIN who persists e.g. +``litellm_settings.success_callback: ["s3://attacker/m.i"]`` via +``/config/update`` would have it merged into the in-memory config +during the next ``load_config`` cycle. The YAML-load chain is active +at that point, so the runtime gate in ``get_instance_fn`` (which +permits remote loads when ``config_file_path`` is non-None) would +pass and ``_load_instance_from_remote_storage`` would exec the +remote module. +""" + +import os +import sys + +import pytest + +sys.path.insert( + 0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../..")) +) + +from litellm.proxy.proxy_server import ( # noqa: E402 + _scrub_db_overlay_remote_module_loads, +) + + +@pytest.mark.parametrize( + "field", + ["callbacks", "success_callback", "failure_callback", "audit_log_callbacks"], +) +def test_litellm_settings_callback_list_strips_remote_urls(field): + overlay = {field: ["langfuse", "s3://attacker/m.i", "gcs://attacker/m.i"]} + cleaned = _scrub_db_overlay_remote_module_loads("litellm_settings", overlay) + assert cleaned[field] == ["langfuse"] + + +@pytest.mark.parametrize( + "field", + [ + "custom_auth", + "custom_key_generate", + "custom_key_update", + "custom_sso", + "custom_ui_sso_sign_in_handler", + ], +) +def test_general_settings_str_field_strips_remote_urls(field): + overlay = {field: "s3://attacker/m.i"} + cleaned = _scrub_db_overlay_remote_module_loads("general_settings", overlay) + assert cleaned[field] is None + + +def test_litellm_settings_post_call_rules_str_stripped(): + overlay = {"post_call_rules": "gcs://attacker/m.i"} + cleaned = _scrub_db_overlay_remote_module_loads("litellm_settings", overlay) + assert cleaned["post_call_rules"] is None + + +def test_custom_provider_map_custom_handler_stripped(): + overlay = { + "custom_provider_map": [ + {"provider": "ok", "custom_handler": "my_module.handler"}, + {"provider": "bad", "custom_handler": "s3://attacker/m.i"}, + ] + } + cleaned = _scrub_db_overlay_remote_module_loads("litellm_settings", overlay) + assert cleaned["custom_provider_map"][0]["custom_handler"] == "my_module.handler" + assert cleaned["custom_provider_map"][1]["custom_handler"] is None + + +def test_litellm_settings_guardrails_v1_callbacks_stripped(): + # v1 guardrail shape: {guardrail_name: {callbacks: [...], default_on: bool}} + overlay = { + "guardrails": [ + { + "prompt_injection": { + "default_on": True, + "callbacks": [ + "lakera_prompt_injection", + "s3://attacker/m.i", + "gcs://attacker/m.i", + ], + } + } + ] + } + cleaned = _scrub_db_overlay_remote_module_loads("litellm_settings", overlay) + assert cleaned["guardrails"][0]["prompt_injection"]["callbacks"] == [ + "lakera_prompt_injection" + ] + + +def test_litellm_settings_guardrails_v2_callbacks_and_guardrail_stripped(): + # v2 shape: {guardrail_name, litellm_params: {guardrail: "module.path", callbacks: [...]}} + overlay = { + "guardrails": [ + { + "guardrail_name": "custom", + "litellm_params": { + "guardrail": "s3://attacker/m.i", + "mode": "pre_call", + "callbacks": ["lakera", "s3://attacker/cb.i"], + }, + } + ] + } + cleaned = _scrub_db_overlay_remote_module_loads("litellm_settings", overlay) + lp = cleaned["guardrails"][0]["litellm_params"] + assert lp["guardrail"] is None + assert lp["callbacks"] == ["lakera"] + assert lp["mode"] == "pre_call" + + +def test_litellm_settings_guardrails_local_dotted_name_preserved(): + overlay = { + "guardrails": [ + { + "guardrail_name": "custom", + "litellm_params": { + "guardrail": "custom_module.MyGuardrail", + "callbacks": ["my_module.cb", "langfuse"], + }, + } + ] + } + cleaned = _scrub_db_overlay_remote_module_loads("litellm_settings", overlay) + lp = cleaned["guardrails"][0]["litellm_params"] + assert lp["guardrail"] == "custom_module.MyGuardrail" + assert lp["callbacks"] == ["my_module.cb", "langfuse"] + + +def test_litellm_settings_guardrails_non_list_passthrough(): + cleaned = _scrub_db_overlay_remote_module_loads( + "litellm_settings", {"guardrails": "not-a-list"} + ) + assert cleaned["guardrails"] == "not-a-list" + + +def test_pass_through_endpoints_target_stripped(): + overlay = { + "pass_through_endpoints": [ + {"path": "/ok", "target": "my_module.legit_handler"}, + {"path": "/bad-s3", "target": "s3://attacker/m.handler"}, + {"path": "/bad-gcs", "target": "gcs://attacker/m.handler"}, + ] + } + cleaned = _scrub_db_overlay_remote_module_loads("general_settings", overlay) + # Legit dotted-name target preserved + assert cleaned["pass_through_endpoints"][0]["target"] == "my_module.legit_handler" + # Both remote URLs stripped to None — entry remains so the path + # registration can still be skipped explicitly downstream + assert cleaned["pass_through_endpoints"][1]["target"] is None + assert cleaned["pass_through_endpoints"][2]["target"] is None + # Sibling fields preserved + assert cleaned["pass_through_endpoints"][0]["path"] == "/ok" + assert cleaned["pass_through_endpoints"][1]["path"] == "/bad-s3" + + +def test_pass_through_endpoints_non_list_passthrough(): + # If pass_through_endpoints is mistyped (not a list), the scrub + # must not raise. + cleaned = _scrub_db_overlay_remote_module_loads( + "general_settings", {"pass_through_endpoints": "not-a-list"} + ) + assert cleaned["pass_through_endpoints"] == "not-a-list" + + +def test_litellm_jwtauth_custom_validate_stripped(): + overlay = { + "litellm_jwtauth": { + "user_id_jwt_field": "sub", + "custom_validate": "s3://attacker/m.validator", + } + } + cleaned = _scrub_db_overlay_remote_module_loads("general_settings", overlay) + assert cleaned["litellm_jwtauth"]["custom_validate"] is None + # Sibling fields preserved. + assert cleaned["litellm_jwtauth"]["user_id_jwt_field"] == "sub" + + +def test_local_dotted_name_preserved(): + # The scrub only targets s3:// / gcs:// scheme prefixes — legitimate + # dotted module names (the documented operator flow) must pass + # through unchanged. + overlay = { + "success_callback": ["langfuse", "my_module.success_handler", "datadog"], + "post_call_rules": "my_module.rule_fn", + } + cleaned = _scrub_db_overlay_remote_module_loads("litellm_settings", overlay) + assert cleaned["success_callback"] == [ + "langfuse", + "my_module.success_handler", + "datadog", + ] + assert cleaned["post_call_rules"] == "my_module.rule_fn" + + +def test_non_dict_overlay_passthrough(): + # Some DB-overlay values are scalars (e.g. ``max_internal_user_budget: + # 100.0``). The scrub must not break those. + assert _scrub_db_overlay_remote_module_loads("litellm_settings", 100.0) == 100.0 + assert _scrub_db_overlay_remote_module_loads("litellm_settings", None) is None + + +def test_unknown_section_passthrough(): + overlay = {"success_callback": ["s3://anything"]} + # ``router_settings`` isn't a section with module-loading fields — + # the scrub leaves it alone. + cleaned = _scrub_db_overlay_remote_module_loads("router_settings", overlay) + assert cleaned == overlay + + +def test_scrub_does_not_mutate_input(): + original = {"success_callback": ["s3://attacker/m.i"]} + _scrub_db_overlay_remote_module_loads("litellm_settings", original) + assert original["success_callback"] == ["s3://attacker/m.i"] diff --git a/tests/test_litellm/proxy/types_utils/test_get_instance_fn_runtime_gate.py b/tests/test_litellm/proxy/types_utils/test_get_instance_fn_runtime_gate.py new file mode 100644 index 00000000000..bf77ef81641 --- /dev/null +++ b/tests/test_litellm/proxy/types_utils/test_get_instance_fn_runtime_gate.py @@ -0,0 +1,113 @@ +""" +Regression tests: ``get_instance_fn`` refuses remote module loading +(``s3://``, ``gcs://``) when invoked without a ``config_file_path``. + +The startup config-file load path passes ``config_file_path`` and is +unaffected — the documented ``litellm_settings.callbacks: +["s3://bucket/module.instance"]`` operator flow continues to work. +""" + +import os +import sys +from unittest.mock import patch + +import pytest + +sys.path.insert( + 0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../../../..")) +) + +from litellm.proxy.types_utils.utils import get_instance_fn # noqa: E402 + + +@pytest.mark.parametrize("scheme", ["s3", "gcs"]) +def test_remote_url_without_config_file_path_is_rejected(scheme): + # The C1-Stage-B attack vector: admin endpoint receives an + # s3:// / gcs:// instance specifier via the request body; no + # ``config_file_path`` is in scope. Must refuse before the + # ``exec_module`` sink is reached. + with pytest.raises(ValueError, match="Remote module loading"): + get_instance_fn(value=f"{scheme}://attacker-bucket/module.instance") + + +def test_remote_url_with_config_file_path_is_allowed(): + # Startup config-file load path: ``config_file_path`` is set, so + # the gate doesn't fire. Documented operator feature must keep + # working. + with patch( + "litellm.proxy.types_utils.utils._load_instance_from_remote_storage", + return_value="loaded", + ) as mock_loader: + result = get_instance_fn( + value="s3://my-bucket/m.inst", + config_file_path="/etc/litellm/config.yaml", + ) + + assert result == "loaded" + mock_loader.assert_called_once_with( + "s3://my-bucket/m.inst", "/etc/litellm/config.yaml" + ) + + +def test_dotted_module_path_is_unaffected_by_gate(): + # Local dotted-name imports — the other branch of get_instance_fn — + # have nothing to do with the remote-URL gate. Regression that the + # gate doesn't accidentally affect them. + with patch( + "litellm.proxy.types_utils.utils.importlib.import_module" + ) as mock_import: + mock_module = type("M", (), {"my_instance": "loaded"}) + mock_import.return_value = mock_module + + result = get_instance_fn(value="my_module.my_instance") + + assert result == "loaded" + + +def test_pass_through_route_threads_config_file_path(): + # ``create_pass_through_route`` must forward ``config_file_path`` so + # an operator with ``custom_handler: s3://...`` declared in + # ``config.yaml`` still resolves at startup. Callers that omit it + # (DB-overlay / runtime admin API) fall through to the gate. + from litellm.proxy.pass_through_endpoints import pass_through_endpoints as pte + + # ``get_instance_fn`` is imported lazily inside the function — patch + # at the source so the deferred import resolves to the mock. + with patch( + "litellm.proxy.types_utils.utils.get_instance_fn", return_value=object() + ) as mock_get: + pte.create_pass_through_route( + endpoint="/x", + target="s3://bucket/mod.inst", + config_file_path="/etc/litellm/config.yaml", + ) + + mock_get.assert_called_once_with( + value="s3://bucket/mod.inst", + config_file_path="/etc/litellm/config.yaml", + ) + + +def test_mcp_tool_registry_threads_config_file_path(): + # MCP tool handlers declared in ``config.yaml`` mcp_tools[].handler + # may legitimately be ``s3://...``; the YAML-load path must thread + # ``config_file_path`` so they resolve. + from litellm.proxy._experimental.mcp_server import tool_registry as tr + + fake_handler = lambda **kwargs: None # noqa: E731 — registry requires callable + with patch.object(tr, "get_instance_fn", return_value=fake_handler) as mock_get: + registry = tr.MCPToolRegistry() + registry.load_tools_from_config( + mcp_tools_config=[ + { + "name": "tool_a", + "description": "d", + "handler": "s3://bucket/mod.handler", + } + ], + config_file_path="/etc/litellm/config.yaml", + ) + + mock_get.assert_called_once_with( + "s3://bucket/mod.handler", "/etc/litellm/config.yaml" + )