mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
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.
This commit is contained in:
parent
1ff1557b96
commit
0e1d0f4742
2 changed files with 171 additions and 23 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue