From 4493c826e7cb7045e9783d633d9a87cee1085290 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Tue, 18 Aug 2026 18:03:34 -0700 Subject: [PATCH] fix(logging): close three secret-leak paths in verbose logging (#37391) * fix(logging): close three secret-leak paths in verbose logging The AWS credential pattern was the only key-name matcher in secret_redaction that skipped optional quotes, so quoted dict-repr values leaked. Fold the three AWS key names into the shared key-name alternation instead. SecretRedactionFilter only scrubs str record attributes, so a dict/list/set passed through extra={...} reached the formatter unredacted. Redact at the formatter boundary so no value shape can bypass it. log_raw_request_response wrote the request curl command to metadata["raw_request"] unredacted, returned an unmasked raw_request_api_base, and fell back to dumping model_call_details whenever api_base was empty. * Update litellm/_logging.py Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * fix(logging): redact JSON log values without breaking the document JsonFormatter redacted the serialized JSON, so a secret-named member collapsed from '"api_key": "sk-..."' to a bare REDACTED token and the line stopped parsing as JSON. Redact before serialization instead: safe_dumps takes an optional value_transform hook (default None, so all other callers are unchanged) and redact_structured_value collapses only the value, leaving the key and surrounding structure intact. JsonFormatter now emits "api_key": "REDACTED" where the formatter unit test expected the already-masked "sk**********". That test bypasses SecretRedactionFilter, which in production collapses the pair before any formatter runs, so the assertion is updated to match real behavior. --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --- litellm/_logging.py | 12 +++-- litellm/litellm_core_utils/litellm_logging.py | 15 +++--- litellm/litellm_core_utils/safe_json_dumps.py | 31 +++++++---- .../litellm_core_utils/secret_redaction.py | 20 ++++++-- .../test_litellm_logging.py | 48 +++++++++++++++++ tests/test_litellm/test_logging.py | 37 +++++++++++++- tests/test_litellm/test_secret_redaction.py | 51 +++++++++++++++++++ 7 files changed, 191 insertions(+), 23 deletions(-) diff --git a/litellm/_logging.py b/litellm/_logging.py index 6add9d79a5b..7d3a30c6d1a 100644 --- a/litellm/_logging.py +++ b/litellm/_logging.py @@ -10,7 +10,7 @@ from typing import Any, Final import litellm 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.litellm_core_utils.secret_redaction import redact_string +from litellm.litellm_core_utils.secret_redaction import redact_string, redact_structured_value set_verbose = False @@ -59,6 +59,12 @@ def _redact_string(value: str) -> str: return redact_string(value) +def _redact_structured_value(key: str | None, value: str) -> str: + if not _ENABLE_SECRET_REDACTION: + return value + return redact_structured_value(key, value) + + def redact_secrets(value: str) -> str: """Public API: redact known secret/credential patterns from an arbitrary string. @@ -265,7 +271,7 @@ class JsonFormatter(Formatter): if record.exc_info: json_record["stacktrace"] = record.exc_text or self.formatException(record.exc_info) - return safe_dumps(json_record) + return safe_dumps(json_record, value_transform=_redact_structured_value) class CorrelationPlainFormatter(logging.Formatter): @@ -276,7 +282,7 @@ class CorrelationPlainFormatter(logging.Formatter): """ def format(self, record: logging.LogRecord) -> str: - formatted: Final = super().format(record) + formatted: Final = _redact_string(super().format(record)) trace_id: Final = getattr(record, "trace_id", None) session_id: Final = getattr(record, "session_id", None) if not trace_id and not session_id: diff --git a/litellm/litellm_core_utils/litellm_logging.py b/litellm/litellm_core_utils/litellm_logging.py index 10e681b816e..946110abf9e 100644 --- a/litellm/litellm_core_utils/litellm_logging.py +++ b/litellm/litellm_core_utils/litellm_logging.py @@ -1094,10 +1094,10 @@ class Logging(LiteLLMLoggingBaseClass): data=additional_args.get("complete_input_dict", {}), ) - _metadata["raw_request"] = str(curl_command) + _metadata["raw_request"] = _redact_string(str(curl_command)) # split up, so it's easier to parse in the UI self.model_call_details["raw_request_typed_dict"] = RawRequestTypedDict( - raw_request_api_base=str(additional_args.get("api_base") or ""), + raw_request_api_base=self._get_masked_api_base(str(additional_args.get("api_base") or "")), raw_request_body=self._get_raw_request_body(additional_args.get("complete_input_dict", {})), # NOTE: setting ignore_sensitive_headers to True will cause # the Authorization header to be leaked when calls to the health @@ -1111,8 +1111,10 @@ class Logging(LiteLLMLoggingBaseClass): self.model_call_details["raw_request_typed_dict"] = RawRequestTypedDict( error=str(e), ) - _metadata["raw_request"] = f"Unable to Log \ + _metadata["raw_request"] = _redact_string( + f"Unable to Log \ raw request: {e}" + ) if getattr(self, "logger_fn", None) and callable(self.logger_fn): try: self.logger_fn( @@ -1206,15 +1208,16 @@ class Logging(LiteLLMLoggingBaseClass): if _is_debugging_on() or self.litellm_request_debug: if json_logs: masked_headers: Final = self._get_masked_headers(headers) + masked_api_base: Final = self._get_masked_api_base(str(api_base or "")) if self.litellm_request_debug: verbose_logger.warning( # .warning ensures this shows up in all environments "POST Request Sent from LiteLLM", - extra={"api_base": {api_base}, **masked_headers}, + extra={"api_base": {masked_api_base}, **masked_headers}, ) else: verbose_logger.debug( "POST Request Sent from LiteLLM", - extra={"api_base": {api_base}, **masked_headers}, + extra={"api_base": {masked_api_base}, **masked_headers}, ) else: headers = additional_args.get("headers", {}) @@ -1254,8 +1257,6 @@ class Logging(LiteLLMLoggingBaseClass): curl_command = "\nRequest Sent from LiteLLM:\n" request_str: Final = additional_args.get("request_str", "") curl_command += request_str - elif api_base == "": - curl_command = str(self.model_call_details) return curl_command def _get_masked_headers(self, headers: dict, ignore_sensitive_headers: bool = False) -> dict: diff --git a/litellm/litellm_core_utils/safe_json_dumps.py b/litellm/litellm_core_utils/safe_json_dumps.py index ebf45ed747c..a1b71593dda 100644 --- a/litellm/litellm_core_utils/safe_json_dumps.py +++ b/litellm/litellm_core_utils/safe_json_dumps.py @@ -1,4 +1,5 @@ import json +from collections.abc import Callable from typing import Any, Final from pydantic import BaseModel @@ -11,20 +12,32 @@ def strip_null_bytes(value: str) -> str: return value.replace("\x00", "") -def safe_dumps(data: Any, max_depth: int = DEFAULT_MAX_RECURSE_DEPTH) -> str: +def safe_dumps( + data: Any, + max_depth: int = DEFAULT_MAX_RECURSE_DEPTH, + value_transform: Callable[[str | None, str], str] | None = None, +) -> str: """ Recursively serialize data while detecting circular references. If a circular reference is detected then a marker string is returned. NUL bytes are stripped from strings to prevent PostgreSQL 22P05 errors. + + value_transform, when given, is applied to every string leaf (and to the + str() fallback for non-serializable objects) with the mapping key the leaf + was reached under, so callers can rewrite values without touching structure. """ - def _serialize(obj: Any, seen: set, depth: int) -> Any: + def _transform(key: str | None, value: str) -> str: + return value if value_transform is None else value_transform(key, value) + + def _serialize(obj: Any, seen: set, depth: int, key: str | None = None) -> Any: # Check for maximum depth. if depth > max_depth: return "MaxDepthExceeded" # Base-case: if it is a primitive, simply return it. if isinstance(obj, str): - return obj.replace("\x00", "") if "\x00" in obj else obj + cleaned = obj.replace("\x00", "") if "\x00" in obj else obj + return _transform(key, cleaned) if isinstance(obj, (int, float, bool, type(None))): return obj # Check for circular reference. @@ -37,30 +50,30 @@ def safe_dumps(data: Any, max_depth: int = DEFAULT_MAX_RECURSE_DEPTH) -> str: for k, v in obj.items(): if isinstance(k, (str)): clean_k = k.replace("\x00", "") if "\x00" in k else k - result[clean_k] = _serialize(v, seen, depth + 1) + result[clean_k] = _serialize(v, seen, depth + 1, clean_k) seen.remove(id(obj)) return result elif isinstance(obj, list): - result = [_serialize(item, seen, depth + 1) for item in obj] + result = [_serialize(item, seen, depth + 1, key) for item in obj] seen.remove(id(obj)) return result elif isinstance(obj, tuple): - result = tuple(_serialize(item, seen, depth + 1) for item in obj) + result = tuple(_serialize(item, seen, depth + 1, key) for item in obj) seen.remove(id(obj)) return result elif isinstance(obj, set): - result = sorted([_serialize(item, seen, depth + 1) for item in obj]) + result = sorted([_serialize(item, seen, depth + 1, key) for item in obj]) seen.remove(id(obj)) return result elif isinstance(obj, BaseModel): dumped: Final = obj.model_dump() - result = _serialize(dumped, seen, depth + 1) + result = _serialize(dumped, seen, depth + 1, key) seen.remove(id(obj)) return result else: # Fall back to string conversion for non-serializable objects. try: - return strip_null_bytes(str(obj)) + return _transform(key, strip_null_bytes(str(obj))) except Exception: return "Unserializable Object" diff --git a/litellm/litellm_core_utils/secret_redaction.py b/litellm/litellm_core_utils/secret_redaction.py index c991a953530..5d5bd547d22 100644 --- a/litellm/litellm_core_utils/secret_redaction.py +++ b/litellm/litellm_core_utils/secret_redaction.py @@ -24,9 +24,6 @@ def _build_secret_patterns() -> "re.Pattern[str]": r"(?:client_secret|azure_password|azure_username)\s+[^\s,'\"})\]{}>]+", # AWS access key IDs r"(?:AKIA|ASIA)[0-9A-Z]{16}", - # AWS secrets / session tokens / access key IDs (key=value) - r"(?:aws_secret_access_key|aws_session_token|aws_access_key_id)" - r"\s*[:=]\s*[A-Za-z0-9/+=]{20,}", # Bearer tokens (OAuth, JWT, etc.) r"Bearer\s+[A-Za-z0-9\-._~+/]{10,}=*", # Basic auth headers @@ -61,6 +58,7 @@ def _build_secret_patterns() -> "re.Pattern[str]": # private_key with PEM-aware value capture r"""private_key['\"]?\s*[:=]\s*['\"]?(?:-----BEGIN[A-Z \-]*PRIVATE KEY-----[\s\S]*?-----END[A-Z \-]*PRIVATE KEY-----|[^\s,'\"})\]{}>]+)""", r"(?:master_key|xai_key|database_url|db_url|connection_string|" + r"aws_secret_access_key|aws_session_token|aws_access_key_id|" r"signing_key|encryption_key|" r"auth_token|access_token|refresh_token|" r"slack_webhook_url|webhook_url|" @@ -83,3 +81,19 @@ _SECRET_RE: Final = _build_secret_patterns() def redact_string(value: str) -> str: """Scrub known secret/credential patterns from *value* and return the result.""" return _SECRET_RE.sub(_REDACTED, value) + + +def redact_structured_value(key: str | None, value: str) -> str: + """Scrub *value* as it appeared under *key* inside a structured record. + + redact_string() replaces a whole ``key: value`` span with REDACTED, which is + fine inside free text but destroys the surrounding syntax when the span is a + JSON member rather than message content. This renders the pair the way a dict + repr would, so the key-name patterns still fire, but collapses only the value + so the caller's structure survives. + """ + scrubbed: Final = redact_string(value) + if scrubbed != value or key is None: + return scrubbed + rendered: Final = f"'{key}': '{value}'" + return _REDACTED if redact_string(rendered) != rendered else value diff --git a/tests/test_litellm/litellm_core_utils/test_litellm_logging.py b/tests/test_litellm/litellm_core_utils/test_litellm_logging.py index 0d54680fa81..54016470f8b 100644 --- a/tests/test_litellm/litellm_core_utils/test_litellm_logging.py +++ b/tests/test_litellm/litellm_core_utils/test_litellm_logging.py @@ -4910,3 +4910,51 @@ def test_payload_without_guardrail_cost_is_unchanged(logging_obj): assert payload is not None assert payload["response_cost"] == pytest.approx(0.0000429) assert payload["cost_breakdown"] is None + + +_AWS_SECRET = "wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY" +_GEMINI_KEY = "AIzaSyC0000000000000000000000000000000" + + +def test_empty_api_base_does_not_dump_call_state(logging_obj): + """Direct (non-HTTP) providers pass api_base='', which used to echo model_call_details.""" + logging_obj.model_call_details["litellm_params"] = { + "api_key": "sk-proj-hunter2hunter2hunter2hunter2", + "aws_secret_access_key": _AWS_SECRET, + } + + curl_command = logging_obj._get_request_curl_command( + api_base="", + headers={}, + additional_args={}, + data={"model": "some-model"}, + ) + + assert "litellm_call_id" not in curl_command + assert _AWS_SECRET not in curl_command + assert "hunter2" not in curl_command + + +def test_pre_call_redacts_and_masks_raw_request(logging_obj): + """log_raw_request_response echoes the request body and api_base back to loggers/UI.""" + metadata = {"user_api_key_alias": "qa-key"} + logging_obj.model_call_details["litellm_params"] = {"metadata": metadata} + logging_obj.log_raw_request_response = True + + logging_obj.pre_call( + input="hi", + api_key="", + additional_args={ + "api_base": f"https://generativelanguage.googleapis.com/v1beta/models/x:generateContent?key={_GEMINI_KEY}", + "headers": {}, + "complete_input_dict": {"aws_secret_access_key": _AWS_SECRET}, + }, + ) + + raw_request = metadata["raw_request"] + assert _AWS_SECRET not in raw_request + assert "REDACTED" in raw_request + + raw_api_base = logging_obj.model_call_details["raw_request_typed_dict"]["raw_request_api_base"] + assert _GEMINI_KEY not in raw_api_base + assert "key=*****" in raw_api_base diff --git a/tests/test_litellm/test_logging.py b/tests/test_litellm/test_logging.py index 9ab362f6cd5..784ec5b6cf4 100644 --- a/tests/test_litellm/test_logging.py +++ b/tests/test_litellm/test_logging.py @@ -177,7 +177,10 @@ def test_json_formatter_parses_embedded_python_dict_repr(): # Python dict parsed and promoted to first-class properties assert obj["model_name"] == "text-embedding-3-large" assert "litellm_params" in obj - assert obj["litellm_params"]["api_key"] == "sk**********" + # Redacted, not passed through: SecretRedactionFilter already collapses this + # pair in the plain path before any formatter sees it, so the JSON path matching + # it is production parity. The key survives because redaction is per-value here. + assert obj["litellm_params"]["api_key"] == "REDACTED" assert obj["litellm_params"]["tpm"] == 1000000 assert obj["litellm_params"]["use_in_pass_through"] is False assert "model_info" in obj @@ -185,6 +188,38 @@ def test_json_formatter_parses_embedded_python_dict_repr(): assert obj["model_info"]["db_model"] is False +def test_json_formatter_output_stays_parseable_when_a_secret_is_redacted(): + """Redaction must collapse the value only, never the surrounding JSON member. + + Redacting the serialized document turned '"api_key": "sk-..."' into a bare + REDACTED token, so the line stopped being valid JSON entirely. + """ + formatter = JsonFormatter() + record = logging.LogRecord( + name="LiteLLM", + level=logging.INFO, + pathname="", + lineno=0, + msg="calling deployment", + args=(), + exc_info=None, + ) + record.deployment = { + "api_key": "sk-abcdefghijklmnopqrstuvwxyz0123456789", + "aws_secret_access_key": "wJalrXUtnFEMIQAfakeKEYbPxRfiCYEXAMPLEKEY", + "aws_region_name": "us-east-1", + "nested": {"tokens": ["Bearer abcdefghijklmnop", "keep-me"]}, + } + + obj = json.loads(formatter.format(record)) + + assert obj["deployment"]["api_key"] == "REDACTED" + assert obj["deployment"]["aws_secret_access_key"] == "REDACTED" + # Non-secret siblings stay legible so the logs remain useful + assert obj["deployment"]["aws_region_name"] == "us-east-1" + assert obj["deployment"]["nested"]["tokens"] == ["REDACTED", "keep-me"] + + def test_json_formatter_includes_component_field(): """ Test that JsonFormatter always emits a 'component' field equal to the logger name. diff --git a/tests/test_litellm/test_secret_redaction.py b/tests/test_litellm/test_secret_redaction.py index a5f3f912339..0188d87dfdb 100644 --- a/tests/test_litellm/test_secret_redaction.py +++ b/tests/test_litellm/test_secret_redaction.py @@ -495,3 +495,54 @@ def test_redaction_survives_uvicorn_logging_reconfiguration(): lg.handlers[:] = handlers lg.setLevel(level) lg.propagate = True + + +def test_aws_credential_redaction_catches_quoted_values(): + """AWS creds appear as quoted dict-repr values, not just bare key=value.""" + cases = ( + "{'aws_secret_access_key': 'wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY'}", + '{"aws_session_token": "IQoJb3JpZ2luX2VjEaCXVzLWVhc3QtMSJHMEUCIQ"}', + "aws_session_token: 'FwoGZXIvYXdzEBYaDHh4eHh4eHh4eHh4eCLLAe'", + "aws_secret_access_key=wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY", + "{'aws_access_key_id': 'not-an-akia-shaped-value'}", + ) + for secret_line in cases: + result = redact_string(secret_line) + assert "REDACTED" in result, f"AWS redaction missed: {secret_line!r}" + assert "wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY" not in result + assert "IQoJb3JpZ2luX2VjEaCXVzLWVhc3QtMSJHMEUCIQ" not in result + + safe = "'aws_region_name': 'us-east-1'" + assert redact_string(safe) == safe + + +@pytest.mark.parametrize( + "extra", + ( + {"api_base": {f"https://host/v1?key={SECRET}"}}, + {"blob": {"authorization": f"Bearer {SECRET}"}}, + {"blob": [f"Bearer {SECRET}"]}, + {"blob": ({"nested": {"deep": SECRET}},)}, + ), + ids=("set", "dict", "list", "nested"), +) +def test_json_formatter_redacts_non_string_extra_values(extra): + """SecretRedactionFilter only scrubs str attrs, so containers must be caught on render.""" + buf = StringIO() + handler = logging.StreamHandler(buf) + handler.setFormatter(JsonFormatter()) + handler.addFilter(_secret_filter) + + logger = logging.getLogger("test_json_extra_redaction") + logger.handlers = [handler] + logger.setLevel(logging.DEBUG) + logger.propagate = False + try: + logger.warning("request sent", extra=extra) + finally: + logger.handlers = [] + + output = buf.getvalue() + assert output.strip(), "no record captured" + assert SECRET not in output, f"non-string extra leaked a secret: {output}" + assert "REDACTED" in output