mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
Merge pull request #31944 from BerriAI/litellm_lit4152_key_url_redaction
fix(proxy): stop leaking master_key and database_url in startup DEBUG logs
This commit is contained in:
yuneng-jiang
GitHub
commit
4bd579cf6f
5 changed files with 477 additions and 34 deletions
|
|
@ -150,7 +150,7 @@ def decrypt_value_helper(
|
|||
verbose_proxy_logger.debug(error_message)
|
||||
return value if return_original_value else None
|
||||
|
||||
verbose_proxy_logger.debug(f"Unable to decrypt value={value} for key: {key}, returning None")
|
||||
verbose_proxy_logger.debug(f"Unable to decrypt value for key: {key}, returning None")
|
||||
if return_original_value:
|
||||
return value
|
||||
else:
|
||||
|
|
|
|||
|
|
@ -63,6 +63,7 @@ from litellm.litellm_core_utils.litellm_logging import (
|
|||
_init_custom_logger_compatible_class,
|
||||
)
|
||||
from litellm.litellm_core_utils.safe_json_dumps import safe_dumps
|
||||
from litellm.litellm_core_utils.safe_json_loads import safe_json_loads
|
||||
from litellm.proxy._types import (
|
||||
UI_TEAM_ID,
|
||||
CallbackDelete,
|
||||
|
|
@ -637,6 +638,43 @@ premium_user_data: Optional["EnterpriseLicenseData"] = _license_check.airgapped_
|
|||
global_max_parallel_request_retries_env: Optional[str] = os.getenv("LITELLM_GLOBAL_MAX_PARALLEL_REQUEST_RETRIES")
|
||||
proxy_state = ProxyState()
|
||||
SENSITIVE_DATA_MASKER = SensitiveDataMasker()
|
||||
|
||||
|
||||
# Secret-bearing general_settings fields the segment masker does not match by
|
||||
# name: database_url and database_extra_connection_params embed DB credentials,
|
||||
# pass_through_endpoints carry upstream Authorization headers, and
|
||||
# alert_to_webhook_url is itself a webhook secret
|
||||
_EXTRA_SECRET_GENERAL_SETTINGS_FIELDS = frozenset(
|
||||
{
|
||||
"database_url",
|
||||
"database_extra_connection_params",
|
||||
"pass_through_endpoints",
|
||||
"alert_to_webhook_url",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
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
|
||||
persisted by `save_worker_config` (a string) or the dict passed directly
|
||||
to `initialize`. Both shapes can carry `master_key`, `database_url`,
|
||||
provider API keys, etc.; passing the raw value to `verbose_proxy_logger`
|
||||
leaks them whenever the last-line-of-defense regex filter is bypassed
|
||||
(`LITELLM_DISABLE_REDACT_SECRETS=true`, an older log sink, a downstream
|
||||
handler that captures records pre-filter). Redact at the source.
|
||||
"""
|
||||
if worker_config is None:
|
||||
return None
|
||||
if isinstance(worker_config, dict):
|
||||
return _redact_secret_values_in_obj(worker_config)
|
||||
parsed = safe_json_loads(worker_config, default=None)
|
||||
if isinstance(parsed, dict):
|
||||
return safe_dumps(_redact_secret_values_in_obj(parsed))
|
||||
return worker_config
|
||||
|
||||
|
||||
if global_max_parallel_request_retries_env is None:
|
||||
global_max_parallel_request_retries: int = 3
|
||||
else:
|
||||
|
|
@ -833,7 +871,7 @@ async def proxy_startup_event(app: FastAPI):
|
|||
### LOAD CONFIG ###
|
||||
worker_config: Optional[Union[str, dict]] = get_secret("WORKER_CONFIG") # type: ignore
|
||||
env_config_yaml: Optional[str] = get_secret_str("CONFIG_FILE_PATH")
|
||||
verbose_proxy_logger.debug("worker_config: %s", worker_config)
|
||||
verbose_proxy_logger.debug("worker_config: %s", _redact_worker_config_for_logging(worker_config))
|
||||
# check if it's a valid file path
|
||||
if env_config_yaml is not None:
|
||||
if os.path.isfile(env_config_yaml) and proxy_config.is_yaml(config_file_path=env_config_yaml):
|
||||
|
|
@ -4273,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):
|
||||
|
|
@ -4288,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
|
||||
|
|
@ -4336,9 +4378,9 @@ class ProxyConfig:
|
|||
### CONNECT TO DATABASE ###
|
||||
database_url = general_settings.get("database_url", None)
|
||||
if database_url and database_url.startswith("os.environ/"):
|
||||
verbose_proxy_logger.debug("GOING INTO LITELLM.GET_SECRET!")
|
||||
verbose_proxy_logger.debug("Resolving database_url via secret manager")
|
||||
database_url = get_secret(database_url)
|
||||
verbose_proxy_logger.debug("RETRIEVED DB URL: %s", database_url)
|
||||
verbose_proxy_logger.debug("Resolved database_url from secret manager")
|
||||
### MASTER KEY ###
|
||||
master_key = general_settings.get("master_key", get_secret("LITELLM_MASTER_KEY", None))
|
||||
|
||||
|
|
@ -4738,7 +4780,7 @@ class ProxyConfig:
|
|||
"""
|
||||
|
||||
_alerting_callbacks = general_settings.get("alerting", None)
|
||||
verbose_proxy_logger.debug(f"_alerting_callbacks: {general_settings}")
|
||||
verbose_proxy_logger.debug("_alerting_callbacks: %s", _alerting_callbacks)
|
||||
if _alerting_callbacks is None:
|
||||
return
|
||||
|
||||
|
|
@ -5646,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_config_param_value_for_logging(param_name, param_value),
|
||||
)
|
||||
|
||||
if param_name is not None and param_value is not None:
|
||||
config = self._update_config_fields(
|
||||
|
|
@ -14227,20 +14273,6 @@ async def update_config_general_settings(
|
|||
return response
|
||||
|
||||
|
||||
# Secret-bearing general_settings fields the segment masker does not match by
|
||||
# name: database_url and database_extra_connection_params embed DB credentials,
|
||||
# pass_through_endpoints carry upstream Authorization headers, and
|
||||
# alert_to_webhook_url is itself a webhook secret
|
||||
_EXTRA_SECRET_GENERAL_SETTINGS_FIELDS = frozenset(
|
||||
{
|
||||
"database_url",
|
||||
"database_extra_connection_params",
|
||||
"pass_through_endpoints",
|
||||
"alert_to_webhook_url",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _is_secret_general_setting_field(field_name: str) -> bool:
|
||||
return field_name in _EXTRA_SECRET_GENERAL_SETTINGS_FIELDS or SENSITIVE_DATA_MASKER.is_sensitive_key(field_name)
|
||||
|
||||
|
|
@ -14270,6 +14302,14 @@ def _redact_secret_values_in_obj(value: JsonValue, depth: int = 0) -> JsonValue:
|
|||
return value
|
||||
|
||||
|
||||
def _redact_config_param_value_for_logging(param_name: Optional[str], param_value: JsonValue) -> JsonValue:
|
||||
if param_name == "environment_variables" and isinstance(param_value, dict):
|
||||
return {key: "REDACTED" for key in param_value}
|
||||
if isinstance(param_value, (dict, list)):
|
||||
return _redact_secret_values_in_obj(param_value)
|
||||
return param_value
|
||||
|
||||
|
||||
def _redact_general_setting_value(field_name: str, value: JsonValue, is_full_admin: bool) -> JsonValue:
|
||||
if is_full_admin:
|
||||
return value
|
||||
|
|
|
|||
|
|
@ -18,9 +18,7 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import (
|
|||
|
||||
def _use_aes(monkeypatch):
|
||||
"""Flip the write-time algorithm to AES-256-GCM for the duration of a test."""
|
||||
monkeypatch.setattr(
|
||||
proxy_server, "general_settings", {"encryption_algorithm": "aes-256-gcm"}
|
||||
)
|
||||
monkeypatch.setattr(proxy_server, "general_settings", {"encryption_algorithm": "aes-256-gcm"})
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
|
|
@ -96,12 +94,7 @@ def test_aes_decrypt_failure_returns_original_when_requested(monkeypatch):
|
|||
_use_aes(monkeypatch)
|
||||
|
||||
garbled = _V2_GCM_PREFIX + "###"
|
||||
assert (
|
||||
decrypt_value_helper(
|
||||
garbled, key="t", exception_type="debug", return_original_value=True
|
||||
)
|
||||
== garbled
|
||||
)
|
||||
assert decrypt_value_helper(garbled, key="t", exception_type="debug", return_original_value=True) == garbled
|
||||
|
||||
|
||||
def test_empty_string_round_trips_under_aes(monkeypatch):
|
||||
|
|
@ -139,10 +132,56 @@ def test_callback_prefix_composes_with_v2(monkeypatch):
|
|||
|
||||
def test_unknown_algorithm_falls_back_to_legacy(monkeypatch):
|
||||
"""An unrecognized encryption_algorithm value does not produce v2 writes."""
|
||||
monkeypatch.setattr(
|
||||
proxy_server, "general_settings", {"encryption_algorithm": "rot13"}
|
||||
)
|
||||
monkeypatch.setattr(proxy_server, "general_settings", {"encryption_algorithm": "rot13"})
|
||||
|
||||
ct = encrypt_value_helper("secret")
|
||||
assert not ct.startswith(_V2_GCM_PREFIX)
|
||||
assert decrypt_value_helper(ct, key="t") == "secret"
|
||||
|
||||
|
||||
def test_decrypt_failure_debug_log_omits_raw_value(monkeypatch):
|
||||
"""Regression for LIT-4152: the decrypt-failure debug breadcrumb must not
|
||||
embed the raw value.
|
||||
|
||||
A DB ``environment_variables`` secret (e.g. a ``DATABASE_URL`` connection
|
||||
string) reaches this path when it cannot be decrypted, for example after a
|
||||
salt or master key change, and previously printed in cleartext when the
|
||||
module regex scrubber was bypassed. The failing key still names the pair so
|
||||
the breadcrumb keeps its debugging value. Uses a dedicated handler rather
|
||||
than caplog because caplog is unreliable under pytest-xdist.
|
||||
"""
|
||||
import logging
|
||||
|
||||
import litellm._logging as _logging_module
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
|
||||
monkeypatch.setattr(_logging_module, "_ENABLE_SECRET_REDACTION", False)
|
||||
|
||||
secret = "postgresql://leak_user:leak_pw_decrypt@leak-host:5432/leak_db"
|
||||
|
||||
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:
|
||||
result = decrypt_value_helper(secret, key="DATABASE_URL", return_original_value=True)
|
||||
rendered = " ".join(record.getMessage() for record in handler.records)
|
||||
finally:
|
||||
verbose_proxy_logger.removeHandler(handler)
|
||||
verbose_proxy_logger.setLevel(original_level)
|
||||
|
||||
assert secret not in rendered, f"raw value leaked in decrypt-failure log: {rendered!r}"
|
||||
assert "leak_pw_decrypt" not in rendered
|
||||
assert any("DATABASE_URL" in record.getMessage() for record in handler.records), (
|
||||
"the failing key should still be named in the breadcrumb"
|
||||
)
|
||||
assert result == secret
|
||||
|
|
|
|||
|
|
@ -212,6 +212,145 @@ def test_save_worker_config_invalid_no_kwargs_yields_empty(monkeypatch):
|
|||
assert os.environ["WORKER_CONFIG"] == "{}"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _redact_worker_config_for_logging (LIT-4152)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
_LIT4152_SECRETS = (
|
||||
"sk-lit4152-regression-master-key-abcdef1234567890",
|
||||
"leak_password_9090",
|
||||
"sk-lit4152-provider-api-key-abcdef",
|
||||
"postgresql://leak_user:leak_password_9090@leak-host.internal:5432/leak_db",
|
||||
)
|
||||
|
||||
|
||||
def _lit4152_worker_config_dict():
|
||||
return {
|
||||
"model": "openai/gpt-4o-mini",
|
||||
"config": "/tmp/c.yaml",
|
||||
"master_key": _LIT4152_SECRETS[0],
|
||||
"database_url": _LIT4152_SECRETS[3],
|
||||
"api_key": _LIT4152_SECRETS[2],
|
||||
"telemetry": True,
|
||||
}
|
||||
|
||||
|
||||
def test__redact_worker_config_for_logging_dict_masks_all_secret_shapes():
|
||||
"""LIT-4152 regression: dict-form worker_config must not embed any raw
|
||||
secret. Covers the segment-matched fields (`master_key`, `api_key`) and the
|
||||
URL-with-credentials field (`database_url`), which the segment masker
|
||||
misses because neither segment matches its sensitive-pattern set.
|
||||
"""
|
||||
from litellm.proxy.proxy_server import _redact_worker_config_for_logging
|
||||
|
||||
redacted = _redact_worker_config_for_logging(_lit4152_worker_config_dict())
|
||||
rendered = repr(redacted)
|
||||
for secret in _LIT4152_SECRETS:
|
||||
assert secret not in rendered, f"leak: {secret} in {rendered!r}"
|
||||
assert isinstance(redacted, dict)
|
||||
assert redacted["model"] == "openai/gpt-4o-mini"
|
||||
assert redacted["telemetry"] is True
|
||||
|
||||
|
||||
def test__redact_worker_config_for_logging_json_string_round_trips_masked():
|
||||
"""Docker/K8s deployments hand the proxy a JSON string via ``WORKER_CONFIG``.
|
||||
Confirm the string path also masks and that the returned value re-parses
|
||||
into a dict with the sensitive fields masked.
|
||||
"""
|
||||
from litellm.proxy.proxy_server import _redact_worker_config_for_logging
|
||||
|
||||
payload = json.dumps(_lit4152_worker_config_dict())
|
||||
redacted = _redact_worker_config_for_logging(payload)
|
||||
assert isinstance(redacted, str)
|
||||
for secret in _LIT4152_SECRETS:
|
||||
assert secret not in redacted, f"leak: {secret} in {redacted!r}"
|
||||
parsed = json.loads(redacted)
|
||||
assert parsed["model"] == "openai/gpt-4o-mini"
|
||||
|
||||
|
||||
def test__redact_worker_config_for_logging_passthrough_for_none_and_non_json_string():
|
||||
"""Non-dict, non-JSON-parseable string is passed through verbatim (nothing
|
||||
to mask) and ``None`` returns ``None``.
|
||||
"""
|
||||
from litellm.proxy.proxy_server import _redact_worker_config_for_logging
|
||||
|
||||
assert _redact_worker_config_for_logging(None) is None
|
||||
assert _redact_worker_config_for_logging("/tmp/some_config.yaml") == "/tmp/some_config.yaml"
|
||||
|
||||
|
||||
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:
|
||||
``alert_to_webhook_url`` is typed as ``Optional[Dict]`` and can nest
|
||||
secret query params under keys the segment masker also misses. Confirm
|
||||
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_worker_config_for_logging
|
||||
|
||||
nested_webhook_secret = "https://hooks.slack.com/services/T0/B0/nested-webhook-secret-xyz"
|
||||
data = {
|
||||
"master_key": "sk-should-be-masked",
|
||||
"alert_to_webhook_url": {"budget_alerts": nested_webhook_secret},
|
||||
"pass_through_endpoints": [
|
||||
{
|
||||
"path": "/upstream",
|
||||
"target": "https://api.provider.com",
|
||||
"headers": {"Authorization": "Bearer nested-token-should-be-gone"},
|
||||
}
|
||||
],
|
||||
"database_extra_connection_params": {"password": "extra-db-password-abc"},
|
||||
}
|
||||
redacted = _redact_worker_config_for_logging(data)
|
||||
rendered = repr(redacted)
|
||||
for secret in (
|
||||
"sk-should-be-masked",
|
||||
nested_webhook_secret,
|
||||
"nested-token-should-be-gone",
|
||||
"extra-db-password-abc",
|
||||
):
|
||||
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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -784,6 +784,70 @@ def test_ProxyConfig__load_alerting_settings_invalid_alerting_raises():
|
|||
pc._load_alerting_settings({"alerting": 12345})
|
||||
|
||||
|
||||
def test_ProxyConfig__load_alerting_settings_does_not_log_general_settings_dict(monkeypatch):
|
||||
"""Regression for LIT-4152.
|
||||
|
||||
``_load_alerting_settings`` used to log ``general_settings`` verbatim in a
|
||||
line labelled ``_alerting_callbacks:``, leaking ``master_key``,
|
||||
``database_url``, and any other secret sitting in ``general_settings`` in
|
||||
cleartext at DEBUG. The fix logs only the alerting callback list.
|
||||
|
||||
The regression check runs with the last-line-of-defense regex scrubber
|
||||
(``SecretRedactionFilter``) DISABLED, since defense in depth is the point.
|
||||
The caller must not construct the leaky string, so consumers of the log
|
||||
stream that bypass the module filter (versions before it existed,
|
||||
``LITELLM_DISABLE_REDACT_SECRETS=true`` operators, downstream handlers
|
||||
that snapshot the record pre-filter) still do not see the secret. Uses a
|
||||
dedicated handler rather than caplog because caplog is unreliable under
|
||||
pytest-xdist.
|
||||
"""
|
||||
import logging
|
||||
|
||||
import litellm._logging as _logging_module
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
|
||||
monkeypatch.setattr(_logging_module, "_ENABLE_SECRET_REDACTION", 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)
|
||||
|
||||
master_key_secret = "sk-lit4152-regression-master-key-abcdef1234567890"
|
||||
db_url_secret = "postgresql://leak_user:leak_password_9090@leak-host.internal:5432/leak_db"
|
||||
settings = {
|
||||
"alerting": ["slack"],
|
||||
"alerting_threshold": 300,
|
||||
"master_key": master_key_secret,
|
||||
"database_url": db_url_secret,
|
||||
}
|
||||
|
||||
handler = LogRecordHandler()
|
||||
handler.setLevel(logging.DEBUG)
|
||||
original_level = verbose_proxy_logger.level
|
||||
verbose_proxy_logger.setLevel(logging.DEBUG)
|
||||
verbose_proxy_logger.addHandler(handler)
|
||||
try:
|
||||
try:
|
||||
ProxyConfig()._load_alerting_settings(settings)
|
||||
except Exception:
|
||||
pass # downstream init may fail without full env; the debug log fires first
|
||||
rendered = " ".join(record.getMessage() for record in handler.records)
|
||||
finally:
|
||||
verbose_proxy_logger.removeHandler(handler)
|
||||
verbose_proxy_logger.setLevel(original_level)
|
||||
|
||||
assert master_key_secret not in rendered, f"master_key leaked in logs: {rendered!r}"
|
||||
assert db_url_secret not in rendered, f"database_url leaked in logs: {rendered!r}"
|
||||
assert "leak_password_9090" not in rendered
|
||||
assert any("['slack']" in r.getMessage() for r in handler.records), (
|
||||
f"expected the alerting callback list to appear in a debug record; got {[r.getMessage() for r in handler.records]!r}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# ProxyConfig.initialize_secret_manager
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -1548,3 +1612,164 @@ 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)
|
||||
monkeypatch.delenv("DATABASE_URL", raising=False)
|
||||
|
||||
def _fake_decrypt_value_helper(value, key, **_kwargs):
|
||||
return value
|
||||
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.decrypt_value_helper", _fake_decrypt_value_helper)
|
||||
|
||||
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"
|
||||
env_db_url_secret = "postgresql://env_leak_user:env_leak_password_9090@env-leak-host.internal:5432/env_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": SimpleNamespace(
|
||||
param_name="environment_variables",
|
||||
param_value={"DATABASE_URL": env_db_url_secret},
|
||||
),
|
||||
}
|
||||
|
||||
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,
|
||||
env_db_url_secret,
|
||||
nested_webhook_secret,
|
||||
"leak_password_9090",
|
||||
"env_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
|
||||
assert merged["environment_variables"]["DATABASE_URL"] == env_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