mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
chore(proxy): refuse remote-URL instance-fn loads outside config-file path
``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.
This commit is contained in:
parent
63a2d1ddc9
commit
dd6eb499e6
4 changed files with 128 additions and 3 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
0
tests/test_litellm/proxy/types_utils/__init__.py
Normal file
0
tests/test_litellm/proxy/types_utils/__init__.py
Normal file
|
|
@ -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"
|
||||
Loading…
Add table
Reference in a new issue