fix(proxy): stop leaking master_key and database_url in startup DEBUG logs - #31944
Conversation
Greptile SummaryThis PR fixes a recurring class of startup/config-load DEBUG log statements in
Confidence Score: 5/5Safe to merge — changes are confined to log call sites and introduce no behavioral differences for the proxy's request or config-apply paths. All six hardened log sites redact correctly via the existing recursive helper or by dropping the value entirely. The new helper functions have no side effects on the actual config values — only the strings passed to verbose_proxy_logger.debug change. Regression tests disable the module-level filter before asserting, so they prove the fix at the source rather than relying on the downstream safety net. Non-secret settings such as num_retries are verified to remain visible, confirming surgical redaction. No functional, API, or schema changes are made. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/proxy_server.py | Six log call sites hardened to redact secrets before they reach a log record; _EXTRA_SECRET_GENERAL_SETTINGS_FIELDS moved earlier so the new top-of-file helpers can reference it; two new functions (_redact_worker_config_for_logging, _redact_config_param_value_for_logging) added with correct forward references. |
| litellm/proxy/common_utils/encrypt_decrypt_utils.py | Single-line fix: removes the raw value from the decrypt-failure DEBUG message, keeping only the key name for debugging signal. |
| tests/test_litellm/proxy/proxy_server/test_lifecycle.py | Adds five focused regression tests for _redact_worker_config_for_logging covering dict, JSON-string, None, nested, and non-string-value shapes; all disable the module regex scrubber to prove source-level fixes. |
| tests/test_litellm/proxy/proxy_server/test_proxy_config.py | Adds three regression tests: _load_alerting_settings no longer logs raw general_settings, _update_config_from_db redacts param values at DEBUG, and the litellm_settings apply loop masks secret keys while keeping plain values visible. |
| tests/test_litellm/proxy/common_utils/test_encrypt_decrypt_utils.py | Adds a decrypt-failure log regression test; cosmetic reformatting of three existing assertions does not weaken any coverage. |
Reviews (4): Last reviewed commit: "fix(proxy): stop the decrypt-failure deb..." | Re-trigger Greptile
Greptile SummaryThis PR fixes three DEBUG log statements in
Confidence Score: 5/5Safe to merge. The change is narrowly scoped to log statements; no request-path logic, no schema changes, and no behavioral changes for callers. Three leaky DEBUG log statements are fixed at the source, each with a dedicated regression test that exercises the fix with the module-level filter disabled. The helper functions handle all realistic input shapes and fall back safely for non-dict inputs. The constant hoisting is a pure refactor with no behavioral change. No existing tests are weakened. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/proxy_server.py | Adds _redact_config_dict_for_logging and _redact_worker_config_for_logging helpers; fixes three leaky DEBUG log statements; hoists _EXTRA_SECRET_GENERAL_SETTINGS_FIELDS to module top so both helpers and _is_secret_general_setting_field share the same definition. Logic is correct and handles all expected input shapes (None, file-path string, JSON string, dict). |
| tests/test_litellm/proxy/proxy_server/test_lifecycle.py | Adds four regression tests for _redact_worker_config_for_logging and _redact_config_dict_for_logging covering dict input, JSON-string input, None/non-JSON passthrough, and non-string webhook/URL values. All tests disable _ENABLE_SECRET_REDACTION to verify the fix at the source rather than relying on the filter. |
| tests/test_litellm/proxy/proxy_server/test_proxy_config.py | Adds a regression test for _load_alerting_settings that captures raw log records with SecretRedactionFilter disabled and asserts neither master_key nor database_url appears while confirming the alerting list is still present. |
Reviews (1): Last reviewed commit: "fix(proxy): stop leaking master_key and ..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
CI status summary Greptile is at 5/5 on both passes. Failures on the checks tab break down as:
|
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e2ede3e. Configure here.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: DB env vars still log secrets
- DB config debug logging now redacts every value for environment_variables rows and the regression test covers DATABASE_URL with source redaction disabled.
You can send follow-ups to the cloud agent here.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 44318b0. Configure here.
44318b0 to
8fe5409
Compare
Live proxy verification — no regressions, no backward-incompatible changesExercised every changed call site on a live proxy (real OpenAI Live results
Backward-compat / regression probes (all pass)
Test suites
Minor, non-blocking observationThe |
… logs Three startup log statements in litellm/proxy/proxy_server.py dumped secret-bearing values in cleartext when the last-line-of-defense regex scrubber was bypassed (LITELLM_DISABLE_REDACT_SECRETS=true, older versions that predated the SecretRedactionFilter, or any downstream handler that snapshots log records before the module filter runs) ProxyConfig._load_alerting_settings logged the whole general_settings dict under a label that only referred to the alerting callbacks; a copy-paste bug that happened to leak master_key, database_url, and every other secret sitting in general_settings. Now logs only the alerting callback list ProxyConfig.load_config logged the resolved DB URL after secret-manager resolution. The line's stated purpose was to confirm the retrieval ran, which does not need the value. Now logs a value-less breadcrumb proxy_startup_event logged the raw WORKER_CONFIG blob, which docker/K8s deployments hand the proxy as a JSON string containing master_key, database_url, and provider API keys. Now routes through _redact_worker_config_for_logging, which combines the segment-matching SensitiveDataMasker (catches master_key, api_key, *_token) with an explicit pass over _EXTRA_SECRET_GENERAL_SETTINGS_FIELDS (catches database_url and other credential-URL fields the segment masker misses) Regression tests disable the module-level SecretRedactionFilter so assertions see the raw record; without the fix they would trip on the secret substring, so a future refactor cannot silently reconstruct the leaky string
…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.
…alue
decrypt_value_helper logged `Unable to decrypt value={value}` at DEBUG, which
printed the raw secret whenever decryption failed (for example after a salt or
master key change). This is the same environment_variables config path the
db-config redaction covers, so a DATABASE_URL connection string could still
leak here when the module regex scrubber is bypassed. Drop the value; the key
already identifies the failing pair.
Regression forces a decrypt failure with the redaction filter disabled and
asserts the raw value never reaches a log record while the key stays visible.
8fe5409 to
bd6ae9e
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bd6ae9e. Configure here.
Relevant issues
Linear ticket
Resolves LIT-4152
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review)
Screenshots / Proof of Fix
Steps run against a live proxy backed by real OpenAI (
gpt-4o-mini), with the last-line-of-defense regex scrubber disabled (LITELLM_DISABLE_REDACT_SECRETS=true) so anything a log call site still writes into a record shows up raw. DistinctiveLEAKMARKERstrings stand in for the secrets so the grep is unambiguous;master_keyandalert_to_webhook_urllive ingeneral_settings, and both a secret-named key and a plain one live inlitellm_settingsRepro config
/tmp/lit4152_live.yaml(no database, so startup does not need Postgres):The debug lines still carry the operational signal, now with the credentials removed:
The real request returns
Ok!, no marker lands in the log stream even with the module regex scrubber off, and a plain operational setting such asnum_retrieskeeps its real value so the debug line stays usefulThe
store_model_in_dbconfig path needs a Postgres instance, so those rows (environment_variablesin particular) are covered by regression tests that drive_update_config_from_dbdirectly with the redaction filter disabled rather than by the live run aboveTest results
Every new redaction regression runs with the module regex scrubber (
_ENABLE_SECRET_REDACTION) disabled, so it proves the fix at the log call site rather than the last-line filter, and each one fails when its fix is reverted.test_secret_redaction.py(the regex net itself) and the broadtest_proxy_server.pysuite stay green, so the change adds redaction without regressing the existing defense-in-depth layer or the wider proxy surfaceType
🐛 Bug Fix
Changes
Several startup and config-load DEBUG log statements in
litellm/proxy/proxy_server.pydumped secret-bearing values whenever the module-levelSecretRedactionFilterwas bypassed. Fixes to log leaks in this codebase have all been reactive per-endpoint (/get/config/callbacks,/model/info, cooldown cache, etc.), so the same bug keeps recurring on a new surface; this change fixes it at the call siteproxy_startup_eventlogged the rawWORKER_CONFIGblob, which docker/K8s deployments hand the proxy as a JSON string containingmaster_key,database_url, and provider API keys. It now routes through_redact_worker_config_for_logging, which delegates to the existing recursive_redact_secret_values_in_obj. Reusing that helper rather than a hand-rolled top-level pass means a credential nested undergeneral_settingsor another config object is redacted at any depth, and depth overrun fails closedProxyConfig._load_alerting_settingslogged the wholegeneral_settingsdict under a label that only referred to the alerting callbacks. Copy-paste bug that happened to leakmaster_key,database_url, and everything else ingeneral_settings. Now logs only the alerting callback listProxyConfig.load_configlogged the resolved DB URL after secret-manager resolution. The stated purpose was to confirm the retrieval ran, which does not need the value. Now logs a value-less breadcrumbProxyConfig._update_config_from_dblogged each DBparam_valueverbatim on thestore_model_in_db=Truepath. Forgeneral_settings,router_settings, andlitellm_settingsthe value now routes through the recursive redactor. Theenvironment_variablesrow is different: its keys are operator-chosen env var names, so a connection string commonly sits underDATABASE_URLorREDIS_URLwhich the key-name matcher cannot recognize (urlis not a sensitive segment, and thedatabase_urlallow-entry is lowercase while the real env var is uppercase). Every value in that row is therefore blanked while the variable names stay visible for signaldecrypt_value_helperloggedUnable to decrypt value={value}at DEBUG, leaking the raw secret on the sameenvironment_variablespath whenever decryption failed (for example after a salt or master key change). It now drops the value; the key already identifies the failing pairThe
litellm_settingsapply loop loggedsetting litellm.<key>=<value>verbatim, leaking secret-named settings such asapi_keyorlangfuse_secret_key. It now routes the value through_redact_general_setting_value, which masks a value only when the key name is secret-bearing and recurses into dict/list, so a plain setting likenum_retriesstill logs its real valueThe
_EXTRA_SECRET_GENERAL_SETTINGS_FIELDSconstant sits alongsideSENSITIVE_DATA_MASKERso the logging and config-update paths share the same listRegression tests in
test_lifecycle.pyandtest_proxy_config.pydisable_ENABLE_SECRET_REDACTIONbefore asserting, so they exercise the source-level fix rather than the last-line-of-defense regex. Coverage includes dict-form and JSON-string-formworker_config,Noneand non-JSON passthrough, secret fields nested under a parent key, the_update_config_from_dbgeneral_settings andenvironment_variablespaths (asserting aDATABASE_URLconnection string never reaches a log record while the variable names stay visible), thedecrypt_value_helperbreadcrumb on a value that fails to decrypt, and thelitellm_settingsloop asserting both that a secret key is redacted and that a plainnum_retriesstays visibleNote
Medium Risk
Touches many DEBUG logging paths on config load and DB overlay; behavior change is redaction-only at log sites, but mistakes could hide useful debug signal or miss a leak path.
Overview
Stops proxy startup/config DEBUG logs from writing credentials when the log redaction filter is off (LIT-4152).
Adds
_redact_worker_config_for_loggingsoWORKER_CONFIG(dict or JSON string) is masked beforeproxy_startup_eventlogs it, using the existing recursive_redact_secret_values_in_obj(including nestedgeneral_settingsand extra secret field names likedatabase_url).ProxyConfigpaths now redact at the call site:_load_alerting_settingslogs only the alerting list (not all ofgeneral_settings);database_urlresolution logs breadcrumbs without the URL;load_configapplies_redact_general_setting_valueforlitellm_settingsdebug lines (secretsREDACTED, plain settings likenum_retriesunchanged);_update_config_from_dblogsparam_valuevia_redact_config_param_value_for_logging(full value redaction forenvironment_variables, recursive redaction elsewhere)._EXTRA_SECRET_GENERAL_SETTINGS_FIELDSis hoisted next to module init for shared use.decrypt_value_helperno longer includes the raw ciphertext/value in the “unable to decrypt” DEBUG message (key name only).Regression tests assert no leaks with
_ENABLE_SECRET_REDACTIONdisabled.Reviewed by Cursor Bugbot for commit bd6ae9e. Bugbot is set up for automated code reviews on this repo. Configure here.