From abbe5d7f8591c85f1ca7ebfa323480d5d7d57a1f Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Mon, 27 Apr 2026 14:50:59 -0700 Subject: [PATCH] fix(proxy): /config/update writes only sent sections, drop store_model_in_db gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The endpoint loaded the full merged YAML+DB config and re-saved every top-level section to LiteLLM_Config rows via save_config(), so a UI toggle of one field persisted unrelated YAML state to DB as a side effect. It also rejected every request when store_model_in_db was False — including the request that would flip the flag to True (chicken-and-egg). Replace save_config with targeted per-section upserts: read the existing litellm_config row, merge in the request, upsert just that row. Sections the caller did not send are not touched. Drop the blanket store_model_in_db guard — the endpoint already requires prisma_client, and the startup-side override at proxy_server.py:6491 picks up general_settings.store_model_in_db=True from the DB on next restart. --- litellm/proxy/proxy_server.py | 144 +++++------ tests/proxy_unit_tests/test_proxy_server.py | 47 ++-- .../test_update_config_endpoint.py | 241 ++++++++++++++++++ 3 files changed, 323 insertions(+), 109 deletions(-) create mode 100644 tests/test_litellm/proxy/management_endpoints/test_update_config_endpoint.py diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 3982d096e6..03bcd4ba1d 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -12557,9 +12557,12 @@ async def update_config( # noqa: PLR0915 user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), ): """ - For Admin UI - allows admin to update config via UI + For Admin UI - allows admin to update config via UI. - Currently supports modifying General Settings + LiteLLM settings + Writes only the sections present in the request body to LiteLLM_Config rows + (one row per top-level section). Sections the caller did not send are left + untouched — this endpoint never persists pre-existing YAML values to DB as + a side effect of an unrelated update. """ global llm_router, llm_model_list, general_settings, proxy_config, proxy_logging_obj, master_key, prisma_client try: @@ -12567,108 +12570,77 @@ async def update_config( # noqa: PLR0915 raise HTTPException( status_code=403, detail="Only proxy admins can update config" ) - import base64 - """ - - Update the ConfigTable DB - - Run 'add_deployment' - """ if prisma_client is None: raise Exception("No DB Connected") - if store_model_in_db is not True: - raise HTTPException( - status_code=500, - detail={ - "error": "Set `'STORE_MODEL_IN_DB='True'` in your env to enable this feature." + async def _read_section(param_name: str) -> dict: + row = await prisma_client.db.litellm_config.find_first( + where={"param_name": param_name} + ) + if row is None or row.param_value is None: + return {} + return dict(row.param_value) + + async def _upsert_section(param_name: str, value: dict) -> None: + serialized = json.dumps(value) + await prisma_client.db.litellm_config.upsert( + where={"param_name": param_name}, + data={ + "create": {"param_name": param_name, "param_value": serialized}, + "update": {"param_value": serialized}, }, ) - updated_settings = config_info.json(exclude_none=True) - updated_settings = prisma_client.jsonify_object(updated_settings) - for k, v in updated_settings.items(): - if k == "router_settings": - await prisma_client.db.litellm_config.upsert( - where={"param_name": k}, - data={ - "create": {"param_name": k, "param_value": v}, - "update": {"param_value": v}, - }, - ) - - ### OLD LOGIC [TODO] MOVE TO DB ### - - # Load existing config - config = await proxy_config.get_config() - verbose_proxy_logger.debug("Loaded config: %s", config) - - # update the general settings + # general_settings: merge per-key, with the alert_to_webhook_url side + # effect of auto-enabling slack alerting. if config_info.general_settings is not None: - config.setdefault("general_settings", {}) - updated_general_settings = config_info.general_settings.dict( - exclude_none=True - ) - - _existing_settings = config["general_settings"] - for k, v in updated_general_settings.items(): - # overwrite existing settings with updated values + existing = await _read_section("general_settings") + updates = config_info.general_settings.dict(exclude_none=True) + for k, v in updates.items(): if k == "alert_to_webhook_url": - # check if slack is already enabled. if not, enable it - if "alerting" not in _existing_settings: - _existing_settings = {"alerting": ["slack"]} - elif isinstance(_existing_settings["alerting"], list): - if "slack" not in _existing_settings["alerting"]: - _existing_settings["alerting"].append("slack") - _existing_settings[k] = v - config["general_settings"] = _existing_settings + if "alerting" not in existing: + existing["alerting"] = ["slack"] + elif ( + isinstance(existing["alerting"], list) + and "slack" not in existing["alerting"] + ): + existing["alerting"].append("slack") + existing[k] = v + await _upsert_section("general_settings", existing) + # environment_variables: encrypt request values, then merge into existing. if config_info.environment_variables is not None: - config.setdefault("environment_variables", {}) - _updated_environment_variables = config_info.environment_variables + existing = await _read_section("environment_variables") + for k, v in config_info.environment_variables.items(): + existing[k] = encrypt_value_helper(value=v) + await _upsert_section("environment_variables", existing) - # encrypt updated_environment_variables # - for k, v in _updated_environment_variables.items(): - encrypted_value = encrypt_value_helper(value=v) - _updated_environment_variables[k] = encrypted_value - - _existing_env_variables = config["environment_variables"] - - for k, v in _updated_environment_variables.items(): - # overwrite existing env variables with updated values - _existing_env_variables[k] = _updated_environment_variables[k] - - # update the litellm settings + # litellm_settings: existing-wins merge (preserving legacy behavior), + # except success_callback is unioned with the request value. if config_info.litellm_settings is not None: - config.setdefault("litellm_settings", {}) + existing = await _read_section("litellm_settings") updated_litellm_settings = config_info.litellm_settings - config["litellm_settings"] = { - **updated_litellm_settings, - **config["litellm_settings"], - } - - # if litellm.success_callback in updated_litellm_settings and config["litellm_settings"] + merged = {**updated_litellm_settings, **existing} if ( "success_callback" in updated_litellm_settings - and "success_callback" in config["litellm_settings"] + and "success_callback" in existing + and isinstance(existing["success_callback"], list) + and isinstance(updated_litellm_settings["success_callback"], list) ): - # check both success callback are lists - if isinstance( - config["litellm_settings"]["success_callback"], list - ) and isinstance(updated_litellm_settings["success_callback"], list): - updated_success_callbacks_normalized = normalize_callback_names( - updated_litellm_settings["success_callback"] - ) - combined_success_callback = ( - config["litellm_settings"]["success_callback"] - + updated_success_callbacks_normalized - ) - combined_success_callback = list(set(combined_success_callback)) - config["litellm_settings"][ - "success_callback" - ] = combined_success_callback + normalized = normalize_callback_names( + updated_litellm_settings["success_callback"] + ) + merged["success_callback"] = list( + set(existing["success_callback"] + normalized) + ) + await _upsert_section("litellm_settings", merged) - # Save the updated config - await proxy_config.save_config(new_config=config) + # router_settings: merge existing + request, request wins. + if config_info.router_settings is not None: + existing = await _read_section("router_settings") + updates = config_info.router_settings.dict(exclude_none=True) + await _upsert_section("router_settings", {**existing, **updates}) await proxy_config.add_deployment( prisma_client=prisma_client, proxy_logging_obj=proxy_logging_obj diff --git a/tests/proxy_unit_tests/test_proxy_server.py b/tests/proxy_unit_tests/test_proxy_server.py index c62c3f41b3..a4aab2ad18 100644 --- a/tests/proxy_unit_tests/test_proxy_server.py +++ b/tests/proxy_unit_tests/test_proxy_server.py @@ -2764,40 +2764,40 @@ async def test_update_config_success_callback_normalization(): import litellm.proxy.proxy_server as proxy_server from litellm.proxy._types import ConfigYAML - # Ensure feature is enabled and prisma_client is set - setattr(proxy_server, "store_model_in_db", True) setattr(proxy_server, "proxy_logging_obj", MagicMock()) + existing_litellm_settings = {"success_callback": ["langfuse"]} + + class FakeRow: + def __init__(self, name, value): + self.param_name = name + self.param_value = value + + upserted = {} + + async def fake_find_first(where=None): + if where and where.get("param_name") == "litellm_settings": + return FakeRow("litellm_settings", existing_litellm_settings) + return None + + async def fake_upsert(where=None, data=None): + upserted[where["param_name"]] = json.loads(data["update"]["param_value"]) + class MockPrisma: def __init__(self): self.db = MagicMock() self.db.litellm_config = MagicMock() - self.db.litellm_config.upsert = AsyncMock() - - # proxy_server.update_config expects this to be sync returning a dict - def jsonify_object(self, obj): - return obj + self.db.litellm_config.find_first = AsyncMock(side_effect=fake_find_first) + self.db.litellm_config.upsert = AsyncMock(side_effect=fake_upsert) setattr(proxy_server, "prisma_client", MockPrisma()) class MockProxyConfig: - def __init__(self): - self.saved_config = None - - async def get_config(self): - # Existing config has one lowercase callback already - return {"litellm_settings": {"success_callback": ["langfuse"]}} - - async def save_config(self, new_config: dict): - self.saved_config = new_config - async def add_deployment(self, prisma_client=None, proxy_logging_obj=None): return None - mock_proxy_config = MockProxyConfig() - setattr(proxy_server, "proxy_config", mock_proxy_config) + setattr(proxy_server, "proxy_config", MockProxyConfig()) - # Update config with mixed-case callbacks - expect normalization to lowercase config_update = ConfigYAML(litellm_settings={"success_callback": ["SQS", "sQs"]}) from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth @@ -2806,9 +2806,10 @@ async def test_update_config_success_callback_normalization(): ) await proxy_server.update_config(config_update, user_api_key_dict=admin_user) - saved = mock_proxy_config.saved_config - assert saved is not None, "save_config was not called" - callbacks = saved["litellm_settings"]["success_callback"] + assert ( + "litellm_settings" in upserted + ), "litellm_config.upsert was not called for litellm_settings" + callbacks = upserted["litellm_settings"]["success_callback"] # Deduped and normalized assert "sqs" in callbacks diff --git a/tests/test_litellm/proxy/management_endpoints/test_update_config_endpoint.py b/tests/test_litellm/proxy/management_endpoints/test_update_config_endpoint.py new file mode 100644 index 0000000000..0bc933145f --- /dev/null +++ b/tests/test_litellm/proxy/management_endpoints/test_update_config_endpoint.py @@ -0,0 +1,241 @@ +""" +Tests for the /config/update endpoint (litellm.proxy.proxy_server.update_config). + +These tests cover the targeted-per-section upsert behavior that replaced the +legacy save_config-based implementation, plus the removal of the +store_model_in_db blanket guard. +""" + +import json +import os +import sys +from unittest.mock import AsyncMock + +import pytest +from fastapi.testclient import TestClient + +sys.path.insert(0, os.path.abspath("../../../..")) + +from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth +from litellm.proxy.auth.user_api_key_auth import user_api_key_auth +from litellm.proxy.proxy_server import app + + +class FakeRow: + def __init__(self, param_name, param_value): + self.param_name = param_name + self.param_value = param_value + + +class FakeLitellmConfig: + def __init__(self, initial_rows=None): + self.rows = dict(initial_rows or {}) + self.upsert_calls = [] + self.find_first = AsyncMock(side_effect=self._find_first) + self.upsert = AsyncMock(side_effect=self._upsert) + + async def _find_first(self, where=None): + if where and "param_name" in where: + name = where["param_name"] + if name in self.rows: + return FakeRow(name, self.rows[name]) + return None + + async def _upsert(self, where=None, data=None): + name = where["param_name"] + # The endpoint calls upsert with json.dumps'd payloads. Some paths in + # the codebase pass dicts directly, so handle both. + raw = data["update"]["param_value"] + value = json.loads(raw) if isinstance(raw, str) else raw + self.rows[name] = value + self.upsert_calls.append((name, value)) + + +class FakeDB: + def __init__(self, initial_rows=None): + self.litellm_config = FakeLitellmConfig(initial_rows=initial_rows) + + +class FakePrismaClient: + def __init__(self, initial_rows=None): + self.db = FakeDB(initial_rows=initial_rows) + self.jsonify_object = lambda obj: obj + + +@pytest.fixture +def admin_auth(): + original = app.dependency_overrides.copy() + app.dependency_overrides[user_api_key_auth] = lambda: UserAPIKeyAuth( + user_id="test_admin", + user_role=LitellmUserRoles.PROXY_ADMIN, + api_key="sk-1234", + ) + yield + app.dependency_overrides = original + + +@pytest.fixture +def non_admin_auth(): + original = app.dependency_overrides.copy() + app.dependency_overrides[user_api_key_auth] = lambda: UserAPIKeyAuth( + user_id="test_user", + user_role=LitellmUserRoles.INTERNAL_USER, + api_key="sk-1234", + ) + yield + app.dependency_overrides = original + + +@pytest.fixture +def patched_proxy(monkeypatch): + """Returns a callable that installs a FakePrismaClient + no-op add_deployment.""" + + def _install(initial_rows=None): + prisma = FakePrismaClient(initial_rows=initial_rows) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", prisma) + # add_deployment is a coroutine method on proxy_config; replace with no-op + from litellm.proxy.proxy_server import proxy_config as real_proxy_config + + monkeypatch.setattr( + real_proxy_config, "add_deployment", AsyncMock(return_value=None) + ) + # Stub encrypt to be identity so we can assert on plain text + monkeypatch.setattr( + "litellm.proxy.proxy_server.encrypt_value_helper", + lambda value, **_: f"enc:{value}", + ) + return prisma + + return _install + + +def test_no_db_returns_500_class_error(admin_auth, monkeypatch): + """When prisma_client is None, the endpoint surfaces a ProxyException.""" + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", None) + client = TestClient(app) + resp = client.post( + "/config/update", json={"general_settings": {"store_model_in_db": True}} + ) + assert resp.status_code >= 400 + # 'No DB Connected' is the message raised inside the endpoint. + assert "No DB" in resp.text or "DB Connected" in resp.text + + +def test_non_admin_rejected(non_admin_auth, patched_proxy): + patched_proxy() + client = TestClient(app) + resp = client.post( + "/config/update", json={"general_settings": {"store_model_in_db": True}} + ) + assert resp.status_code in (401, 403) + + +def test_can_flip_store_model_in_db_when_currently_false( + admin_auth, patched_proxy, monkeypatch +): + """ + Regression: previously the endpoint refused all writes when the global + store_model_in_db flag was False, blocking even the request that would + flip it to True. After this fix, the request succeeds and the flag is + persisted to the general_settings row. + """ + monkeypatch.setattr("litellm.proxy.proxy_server.store_model_in_db", False) + prisma = patched_proxy() + + client = TestClient(app) + resp = client.post( + "/config/update", json={"general_settings": {"store_model_in_db": True}} + ) + + assert resp.status_code == 200 + assert ( + prisma.db.litellm_config.rows["general_settings"]["store_model_in_db"] is True + ) + + +def test_only_sent_section_is_written(admin_auth, patched_proxy): + """ + A request that only touches general_settings must not write + litellm_settings, environment_variables, or router_settings rows. + """ + prisma = patched_proxy( + initial_rows={ + "litellm_settings": {"drop_params": True}, + "environment_variables": {"FOO": "enc:bar"}, + } + ) + + client = TestClient(app) + resp = client.post( + "/config/update", + json={"general_settings": {"store_prompts_in_spend_logs": True}}, + ) + + assert resp.status_code == 200 + written_param_names = {name for name, _ in prisma.db.litellm_config.upsert_calls} + assert written_param_names == {"general_settings"} + # Untouched rows preserved. + assert prisma.db.litellm_config.rows["litellm_settings"] == {"drop_params": True} + assert prisma.db.litellm_config.rows["environment_variables"] == {"FOO": "enc:bar"} + + +def test_environment_variables_encrypted_before_write(admin_auth, patched_proxy): + prisma = patched_proxy() + client = TestClient(app) + resp = client.post( + "/config/update", + json={"environment_variables": {"OPENAI_API_KEY": "sk-secret"}}, + ) + + assert resp.status_code == 200 + stored = prisma.db.litellm_config.rows["environment_variables"] + assert stored == {"OPENAI_API_KEY": "enc:sk-secret"} + + +def test_success_callback_unioned_with_existing(admin_auth, patched_proxy): + prisma = patched_proxy( + initial_rows={"litellm_settings": {"success_callback": ["langfuse"]}} + ) + + client = TestClient(app) + resp = client.post( + "/config/update", + json={"litellm_settings": {"success_callback": ["prometheus"]}}, + ) + + assert resp.status_code == 200 + stored = prisma.db.litellm_config.rows["litellm_settings"]["success_callback"] + assert set(stored) == {"langfuse", "prometheus"} + + +def test_alert_to_webhook_url_enables_slack_alerting(admin_auth, patched_proxy): + prisma = patched_proxy() + client = TestClient(app) + resp = client.post( + "/config/update", + json={ + "general_settings": { + "alert_to_webhook_url": {"spend_reports": "https://hooks/foo"} + } + }, + ) + + assert resp.status_code == 200 + stored = prisma.db.litellm_config.rows["general_settings"] + assert stored["alerting"] == ["slack"] + assert stored["alert_to_webhook_url"] == {"spend_reports": "https://hooks/foo"} + + +def test_router_settings_merged_with_existing(admin_auth, patched_proxy): + prisma = patched_proxy( + initial_rows={"router_settings": {"num_retries": 3, "timeout": 10}} + ) + + client = TestClient(app) + resp = client.post("/config/update", json={"router_settings": {"num_retries": 5}}) + + assert resp.status_code == 200 + stored = prisma.db.litellm_config.rows["router_settings"] + # New value wins, untouched key preserved. + assert stored["num_retries"] == 5 + assert stored["timeout"] == 10