diff --git a/docs/my-website/docs/oidc.md b/docs/my-website/docs/oidc.md index 23eb431b7e..c4b82a08d1 100644 --- a/docs/my-website/docs/oidc.md +++ b/docs/my-website/docs/oidc.md @@ -57,12 +57,31 @@ oidc/config_name_here/ #### Unofficial Providers (not recommended) -For the unofficial `file` provider, you can use the following format: +For the unofficial `file` provider, you can use the following format +(note the double slash — the path after `oidc/file/` must be absolute): ``` -oidc/file/home/user/dave/this_is_a_file_with_a_token.txt +oidc/file//var/run/secrets/my-token ``` +For safety, the resolved path must live inside an allowed credential +directory. By default the following directories are allowed: + +- `/var/run/secrets` +- `/run/secrets` + +If your deployment mounts credentials elsewhere, set the +`LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS` environment variable to a +comma-separated list of absolute directories. The value replaces the +default list, so include the defaults if you still need them: + +```bash +export LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS="/var/run/secrets,/etc/litellm/creds" +``` + +Paths that resolve (after following symlinks and `..`) outside the +allowlist are rejected. + For the unofficial `env`, use the following format, where `SECRET_TOKEN` is the name of the environment variable that contains the token: ``` diff --git a/docs/my-website/docs/proxy/config_settings.md b/docs/my-website/docs/proxy/config_settings.md index db38cf5426..fa7b73f6c4 100644 --- a/docs/my-website/docs/proxy/config_settings.md +++ b/docs/my-website/docs/proxy/config_settings.md @@ -830,6 +830,7 @@ router_settings: | LITELLM_KEY_ROTATION_LOCK_TTL_SECONDS | TTL in seconds for the distributed lock used by the key rotation job. Default is 600 (10 minutes). | LITELLM_LICENSE | License key for LiteLLM usage | LITELLM_LOCAL_ANTHROPIC_BETA_HEADERS | Set to `True` to use the local bundled Anthropic beta headers config only, disabling remote fetching. Default is `False` +| LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS | Comma-separated list of absolute directories from which the `oidc/file/` provider is permitted to read token files. Defaults to `/var/run/secrets,/run/secrets`. | LITELLM_LOCAL_BLOG_POSTS | When set to `True`, uses the local bundled blog posts only, disabling remote fetching from GitHub. Default is `False` | LITELLM_LOCAL_MODEL_COST_MAP | Local configuration for model cost mapping in LiteLLM | LITELLM_LOCAL_POLICY_TEMPLATES | When set to "true", uses local backup policy templates instead of fetching from GitHub. Policy templates are fetched from https://raw.githubusercontent.com/BerriAI/litellm/main/policy_templates.json by default, with automatic fallback to local backup on failure diff --git a/litellm/litellm_core_utils/initialize_dynamic_callback_params.py b/litellm/litellm_core_utils/initialize_dynamic_callback_params.py index ff521d4780..b80907e8ec 100644 --- a/litellm/litellm_core_utils/initialize_dynamic_callback_params.py +++ b/litellm/litellm_core_utils/initialize_dynamic_callback_params.py @@ -1,7 +1,12 @@ from typing import Dict, Optional -from litellm.secret_managers.main import get_secret_str + +from litellm._logging import verbose_logger from litellm.types.utils import StandardCallbackDynamicParams + +def _is_env_reference(value: object) -> bool: + return isinstance(value, str) and "os.environ/" in value + # Hardcoded list of supported callback params to avoid runtime inspection issues with TypedDict _supported_callback_params = [ "langfuse_public_key", @@ -46,12 +51,14 @@ def initialize_standard_callback_dynamic_params( for param in _supported_callback_params: if param in kwargs: _param_value = kwargs.get(param) - if ( - _param_value is not None - and isinstance(_param_value, str) - and "os.environ/" in _param_value - ): - _param_value = get_secret_str(secret_name=_param_value) + if _is_env_reference(_param_value): + verbose_logger.warning( + "Dropping callback param '%s': os.environ/ references " + "in request-supplied parameters are not resolved. " + "Configure this value server-side instead.", + param, + ) + continue standard_callback_dynamic_params[param] = _param_value # type: ignore # 2. Fallback: check "metadata" or "litellm_params" -> "metadata" @@ -64,12 +71,15 @@ def initialize_standard_callback_dynamic_params( for param in _supported_callback_params: if param not in standard_callback_dynamic_params and param in metadata: _param_value = metadata.get(param) - if ( - _param_value is not None - and isinstance(_param_value, str) - and "os.environ/" in _param_value - ): - _param_value = get_secret_str(secret_name=_param_value) + if _is_env_reference(_param_value): + verbose_logger.warning( + "Dropping callback param '%s' from metadata: " + "os.environ/ references in request-supplied " + "parameters are not resolved. Configure this " + "value server-side instead.", + param, + ) + continue standard_callback_dynamic_params[param] = _param_value # type: ignore return standard_callback_dynamic_params diff --git a/litellm/proxy/health_endpoints/_health_endpoints.py b/litellm/proxy/health_endpoints/_health_endpoints.py index 8a09edfd4c..8fd19548cb 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -5,7 +5,7 @@ import os import time import traceback from datetime import datetime, timedelta -from typing import Any, Dict, Literal, Optional, Union, cast +from typing import Any, Dict, Iterable, Literal, Optional, Union, cast import fastapi from fastapi import APIRouter, Depends, HTTPException, Request, Response, status @@ -36,79 +36,43 @@ from litellm.proxy.health_check import ( from litellm.proxy.middleware.in_flight_requests_middleware import ( get_in_flight_requests, ) -from litellm.secret_managers.main import get_secret #### Health ENDPOINTS #### -def _resolve_os_environ_variables(params: dict) -> dict: +def _reject_os_environ_references(params: dict) -> None: """ - Resolve ``os.environ/`` environment variables in ``litellm_params``. - - This walks the input dict/list structure iteratively (no Python recursion) to - avoid unbounded recursion / stack overflows on deeply nested inputs. + Validate that the provided params do not contain any ``os.environ/`` + references. Values with that prefix are expected to come only from + server-side configuration (already resolved before reaching here). If a + request-supplied value still carries the prefix, raise ``HTTPException``. """ if not isinstance(params, dict): - return params + return - # Use an explicit stack to avoid recursion and handle nested dicts/lists. - # We also keep a `seen` set to guard against accidental cycles. - resolved_root: dict = {} - stack: list[tuple[object, object]] = [(params, resolved_root)] + stack: list[object] = [params] seen: set[int] = {id(params)} while stack: - src, dst = stack.pop() + src = stack.pop() + if isinstance(src, dict): + values: Iterable[object] = src.values() + elif isinstance(src, list): + values = src + else: + continue - if isinstance(src, dict) and isinstance(dst, dict): - for key, value in src.items(): - # Direct string replacement for os.environ/ references - if isinstance(value, str) and value.startswith("os.environ/"): - dst[key] = get_secret(value) - elif isinstance(value, dict): - if id(value) in seen: - # Cycle detected – keep a shallow copy reference to prevent infinite loops - dst[key] = {} - continue - seen.add(id(value)) - new_dict: dict = {} - dst[key] = new_dict - stack.append((value, new_dict)) - elif isinstance(value, list): - if id(value) in seen: - dst[key] = [] - continue - seen.add(id(value)) - new_list: list = [] - dst[key] = new_list - stack.append((value, new_list)) - else: - dst[key] = value - - elif isinstance(src, list) and isinstance(dst, list): - for item in src: - if isinstance(item, str) and item.startswith("os.environ/"): - dst.append(get_secret(item)) - elif isinstance(item, dict): - if id(item) in seen: - dst.append({}) - continue - seen.add(id(item)) - new_dict = {} - dst.append(new_dict) - stack.append((item, new_dict)) - elif isinstance(item, list): - if id(item) in seen: - dst.append([]) - continue - seen.add(id(item)) - new_list = [] - dst.append(new_list) - stack.append((item, new_list)) - else: - dst.append(item) - - return resolved_root + for value in values: + if isinstance(value, str) and value.startswith("os.environ/"): + raise HTTPException( + status_code=400, + detail={ + "error": "Environment variable references are not permitted in request parameters." + }, + ) + if isinstance(value, (dict, list)) and id(value) not in seen: + seen.add(id(value)) + stack.append(value) def get_callback_identifier(callback): @@ -1510,6 +1474,10 @@ async def test_model_connection( # Get model name from litellm_params request_litellm_params = litellm_params or {} + # Reject request-supplied os.environ/ references. Config values are + # already resolved before reaching this endpoint; any remaining + # reference must have come from the request body. + _reject_os_environ_references(request_litellm_params) model_name = request_litellm_params.get("model") # Look up model configuration from router if model name is provided @@ -1546,11 +1514,7 @@ 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 - merged_litellm_params = {**config_litellm_params, **request_litellm_params} - - # Resolve os.environ/ environment variables in any remaining request params - # This handles cases where user explicitly passes os.environ/ values to override config - litellm_params = _resolve_os_environ_variables(merged_litellm_params) + litellm_params = {**config_litellm_params, **request_litellm_params} ## Auth check await ModelManagementAuthChecks.can_user_make_model_call( diff --git a/litellm/secret_managers/main.py b/litellm/secret_managers/main.py index 2aca1cd9dd..a560f5222b 100644 --- a/litellm/secret_managers/main.py +++ b/litellm/secret_managers/main.py @@ -16,6 +16,52 @@ from litellm.secret_managers.secret_manager_handler import get_secret_from_manag oidc_cache = DualCache() +_DEFAULT_OIDC_ALLOWED_CREDENTIAL_DIRS = ("/var/run/secrets", "/run/secrets") + + +def _get_oidc_allowed_credential_dirs() -> list[str]: + """ + Return the absolute, normalized list of directories from which + ``oidc/file/`` is permitted to read token files. + + Defaults to standard container credential mount points. Operators can + override via the ``LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS`` environment + variable (comma-separated list of absolute paths). + """ + override = os.getenv("LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS") + raw_dirs = ( + [d.strip() for d in override.split(",") if d.strip()] + if override + else list(_DEFAULT_OIDC_ALLOWED_CREDENTIAL_DIRS) + ) + return [os.path.realpath(d) for d in raw_dirs] + + +def _resolve_oidc_file_path(requested_path: str) -> str: + """ + Resolve ``requested_path`` and verify it falls within one of the allowed + credential directories. Raises ``ValueError`` otherwise. + """ + if not os.path.isabs(requested_path): + raise ValueError( + "oidc/file path must be absolute. Use the format " + "'oidc/file//var/run/secrets/' (note the leading slash " + "after 'oidc/file/')." + ) + resolved = os.path.realpath(requested_path) + for allowed in _get_oidc_allowed_credential_dirs(): + try: + if os.path.commonpath([resolved, allowed]) == allowed: + return resolved + except ValueError: + # commonpath raises when paths are on different drives (Windows); + # treat as not-matching and continue. + continue + raise ValueError( + "oidc/file path is outside the allowed credential directories. " + "Set LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS to extend the allowlist." + ) + def _get_oidc_http_handler(timeout: Optional[httpx.Timeout] = None) -> HTTPHandler: """ @@ -196,8 +242,9 @@ def get_secret( # noqa: PLR0915 oidc_token = f.read() return oidc_token elif oidc_provider == "file": - # Load token from a file - with open(oidc_aud, "r") as f: + # Load token from a file within an allowed credential directory. + safe_path = _resolve_oidc_file_path(oidc_aud) + with open(safe_path, "r") as f: oidc_token = f.read() return oidc_token elif oidc_provider == "env": diff --git a/tests/litellm_utils_tests/test_secret_manager.py b/tests/litellm_utils_tests/test_secret_manager.py index a1190193ea..de35caec3f 100644 --- a/tests/litellm_utils_tests/test_secret_manager.py +++ b/tests/litellm_utils_tests/test_secret_manager.py @@ -183,13 +183,14 @@ def test_oidc_env_variable(): del os.environ[env_var_name] -def test_oidc_file(): - # Create a temporary file - with tempfile.NamedTemporaryFile(mode="w+") as temp_file: +def test_oidc_file(monkeypatch): + # Create a temporary file inside a directory added to the allowlist. + with tempfile.TemporaryDirectory() as temp_dir: + monkeypatch.setenv("LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS", temp_dir) + temp_file_path = os.path.join(temp_dir, "token.txt") secret_value = "secret-" + uuid4().hex - temp_file.write(secret_value) - temp_file.flush() - temp_file_path = temp_file.name + with open(temp_file_path, "w") as temp_file: + temp_file.write(secret_value) secret_val = get_secret(f"oidc/file/{temp_file_path}") 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 bc3aec5899..097de13df1 100644 --- a/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py +++ b/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py @@ -450,10 +450,10 @@ async def test_test_model_connection_loads_config_from_router(): params["messages"] = [{"role": "user", "content": "test"}] return params - # Mock _resolve_os_environ_variables - def mock_resolve_os_environ(params): - return params - + # Mock _reject_os_environ_references + def mock_reject_os_environ(params): + return None + with patch( "litellm.proxy.proxy_server.prisma_client", mock_prisma_client, @@ -476,8 +476,8 @@ async def test_test_model_connection_loads_config_from_router(): "litellm.proxy.health_endpoints._health_endpoints._update_litellm_params_for_health_check", mock_update_params, ), patch( - "litellm.proxy.health_endpoints._health_endpoints._resolve_os_environ_variables", - mock_resolve_os_environ, + "litellm.proxy.health_endpoints._health_endpoints._reject_os_environ_references", + mock_reject_os_environ, ): # Call the endpoint with only model name (no credentials) result = await health_test_model_connection( diff --git a/tests/test_litellm/secret_managers/test_secret_managers_main.py b/tests/test_litellm/secret_managers/test_secret_managers_main.py index 4a6e303586..d90b68198b 100644 --- a/tests/test_litellm/secret_managers/test_secret_managers_main.py +++ b/tests/test_litellm/secret_managers/test_secret_managers_main.py @@ -199,9 +199,10 @@ def test_oidc_azure_ad_token_success(mock_get_azure_ad_token_provider, monkeypat mock_token_provider.assert_called_once_with() -def test_oidc_file_success(tmp_path): +def test_oidc_file_success(tmp_path, monkeypatch): token_file = tmp_path / "token.txt" token_file.write_text("file_token") + monkeypatch.setenv("LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS", str(tmp_path)) secret_name = f"oidc/file/{token_file}" result = get_secret(secret_name) @@ -209,6 +210,24 @@ def test_oidc_file_success(tmp_path): assert result == "file_token" +def test_oidc_file_rejects_path_outside_allowlist(tmp_path, monkeypatch): + outside_file = tmp_path / "outside.txt" + outside_file.write_text("should_not_read") + # Allowlist a different directory. + allowed_dir = tmp_path / "allowed" + allowed_dir.mkdir() + monkeypatch.setenv("LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS", str(allowed_dir)) + + with pytest.raises(ValueError, match="outside the allowed credential directories"): + get_secret(f"oidc/file/{outside_file}") + + +def test_oidc_file_rejects_relative_path(tmp_path, monkeypatch): + monkeypatch.setenv("LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS", str(tmp_path)) + with pytest.raises(ValueError, match="must be absolute"): + get_secret("oidc/file/relative/path/token") + + def test_oidc_env_success(mock_env): mock_env["CUSTOM_TOKEN"] = "env_token"