-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
fix: global secret redaction via root logger + key-name-based pattern matching #24305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,6 +52,17 @@ def _build_secret_patterns() -> re.Pattern: | |
| r"(?<=://)[^\s'\"]*:[^\s'\"@]+(?=@)", | ||
| # Databricks personal access tokens | ||
| r"dapi[0-9a-f]{32}", | ||
| # ── Key-name-based redaction ── | ||
| # Catches secrets inside dicts/config dumps by matching on the KEY name | ||
| # regardless of what the value looks like. | ||
| # e.g. 'master_key': 'any-value-here', "database_url": "postgres://..." | ||
| r"(?:master_key|database_url|db_url|connection_string|" | ||
| r"private_key|signing_key|encryption_key|" | ||
| r"auth_token|access_token|refresh_token|" | ||
| r"slack_webhook_url|webhook_url|" | ||
| r"database_connection_string|" | ||
| r"huggingface_token|jwt_secret)" | ||
| r"""['\"]?\s*[:=]\s*['\"]?[^\s,'\"})\]{}>]+""", | ||
|
Comment on lines
+59
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new pattern captures While this is secure, it makes logs less useful for debugging because you can't tell which field was redacted. A more targeted approach would capture and preserve the key name while replacing only the value, using a capture group: r"((?:master_key|database_url|db_url|...)(?:['\"]?\s*[:=]\s*['\"]?))[^\s,'\"})\]{}>]+"with a substitution like
Comment on lines
+59
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The value-matching portion An alternative branch that also matches fully-quoted values spanning spaces would guard against this: r"""['\"]?\s*[:=]\s*(?:'[^']*'|\"[^\"]*\"|[^\s,'\"})\]{}>]+)"""This matches either a single-quoted string, a double-quoted string, or the existing bare-token form.
Comment on lines
+59
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The alternation has no Adding a negative lookbehind before the alternation prevents these false triggers: r"(?<![A-Za-z_])(?:master_key|database_url|db_url|connection_string|" |
||
| ] | ||
| return re.compile("|".join(patterns), re.IGNORECASE) | ||
|
|
||
|
|
@@ -272,7 +283,7 @@ def async_json_exception_handler(loop, context): | |
| verbose_router_logger = logging.getLogger("LiteLLM Router") | ||
| verbose_logger = logging.getLogger("LiteLLM") | ||
|
|
||
| # Add the handler to the logger | ||
| # Add the handler to the loggers | ||
| verbose_router_logger.addHandler(handler) | ||
| verbose_proxy_logger.addHandler(handler) | ||
| verbose_logger.addHandler(handler) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -213,3 +213,52 @@ def test_json_excepthook_redacts_traceback_secrets(): | |
| output = h.formatter.format(record) | ||
| assert SECRET not in output | ||
| assert "REDACTED" in output | ||
|
|
||
|
|
||
| def test_key_name_redaction_catches_secrets_in_dict_repr(): | ||
| """Secrets inside dict repr strings are redacted based on key names.""" | ||
| cases = [ | ||
| # Python dict repr (the exact leak format from the bug report) | ||
| "param_name=general_settings, param_value={'master_key': 'my-random-secret-key-1234', 'enable_jwt_auth': True}", | ||
| # database_url | ||
| "'database_url': 'postgres://admin:password@db.example.com:5432/litellm'", | ||
| # JSON format | ||
| '"database_url": "postgres://admin:password@db.example.com:5432/litellm"', | ||
| # access_token | ||
| "'access_token': 'some-opaque-token-value'", | ||
| # refresh_token | ||
| "refresh_token=my-refresh-tok-12345", | ||
| # auth_token | ||
| "'auth_token': 'random-auth-value'", | ||
| # slack_webhook_url | ||
| "'slack_webhook_url': 'https://hooks.slack.com/services/T00/B00/xxx'", | ||
| ] | ||
| for secret_line in cases: | ||
| result = _redact_string(secret_line) | ||
| assert "REDACTED" in result, f"Key-name redaction missed: {secret_line!r}" | ||
|
|
||
| # Non-sensitive keys should NOT be redacted | ||
| safe = "'enable_jwt_auth': True, 'store_model_in_db': True" | ||
| assert _redact_string(safe) == safe | ||
|
Comment on lines
+218
to
+242
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The patterns Consider adding at least one case per new pattern, for example: "'private_key': 'BEGIN PRIVATE KEY...'",
"'signing_key': 'hmac-sha256-secret'",
"'encryption_key': 'aes256-key-value'",
"'webhook_url': 'https://example.com/hook/abc123'",
"'jwt_secret': 'super-secret-jwt'",
"'huggingface_token': 'hf_abcdefXYZ123'", |
||
|
|
||
|
|
||
| def test_key_name_redaction_in_general_settings_dict(): | ||
| """End-to-end: secrets inside a general_settings dict dump are redacted | ||
| when logged through the named litellm loggers.""" | ||
|
|
||
| def log_messages(): | ||
| general_settings = { | ||
| "master_key": "my-random-secret-key-1234", | ||
| "database_url": "postgres://admin:password@db.example.com:5432/litellm", | ||
| "enable_jwt_auth": True, | ||
| "store_model_in_db": True, | ||
| } | ||
| verbose_proxy_logger.debug( | ||
| f"param_name=general_settings, param_value={general_settings}" | ||
| ) | ||
|
|
||
| output = _capture_logger_output(log_messages) | ||
| assert "my-random-secret-key-1234" not in output | ||
| assert "REDACTED" in output | ||
| # Non-sensitive values should survive | ||
| assert "enable_jwt_auth" in output | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
database_connection_stringis unreachable — subsumed byconnection_stringBecause there are no word-boundary anchors, the engine scans each character position in the input. When it reaches the
cindatabase_connection_string: …, the earlier alternativeconnection_stringalready matches, sodatabase_connection_stringis never the winning alternative. The entry can be removed without any behavioral change.