From 64d6a1518238c71dabe1fd9b181a21e82ffeee8e Mon Sep 17 00:00:00 2001 From: yucheng-berriai Date: Thu, 2 Jul 2026 11:24:58 -0700 Subject: [PATCH] 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. --- litellm/proxy/proxy_server.py | 42 ++--- .../proxy/proxy_server/test_lifecycle.py | 43 ++++- .../proxy/proxy_server/test_proxy_config.py | 150 ++++++++++++++++++ 3 files changed, 204 insertions(+), 31 deletions(-) diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index eda7a1b507a..e7ccfc883c3 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -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( diff --git a/tests/test_litellm/proxy/proxy_server/test_lifecycle.py b/tests/test_litellm/proxy/proxy_server/test_lifecycle.py index 842b372ba83..a3f5049ef1d 100644 --- a/tests/test_litellm/proxy/proxy_server/test_lifecycle.py +++ b/tests/test_litellm/proxy/proxy_server/test_lifecycle.py @@ -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 # --------------------------------------------------------------------------- diff --git a/tests/test_litellm/proxy/proxy_server/test_proxy_config.py b/tests/test_litellm/proxy/proxy_server/test_proxy_config.py index 78ec2813c61..08b825f41d4 100644 --- a/tests/test_litellm/proxy/proxy_server/test_proxy_config.py +++ b/tests/test_litellm/proxy/proxy_server/test_proxy_config.py @@ -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.=`` 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}" + )