diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 126988fdc6d..607fc2f783d 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -8501,7 +8501,7 @@ class ProxyConfig: unmanaged: Final = frozenset(self.settings.unmanaged_db_keys()) for key in sorted(unmanaged - self._warned_unmanaged_keys): verbose_proxy_logger.warning( - "general_settings.%s is stored in the database, but no settings API manages it, so the stored value " + "general_settings.%s is stored in the database, but the proxy does not apply that stored value, so it " "is ignored. %s", key, "The config file's value applies. Remove the stored copy with POST /config/field/delete." @@ -20411,7 +20411,9 @@ async def _persist_general_settings_ui_litellm_field( return {"message": f"Field {field_name} updated", "status": "success"} -async def _reset_general_settings_ui_litellm_field(field_name: str, user_api_key_dict: UserAPIKeyAuth) -> dict: +async def _reset_general_settings_ui_litellm_field( + field_name: str, user_api_key_dict: UserAPIKeyAuth +) -> dict[str, str]: default_value: Final = _general_settings_ui_litellm_default(_GENERAL_SETTINGS_UI_LITELLM_FIELDS[field_name]) proxy_config.reject_config_owned_writes(section_name="litellm_settings", changed_keys={field_name: default_value}) config: Final = await proxy_config.get_config() @@ -20637,18 +20639,15 @@ async def delete_config_general_settings( ) if data.field_name in _GENERAL_SETTINGS_UI_LITELLM_FIELDS: - return await _reset_general_settings_ui_litellm_field(data.field_name, user_api_key_dict) + reset: Final = await _reset_general_settings_ui_litellm_field(data.field_name, user_api_key_dict) + stored_copy: Final = await _stored_general_settings() + if stored_copy is not None and data.field_name in stored_copy: + await _remove_stored_general_setting(data.field_name, stored_copy, user_api_key_dict) + return reset - db_general_settings: Final[_ConfigParamRow | None] = await _config_param_table(prisma_client).find_first( - where={"param_name": "general_settings"} - ) - stored_keys: Final[frozenset[str]] = ( - frozenset(dict(db_general_settings.param_value)) - if db_general_settings is not None and db_general_settings.param_value is not None - else frozenset() - ) + stored: Final = await _stored_general_settings() - if data.field_name not in ConfigGeneralSettings.model_fields and data.field_name not in stored_keys: + if data.field_name not in ConfigGeneralSettings.model_fields and (stored is None or data.field_name not in stored): raise HTTPException( status_code=400, detail={"error": f"Invalid field={data.field_name} passed in."}, @@ -20657,39 +20656,49 @@ async def delete_config_general_settings( if data.field_name in _db_managed_general_settings_keys(): proxy_config.reject_config_owned_deletes(section_name="general_settings", keys=(data.field_name,)) - if db_general_settings is None or db_general_settings.param_value is None: + if stored is None: raise HTTPException( status_code=400, detail={"error": f"Field name={data.field_name} not in config"}, ) - else: - general_settings = dict(db_general_settings.param_value) - before_general_settings: Final = copy.deepcopy(general_settings) + return await _remove_stored_general_setting(data.field_name, stored, user_api_key_dict) - ## update db - general_settings.pop(data.field_name, None) +async def _stored_general_settings() -> Mapping[str, SettingsJsonValue] | None: + config_table: Final[TableActions[_ConfigParamValueRow]] = _config_param_table(prisma_client) + row: Final = await config_table.find_first(where={"param_name": "general_settings"}) + if row is None or row.param_value is None: + return None + value: Final = ( + cast(object, json.loads(row.param_value)) # cast-ok: json.loads is untyped; the row is opaque JSON + if isinstance(row.param_value, str) + else row.param_value + ) + return _SETTINGS_MAPPING.validate_python(value) if isinstance(value, Mapping) else None + +async def _remove_stored_general_setting( + field_name: str, stored: Mapping[str, SettingsJsonValue], user_api_key_dict: UserAPIKeyAuth +) -> _ConfigParamValueRow: + remaining: Final = {key: value for key, value in stored.items() if key != field_name} response: Final = await ConfigRepository(prisma_client).table.upsert( where={"param_name": "general_settings"}, data={ "create": { "param_name": "general_settings", - "param_value": json.dumps(general_settings), + "param_value": json.dumps(remaining), }, - "update": {"param_value": json.dumps(general_settings)}, + "update": {"param_value": json.dumps(remaining)}, }, ) await invalidate_config_param("general_settings") - proxy_config.settings.apply_db_row("general_settings", general_settings) - if is_resource_list("general_settings", data.field_name): - stored_endpoints: Final = general_settings.get("pass_through_endpoints") + proxy_config.settings.apply_db_row("general_settings", remaining) + if is_resource_list("general_settings", field_name): + stored_endpoints: Final = remaining.get("pass_through_endpoints") await proxy_config.serve_pass_through_endpoints(stored_endpoints if isinstance(stored_endpoints, list) else ()) asyncio.create_task( - create_config_audit_log( - "general_settings", "deleted", before_general_settings, general_settings, user_api_key_dict - ) + create_config_audit_log("general_settings", "deleted", dict(stored), remaining, user_api_key_dict) ) return response diff --git a/tests/integration/configuration/test_effective_settings.py b/tests/integration/configuration/test_effective_settings.py index ca016755ef5..1b0086c619a 100644 --- a/tests/integration/configuration/test_effective_settings.py +++ b/tests/integration/configuration/test_effective_settings.py @@ -1754,4 +1754,4 @@ def test_a_stored_general_setting_no_settings_api_manages_stays_off_and_the_rest if entry["field_name"] == "store_prompts_in_spend_logs" ) assert (stored_prompts["field_value"], stored_prompts["source"]) == (True, "db") - assert "general_settings.enforce_user_param is stored in the database, but no settings API manages it" in log + assert "general_settings.enforce_user_param is stored in the database, but the proxy does not apply" in log diff --git a/tests/unit/proxy/proxy_server/test_proxy_config.py b/tests/unit/proxy/proxy_server/test_proxy_config.py index b3718028de5..a211b4ed920 100644 --- a/tests/unit/proxy/proxy_server/test_proxy_config.py +++ b/tests/unit/proxy/proxy_server/test_proxy_config.py @@ -99,7 +99,9 @@ def test_proxy_config_upserts_existing_deployment_without_duplicating_it(monkeyp @pytest.mark.asyncio -async def test_proxy_config_preserves_models_when_config_read_fails(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: +async def test_proxy_config_preserves_models_when_config_read_fails( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: from litellm.proxy import proxy_server router = litellm.Router(model_list=[_deployment("config-deployment").to_json(exclude_none=True)]) @@ -132,6 +134,8 @@ async def test_proxy_config_keeps_database_models_and_deletes_stale_models( assert desired == frozenset({"kept-deployment"}) assert router.get_model_ids() == ["kept-deployment"] + + from tests._master_key import MASTER_KEY @@ -147,7 +151,6 @@ async def test_proxy_config_loads_lens_store_from_yaml(tmp_path, monkeypatch) -> assert is_lens_tracing_enabled(settings["tracing"], {}) is True - @pytest.mark.asyncio @pytest.mark.parametrize("shutdown_error", [False, True]) async def test_tracing_config_automatically_exports_spend_without_a_storage_dependency( @@ -173,7 +176,9 @@ async def test_tracing_config_automatically_exports_spend_without_a_storage_depe outcome: Final = pytest.raises(RuntimeError, match="shutdown failure") if shutdown_error else nullcontext() with outcome: async with manage_tracing(enabled=True, client_factory=client): - logger: Final = next(callback for callback in litellm._async_success_callback if isinstance(callback, LensExporter)) + logger: Final = next( + callback for callback in litellm._async_success_callback if isinstance(callback, LensExporter) + ) await logger.async_log_success_event( {"standard_logging_object": {"id": "response-1", "response_cost": 0.25}}, None, None, None ) @@ -2579,7 +2584,6 @@ async def test_load_config_rejects_non_boolean_deny_by_default( await ProxyConfig().load_config(router=None, config_file_path=str(config_file)) - @pytest.mark.asyncio @pytest.mark.parametrize( "yaml_value", ["files-api-2025-04-14", "[files-api-2025-04-14, 7]"], ids=["bare-string", "non-string-entry"] @@ -2599,6 +2603,7 @@ async def test_load_config_rejects_a_malformed_vertex_drop_anthropic_betas_list( with pytest.raises(ValidationError, match="vertex_ai_pass_through_drop_anthropic_betas"): await ProxyConfig().load_config(router=None, config_file_path=str(config_file)) + @pytest.mark.asyncio async def test_ProxyConfig_load_config_resolves_router_settings_plugins(tmp_path, monkeypatch): """Regression: router_settings.plugins dotted-path strings must be resolved to @@ -4372,8 +4377,8 @@ async def test_ProxyConfig__update_general_settings_ignores_a_stored_key_no_sett assert "enforce_user_param" not in proxy_server.general_settings warnings: Final = [record.getMessage() for record in caplog.records if "enforce_user_param" in record.getMessage()] assert warnings == [ - "general_settings.enforce_user_param is stored in the database, but no settings API manages it, so the " - "stored value is ignored. Set it in the config file to use it, or remove it with POST /config/field/delete." + "general_settings.enforce_user_param is stored in the database, but the proxy does not apply that stored " + "value, so it is ignored. Set it in the config file to use it, or remove it with POST /config/field/delete." ] @@ -4392,8 +4397,8 @@ async def test_ProxyConfig__update_general_settings_warns_about_an_ignored_store assert proxy_server.general_settings["enforce_user_param"] is False warnings: Final = [record.getMessage() for record in caplog.records if "enforce_user_param" in record.getMessage()] assert warnings == [ - "general_settings.enforce_user_param is stored in the database, but no settings API manages it, so the " - "stored value is ignored. The config file's value applies. Remove the stored copy with POST " + "general_settings.enforce_user_param is stored in the database, but the proxy does not apply that stored " + "value, so it is ignored. The config file's value applies. Remove the stored copy with POST " "/config/field/delete." ] @@ -5008,7 +5013,9 @@ async def test_ProxyConfig__update_config_from_db_keeps_keys_the_config_file_omi @pytest.mark.asyncio @pytest.mark.parametrize("ui_settings_already_synced", [False, True]) -async def test_ProxyConfig_add_deployment_continues_after_null_pass_through_endpoints(monkeypatch, ui_settings_already_synced): +async def test_ProxyConfig_add_deployment_continues_after_null_pass_through_endpoints( + monkeypatch, ui_settings_already_synced +): from litellm.proxy import proxy_server pc = ProxyConfig() @@ -5213,9 +5220,7 @@ def revision_test_environment(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv(key) -def _environment_variables_row( - values: Mapping[str, str | None] | None, revision: int | None -) -> SimpleNamespace | None: +def _environment_variables_row(values: Mapping[str, str | None] | None, revision: int | None) -> SimpleNamespace | None: if values is None: return None return SimpleNamespace( @@ -5401,9 +5406,7 @@ async def test_environment_variables_row_read_before_an_overlapping_newer_sync_i await newer_sync_finished.wait() older_sync: Final = asyncio.create_task( - _sync_from( - pc, _config_database(read_older_row, AsyncMock(return_value=older_row), wait_for_the_newer_sync) - ) + _sync_from(pc, _config_database(read_older_row, AsyncMock(return_value=older_row), wait_for_the_newer_sync)) ) await older_environment_row_read.wait() await _sync_from(pc, _rows_database(newer_row, newer_row)) diff --git a/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py b/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py index f7f3b3a8c8b..501c551d6b8 100644 --- a/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py +++ b/tests/unit/proxy/test_proxy_server_endpoints_and_startup.py @@ -1792,9 +1792,7 @@ class _DatabaseWithOneStoredCredential: @pytest.mark.asyncio -@pytest.mark.parametrize( - "encrypted_with, asks_to_migrate", [(PUBLICLY_KNOWN_KEY, True), ("sk-some-other-key", False)] -) +@pytest.mark.parametrize("encrypted_with, asks_to_migrate", [(PUBLICLY_KNOWN_KEY, True), ("sk-some-other-key", False)]) async def test_proxy_startup_asks_to_migrate_only_when_the_database_holds_values_under_the_unsafe_key( monkeypatch, tmp_path, encrypted_with, asks_to_migrate ): @@ -1877,9 +1875,7 @@ async def test_proxy_startup_names_the_config_file_that_set_the_unsafe_key(monke from litellm.proxy.auth.master_key_boot_check import UnsafeMasterKeyError from litellm.proxy.proxy_server import proxy_startup_event - config_path, announced = _boot_with_general_settings( - monkeypatch, tmp_path, {"master_key": PUBLICLY_KNOWN_KEY} - ) + config_path, announced = _boot_with_general_settings(monkeypatch, tmp_path, {"master_key": PUBLICLY_KNOWN_KEY}) with pytest.raises(UnsafeMasterKeyError): async with proxy_startup_event(FastAPI()): @@ -8104,10 +8100,18 @@ async def test_update_general_settings_db_pass_through_endpoint_overrides_yaml_e request.headers = {} request.query_params = {} - settings: Final = patch("litellm.proxy.proxy_server.general_settings", {"pass_through_endpoints": [yaml_endpoint]}) # test-quality-ok: the method reads this module global; no injection seam - yaml_endpoints: Final = patch("litellm.proxy.proxy_server.config_passthrough_endpoints", [yaml_endpoint]) # test-quality-ok: module global holding the YAML endpoints the fix merges in - initialize: Final = patch("litellm.proxy.proxy_server.initialize_pass_through_endpoints", AsyncMock()) # test-quality-ok: route registration needs the FastAPI app; auth is the observable here - master_key: Final = patch("litellm.proxy.proxy_server.master_key", MASTER_KEY) # test-quality-ok: a set master key is what makes a missing Authorization header a 401 + settings: Final = patch( + "litellm.proxy.proxy_server.general_settings", {"pass_through_endpoints": [yaml_endpoint]} + ) # test-quality-ok: the method reads this module global; no injection seam + yaml_endpoints: Final = patch( + "litellm.proxy.proxy_server.config_passthrough_endpoints", [yaml_endpoint] + ) # test-quality-ok: module global holding the YAML endpoints the fix merges in + initialize: Final = patch( + "litellm.proxy.proxy_server.initialize_pass_through_endpoints", AsyncMock() + ) # test-quality-ok: route registration needs the FastAPI app; auth is the observable here + master_key: Final = patch( + "litellm.proxy.proxy_server.master_key", MASTER_KEY + ) # test-quality-ok: a set master key is what makes a missing Authorization header a 401 with settings, yaml_endpoints, initialize, master_key: await ProxyConfig()._update_general_settings(db_general_settings={"pass_through_endpoints": [db_endpoint]}) @@ -12220,8 +12224,6 @@ async def test_update_config_field_prompt_caching_rejects_invalid(monkeypatch, f async def test_reset_config_field_restores_type_default(monkeypatch, field_name, expected_default): """Reset must restore each field's own default. Blanket None would leave the boolean flag set to None, which is not a bool and would read as neither on nor off.""" - from unittest.mock import MagicMock - import litellm.proxy.proxy_server as ps from litellm.proxy._types import ( ConfigFieldDelete, @@ -12238,9 +12240,10 @@ async def test_reset_config_field_restores_type_default(monkeypatch, field_name, async def fake_save_config(new_config=None): saved.update(new_config or {}) + fake_prisma = _fake_prisma_with_config({"store_prompts_in_spend_logs": True}) monkeypatch.setattr(ps.proxy_config, "get_config", fake_get_config) monkeypatch.setattr(ps.proxy_config, "save_config", fake_save_config) - monkeypatch.setattr(ps, "prisma_client", MagicMock()) + monkeypatch.setattr(ps, "prisma_client", fake_prisma) monkeypatch.setattr(litellm, "store_audit_logs", False) monkeypatch.setattr(litellm, field_name, "stale") @@ -12252,6 +12255,7 @@ async def test_reset_config_field_restores_type_default(monkeypatch, field_name, assert getattr(litellm, field_name) is expected_default assert field_name not in saved["litellm_settings"] + fake_prisma.db.litellm_config.upsert.assert_not_awaited() @pytest.mark.parametrize("bad_value", [0, -0.1, 1.5, True]) @@ -15470,6 +15474,90 @@ async def test_delete_config_general_settings_removes_an_ignored_stored_key_the_ assert pc.settings.unmanaged_db_keys() == () +@pytest.mark.asyncio +async def test_delete_config_general_settings_removes_a_stored_general_settings_copy_of_a_ui_litellm_field( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from litellm.proxy._types import ConfigFieldDelete + from litellm.proxy.proxy_server import ProxyConfig, delete_config_general_settings + + pc: Final = ProxyConfig() + pc._load_yaml_settings_stores({"general_settings": {}}) + stored_row: Final = {"budget_rollover": True, "store_prompts_in_spend_logs": True} + pc.settings.apply_db_row("general_settings", stored_row) + fake_prisma: Final = _fake_prisma_with_config(stored_row) + + async def litellm_settings_with_rollover() -> dict[str, dict[str, bool]]: + return {"litellm_settings": {"budget_rollover": True}} + + async def discard_save(new_config: object = None) -> None: + return None + + monkeypatch.setattr(pc, "get_config", litellm_settings_with_rollover) + monkeypatch.setattr(pc, "save_config", discard_save) + monkeypatch.setattr(proxy_server_module, "proxy_config", pc) + monkeypatch.setattr(proxy_server_module, "prisma_client", fake_prisma) + monkeypatch.setattr(litellm, "budget_rollover", True) + + admin: Final = UserAPIKeyAuth(api_key="hashed-admin", user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN) + await delete_config_general_settings( + data=ConfigFieldDelete(field_name="budget_rollover", config_type="general_settings"), + user_api_key_dict=admin, + ) + + written: Final = json.loads(fake_prisma.db.litellm_config.upsert.await_args.kwargs["data"]["update"]["param_value"]) + assert written == {"store_prompts_in_spend_logs": True} + assert litellm.budget_rollover is False + assert pc.settings.unmanaged_db_keys() == () + + +@pytest.mark.asyncio +async def test_delete_config_general_settings_refuses_an_unknown_key_when_the_stored_row_is_a_json_string( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from litellm.proxy._types import ConfigFieldDelete + from litellm.proxy.proxy_server import ProxyConfig, delete_config_general_settings + + pc: Final = ProxyConfig() + pc._load_yaml_settings_stores({"general_settings": {}}) + fake_prisma: Final = _fake_prisma_with_config('{"store_prompts_in_spend_logs": true}') + monkeypatch.setattr(proxy_server_module, "proxy_config", pc) + monkeypatch.setattr(proxy_server_module, "prisma_client", fake_prisma) + + admin: Final = UserAPIKeyAuth(api_key="hashed-admin", user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN) + with pytest.raises(HTTPException) as refused: + await delete_config_general_settings( + data=ConfigFieldDelete(field_name="not_a_setting", config_type="general_settings"), + user_api_key_dict=admin, + ) + + assert refused.value.status_code == 400 + fake_prisma.db.litellm_config.upsert.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_delete_config_general_settings_removes_a_key_from_a_json_string_row( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from litellm.proxy._types import ConfigFieldDelete + from litellm.proxy.proxy_server import ProxyConfig, delete_config_general_settings + + pc: Final = ProxyConfig() + pc._load_yaml_settings_stores({"general_settings": {}}) + fake_prisma: Final = _fake_prisma_with_config('{"max_request_size_mb": 42, "store_prompts_in_spend_logs": true}') + monkeypatch.setattr(proxy_server_module, "proxy_config", pc) + monkeypatch.setattr(proxy_server_module, "prisma_client", fake_prisma) + + admin: Final = UserAPIKeyAuth(api_key="hashed-admin", user_id="admin-1", user_role=LitellmUserRoles.PROXY_ADMIN) + await delete_config_general_settings( + data=ConfigFieldDelete(field_name="max_request_size_mb", config_type="general_settings"), + user_api_key_dict=admin, + ) + + written: Final = json.loads(fake_prisma.db.litellm_config.upsert.await_args.kwargs["data"]["update"]["param_value"]) + assert written == {"store_prompts_in_spend_logs": True} + + @pytest.mark.asyncio async def test_delete_config_general_settings_refuses_a_key_neither_managed_nor_stored( monkeypatch: pytest.MonkeyPatch,