From 7353b779c2198b24f9e7146751e449fe4d4e39ba Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Sat, 19 Sep 2026 10:17:30 -0700 Subject: [PATCH] feat(proxy): say when a stored setting is ignored because the config file owns it The config file winning over the database was silent. An admin who had set a value through the UI and later pinned the same key in the file saw their stored value quietly stop applying, with nothing said at boot and nothing said when a later write was refused. Startup now warns once per key whose stored value differs from the file's, naming the key and what to do about it. The refusal raised on a write to a config-owned key carries the same sentence, so the log and the 400 read identically, and both call out that a stored value exists and will never be applied. The /config/update refusal gained the same detail. Keys the file does not declare are untouched: the database still owns them, and a stored value equal to the file's is not worth a warning. --- litellm/proxy/config_resolvers/__init__.py | 4 +- .../proxy/config_resolvers/settings_store.py | 38 +++++++++++++--- litellm/proxy/proxy_server.py | 24 +++++++++- .../proxy_setting_endpoints.py | 6 +-- .../config_resolvers/test_settings_store.py | 44 +++++++++++++++++++ tests/test_litellm/proxy/test_proxy_server.py | 31 +++++++++++++ 6 files changed, 132 insertions(+), 15 deletions(-) diff --git a/litellm/proxy/config_resolvers/__init__.py b/litellm/proxy/config_resolvers/__init__.py index ebd339b34c3..eee760df458 100644 --- a/litellm/proxy/config_resolvers/__init__.py +++ b/litellm/proxy/config_resolvers/__init__.py @@ -5,6 +5,6 @@ from litellm.proxy.config_resolvers._descriptors import ( FieldSource, resolve_fields, ) -from litellm.proxy.config_resolvers.settings_store import SettingsStore +from litellm.proxy.config_resolvers.settings_store import SettingsStore, config_ownership_message -__all__ = ("FieldDescriptor", "FieldSource", "SettingsStore", "resolve_fields") +__all__ = ("FieldDescriptor", "FieldSource", "SettingsStore", "config_ownership_message", "resolve_fields") diff --git a/litellm/proxy/config_resolvers/settings_store.py b/litellm/proxy/config_resolvers/settings_store.py index 345f00c35a5..90f1da76bf6 100644 --- a/litellm/proxy/config_resolvers/settings_store.py +++ b/litellm/proxy/config_resolvers/settings_store.py @@ -19,10 +19,21 @@ from litellm.proxy.config_resolvers.settings_rules import ( 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") + def __init__(self, section: Section, key: str, *, shadows_db_value: bool = False) -> None: + super().__init__(config_ownership_message(section=section, key=key, shadows_db_value=shadows_db_value)) self.section: Final = section self.key: Final = key + self.shadows_db_value: Final = shadows_db_value + + +def config_ownership_message(*, section: Section, key: str, shadows_db_value: bool) -> str: + stored: Final = ( + " The value stored in the database for it is ignored and will never be applied." if shadows_db_value else "" + ) + return ( + f"{section}.{key} is set in the config file, so the config file owns it and it cannot be changed " + f"here.{stored} Edit the config file to change it, or remove it from the file to let the database own it." + ) _EMPTY_VALUES: Final[Mapping[str, JsonValue]] = MappingProxyType({}) @@ -54,6 +65,13 @@ class SettingsStore(MutableMapping[str, JsonValue]): ) ) + def shadowed_db_keys(self) -> tuple[str, ...]: + """Keys the config file owns whose stored value differs, so the stored one never reaches a reader.""" + return tuple(sorted(key for key in self._yaml_values if self._db_value_is_shadowed(key))) + + def shadows_db_value(self, key: str) -> bool: + return self.owned_by_config(key) and self._db_value_is_shadowed(key) + def apply_db_row(self, row: DbRow, db_row: Mapping[str, JsonValue]) -> None: previous_row: Final = self._database_rows.get(row, _EMPTY_VALUES) self._database_rows = MappingProxyType({**self._database_rows, row: MappingProxyType(dict(db_row))}) @@ -81,7 +99,7 @@ class SettingsStore(MutableMapping[str, JsonValue]): def __setitem__(self, key: str, value: JsonValue) -> None: if self.owned_by_config(key) and value != self.get(key): - raise ConfigOwnedKeyError(self._section, key) + raise ConfigOwnedKeyError(self._section, key, shadows_db_value=self._db_value_is_shadowed(key)) self._runtime_values = MappingProxyType({**self._runtime_values, key: value}) self._deleted_runtime_keys = self._deleted_runtime_keys - frozenset((key,)) @@ -89,7 +107,7 @@ class SettingsStore(MutableMapping[str, JsonValue]): if key not in self: raise KeyError(key) if self.owned_by_config(key): - raise ConfigOwnedKeyError(self._section, key) + raise ConfigOwnedKeyError(self._section, key, shadows_db_value=self._db_value_is_shadowed(key)) self._runtime_values = MappingProxyType( {key_: value for key_, value in self._runtime_values.items() if key_ != key} ) @@ -136,8 +154,14 @@ class SettingsStore(MutableMapping[str, JsonValue]): ) ) - def _resolution_for(self, key: str) -> Resolved: + def _db_value(self, key: str) -> SettingValue: rule: Final = rule_for(self._section, key) + return self._database_rows.get(rule.db_row, _EMPTY_VALUES).get(key, ABSENT) + + def _db_value_is_shadowed(self, key: str) -> bool: + db_value: Final = self._db_value(key) + return not isinstance(db_value, Absent) and db_value is not None and db_value != self.get(key) + + def _resolution_for(self, key: str) -> Resolved: yaml_value: Final[SettingValue] = self._yaml_values.get(key, ABSENT) - db_value: Final[SettingValue] = self._database_rows.get(rule.db_row, _EMPTY_VALUES).get(key, ABSENT) - return resolve(yaml_value, db_value) + return resolve(yaml_value, self._db_value(key)) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 6741b8b56c2..c6ce2e61b3c 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -447,7 +447,7 @@ from litellm.proxy.common_utils.user_api_key_cache import ( project_spend_counter_key, tag_cache_key, ) -from litellm.proxy.config_resolvers import SettingsStore, resolve_fields +from litellm.proxy.config_resolvers import SettingsStore, config_ownership_message, resolve_fields from litellm.proxy.config_resolvers.alerting import ( EMAIL_DESCRIPTORS, MS_TEAMS_DESCRIPTORS, @@ -4907,6 +4907,7 @@ class ProxyConfig: self.router_settings: Final[SettingsStore] = SettingsStore("router_settings") self.litellm_settings: Final[SettingsStore] = SettingsStore("litellm_settings") self.environment_variables: Final[SettingsStore] = SettingsStore("environment_variables") + self._warned_shadowed_keys: frozenset[tuple[Section, str]] = frozenset() self._settings_stores: Final[Mapping[Section, SettingsStore]] = MappingProxyType( { "general_settings": self.settings, @@ -5128,12 +5129,20 @@ class ProxyConfig: f"key '{rejected[0]}' is" if len(rejected) == 1 else f"keys {', '.join(repr(key) for key in rejected)} are" ) pronoun: Final = "it" if len(rejected) == 1 else "them" + shadowed: Final = tuple(key for key in rejected if store.shadows_db_value(key)) + stored: Final = ( + f" The {'value' if len(shadowed) == 1 else 'values'} already stored in the database for " + f"{', '.join(shadowed)} {'is' if len(shadowed) == 1 else 'are'} ignored and will never be applied." + if shadowed + else "" + ) raise HTTPException( status_code=400, detail={ - "error": f"{section_name} {subject} set in the config file and cannot be changed here", + "error": f"{section_name} {subject} set in the config file and cannot be changed here.{stored}", "keys": list(rejected), "section": section_name, + "stored_database_values_ignored": list(shadowed), "resolution": ( f"edit {user_config_file_path} to change {pronoun}, " f"or remove {pronoun} from the file to let the database own {pronoun}" @@ -7430,8 +7439,19 @@ class ProxyConfig: self._prepared_db_settings_values(section, param_value), ) + self._warn_about_shadowed_db_settings() return self._config_with_resolved_settings(config) + def _warn_about_shadowed_db_settings(self) -> None: + shadowed: Final[frozenset[tuple[Section, str]]] = frozenset( + (section, key) for section, store in self._settings_stores.items() for key in store.shadowed_db_keys() + ) + for section, key in sorted(shadowed - self._warned_shadowed_keys): + verbose_proxy_logger.warning( + "%s", config_ownership_message(section=section, key=key, shadows_db_value=True) + ) + self._warned_shadowed_keys = shadowed + def _prepared_db_settings_values(self, section: Section, value: object) -> Mapping[str, SettingsJsonValue]: if section == "environment_variables": decrypted: Final = self._decrypt_and_set_db_env_variables( diff --git a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py index 7c2abce60e2..b2baef126e9 100644 --- a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py +++ b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py @@ -497,12 +497,10 @@ def _store_allowed_ips(general_settings: MutableMapping[str, object], allowed_ip 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", + "error": str(owned), "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" - ), + "stored_database_value_ignored": owned.shadows_db_value, }, ) from owned 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 1182bcdce3c..daf6609325e 100644 --- a/tests/test_litellm/proxy/config_resolvers/test_settings_store.py +++ b/tests/test_litellm/proxy/config_resolvers/test_settings_store.py @@ -315,3 +315,47 @@ def test_settings_store_still_accepts_a_write_to_a_key_the_config_does_not_own() store["max_parallel_requests"] = 7 assert store["max_parallel_requests"] == 7 + + +def test_settings_store_reports_a_config_owned_key_whose_stored_value_differs() -> None: + store: Final = SettingsStore("general_settings") + store.load_yaml({"allowed_ips": ["1.2.3.4"]}) + store.apply_db_row("general_settings", {"allowed_ips": ["1.2.3.4", "5.6.7.8"], "max_parallel_requests": 7}) + + assert store.shadowed_db_keys() == ("allowed_ips",) + assert store.shadows_db_value("allowed_ips") is True + assert store.shadows_db_value("max_parallel_requests") is False + assert store["max_parallel_requests"] == 7 + + +def test_settings_store_reports_no_shadowing_when_the_stored_value_agrees() -> None: + store: Final = SettingsStore("general_settings") + store.load_yaml({"allowed_ips": ["1.2.3.4"]}) + store.apply_db_row("general_settings", {"allowed_ips": ["1.2.3.4"]}) + + assert store.shadowed_db_keys() == () + assert store.shadows_db_value("allowed_ips") is False + + +def test_settings_store_says_the_stored_value_is_ignored_when_it_refuses_a_write() -> None: + store: Final = SettingsStore("general_settings") + store.load_yaml({"allowed_ips": ["1.2.3.4"]}) + store.apply_db_row("general_settings", {"allowed_ips": ["1.2.3.4", "5.6.7.8"]}) + + with pytest.raises(ConfigOwnedKeyError) as refused: + store["allowed_ips"] = ["9.9.9.9"] + + assert refused.value.shadows_db_value is True + assert "stored in the database" in str(refused.value) + + +def test_settings_store_refusal_stays_quiet_about_the_database_when_nothing_is_stored() -> None: + store: Final = SettingsStore("general_settings") + store.load_yaml({"allowed_ips": ["1.2.3.4"]}) + + with pytest.raises(ConfigOwnedKeyError) as refused: + store["allowed_ips"] = ["9.9.9.9"] + + assert refused.value.shadows_db_value is False + assert "stored in the database" not in str(refused.value) + assert "config file" in str(refused.value) diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index 811d11bf0bf..935cc6ad8b7 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -5516,6 +5516,37 @@ async def test_router_settings_reload_keeps_db_values_writable(tmp_path, monkeyp assert proxy_config.router_settings.rejected_writes({"disable_cooldowns": False}) == ("disable_cooldowns",) +@pytest.mark.asyncio +async def test_boot_warns_that_a_shadowed_database_value_will_never_apply(tmp_path, monkeypatch, caplog): + from litellm.proxy.proxy_server import ProxyConfig + + config_path: Final = tmp_path / "config.yaml" + config_path.write_text( + yaml.safe_dump({"model_list": [], "general_settings": {"allowed_ips": ["1.2.3.4"], "max_file_size_mb": 5}}) + ) + db_row: Final = types.SimpleNamespace(param_value={"allowed_ips": ["1.2.3.4", "5.6.7.8"], "max_parallel_requests": 7}) + + async def read_config_row(_prisma_client, param_name): + return db_row if param_name == "general_settings" else None + + monkeypatch.setattr(proxy_server_module, "get_config_param", read_config_row) + monkeypatch.setattr(proxy_server_module, "prisma_client", MagicMock()) + monkeypatch.setattr(proxy_server_module, "store_model_in_db", True) + monkeypatch.setattr(proxy_server_module, "user_config_file_path", None) + proxy_config: Final = ProxyConfig() + + with caplog.at_level(logging.WARNING, logger="LiteLLM Proxy"): + await proxy_config.get_config(config_file_path=str(config_path)) + + warnings: Final = " ".join(record.getMessage() for record in caplog.records) + assert "allowed_ips" in warnings + assert "ignored" in warnings + assert "max_parallel_requests" not in warnings + assert "max_file_size_mb" not in warnings + assert proxy_config.settings["allowed_ips"] == ["1.2.3.4"] + assert proxy_config.settings["max_parallel_requests"] == 7 + + @pytest.mark.asyncio async def test_model_info_v1_oci_secrets_not_leaked(): """