From dd6eb499e6f0c955d3af3723dc96fbf39ac45817 Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Wed, 13 May 2026 01:05:03 +0000 Subject: [PATCH] chore(proxy): refuse remote-URL instance-fn loads outside config-file path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ``get_instance_fn`` previously routed any ``s3://`` / ``gcs://`` value into ``_load_instance_from_remote_storage`` regardless of how the value got there. The function ultimately calls ``spec.loader.exec_module(module)`` — Python in the proxy process. On admin-callable endpoints that accept a ``target`` / ``custom_handler`` field from the request body (e.g. ``/config/pass_through_endpoint``, custom-callback registration), that is a one-step admin-to-RCE primitive: any future privilege-escalation bug becomes immediate code execution. The documented operator flow for remote-module loading is ``litellm_settings.callbacks: ["s3://bucket/module.instance"]`` in ``config.yaml``. That path always carries the YAML's ``config_file_path`` through to ``get_instance_fn``. Use the presence of ``config_file_path`` as the discriminator: refuse remote URLs when it is absent (the request-body path) unless the operator explicitly opts back in via ``LITELLM_ALLOW_REMOTE_INSTANCE_FN_FROM_API=true``. The three success/failure/audit-log callback-loop call sites in ``proxy_server.py:load_config`` were already running inside the startup config-file load but had stopped threading ``config_file_path`` through. Pass it through so the documented ``s3://`` callback flow continues to work unchanged. Tests cover: remote URL without ``config_file_path`` raises; remote URL with the opt-in env reaches the loader; remote URL with ``config_file_path`` passes (documented startup flow); local dotted-name imports unaffected. --- litellm/proxy/proxy_server.py | 15 ++- litellm/proxy/types_utils/utils.py | 24 +++++ .../proxy/types_utils/__init__.py | 0 .../test_get_instance_fn_runtime_gate.py | 92 +++++++++++++++++++ 4 files changed, 128 insertions(+), 3 deletions(-) create mode 100644 tests/test_litellm/proxy/types_utils/__init__.py create mode 100644 tests/test_litellm/proxy/types_utils/test_get_instance_fn_runtime_gate.py diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 4a538a28e03..675988c879c 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -3799,7 +3799,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: @@ -3827,7 +3830,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: @@ -3848,7 +3854,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) diff --git a/litellm/proxy/types_utils/utils.py b/litellm/proxy/types_utils/utils.py index 676d7fb51b2..2786564152a 100644 --- a/litellm/proxy/types_utils/utils.py +++ b/litellm/proxy/types_utils/utils.py @@ -5,12 +5,36 @@ import os from typing import Any, Callable, Literal, Optional, get_type_hints +def _allow_remote_instance_fn_from_api() -> bool: + return os.getenv("LITELLM_ALLOW_REMOTE_INSTANCE_FN_FROM_API", "").lower() in ( + "1", + "true", + "yes", + ) + + def get_instance_fn(value: str, config_file_path: Optional[str] = None) -> Any: module_name = value instance_name = None 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``. Refuse unless the operator explicitly opts + # back in via ``LITELLM_ALLOW_REMOTE_INSTANCE_FN_FROM_API``. + if config_file_path is None and not _allow_remote_instance_fn_from_api(): + 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. To opt into runtime API " + "loading, set " + "``LITELLM_ALLOW_REMOTE_INSTANCE_FN_FROM_API=true``." + ) return _load_instance_from_remote_storage(value, config_file_path) # Split the path by dots to separate module from instance 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_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..fd6b6c6166b --- /dev/null +++ b/tests/test_litellm/proxy/types_utils/test_get_instance_fn_runtime_gate.py @@ -0,0 +1,92 @@ +""" +Regression tests: ``get_instance_fn`` refuses remote module loading +(``s3://``, ``gcs://``) when invoked without a ``config_file_path`` +unless the operator explicitly opts in via +``LITELLM_ALLOW_REMOTE_INSTANCE_FN_FROM_API``. + +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.fixture(autouse=True) +def _strip_opt_in_env(monkeypatch): + monkeypatch.delenv("LITELLM_ALLOW_REMOTE_INSTANCE_FN_FROM_API", raising=False) + + +@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 POSTs 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") + + +@pytest.mark.parametrize("scheme", ["s3", "gcs"]) +def test_remote_url_with_opt_in_env_is_allowed_to_reach_loader(scheme, monkeypatch): + # When the operator explicitly opts in, the runtime path is allowed + # to reach ``_load_instance_from_remote_storage`` — which then makes + # its own decisions / fails on AWS auth / etc. We only verify the + # gate doesn't fire before the loader is reached. + monkeypatch.setenv("LITELLM_ALLOW_REMOTE_INSTANCE_FN_FROM_API", "true") + + with patch( + "litellm.proxy.types_utils.utils._load_instance_from_remote_storage", + return_value="loaded", + ) as mock_loader: + result = get_instance_fn(value=f"{scheme}://my-bucket/m.inst") + + assert result == "loaded" + mock_loader.assert_called_once_with(f"{scheme}://my-bucket/m.inst", None) + + +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 without the opt-in env. + 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"