mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
fix(proxy): redact secrets on the db-config and litellm_settings log paths too
Reuse the existing recursive `_redact_secret_values_in_obj` for the worker config log instead of a hand-rolled top-level pass, so a credential nested under general_settings is masked at any depth and depth overrun fails closed. Route the `_update_config_from_db` param_value log (the store_model_in_db path) and the litellm_settings apply-loop log through the same redactors, so master_key, database_url, and secret-named settings such as api_key stop leaking at DEBUG when the module regex scrubber is bypassed. A plain setting like num_retries still logs its real value. Regression tests disable _ENABLE_SECRET_REDACTION and cover the nested worker config shape, the db-config path, and the litellm_settings loop in both directions.
This commit is contained in:
parent
680f15f1e3
commit
64d6a15182
3 changed files with 204 additions and 31 deletions
|
|
@ -654,29 +654,7 @@ _EXTRA_SECRET_GENERAL_SETTINGS_FIELDS = frozenset(
|
|||
)
|
||||
|
||||
|
||||
_EXTRA_SECRET_MASK = "REDACTED"
|
||||
|
||||
|
||||
def _redact_config_dict_for_logging(data: dict[str, object]) -> dict[str, object]:
|
||||
"""Mask secret-bearing entries in a proxy config-shaped dict.
|
||||
|
||||
Combines the segment-matching masker (catches `master_key`, `api_key`,
|
||||
`*_token`, etc.) with an explicit whole-value replacement for the
|
||||
URL/webhook fields that embed credentials but do not contain any
|
||||
sensitive-pattern segment (e.g. `database_url` splits into
|
||||
`['database', 'url']`, so segment matching misses it). The whole-value
|
||||
replacement covers non-string shapes (`alert_to_webhook_url` is a dict,
|
||||
`pass_through_endpoints` is a list), which the string-only
|
||||
`mask_sensitive_keys` helper would otherwise pass through unchanged.
|
||||
"""
|
||||
segment_masked = SENSITIVE_DATA_MASKER.mask_dict(data)
|
||||
return {
|
||||
key: _EXTRA_SECRET_MASK if key in _EXTRA_SECRET_GENERAL_SETTINGS_FIELDS and value is not None else value
|
||||
for key, value in segment_masked.items()
|
||||
}
|
||||
|
||||
|
||||
def _redact_worker_config_for_logging(worker_config: str | dict[str, object] | None) -> str | dict[str, object] | None:
|
||||
def _redact_worker_config_for_logging(worker_config: str | dict[str, JsonValue] | None) -> JsonValue:
|
||||
"""Mask sensitive fields in the worker config before it enters a log record.
|
||||
|
||||
`worker_config` reaches `proxy_startup_event` as either the JSON blob
|
||||
|
|
@ -690,10 +668,10 @@ def _redact_worker_config_for_logging(worker_config: str | dict[str, object] | N
|
|||
if worker_config is None:
|
||||
return None
|
||||
if isinstance(worker_config, dict):
|
||||
return _redact_config_dict_for_logging(worker_config)
|
||||
return _redact_secret_values_in_obj(worker_config)
|
||||
parsed = safe_json_loads(worker_config, default=None)
|
||||
if isinstance(parsed, dict):
|
||||
return safe_dumps(_redact_config_dict_for_logging(parsed))
|
||||
return safe_dumps(_redact_secret_values_in_obj(parsed))
|
||||
return worker_config
|
||||
|
||||
|
||||
|
|
@ -4333,7 +4311,9 @@ class ProxyConfig:
|
|||
raise Exception(
|
||||
f"team_id missing from default_team_settings at index={idx}\npassed in value={type(team_setting)}"
|
||||
)
|
||||
verbose_proxy_logger.debug(f"{blue_color_code} setting litellm.{key}={value}{reset_color_code}")
|
||||
verbose_proxy_logger.debug(
|
||||
f"{blue_color_code} setting litellm.{key}={_redact_general_setting_value(key, value, is_full_admin=False)}{reset_color_code}"
|
||||
)
|
||||
setattr(litellm, key, value)
|
||||
elif key == "upperbound_key_generate_params":
|
||||
if value is not None and isinstance(value, dict):
|
||||
|
|
@ -4348,7 +4328,9 @@ class ProxyConfig:
|
|||
litellm._turn_on_json()
|
||||
verbose_proxy_logger.debug(f"{blue_color_code} Enabled JSON logging via config{reset_color_code}")
|
||||
else:
|
||||
verbose_proxy_logger.debug(f"{blue_color_code} setting litellm.{key}={value}{reset_color_code}")
|
||||
verbose_proxy_logger.debug(
|
||||
f"{blue_color_code} setting litellm.{key}={_redact_general_setting_value(key, value, is_full_admin=False)}{reset_color_code}"
|
||||
)
|
||||
setattr(litellm, key, value)
|
||||
if key == "request_timeout":
|
||||
litellm.request_timeout_explicitly_set = True
|
||||
|
|
@ -5706,7 +5688,11 @@ class ProxyConfig:
|
|||
|
||||
param_name = getattr(response, "param_name", None)
|
||||
param_value = getattr(response, "param_value", None)
|
||||
verbose_proxy_logger.debug(f"param_name={param_name}, param_value={param_value}")
|
||||
verbose_proxy_logger.debug(
|
||||
"param_name=%s, param_value=%s",
|
||||
param_name,
|
||||
_redact_secret_values_in_obj(param_value) if isinstance(param_value, (dict, list)) else param_value,
|
||||
)
|
||||
|
||||
if param_name is not None and param_value is not None:
|
||||
config = self._update_config_fields(
|
||||
|
|
|
|||
|
|
@ -279,7 +279,7 @@ def test__redact_worker_config_for_logging_passthrough_for_none_and_non_json_str
|
|||
assert _redact_worker_config_for_logging("/tmp/some_config.yaml") == "/tmp/some_config.yaml"
|
||||
|
||||
|
||||
def test__redact_config_dict_for_logging_masks_non_string_url_webhook_values():
|
||||
def test__redact_worker_config_for_logging_masks_non_string_url_webhook_values():
|
||||
"""The URL/webhook fields the segment masker cannot catch by key name
|
||||
(``alert_to_webhook_url``, ``pass_through_endpoints``,
|
||||
``database_extra_connection_params``) can hold non-string shapes:
|
||||
|
|
@ -288,7 +288,7 @@ def test__redact_config_dict_for_logging_masks_non_string_url_webhook_values():
|
|||
the whole value is replaced regardless of shape so a nested webhook or
|
||||
Bearer token under a non-segment-matched key does not slip through.
|
||||
"""
|
||||
from litellm.proxy.proxy_server import _redact_config_dict_for_logging
|
||||
from litellm.proxy.proxy_server import _redact_worker_config_for_logging
|
||||
|
||||
nested_webhook_secret = "https://hooks.slack.com/services/T0/B0/nested-webhook-secret-xyz"
|
||||
data = {
|
||||
|
|
@ -303,7 +303,7 @@ def test__redact_config_dict_for_logging_masks_non_string_url_webhook_values():
|
|||
],
|
||||
"database_extra_connection_params": {"password": "extra-db-password-abc"},
|
||||
}
|
||||
redacted = _redact_config_dict_for_logging(data)
|
||||
redacted = _redact_worker_config_for_logging(data)
|
||||
rendered = repr(redacted)
|
||||
for secret in (
|
||||
"sk-should-be-masked",
|
||||
|
|
@ -314,6 +314,43 @@ def test__redact_config_dict_for_logging_masks_non_string_url_webhook_values():
|
|||
assert secret not in rendered, f"leak: {secret} in {rendered!r}"
|
||||
|
||||
|
||||
def test__redact_worker_config_for_logging_masks_nested_secret_fields():
|
||||
"""LIT-4152 nested regression: the URL/webhook credential fields the segment
|
||||
masker cannot catch by name (``database_url``,
|
||||
``database_extra_connection_params``, ``pass_through_endpoints``,
|
||||
``alert_to_webhook_url``) must be redacted at any depth, not just the top
|
||||
level. A worker_config that nests ``general_settings`` under a parent key
|
||||
must not leak a nested ``database_url`` or webhook secret; the earlier
|
||||
top-level-only redaction would have passed these through raw.
|
||||
"""
|
||||
from litellm.proxy.proxy_server import _redact_worker_config_for_logging
|
||||
|
||||
nested_db_url = "postgresql://nested_user:nested_pw_4152@nested-host:5432/db"
|
||||
nested_webhook = "https://hooks.slack.com/services/T0/B0/nested-4152-webhook"
|
||||
nested_extra_pw = "nested-extra-conn-pw-4152"
|
||||
nested_bearer = "Bearer nested-passthrough-token-4152"
|
||||
data = {
|
||||
"config": {
|
||||
"general_settings": {
|
||||
"database_url": nested_db_url,
|
||||
"database_extra_connection_params": {"password": nested_extra_pw},
|
||||
"alert_to_webhook_url": {"budget_alerts": nested_webhook},
|
||||
"pass_through_endpoints": [
|
||||
{"path": "/up", "headers": {"Authorization": nested_bearer}}
|
||||
],
|
||||
}
|
||||
}
|
||||
}
|
||||
redacted = _redact_worker_config_for_logging(data)
|
||||
rendered = repr(redacted)
|
||||
for secret in (nested_db_url, nested_webhook, nested_extra_pw, nested_bearer):
|
||||
assert secret not in rendered, f"nested leak: {secret} in {rendered!r}"
|
||||
|
||||
inner = redacted["config"]["general_settings"]
|
||||
assert inner["database_url"] == "REDACTED"
|
||||
assert inner["pass_through_endpoints"] == "REDACTED"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# initialize
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -1612,3 +1612,153 @@ def test_ProxyConfig__update_config_fields_invalid_param_raises():
|
|||
with pytest.raises(Exception):
|
||||
# Missing required arg.
|
||||
pc._update_config_fields(current_config={}, param_name="general_settings") # type: ignore[call-arg]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ProxyConfig._update_config_from_db
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ProxyConfig__update_config_from_db_does_not_log_general_settings_secrets(
|
||||
monkeypatch,
|
||||
):
|
||||
"""Regression for LIT-4152 on the store_model_in_db path.
|
||||
|
||||
``_update_config_from_db`` logged each DB ``param_value`` verbatim at DEBUG;
|
||||
for ``general_settings`` that value is the whole dict, leaking ``master_key``
|
||||
and ``database_url`` the same way the startup config load did. The value now
|
||||
routes through the recursive redactor. Asserted with the module regex
|
||||
scrubber (``_ENABLE_SECRET_REDACTION``) disabled so the caller itself must
|
||||
not build the leaky string. The merge into the returned config must still
|
||||
carry the raw values, proving only the log record is redacted.
|
||||
"""
|
||||
import logging
|
||||
|
||||
import litellm._logging as _logging_module
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
|
||||
monkeypatch.setattr(_logging_module, "_ENABLE_SECRET_REDACTION", False)
|
||||
|
||||
master_key_secret = "sk-lit4152-db-path-master-key-abcdef1234567890"
|
||||
db_url_secret = "postgresql://leak_user:leak_password_9090@leak-host.internal:5432/leak_db"
|
||||
nested_webhook_secret = "https://hooks.slack.com/services/T0/B0/db-path-webhook-secret"
|
||||
|
||||
responses = {
|
||||
"general_settings": SimpleNamespace(
|
||||
param_name="general_settings",
|
||||
param_value={
|
||||
"master_key": master_key_secret,
|
||||
"database_url": db_url_secret,
|
||||
"alert_to_webhook_url": {"budget_alerts": nested_webhook_secret},
|
||||
},
|
||||
),
|
||||
"router_settings": None,
|
||||
"litellm_settings": None,
|
||||
"environment_variables": None,
|
||||
}
|
||||
|
||||
async def _fake_get_config_param(prisma_client, key):
|
||||
return responses[key]
|
||||
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.get_config_param", _fake_get_config_param
|
||||
)
|
||||
|
||||
class LogRecordHandler(logging.Handler):
|
||||
def __init__(self) -> None:
|
||||
super().__init__()
|
||||
self.records: list[logging.LogRecord] = []
|
||||
|
||||
def emit(self, record: logging.LogRecord) -> None:
|
||||
self.records.append(record)
|
||||
|
||||
handler = LogRecordHandler()
|
||||
handler.setLevel(logging.DEBUG)
|
||||
original_level = verbose_proxy_logger.level
|
||||
verbose_proxy_logger.setLevel(logging.DEBUG)
|
||||
verbose_proxy_logger.addHandler(handler)
|
||||
try:
|
||||
merged = await ProxyConfig()._update_config_from_db(
|
||||
prisma_client=MagicMock(),
|
||||
config={"general_settings": {}},
|
||||
store_model_in_db=True,
|
||||
)
|
||||
rendered = " ".join(record.getMessage() for record in handler.records)
|
||||
finally:
|
||||
verbose_proxy_logger.removeHandler(handler)
|
||||
verbose_proxy_logger.setLevel(original_level)
|
||||
|
||||
for secret in (
|
||||
master_key_secret,
|
||||
db_url_secret,
|
||||
nested_webhook_secret,
|
||||
"leak_password_9090",
|
||||
):
|
||||
assert secret not in rendered, f"leak: {secret} in {rendered!r}"
|
||||
assert merged["general_settings"]["master_key"] == master_key_secret
|
||||
assert merged["general_settings"]["database_url"] == db_url_secret
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ProxyConfig_load_config_redacts_secret_litellm_setting_keeps_plain(
|
||||
tmp_path, monkeypatch
|
||||
):
|
||||
"""Regression for LIT-4152 on the ``litellm_settings`` apply loop.
|
||||
|
||||
``load_config`` logged ``setting litellm.<key>=<value>`` verbatim at DEBUG,
|
||||
so a secret-bearing setting such as ``api_key`` leaked in cleartext. The
|
||||
value now routes through ``_redact_general_setting_value``. Crucially the
|
||||
redaction must be surgical: a secret-named key is masked, but a plain
|
||||
operational setting like ``num_retries`` must still log its real value, so
|
||||
the debug line keeps its signal. Asserted with the module regex scrubber
|
||||
(``_ENABLE_SECRET_REDACTION``) disabled.
|
||||
"""
|
||||
import logging
|
||||
|
||||
import litellm._logging as _logging_module
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
|
||||
monkeypatch.setattr(_logging_module, "_ENABLE_SECRET_REDACTION", False)
|
||||
|
||||
api_key_secret = "sk-lit4152-litellm-settings-secret-abcdef1234567890"
|
||||
f = tmp_path / "c.yaml"
|
||||
f.write_text(
|
||||
"model_list: []\n"
|
||||
"general_settings: {}\n"
|
||||
"litellm_settings:\n"
|
||||
f" api_key: {api_key_secret}\n"
|
||||
" num_retries: 7\n"
|
||||
)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", None)
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.store_model_in_db", False)
|
||||
monkeypatch.delenv("LITELLM_CONFIG_BUCKET_NAME", raising=False)
|
||||
|
||||
class LogRecordHandler(logging.Handler):
|
||||
def __init__(self) -> None:
|
||||
super().__init__()
|
||||
self.records: list[logging.LogRecord] = []
|
||||
|
||||
def emit(self, record: logging.LogRecord) -> None:
|
||||
self.records.append(record)
|
||||
|
||||
handler = LogRecordHandler()
|
||||
handler.setLevel(logging.DEBUG)
|
||||
original_level = verbose_proxy_logger.level
|
||||
original_api_key = getattr(litellm, "api_key", None)
|
||||
original_num_retries = getattr(litellm, "num_retries", None)
|
||||
verbose_proxy_logger.setLevel(logging.DEBUG)
|
||||
verbose_proxy_logger.addHandler(handler)
|
||||
try:
|
||||
await ProxyConfig().load_config(router=None, config_file_path=str(f))
|
||||
rendered = " ".join(record.getMessage() for record in handler.records)
|
||||
finally:
|
||||
verbose_proxy_logger.removeHandler(handler)
|
||||
verbose_proxy_logger.setLevel(original_level)
|
||||
litellm.api_key = original_api_key
|
||||
litellm.num_retries = original_num_retries
|
||||
|
||||
assert api_key_secret not in rendered, f"api_key leaked in logs: {rendered!r}"
|
||||
assert "num_retries=7" in rendered, (
|
||||
f"non-secret num_retries value was over-redacted; expected it visible in {rendered!r}"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue