From 06a0d4498a03c7b0362fa12965edd7307c60648d Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Sat, 11 Apr 2026 21:41:41 -0700 Subject: [PATCH 1/3] fix: tighten handling of environment references in request parameters - Reject os.environ/ references supplied via /health/test_connection request params instead of resolving them; config-sourced values are already resolved before reaching the endpoint. - Skip os.environ/ references in dynamic callback params loaded from per-request metadata. - Constrain oidc/file/ to an allowed credential directory allowlist (defaults to /var/run/secrets and /run/secrets, overridable via LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS). --- docs/my-website/docs/oidc.md | 20 +++- .../initialize_dynamic_callback_params.py | 23 ++--- .../health_endpoints/_health_endpoints.py | 94 ++++++------------- litellm/secret_managers/main.py | 45 ++++++++- .../test_secret_manager.py | 13 +-- .../health_endpoints/test_health_endpoints.py | 10 +- .../test_secret_managers_main.py | 15 ++- 7 files changed, 128 insertions(+), 92 deletions(-) diff --git a/docs/my-website/docs/oidc.md b/docs/my-website/docs/oidc.md index 23eb431b7e..4ff045ada7 100644 --- a/docs/my-website/docs/oidc.md +++ b/docs/my-website/docs/oidc.md @@ -60,9 +60,27 @@ oidc/config_name_here/ For the unofficial `file` provider, you can use the following format: ``` -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/litellm/litellm_core_utils/initialize_dynamic_callback_params.py b/litellm/litellm_core_utils/initialize_dynamic_callback_params.py index ff521d4780..bb31ba7510 100644 --- a/litellm/litellm_core_utils/initialize_dynamic_callback_params.py +++ b/litellm/litellm_core_utils/initialize_dynamic_callback_params.py @@ -1,7 +1,10 @@ from typing import Dict, Optional -from litellm.secret_managers.main import get_secret_str 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 +49,10 @@ 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): + # Skip request-supplied environment references; these must + # come from server-side configuration only. + continue standard_callback_dynamic_params[param] = _param_value # type: ignore # 2. Fallback: check "metadata" or "litellm_params" -> "metadata" @@ -64,12 +65,8 @@ 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): + 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..2cb9a136e1 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -36,79 +36,45 @@ 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) -> dict: """ - 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, reject it. """ if not isinstance(params, dict): return params - # 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): + iterable = src.values() + elif isinstance(src, list): + iterable = 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 + for value in iterable: + 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) - 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 + return params def get_callback_identifier(callback): @@ -1510,6 +1476,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 +1516,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..10042b6d46 100644 --- a/litellm/secret_managers/main.py +++ b/litellm/secret_managers/main.py @@ -16,6 +16,46 @@ 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. + """ + 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 +236,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..7e647d7e9f 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): + # Mock _reject_os_environ_references + def mock_reject_os_environ(params): return params - + 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..89155d51b5 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,18 @@ 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_env_success(mock_env): mock_env["CUSTOM_TOKEN"] = "env_token" From 6baee0dfcbe39f4a789a4f09872f321d52414298 Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Sat, 11 Apr 2026 21:52:39 -0700 Subject: [PATCH 2/3] address review feedback - Log a warning when dropping callback params that carry os.environ/ references so operators notice the misconfiguration. - Require absolute paths in oidc/file/ and correct the documented example to use the leading-slash form. - Drop the unused return value from _reject_os_environ_references. --- docs/my-website/docs/oidc.md | 5 +++-- .../initialize_dynamic_callback_params.py | 17 +++++++++++++++-- .../proxy/health_endpoints/_health_endpoints.py | 8 +++----- litellm/secret_managers/main.py | 6 ++++++ .../health_endpoints/test_health_endpoints.py | 2 +- .../test_secret_managers_main.py | 6 ++++++ 6 files changed, 34 insertions(+), 10 deletions(-) diff --git a/docs/my-website/docs/oidc.md b/docs/my-website/docs/oidc.md index 4ff045ada7..c4b82a08d1 100644 --- a/docs/my-website/docs/oidc.md +++ b/docs/my-website/docs/oidc.md @@ -57,10 +57,11 @@ 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/var/run/secrets/my-token +oidc/file//var/run/secrets/my-token ``` For safety, the resolved path must live inside an allowed credential diff --git a/litellm/litellm_core_utils/initialize_dynamic_callback_params.py b/litellm/litellm_core_utils/initialize_dynamic_callback_params.py index bb31ba7510..b80907e8ec 100644 --- a/litellm/litellm_core_utils/initialize_dynamic_callback_params.py +++ b/litellm/litellm_core_utils/initialize_dynamic_callback_params.py @@ -1,4 +1,6 @@ from typing import Dict, Optional + +from litellm._logging import verbose_logger from litellm.types.utils import StandardCallbackDynamicParams @@ -50,8 +52,12 @@ def initialize_standard_callback_dynamic_params( if param in kwargs: _param_value = kwargs.get(param) if _is_env_reference(_param_value): - # Skip request-supplied environment references; these must - # come from server-side configuration only. + 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 @@ -66,6 +72,13 @@ def initialize_standard_callback_dynamic_params( if param not in standard_callback_dynamic_params and param in metadata: _param_value = metadata.get(param) 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 diff --git a/litellm/proxy/health_endpoints/_health_endpoints.py b/litellm/proxy/health_endpoints/_health_endpoints.py index 2cb9a136e1..a6caf9d080 100644 --- a/litellm/proxy/health_endpoints/_health_endpoints.py +++ b/litellm/proxy/health_endpoints/_health_endpoints.py @@ -40,15 +40,15 @@ from litellm.proxy.middleware.in_flight_requests_middleware import ( #### Health ENDPOINTS #### -def _reject_os_environ_references(params: dict) -> dict: +def _reject_os_environ_references(params: dict) -> None: """ 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, reject it. + request-supplied value still carries the prefix, raise ``HTTPException``. """ if not isinstance(params, dict): - return params + return stack: list[object] = [params] seen: set[int] = {id(params)} @@ -74,8 +74,6 @@ def _reject_os_environ_references(params: dict) -> dict: seen.add(id(value)) stack.append(value) - return params - def get_callback_identifier(callback): """ diff --git a/litellm/secret_managers/main.py b/litellm/secret_managers/main.py index 10042b6d46..a560f5222b 100644 --- a/litellm/secret_managers/main.py +++ b/litellm/secret_managers/main.py @@ -42,6 +42,12 @@ 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: 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 7e647d7e9f..097de13df1 100644 --- a/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py +++ b/tests/test_litellm/proxy/health_endpoints/test_health_endpoints.py @@ -452,7 +452,7 @@ async def test_test_model_connection_loads_config_from_router(): # Mock _reject_os_environ_references def mock_reject_os_environ(params): - return params + return None with patch( "litellm.proxy.proxy_server.prisma_client", 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 89155d51b5..d90b68198b 100644 --- a/tests/test_litellm/secret_managers/test_secret_managers_main.py +++ b/tests/test_litellm/secret_managers/test_secret_managers_main.py @@ -222,6 +222,12 @@ def test_oidc_file_rejects_path_outside_allowlist(tmp_path, monkeypatch): 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" From 41849a540d61a7f61a3f8ba0ddb8f5fcdf69c89e Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Sat, 11 Apr 2026 22:17:32 -0700 Subject: [PATCH 3/3] document new env var and fix type hint - Add LITELLM_OIDC_ALLOWED_CREDENTIAL_DIRS to the environment variables reference so the documentation test passes. - Annotate the values variable in _reject_os_environ_references so it accepts both dict.values() and list iterables. --- docs/my-website/docs/proxy/config_settings.md | 1 + litellm/proxy/health_endpoints/_health_endpoints.py | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) 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/proxy/health_endpoints/_health_endpoints.py b/litellm/proxy/health_endpoints/_health_endpoints.py index a6caf9d080..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 @@ -56,13 +56,13 @@ def _reject_os_environ_references(params: dict) -> None: while stack: src = stack.pop() if isinstance(src, dict): - iterable = src.values() + values: Iterable[object] = src.values() elif isinstance(src, list): - iterable = src + values = src else: continue - for value in iterable: + for value in values: if isinstance(value, str) and value.startswith("os.environ/"): raise HTTPException( status_code=400,