Merge pull request #41931 from BerriAI/litellm_settings_store_immutable

fix(proxy): refuse runtime writes to config-owned settings

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(

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,

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()

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

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

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