diff --git a/litellm/proxy/config_resolvers/settings_store.py b/litellm/proxy/config_resolvers/settings_store.py index d4ca0e87d2b..345f00c35a5 100644 --- a/litellm/proxy/config_resolvers/settings_store.py +++ b/litellm/proxy/config_resolvers/settings_store.py @@ -17,6 +17,14 @@ from litellm.proxy.config_resolvers.settings_rules import ( rule_for, ) + +class ConfigOwnedKeyError(RuntimeError): + def __init__(self, section: Section, key: str) -> None: + super().__init__(f"{section}.{key} is set in the config file and cannot be changed at runtime") + self.section: Final = section + self.key: Final = key + + _EMPTY_VALUES: Final[Mapping[str, JsonValue]] = MappingProxyType({}) _EMPTY_ROWS: Final[Mapping[DbRow, Mapping[str, JsonValue]]] = MappingProxyType({}) @@ -72,8 +80,8 @@ class SettingsStore(MutableMapping[str, JsonValue]): return resolved.value def __setitem__(self, key: str, value: JsonValue) -> None: - if self.owned_by_config(key): - return + if self.owned_by_config(key) and value != self.get(key): + raise ConfigOwnedKeyError(self._section, key) self._runtime_values = MappingProxyType({**self._runtime_values, key: value}) self._deleted_runtime_keys = self._deleted_runtime_keys - frozenset((key,)) @@ -81,7 +89,7 @@ class SettingsStore(MutableMapping[str, JsonValue]): if key not in self: raise KeyError(key) if self.owned_by_config(key): - return + raise ConfigOwnedKeyError(self._section, key) self._runtime_values = MappingProxyType( {key_: value for key_, value in self._runtime_values.items() if key_ != key} ) @@ -109,12 +117,13 @@ class SettingsStore(MutableMapping[str, JsonValue]): self._deleted_runtime_keys = frozenset() def _clear_runtime_keys(self, keys: frozenset[str]) -> None: - if not keys: + stale: Final = frozenset(key for key in keys if not self.owned_by_config(key)) + if not stale: return self._runtime_values = MappingProxyType( - {key: value for key, value in self._runtime_values.items() if key not in keys} + {key: value for key, value in self._runtime_values.items() if key not in stale} ) - self._deleted_runtime_keys = self._deleted_runtime_keys - keys + self._deleted_runtime_keys = self._deleted_runtime_keys - stale def _keys(self) -> tuple[str, ...]: return tuple( diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 36fdea605c2..6741b8b56c2 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -5234,20 +5234,27 @@ class ProxyConfig: verbose_proxy_logger.warning("Maximum recursion depth (%s) reached while processing config.", max_depth) return config - for key, value in config.items(): - if isinstance(value, dict): - config[key] = self._check_for_os_environ_vars(config=value, depth=depth + 1, max_depth=max_depth) - elif isinstance(value, list): - for item in value: - if isinstance(item, dict): - item = self._check_for_os_environ_vars(config=item, depth=depth + 1, max_depth=max_depth) - # if the value is a string and starts with "os.environ/" - then it's an environment variable - elif isinstance(value, str) and value.startswith("os.environ/"): - resolved = get_secret(value) - if resolved is None and secret_manager_would_be_consulted(value): - verbose_proxy_logger.warning("%s is absent from the configured secret manager", value) - config[key] = resolved - return config + return { # mutable-ok: callers deep-copy and mutate this, and a mappingproxy cannot be deep-copied + key: self._resolved_config_value(value=value, depth=depth, max_depth=max_depth) + for key, value in config.items() + } + + def _resolved_config_value(self, value: object, depth: int, max_depth: int) -> object: + if isinstance(value, dict): + return self._check_for_os_environ_vars(config=value, depth=depth + 1, max_depth=max_depth) + if isinstance(value, list): + return [ # mutable-ok: config values round-trip through json, where a tuple is not a list + self._check_for_os_environ_vars(config=item, depth=depth + 1, max_depth=max_depth) + if isinstance(item, dict) + else item + for item in value + ] + if isinstance(value, str) and value.startswith("os.environ/"): + resolved: Final = get_secret(value) + if resolved is None and secret_manager_would_be_consulted(value): + verbose_proxy_logger.warning("%s is absent from the configured secret manager", value) + return resolved + return value def _initialize_secret_manager_from_raw_config( self, config: Mapping[str, object], config_file_path: str | None @@ -7339,7 +7346,9 @@ class ProxyConfig: "disable_auto_add_proxy_admin_to_teams", "apply_user_budget_to_team_keys", ): - if key in db_values and (value := self.settings.get(key)) is not None: + if key not in db_values or self.settings.owned_by_config(key): + continue + if (value := self.settings.get(key)) is not None: self.settings[key] = coerce_bool(value) async def _apply_cache_size_setting( @@ -7349,21 +7358,24 @@ class ProxyConfig: ) -> None: if "user_api_key_cache_max_size" not in db_values and not cache_size_was_db: return + writable: Final = not self.settings.owned_by_config("user_api_key_cache_max_size") cache_value: Final = self.settings.get("user_api_key_cache_max_size") try: cache_max_size: Final = ConfigGeneralSettings.model_validate( MappingProxyType({"user_api_key_cache_max_size": cache_value}) ).user_api_key_cache_max_size except ValidationError: - self.settings.pop("user_api_key_cache_max_size", None) + if writable: + self.settings.pop("user_api_key_cache_max_size", None) verbose_proxy_logger.warning( "Ignoring invalid general_settings.user_api_key_cache_max_size=%r from the DB", cache_value ) return - if cache_max_size is None: - self.settings.pop("user_api_key_cache_max_size", None) - else: - self.settings["user_api_key_cache_max_size"] = cache_max_size + if writable: + if cache_max_size is None: + self.settings.pop("user_api_key_cache_max_size", None) + else: + self.settings["user_api_key_cache_max_size"] = cache_max_size user_api_key_cache.update_in_memory_max_size(cache_max_size) async def _apply_store_model_in_db_setting(self, db_values: Mapping[str, SettingsJsonValue]) -> None: @@ -7375,7 +7387,8 @@ class ProxyConfig: return normalized: Final = coerce_bool(value) store_model_in_db = normalized if isinstance(normalized, bool) else bool(normalized) - self.settings["store_model_in_db"] = store_model_in_db + if not self.settings.owned_by_config("store_model_in_db"): + self.settings["store_model_in_db"] = store_model_in_db async def _apply_retention_settings( self, diff --git a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py index 75431383fbd..7c2abce60e2 100644 --- a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py +++ b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py @@ -3,7 +3,7 @@ import asyncio import json import os from collections import Counter -from collections.abc import Mapping, Sequence +from collections.abc import Mapping, MutableMapping, Sequence from types import MappingProxyType from typing import ( Final, @@ -24,6 +24,7 @@ from litellm.litellm_core_utils.sensitive_data_masker import mask_sensitive_keys from litellm.proxy._experimental.mcp_server.tool_search import MCP_TOOL_SEARCH_SETTINGS_KEY from litellm.proxy._types import * from litellm.proxy.auth.user_api_key_auth import user_api_key_auth +from litellm.proxy.config_resolvers.settings_store import ConfigOwnedKeyError from litellm.proxy.config_resolvers.sso import ( SSO_FIELD_ENV_VARS, SSO_SECRET_FIELDS, @@ -489,6 +490,23 @@ async def get_allowed_ips(): return {"data": _allowed_ip} +def _store_allowed_ips(general_settings: MutableMapping[str, object], allowed_ips: Sequence[str]) -> None: + try: + general_settings["allowed_ips"] = list(allowed_ips) # mutable-ok: compared against the file's own list + except ConfigOwnedKeyError as owned: + raise HTTPException( + status_code=400, + detail={ # mutable-ok: HTTPException serializes its detail as json + "error": f"{owned.section} key '{owned.key}' is set in the config file and cannot be changed here", + "keys": (owned.key,), + "section": owned.section, + "resolution": ( + "edit the config file to change it, or remove it from the file to let the database own it" + ), + }, + ) from owned + + @router.post( "/add/allowed_ip", tags=["Budget & Spend Tracking"], @@ -509,12 +527,10 @@ async def add_allowed_ip( if prisma_client is None: raise Exception("No DB Connected") - _allowed_ips: Final[list] = general_settings.get("allowed_ips", []) - if ip_address.ip not in _allowed_ips: - _allowed_ips.append(ip_address.ip) - general_settings["allowed_ips"] = _allowed_ips - else: + _allowed_ips: Final[Sequence[str]] = general_settings.get("allowed_ips") or () + if ip_address.ip in _allowed_ips: raise HTTPException(status_code=400, detail="IP address already exists") + _store_allowed_ips(general_settings, (*_allowed_ips, ip_address.ip)) if store_model_in_db is not True: raise HTTPException( @@ -568,12 +584,10 @@ async def delete_allowed_ip( proxy_config, ) - _allowed_ips: Final[list] = general_settings.get("allowed_ips", []) - if ip_address.ip in _allowed_ips: - _allowed_ips.remove(ip_address.ip) - general_settings["allowed_ips"] = _allowed_ips - else: + _allowed_ips: Final[Sequence[str]] = general_settings.get("allowed_ips") or () + if ip_address.ip not in _allowed_ips: raise HTTPException(status_code=404, detail="IP address not found") + _store_allowed_ips(general_settings, tuple(ip for ip in _allowed_ips if ip != ip_address.ip)) # Load existing config config: Final = await proxy_config.get_config() diff --git a/tests/test_litellm/proxy/config_resolvers/test_settings_store.py b/tests/test_litellm/proxy/config_resolvers/test_settings_store.py index 88ec382b013..1182bcdce3c 100644 --- a/tests/test_litellm/proxy/config_resolvers/test_settings_store.py +++ b/tests/test_litellm/proxy/config_resolvers/test_settings_store.py @@ -6,7 +6,7 @@ from unittest.mock import patch import pytest from litellm.proxy.config_resolvers.settings_rules import JsonValue -from litellm.proxy.config_resolvers.settings_store import SettingsStore +from litellm.proxy.config_resolvers.settings_store import ConfigOwnedKeyError, SettingsStore def test_settings_store_matches_plain_dict_mapping_operations() -> None: @@ -86,7 +86,6 @@ def test_settings_store_keeps_unaffected_runtime_values_on_a_db_row_refresh() -> def test_settings_store_keeps_a_config_owned_key_when_a_db_row_disagrees() -> None: store: Final = SettingsStore("general_settings") store.load_yaml({"changed": "config"}) - store.apply_runtime_values({"changed": "resolved-config"}) store.apply_db_row("general_settings", {"changed": "database"}) @@ -94,6 +93,17 @@ def test_settings_store_keeps_a_config_owned_key_when_a_db_row_disagrees() -> No assert store.source("changed") == "config" +def test_settings_store_keeps_the_resolved_value_of_a_config_owned_key_across_a_db_row() -> None: + store: Final = SettingsStore("general_settings") + store.load_yaml({"changed": "os.environ/SETTING"}) + store.apply_runtime_values({"changed": "resolved-config"}) + + store.apply_db_row("general_settings", {"changed": "database"}) + + assert store["changed"] == "resolved-config" + assert store.source("changed") == "config" + + def test_settings_store_removes_only_runtime_values_affected_by_a_cleared_db_row() -> None: store: Final = SettingsStore("general_settings") store.load_yaml({"template": "os.environ/SETTING"}) @@ -162,13 +172,27 @@ def test_settings_store_refuses_a_runtime_write_to_a_config_owned_key() -> None: store: Final = SettingsStore("general_settings") store.load_yaml({"max_parallel_requests": 3}) - store["max_parallel_requests"] = 11 - del store["max_parallel_requests"] + with pytest.raises(ConfigOwnedKeyError) as write: + store["max_parallel_requests"] = 11 + with pytest.raises(ConfigOwnedKeyError): + del store["max_parallel_requests"] + assert "max_parallel_requests" in str(write.value) assert store["max_parallel_requests"] == 3 assert store.source("max_parallel_requests") == "config" +def test_settings_store_accepts_a_write_that_does_not_change_a_config_owned_value() -> None: + store: Final = SettingsStore("general_settings") + store.load_yaml({"master_key": "os.environ/MASTER_KEY"}) + store.apply_runtime_values({"master_key": "sk-resolved"}) + + store["master_key"] = "sk-resolved" + + assert store["master_key"] == "sk-resolved" + assert store.source("master_key") == "config" + + @pytest.mark.timeout(10) def test_settings_store_clear_removes_every_key_the_config_file_does_not_own() -> None: store: Final = SettingsStore("general_settings") @@ -282,3 +306,12 @@ def test_settings_store_starts_with_an_unset_source() -> None: store: Final = SettingsStore("general_settings") assert store.source("unknown") == "unset" + + +def test_settings_store_still_accepts_a_write_to_a_key_the_config_does_not_own() -> None: + store: Final = SettingsStore("general_settings") + store.load_yaml({"allowed_ips": ["1.2.3.4"]}) + + store["max_parallel_requests"] = 7 + + assert store["max_parallel_requests"] == 7 diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index e2e2045e826..811d11bf0bf 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -3342,6 +3342,94 @@ async def test_load_config_rejects_malformed_role_permissions(tmp_path): await ProxyConfig().load_config(router=MagicMock(), config_file_path=str(config_file)) +def test_os_environ_resolution_leaves_the_config_layer_holding_the_reference(monkeypatch): + from litellm.proxy.proxy_server import ProxyConfig + + monkeypatch.setenv("PROOF_NESTED_SECRET", "sk-nested-value") + proxy_config: Final = ProxyConfig() + config: Final = { + "general_settings": { + "master_key": "os.environ/PROOF_NESTED_SECRET", + "coordination_redis": {"password": "os.environ/PROOF_NESTED_SECRET"}, + } + } + + proxy_config._load_yaml_settings_stores(config) + resolved: Final = proxy_config._check_for_os_environ_vars( + config=proxy_config._config_with_resolved_settings(config) + ) + + assert resolved["general_settings"]["coordination_redis"]["password"] == "sk-nested-value" + assert resolved["general_settings"]["master_key"] == "sk-nested-value" + assert proxy_config.settings.config_value("master_key") == "os.environ/PROOF_NESTED_SECRET" + assert proxy_config.settings.config_value("coordination_redis") == { + "password": "os.environ/PROOF_NESTED_SECRET" + } + + +def test_os_environ_resolution_reaches_dicts_nested_in_a_list(monkeypatch): + from litellm.proxy.proxy_server import ProxyConfig + + monkeypatch.setenv("PROOF_LIST_SECRET", "sk-list-value") + config: Final = {"model_list": [{"litellm_params": {"api_key": "os.environ/PROOF_LIST_SECRET"}}]} + + resolved: Final = ProxyConfig()._check_for_os_environ_vars(config=config) + + assert resolved["model_list"][0]["litellm_params"]["api_key"] == "sk-list-value" + + +@pytest.mark.parametrize("config_cache_size", ("not-a-number", "7")) +@pytest.mark.asyncio +async def test_db_reload_finishes_when_the_config_owns_a_setting_the_db_also_sets(monkeypatch, config_cache_size): + import litellm + from litellm.proxy import proxy_server as proxy_server_module + from litellm.proxy.proxy_server import ProxyConfig + + monkeypatch.setattr(litellm, "user_url_allowed_hosts", [], raising=False) + monkeypatch.setattr(proxy_server_module, "user_api_key_cache", MagicMock(), raising=False) + proxy_config: Final = ProxyConfig() + proxy_config.settings.load_yaml( + { + "store_prompts_in_spend_logs": "os.environ/PROOF_FLAG", + "store_model_in_db": "os.environ/PROOF_FLAG", + "user_api_key_cache_max_size": config_cache_size, + } + ) + monkeypatch.setattr(proxy_server_module, "general_settings", proxy_config.settings, raising=False) + + await proxy_config._update_general_settings( + { + "store_prompts_in_spend_logs": False, + "store_model_in_db": False, + "user_api_key_cache_max_size": 5, + "user_url_allowed_hosts": ["proof.example.com"], + } + ) + + assert litellm.user_url_allowed_hosts == ["proof.example.com"] + assert proxy_config.settings["store_prompts_in_spend_logs"] == "os.environ/PROOF_FLAG" + assert proxy_config.settings["store_model_in_db"] == "os.environ/PROOF_FLAG" + assert proxy_config.settings["user_api_key_cache_max_size"] == config_cache_size + + +@pytest.mark.asyncio +async def test_db_reload_keeps_the_resolved_value_of_a_config_owned_env_reference(monkeypatch): + from litellm.proxy import proxy_server as proxy_server_module + from litellm.proxy.proxy_server import ProxyConfig + + monkeypatch.setattr(proxy_server_module, "user_api_key_cache", MagicMock(), raising=False) + monkeypatch.setattr(proxy_server_module, "store_model_in_db", True, raising=False) + proxy_config: Final = ProxyConfig() + proxy_config.settings.load_yaml({"store_model_in_db": "os.environ/PROOF_STORE_FLAG"}) + proxy_config.settings.apply_runtime_values({"store_model_in_db": True}) + monkeypatch.setattr(proxy_server_module, "general_settings", proxy_config.settings, raising=False) + + await proxy_config._update_general_settings({"store_model_in_db": True}) + + assert proxy_config.settings["store_model_in_db"] is True + assert proxy_server_module.store_model_in_db is True + + def test_max_ui_session_budget_default_is_one_dollar(): """LIT-4662: the dashboard session budget default is a product decision; the old 0.25 default locked admins out of auto router Test Connection and the diff --git a/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py b/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py index 9860d1bf94a..58201bd14ce 100644 --- a/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py +++ b/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py @@ -2662,6 +2662,55 @@ def test_delete_allowed_ip_writes_deleted_audit_log(monkeypatch): app.dependency_overrides.pop(user_api_key_auth, None) +@pytest.mark.parametrize("route", ["/add/allowed_ip", "/delete/allowed_ip"]) +def test_allowed_ip_routes_refuse_a_config_owned_list_with_a_clear_400(route, monkeypatch): + from unittest.mock import AsyncMock, MagicMock + + import litellm.proxy.proxy_server as proxy_server_module + from litellm.proxy._types import UserAPIKeyAuth + from litellm.proxy.auth.user_api_key_auth import user_api_key_auth + from litellm.proxy.config_resolvers.settings_store import SettingsStore + + store = SettingsStore("general_settings") + store.load_yaml({"allowed_ips": ["203.0.113.77"]}) + saved = [] + + fake_prisma = MagicMock() + fake_prisma.db.litellm_auditlog.create = AsyncMock() + + async def _get_config(): + return {"general_settings": {"allowed_ips": ["203.0.113.77"]}} + + async def _save_config(new_config=None): + saved.append(new_config) + return new_config + + monkeypatch.setattr(proxy_server_module, "prisma_client", fake_prisma) + monkeypatch.setattr(proxy_server_module, "store_model_in_db", True) + monkeypatch.setattr(proxy_server_module, "general_settings", store) + monkeypatch.setattr(proxy_server_module.proxy_config, "get_config", _get_config) + monkeypatch.setattr(proxy_server_module.proxy_config, "save_config", _save_config) + + async def _admin_auth(): + return UserAPIKeyAuth( + user_id="config-admin", + api_key="hashed-admin-key", + user_role=LitellmUserRoles.PROXY_ADMIN, + ) + + app.dependency_overrides[user_api_key_auth] = _admin_auth + try: + ip = "198.51.100.9" if route == "/add/allowed_ip" else "203.0.113.77" + resp = client.post(route, json={"ip": ip}) + + assert resp.status_code == 400, resp.text + assert "allowed_ips" in resp.text + assert list(store["allowed_ips"]) == ["203.0.113.77"] + assert saved == [] + finally: + app.dependency_overrides.pop(user_api_key_auth, None) + + def test_update_ui_theme_settings_writes_audit_log(mock_proxy_config, monkeypatch): """Updating the UI theme must be audited under ui_theme_config.""" from unittest.mock import AsyncMock, MagicMock