mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(proxy): reject config includes that escape the config directory
resolve_include_file_path used to join and abspath include entries with no boundary check, so a relative path with `..` (or an absolute path outside the config dir) could be read and merged into the proxy config. Keep includes under the root config directory and fail closed with ValueError before the file is opened. Fixes #43480
This commit is contained in:
parent
2c9b0e00ac
commit
c5b120f0c3
3 changed files with 106 additions and 3 deletions
|
|
@ -8,6 +8,25 @@ from litellm._logging import verbose_proxy_logger
|
|||
INCLUDE_KEY: Final = "include"
|
||||
|
||||
|
||||
def _path_is_within_directory(path: str, directory: str) -> bool:
|
||||
"""True when ``path`` is ``directory`` itself or a file under it (after abspath)."""
|
||||
try:
|
||||
return os.path.commonpath([directory, path]) == directory
|
||||
except ValueError:
|
||||
# Different drives on Windows — never treat that as inside the config dir.
|
||||
return False
|
||||
|
||||
|
||||
def _reject_include_outside_config_dir(resolved: str, root_config_path: str, include_file: str) -> str:
|
||||
config_dir: Final = os.path.abspath(os.path.dirname(root_config_path))
|
||||
if _path_is_within_directory(resolved, config_dir):
|
||||
return resolved
|
||||
raise ValueError(
|
||||
f"Config include '{include_file}' resolves to {resolved}, which is outside the config "
|
||||
f"directory {config_dir}. Include paths must stay under the config directory."
|
||||
)
|
||||
|
||||
|
||||
def resolve_include_file_path(include_file: str, declared_in: str, root_config_path: str) -> str:
|
||||
"""
|
||||
Resolve one `include` entry to the file it names, next to the config that declares it.
|
||||
|
|
@ -15,11 +34,14 @@ def resolve_include_file_path(include_file: str, declared_in: str, root_config_p
|
|||
A config written before nested entries resolved this way can name a file sitting next to the root
|
||||
config instead, so that file is still read, with a warning naming where it was found. When both
|
||||
files exist the one next to the declaring config wins and the other is named in a warning.
|
||||
|
||||
Resolved paths that climb out of the root config directory (``..`` or an absolute path outside
|
||||
it) are rejected before the file is opened.
|
||||
"""
|
||||
declared_relative: Final = os.path.abspath(os.path.join(os.path.dirname(declared_in), include_file))
|
||||
root_relative: Final = os.path.abspath(os.path.join(os.path.dirname(root_config_path), include_file))
|
||||
if root_relative == declared_relative or not os.path.exists(root_relative):
|
||||
return declared_relative
|
||||
return _reject_include_outside_config_dir(declared_relative, root_config_path, include_file)
|
||||
|
||||
if not os.path.exists(declared_relative):
|
||||
verbose_proxy_logger.warning(
|
||||
|
|
@ -29,7 +51,7 @@ def resolve_include_file_path(include_file: str, declared_in: str, root_config_p
|
|||
declared_in,
|
||||
root_relative,
|
||||
)
|
||||
return root_relative
|
||||
return _reject_include_outside_config_dir(root_relative, root_config_path, include_file)
|
||||
|
||||
verbose_proxy_logger.warning(
|
||||
"Config include '%s' declared in %s matches two files. %s sits next to that config and was read, "
|
||||
|
|
@ -39,7 +61,7 @@ def resolve_include_file_path(include_file: str, declared_in: str, root_config_p
|
|||
declared_relative,
|
||||
root_relative,
|
||||
)
|
||||
return declared_relative
|
||||
return _reject_include_outside_config_dir(declared_relative, root_config_path, include_file)
|
||||
|
||||
|
||||
class IncludeResolver(Protocol):
|
||||
|
|
|
|||
65
tests/unit/proxy/common_utils/test_config_includes.py
Normal file
65
tests/unit/proxy/common_utils/test_config_includes.py
Normal file
|
|
@ -0,0 +1,65 @@
|
|||
"""Unit tests for litellm.proxy.common_utils.config_includes."""
|
||||
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
from litellm.proxy.common_utils.config_includes import resolve_include_file_path
|
||||
|
||||
|
||||
def test_resolve_include_rejects_path_outside_config_dir(tmp_path):
|
||||
"""
|
||||
Include entries that climb out of the root config directory must be rejected
|
||||
before the file is opened (regression for #43480).
|
||||
"""
|
||||
cfg_dir = tmp_path / "cfg"
|
||||
cfg_dir.mkdir()
|
||||
root_config = cfg_dir / "config.yaml"
|
||||
root_config.write_text("model_list: []\n")
|
||||
|
||||
# Same shape as the issue report: ../../etc/passwd from cfg/config.yaml
|
||||
with pytest.raises(ValueError, match="outside the config directory"):
|
||||
resolve_include_file_path("../../etc/passwd", str(root_config), str(root_config))
|
||||
|
||||
# Absolute path outside the config dir
|
||||
with pytest.raises(ValueError, match="outside the config directory"):
|
||||
resolve_include_file_path("/etc/passwd", str(root_config), str(root_config))
|
||||
|
||||
|
||||
def test_resolve_include_allows_file_under_config_dir(tmp_path):
|
||||
"""Sibling and nested includes under the config directory still resolve."""
|
||||
cfg_dir = tmp_path / "cfg"
|
||||
nested = cfg_dir / "nested"
|
||||
nested.mkdir(parents=True)
|
||||
root_config = cfg_dir / "config.yaml"
|
||||
root_config.write_text("model_list: []\n")
|
||||
sibling = cfg_dir / "models.yaml"
|
||||
sibling.write_text("model_list: []\n")
|
||||
nested_include = nested / "extra.yaml"
|
||||
nested_include.write_text("model_list: []\n")
|
||||
|
||||
assert resolve_include_file_path("models.yaml", str(root_config), str(root_config)) == str(
|
||||
sibling.resolve()
|
||||
)
|
||||
# Nested config may climb one level with .. and still stay under cfg/
|
||||
assert resolve_include_file_path(
|
||||
"../models.yaml", str(nested_include), str(root_config)
|
||||
) == str(sibling.resolve())
|
||||
|
||||
|
||||
def test_resolve_include_rejects_escape_via_legacy_root_fallback(tmp_path):
|
||||
"""
|
||||
When the include is missing next to a nested declarer but present next to the
|
||||
root, the legacy fallback must still refuse a path that left the config dir.
|
||||
"""
|
||||
cfg_dir = tmp_path / "cfg"
|
||||
nested = cfg_dir / "nested"
|
||||
nested.mkdir(parents=True)
|
||||
root_config = cfg_dir / "config.yaml"
|
||||
root_config.write_text("model_list: []\n")
|
||||
nested_config = nested / "part.yaml"
|
||||
nested_config.write_text("model_list: []\n")
|
||||
|
||||
# declared next to nested; ../../outside.yaml from nested → tmp_path/outside.yaml
|
||||
with pytest.raises(ValueError, match="outside the config directory"):
|
||||
resolve_include_file_path("../../outside.yaml", str(nested_config), str(root_config))
|
||||
|
|
@ -158,6 +158,22 @@ async def test_multiple_includes():
|
|||
assert config["litellm_settings"]["callbacks"] == ["prometheus"]
|
||||
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_includes_rejects_escaping_include(tmp_path):
|
||||
"""ProxyConfig._process_includes must fail closed on escaping include paths."""
|
||||
cfg_dir = tmp_path / "cfg"
|
||||
cfg_dir.mkdir()
|
||||
outside = tmp_path / "outside.yaml"
|
||||
outside.write_text("model_list:\n - model_name: leaked\n litellm_params:\n model: openai/gpt-4o-mini\n")
|
||||
root_config = cfg_dir / "config.yaml"
|
||||
root_config.write_text("include:\n - ../outside.yaml\n\nmodel_list: []\n")
|
||||
|
||||
proxy_config_instance = ProxyConfig()
|
||||
with pytest.raises(ValueError, match="outside the config directory"):
|
||||
await proxy_config_instance.get_config(config_file_path=str(root_config))
|
||||
|
||||
|
||||
def test_add_callbacks_from_db_config():
|
||||
"""Test that callbacks are added correctly and duplicates are prevented"""
|
||||
# Setup
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue