From 0e1d0f474279bee96368e0d7ebe714188bf4d7f8 Mon Sep 17 00:00:00 2001 From: mubashir1osmani Date: Wed, 24 Jun 2026 19:19:08 -0700 Subject: [PATCH] fix(proxy): stop double-decrypting email/slack alerting env vars in get_config (#31117) * fix(proxy): stop double-decrypting email/slack alerting env vars in get_config proxy_config.get_config() already returns environment_variables decrypted (the DB overlay decrypts them in _update_config_fields, and YAML values are plaintext), so the /get/config/callbacks slack and email blocks were running decrypt_value_helper() a second time on plaintext. That second decrypt always failed and the helper swallowed the error and returned None, so every SMTP_* field came back blank when the Admin UI reloaded the email settings, and the proxy logged a misleading "Did your master_key/salt key change recently?" error even when nothing changed. Consume the already-decrypted values directly, matching process_callback's handling of the same dict for langfuse/datadog/etc. Sensitive-value masking is preserved. Fixes #19221 * fix(proxy): preserve a cleared slack webhook instead of falling back to OS env Use an explicit is-not-None guard rather than truthiness when deciding whether to fall back to os.getenv for SLACK_WEBHOOK_URL. With `or`, a webhook the admin cleared (stored as "") is falsy and would surface a stale SLACK_WEBHOOK_URL from the OS environment; only a truly absent key should trigger the OS lookup. No decryption is reintroduced. --- litellm/proxy/proxy_server.py | 34 ++-- tests/test_litellm/proxy/test_proxy_server.py | 160 ++++++++++++++++++ 2 files changed, 171 insertions(+), 23 deletions(-) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 50db86c59ef..59ddd0934d0 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -15666,8 +15666,6 @@ async def get_config(): """ global llm_router, llm_model_list, general_settings, proxy_config, proxy_logging_obj, master_key try: - import base64 - all_available_callbacks = AllCallbacks() config_data = await proxy_config.get_config() @@ -15733,18 +15731,14 @@ async def get_config(): _slack_vars = [ "SLACK_WEBHOOK_URL", ] - _slack_env_vars = {} - for _var in _slack_vars: - env_variable = environment_variables.get(_var, None) - if env_variable is None: - _value = os.getenv("SLACK_WEBHOOK_URL", None) - _slack_env_vars[_var] = _value - else: - # decode + decrypt the value - _decrypted_value = decrypt_value_helper( - value=env_variable, key=_var - ) - _slack_env_vars[_var] = _decrypted_value + _slack_env_vars = { + _var: ( + value + if (value := environment_variables.get(_var)) is not None + else os.getenv(_var) + ) + for _var in _slack_vars + } _slack_env_vars = mask_sensitive_keys( _slack_env_vars, _ALERTING_SENSITIVE_VARS ) @@ -15775,15 +15769,9 @@ async def get_config(): "EMAIL_LOGO_URL", "EMAIL_SUPPORT_CONTACT", ] - _email_env_vars = {} - for _var in _email_vars: - env_variable = environment_variables.get(_var, None) - if env_variable is None: - _email_env_vars[_var] = None - else: - # decode + decrypt the value - _decrypted_value = decrypt_value_helper(value=env_variable, key=_var) - _email_env_vars[_var] = _decrypted_value + _email_env_vars = { + _var: environment_variables.get(_var) for _var in _email_vars + } _email_env_vars = mask_sensitive_keys(_email_env_vars, _ALERTING_SENSITIVE_VARS) alerting_data.append( diff --git a/tests/test_litellm/proxy/test_proxy_server.py b/tests/test_litellm/proxy/test_proxy_server.py index a203fcc7ec0..fb951e922fc 100644 --- a/tests/test_litellm/proxy/test_proxy_server.py +++ b/tests/test_litellm/proxy/test_proxy_server.py @@ -857,6 +857,166 @@ def test_get_config_custom_callback_api_env_vars(monkeypatch): } +def test_get_config_returns_email_settings(monkeypatch): + """ + Regression for https://github.com/BerriAI/litellm/issues/19221 + + proxy_config.get_config() already returns environment_variables decrypted + (the DB-overlay path decrypts them, and YAML values are plaintext). The + /get/config/callbacks email block must therefore surface those values as-is + instead of decrypting a second time. The old code ran decrypt_value_helper() + on the already-plaintext value, which failed and returned None, so every + SMTP_* field came back blank on UI refresh. + """ + from litellm.proxy.proxy_server import app, proxy_config, user_api_key_auth + + smtp_password = "super-secret-app-password" + config_data = { + "litellm_settings": {}, + "general_settings": {"alerting": ["email"]}, + "environment_variables": { + "SMTP_HOST": "smtp.resend.com", + "SMTP_PORT": "587", + "SMTP_USERNAME": "resend", + "SMTP_PASSWORD": smtp_password, + "SMTP_SENDER_EMAIL": "alerts@example.com", + "TEST_EMAIL_ADDRESS": "admin@example.com", + }, + } + + mock_router = MagicMock() + mock_router.get_settings.return_value = {} + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", mock_router) + monkeypatch.setattr(proxy_config, "get_config", AsyncMock(return_value=config_data)) + + original_overrides = app.dependency_overrides.copy() + app.dependency_overrides[user_api_key_auth] = lambda: MagicMock() + + client = TestClient(app) + try: + response = client.get("/get/config/callbacks") + finally: + app.dependency_overrides = original_overrides + + assert response.status_code == 200 + email_alert = next( + (a for a in response.json()["alerts"] if a["name"] == "email"), None + ) + assert email_alert is not None + variables = email_alert["variables"] + + # Non-sensitive fields round-trip verbatim (None before the fix). + assert variables["SMTP_HOST"] == "smtp.resend.com" + assert variables["SMTP_PORT"] == "587" + assert variables["SMTP_USERNAME"] == "resend" + assert variables["SMTP_SENDER_EMAIL"] == "alerts@example.com" + assert variables["TEST_EMAIL_ADDRESS"] == "admin@example.com" + + # Password is present but masked: never None, never the raw secret. + assert variables["SMTP_PASSWORD"] is not None + assert variables["SMTP_PASSWORD"] != smtp_password + assert "*" in variables["SMTP_PASSWORD"] + + +def test_get_config_returns_slack_webhook(monkeypatch): + """ + Same double-decryption regression as the email block (issue #19221): the + slack alerting block must surface the already-decrypted SLACK_WEBHOOK_URL + rather than decrypting it again into None. + """ + from litellm.proxy.proxy_server import app, proxy_config, user_api_key_auth + + webhook_url = "https://hooks.slack.com/services/T00000/B00000/abcdefghijklmnop" + config_data = { + "litellm_settings": {}, + "general_settings": {"alerting": ["slack"]}, + "environment_variables": {"SLACK_WEBHOOK_URL": webhook_url}, + } + + mock_router = MagicMock() + mock_router.get_settings.return_value = {} + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", mock_router) + + mock_logging = MagicMock() + mock_logging.slack_alerting_instance.alert_types = ["budget_alerts"] + mock_logging.slack_alerting_instance._all_possible_alert_types.return_value = [ + "budget_alerts" + ] + mock_logging.slack_alerting_instance.alert_to_webhook_url = {} + monkeypatch.setattr("litellm.proxy.proxy_server.proxy_logging_obj", mock_logging) + monkeypatch.setattr(proxy_config, "get_config", AsyncMock(return_value=config_data)) + + original_overrides = app.dependency_overrides.copy() + app.dependency_overrides[user_api_key_auth] = lambda: MagicMock() + + client = TestClient(app) + try: + response = client.get("/get/config/callbacks") + finally: + app.dependency_overrides = original_overrides + + assert response.status_code == 200 + slack_alert = next( + (a for a in response.json()["alerts"] if a["name"] == "slack"), None + ) + assert slack_alert is not None + masked_url = slack_alert["variables"]["SLACK_WEBHOOK_URL"] + + # Masked, but derived from the real URL (None before the fix). + assert masked_url is not None + assert masked_url != webhook_url + assert masked_url.startswith("http") + assert "*" in masked_url + + +def test_get_config_cleared_slack_webhook_not_overridden_by_os_env(monkeypatch): + """ + A webhook the admin cleared is stored as "" in environment_variables. The + slack block must surface that empty value, not silently fall back to a + SLACK_WEBHOOK_URL still present in the OS environment (which truthiness-based + `or` would do). Only a truly absent key should trigger the os.getenv lookup. + """ + from litellm.proxy.proxy_server import app, proxy_config, user_api_key_auth + + monkeypatch.setenv( + "SLACK_WEBHOOK_URL", "https://hooks.slack.com/services/STALE/OS/ENVVALUE" + ) + config_data = { + "litellm_settings": {}, + "general_settings": {"alerting": ["slack"]}, + "environment_variables": {"SLACK_WEBHOOK_URL": ""}, + } + + mock_router = MagicMock() + mock_router.get_settings.return_value = {} + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", mock_router) + + mock_logging = MagicMock() + mock_logging.slack_alerting_instance.alert_types = ["budget_alerts"] + mock_logging.slack_alerting_instance._all_possible_alert_types.return_value = [ + "budget_alerts" + ] + mock_logging.slack_alerting_instance.alert_to_webhook_url = {} + monkeypatch.setattr("litellm.proxy.proxy_server.proxy_logging_obj", mock_logging) + monkeypatch.setattr(proxy_config, "get_config", AsyncMock(return_value=config_data)) + + original_overrides = app.dependency_overrides.copy() + app.dependency_overrides[user_api_key_auth] = lambda: MagicMock() + + client = TestClient(app) + try: + response = client.get("/get/config/callbacks") + finally: + app.dependency_overrides = original_overrides + + assert response.status_code == 200 + slack_alert = next( + (a for a in response.json()["alerts"] if a["name"] == "slack"), None + ) + assert slack_alert is not None + assert slack_alert["variables"]["SLACK_WEBHOOK_URL"] == "" + + # Mock Prisma class MockPrisma: def __init__(self, database_url=None, proxy_logging_obj=None, http_client=None):