From 238848f2449015712119ad0ba89622022da48b2f Mon Sep 17 00:00:00 2001 From: lzhan011 <35493221+lzhan011@users.noreply.github.com> Date: Mon, 5 Oct 2026 05:44:07 -0500 Subject: [PATCH 1/6] fix(cache): redact Redis write values from error logs and telemetry redis-py annotates every failed pipeline command with the command it sent, arguments included: Command # 1 (SET litellm:cache: {"messages":[...]} EX 60) of pipeline caused error: WRONGTYPE Operation against a key holding the wrong kind of value `uv.lock` pins redis-py 5.3.1, whose `Pipeline.annotate_exception` does not truncate that text at all, so `str(exc)` carries the whole cache key and the serialized prompt/response into both sinks this exception reaches: the log line, and the `async_service_failure_hook` payload that Datadog and OTel export. A tenant's cacheable content therefore leaves the process on any pipeline rejection. `redact_redis_error()` keeps the parts with diagnostic value -- which command, and the server's own reason ("WRONGTYPE ...", "OOM ...") -- and replaces the arguments. The command is matched greedily so the split lands on the *last* " of pipeline caused error: ", the one redis-py wrote, which stops a cache value containing that literal text from shifting the boundary. Applied in two places rather than at the call sites: * `log_redis_failure()` -- every Redis failure line in redis_cache.py already goes through it, so one redaction covers all of them and any call site added later. * `ServiceLogging.async_service_failure_hook()`, gated on `service == ServiceTypes.REDIS` -- redis_cache.py hands that hook an exception from ~16 places, and a new one must not be able to leak by being written without the redaction. The OTel branch no longer re-derives `str(error)`, which would have undone it. Prometheus still gets the raw exception; it labels by class and never reads the message. Rebuilt on `main`: this PR was opened against the since-deleted `litellm_internal_staging`, and the original patch replaced the logging at seven call sites. `main` has since centralised them into `log_redis_failure`, so the logging half is now one change in one function. `main` also stopped logging the cache *value* as a separate argument, which was the other half of the original report -- that part is already fixed upstream and is not re-litigated here; what remains is the annotated exception. Seven tests in tests/unit/caching/test_redis_cache.py, including a precondition that builds the exception with redis-py's own `annotate_exception` and asserts the value really is in the raw message (skipped with the version named on a redis-py that truncates, so the suite stays honest on both). The two redaction tests fail with the redaction removed; 100 pass with it. --- litellm/_service_logger.py | 18 +++- litellm/caching/redis_cache.py | 56 +++++++++++-- tests/unit/caching/test_redis_cache.py | 109 +++++++++++++++++++++++++ 3 files changed, 176 insertions(+), 7 deletions(-) diff --git a/litellm/_service_logger.py b/litellm/_service_logger.py index 24625382d0c..7688968b9dc 100644 --- a/litellm/_service_logger.py +++ b/litellm/_service_logger.py @@ -335,6 +335,17 @@ class ServiceLogging(CustomLogger): elif isinstance(error, str): error_message = error + if service == ServiceTypes.REDIS: + # redis-py annotates a failed pipeline command with the command it + # sent, key and value included, so the raw message carries cached + # prompt/response text into Datadog and OTel. Redacted here rather + # than at each call site: `redis_cache` hands this hook an exception + # from ~16 places and a new one must not be able to leak by being + # written without the redaction. + from litellm.caching.redis_cache import redact_redis_error # noqa: PLC0415 + + error_message = redact_redis_error(error) if isinstance(error, Exception) else error_message + payload: Final = ServiceLoggerPayload( is_error=True, error=error_message, @@ -369,8 +380,11 @@ class ServiceLogging(CustomLogger): else: _otel_logger_to_use = self._resolve_otel_service_logger(callback) - if not isinstance(error, str): - error = str(error) + # `error_message`, not `str(error)`: re-deriving it here would + # undo the redaction above. Prometheus still gets the raw + # exception, which it labels by class and never reads the + # message of. + error = error_message # See the success hook: no parent gate, so background failures # are traced too. V1 no-ops without a parent; V2 emits a root. diff --git a/litellm/caching/redis_cache.py b/litellm/caching/redis_cache.py index 2ee4bff6112..68d711fc98c 100644 --- a/litellm/caching/redis_cache.py +++ b/litellm/caching/redis_cache.py @@ -16,6 +16,7 @@ import inspect import itertools import json import logging +import re import threading import time from collections.abc import Awaitable, Callable, Iterator, Sequence @@ -528,28 +529,73 @@ class _RedisTimeoutLogThrottle: _redis_timeout_log_throttle: Final = _RedisTimeoutLogThrottle(REDIS_TIMEOUT_LOG_INTERVAL) +# redis-py annotates every failed pipeline command with the command it sent, +# arguments included: ``Command # 1 (SET EX 60) of pipeline caused +# error: `` (``Pipeline.annotate_exception`` in redis/client.py, +# redis/asyncio/client.py and redis/cluster.py). ``uv.lock`` pins a redis-py +# whose ``annotate_exception`` does not truncate that text at all, so +# ``str(exc)`` carries the whole cache key and the serialized prompt/response +# into every sink this exception reaches. +# +# ``command`` is matched greedily so the split happens at the LAST +# " of pipeline caused error: ", which is the one redis-py wrote. A cache value +# containing that literal text therefore cannot shift the boundary and smuggle +# itself into the part that is kept. +_PIPELINE_COMMAND_ANNOTATION: Final = re.compile( + r"^Command # (?P\d+) \((?P.*)\) of pipeline caused error: (?P.*)$", + re.DOTALL, +) + +_REDACTED: Final = "" + + +def redact_redis_error(exc: BaseException) -> str: + """``str(exc)`` with any redis-py pipeline command arguments removed. + + Only the command name is kept (``SET``, ``INCRBY``, ...) because that is the + part with diagnostic value; the key and value that follow it are not. The + trailing detail is the server's own error string ("WRONGTYPE ...", "OOM + ..."), which redis never echoes argument data into, so it is preserved. + + Messages that are not pipeline annotations are returned unchanged: redis-py + builds those from the server reply alone. + """ + message: Final = str(exc) + match: Final = _PIPELINE_COMMAND_ANNOTATION.match(message) + if match is None: + return message + command_name: Final = match["command"].split(" ", 1)[0].strip() or _REDACTED + return ( + f"Command # {match['index']} ({command_name} {_REDACTED}) " + f"of pipeline caused error: {match['detail']}" + ) + + def log_redis_failure( logger: logging.Logger, level: int, message: str, exc: BaseException, with_traceback: bool = False ) -> None: + # Every Redis failure line in this module goes through here, so redacting + # once covers all of them -- and any call site added later. + reason: Final = redact_redis_error(exc) if isinstance(exc, RedisCircuitBreakerOpenError): - logger.debug("%s: %s", message, exc, stacklevel=2) + logger.debug("%s: %s", message, reason, stacklevel=2) return exc_info: Final = exc if with_traceback else None if not is_redis_timeout_failure(exc): - logger.log(level, "%s: %s", message, exc, exc_info=exc_info, stacklevel=2) + logger.log(level, "%s: %s", message, reason, exc_info=exc_info, stacklevel=2) return suppressed: Final = _redis_timeout_log_throttle.admit() if suppressed is None: - logger.debug("%s: %s", message, exc, stacklevel=2) + logger.debug("%s: %s", message, reason, stacklevel=2) return if suppressed == 0: - logger.log(level, "%s: %s", message, exc, exc_info=exc_info, stacklevel=2) + logger.log(level, "%s: %s", message, reason, exc_info=exc_info, stacklevel=2) return logger.log( level, "%s: %s (%d more Redis timeouts since the previous Redis timeout line were logged at DEBUG)", message, - exc, + reason, suppressed, exc_info=exc_info, stacklevel=2, diff --git a/tests/unit/caching/test_redis_cache.py b/tests/unit/caching/test_redis_cache.py index 5db11a67564..b94cfb5843c 100644 --- a/tests/unit/caching/test_redis_cache.py +++ b/tests/unit/caching/test_redis_cache.py @@ -9,6 +9,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest import litellm +from litellm._logging import verbose_logger from litellm._service_logger import ServiceLogging from litellm.caching.redis_cache import RedisCache, RedisCircuitBreakerOpenError @@ -1611,3 +1612,111 @@ def test_call_stack_info_skips_native_lifecycle_frames(): return native_drive() assert anthropic_messages() == "anthropic_messages <- test_call_stack_info_skips_native_lifecycle_frames" + + +SENSITIVE_CACHE_KEY = "litellm:cache:customer-secret-cache-key" +SENSITIVE_CACHE_VALUE = '{"messages":[{"role":"user","content":"private customer message"}]}' + + +def _annotated_pipeline_error(key=SENSITIVE_CACHE_KEY, value=SENSITIVE_CACHE_VALUE): + """The exception redis-py raises for a failed pipeline command. + + Built with redis-py's own ``annotate_exception`` rather than a hand-written + string, so this stays honest about the message shape the library produces. + """ + import redis.asyncio as async_redis + from redis.exceptions import ResponseError + + pipeline = async_redis.Redis().pipeline(transaction=False) + error = ResponseError("WRONGTYPE Operation against a key holding the wrong kind of value") + pipeline.annotate_exception(error, 1, ("SET", key, value, "EX", "60")) + return error + + +def test_the_annotation_really_does_carry_the_key_and_value(): + """Precondition: on the pinned redis-py there is something to leak. + + How much `annotate_exception` includes is a redis-py detail: the version + `uv.lock` pins does not truncate the command at all, while later releases + cap it. The redaction is worth having either way, so this asserts the + premise only on a redis-py that actually exposes the value, and says so + rather than silently passing on one that does not. + """ + raw = str(_annotated_pipeline_error()) + if SENSITIVE_CACHE_KEY not in raw: + import redis as _redis + + pytest.skip(f"redis-py {_redis.__version__} truncates the pipeline annotation") + assert "private customer message" in raw + + +def test_redact_redis_error_drops_the_command_arguments(): + from litellm.caching.redis_cache import redact_redis_error + + redacted = redact_redis_error(_annotated_pipeline_error()) + + assert SENSITIVE_CACHE_KEY not in redacted + assert "private customer message" not in redacted + # The parts worth keeping survive: which command, and the server's reason. + assert "SET" in redacted + assert "WRONGTYPE" in redacted + + +def test_redact_redis_error_leaves_a_plain_message_alone(): + from litellm.caching.redis_cache import redact_redis_error + + assert redact_redis_error(ValueError("OOM command not allowed")) == "OOM command not allowed" + + +def test_a_value_containing_the_annotation_text_cannot_shift_the_boundary(): + """The split is on the LAST separator, the one redis-py wrote.""" + from litellm.caching.redis_cache import redact_redis_error + + hostile = "x of pipeline caused error: " + SENSITIVE_CACHE_VALUE + redacted = redact_redis_error(_annotated_pipeline_error(value=hostile)) + + assert "private customer message" not in redacted + + +def test_log_redis_failure_redacts_the_line_it_writes(caplog): + import logging as _logging + + from litellm.caching.redis_cache import log_redis_failure + + with caplog.at_level(_logging.ERROR, logger="LiteLLM"): + log_redis_failure( + verbose_logger, + _logging.ERROR, + "LiteLLM Redis Caching: async set_cache_pipeline() - Got exception from REDIS", + _annotated_pipeline_error(), + ) + + assert SENSITIVE_CACHE_KEY not in caplog.text + assert "private customer message" not in caplog.text + assert "WRONGTYPE" in caplog.text + + +@pytest.mark.asyncio +async def test_service_failure_hook_redacts_before_telemetry(): + """Datadog/OTel must not receive the annotated command either.""" + from litellm._service_logger import ServiceLogging + from litellm.types.services import ServiceTypes + + dd_logger = MagicMock() + dd_logger.async_service_failure_hook = AsyncMock() + service_logging = ServiceLogging() + service_logging.dd_logger = dd_logger + + with patch.object(litellm, "service_callback", ["datadog"]): + await service_logging.async_service_failure_hook( + service=ServiceTypes.REDIS, + duration=0.1, + error=_annotated_pipeline_error(), + call_type="async_set_cache_pipeline", + ) + + kwargs = dd_logger.async_service_failure_hook.call_args.kwargs + assert SENSITIVE_CACHE_KEY not in kwargs["error"] + assert "private customer message" not in kwargs["error"] + assert SENSITIVE_CACHE_KEY not in kwargs["payload"].error + assert "private customer message" not in kwargs["payload"].error From eca4d13c410aa9ddeef22f3239f38c4da9c396a7 Mon Sep 17 00:00:00 2001 From: lzhan011 <35493221+lzhan011@users.noreply.github.com> Date: Mon, 5 Oct 2026 12:35:26 -0500 Subject: [PATCH 2/6] style(cache): format the redaction helper the way CI's ruff does `lint` failed on `ruff format --check`: CI pins ruff 0.15.3, which joins the return expression into one line where the ruff I formatted with locally (0.16.5) split it across two. Would reformat: litellm/caching/redis_cache.py 1 file would be reformatted, 1 file already formatted No behaviour change -- reformatted with 0.15.3 so the check agrees. `ruff check` passes on all three files, and tests/unit/caching/test_redis_cache.py still reports 100 passed. --- litellm/caching/redis_cache.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/litellm/caching/redis_cache.py b/litellm/caching/redis_cache.py index 68d711fc98c..a156fe85cf0 100644 --- a/litellm/caching/redis_cache.py +++ b/litellm/caching/redis_cache.py @@ -565,10 +565,7 @@ def redact_redis_error(exc: BaseException) -> str: if match is None: return message command_name: Final = match["command"].split(" ", 1)[0].strip() or _REDACTED - return ( - f"Command # {match['index']} ({command_name} {_REDACTED}) " - f"of pipeline caused error: {match['detail']}" - ) + return f"Command # {match['index']} ({command_name} {_REDACTED}) of pipeline caused error: {match['detail']}" def log_redis_failure( From 68e93eae6a03ee709d598540ba315974465ecad6 Mon Sep 17 00:00:00 2001 From: lzhan011 <35493221+lzhan011@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:40:37 -0500 Subject: [PATCH 3/6] style(cache): give the deferred import's noqa a reason, as the gate requires `lint` was still red after the format fix, on a different rule this time: FAIL: LIT-rule totals exceed their limit (base 1238cfe90f): LIT003: total 263 over limit 261 (this change added 1) litellm/_service_logger.py:345 LIT003 is `scripts/check_type_discipline.py`'s "noqa requires rule codes or a reason". The deferred import carried `# noqa: PLC0415` with no reason, so it counted against the budget; it now says why the import is deferred (a module-scope import would be circular, since redis_cache imports this module). Two things worth recording, both found by running the gate rather than guessing: * A first attempt put the explanation on its own comment line above the import. That line contained the word "noqa", which `scan_comments` reads as a *bare* suppression -- so the explanatory comment created a second LIT003 instead of clearing the first. The reason has to live on the suppression line itself. * `ruff check` then wanted I001 on the longer line, and its fix wraps the import in parentheses with the comment on the imported name. `ruff format --check`, `ruff check` and the LIT003 scan are all clean on the three files now, and tests/unit/caching/test_redis_cache.py still reports 100 passed. --- litellm/_service_logger.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/litellm/_service_logger.py b/litellm/_service_logger.py index 7688968b9dc..9f81e6bdb51 100644 --- a/litellm/_service_logger.py +++ b/litellm/_service_logger.py @@ -342,7 +342,9 @@ class ServiceLogging(CustomLogger): # than at each call site: `redis_cache` hands this hook an exception # from ~16 places and a new one must not be able to leak by being # written without the redaction. - from litellm.caching.redis_cache import redact_redis_error # noqa: PLC0415 + from litellm.caching.redis_cache import ( + redact_redis_error, # noqa: PLC0415 # redis_cache imports this module; a module-scope import would be circular + ) error_message = redact_redis_error(error) if isinstance(error, Exception) else error_message From eeec57d7c3e1c3b7aad6a70efa05ae4cde1bd2d3 Mon Sep 17 00:00:00 2001 From: lzhan011 <35493221+lzhan011@users.noreply.github.com> Date: Sat, 10 Oct 2026 00:32:52 -0500 Subject: [PATCH 4/6] style(cache): mark the redaction's error_message write as a deliberate rebind `lint` / `python` went red on the type-discipline gate after the merge: FAIL: LIT-rule totals grew past their merge-base count (base 3069a467d0): LIT010: total 16257 over ceiling 16256 (this change added 1) Localised by counting the rule per file against the merge base rather than guessing: `litellm/caching/redis_cache.py` is unchanged at 101, and `litellm/_service_logger.py` goes 10 -> 11. The one added is the redaction's write to `error_message`, which LIT010 reads as an un-`Final` assignment. `error_message` is deliberately rebound -- the three assignments above it in the same function are rebinds of the same name -- so the sanctioned suppression is `# rebind-ok`, not a `Final` annotation (which would be wrong here and would not compile). Recording a wrong turn, because it is the same shape as the LIT003 one earlier in this PR: putting `# rebind-ok: ...` on its own comment line above the statement did not clear it. The count went 11 -> 12, because the scanner reads the comment as a suppression attached to nothing and the statement is still unsuppressed. It has to sit on the statement itself. LIT010 is back to the merge-base count of 10 and LIT003 is 0 in that file; `ruff format --check` and `ruff check` are clean; tests/unit/caching/ test_redis_cache.py reports 101 passed. --- litellm/_service_logger.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/litellm/_service_logger.py b/litellm/_service_logger.py index 9f81e6bdb51..688ce641283 100644 --- a/litellm/_service_logger.py +++ b/litellm/_service_logger.py @@ -346,7 +346,9 @@ class ServiceLogging(CustomLogger): redact_redis_error, # noqa: PLC0415 # redis_cache imports this module; a module-scope import would be circular ) - error_message = redact_redis_error(error) if isinstance(error, Exception) else error_message + error_message = ( # rebind-ok: narrows the message derived above, like the three assignments that produced it + redact_redis_error(error) if isinstance(error, Exception) else error_message + ) payload: Final = ServiceLoggerPayload( is_error=True, From 26631bca72752145eef98164464044dce6940fe9 Mon Sep 17 00:00:00 2001 From: LeizhenZhang Date: Mon, 2 Nov 2026 18:32:19 -0600 Subject: [PATCH 5/6] fix(cache): redact Redis exceptions in traceback output Signed-off-by: LeizhenZhang --- litellm/caching/redis_cache.py | 2 +- tests/unit/caching/test_redis_cache.py | 33 ++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/litellm/caching/redis_cache.py b/litellm/caching/redis_cache.py index 3f536a660c1..920111bfb36 100644 --- a/litellm/caching/redis_cache.py +++ b/litellm/caching/redis_cache.py @@ -555,7 +555,7 @@ def log_redis_failure( if isinstance(exc, RedisCircuitBreakerOpenError): logger.debug("%s: %s", message, reason, stacklevel=2) return - exc_info: Final = exc if with_traceback else None + exc_info: Final = (type(exc), Exception(reason), exc.__traceback__) if with_traceback else None if not is_redis_timeout_failure(exc): logger.log(level, "%s: %s", message, reason, exc_info=exc_info, stacklevel=2) return diff --git a/tests/unit/caching/test_redis_cache.py b/tests/unit/caching/test_redis_cache.py index 1a1ac62ddc6..102e31ce49f 100644 --- a/tests/unit/caching/test_redis_cache.py +++ b/tests/unit/caching/test_redis_cache.py @@ -1034,8 +1034,9 @@ def test_prometheus_services_logger_starts_from_the_breakers_already_built(monke monkeypatch.setattr(litellm, "service_callback", ["prometheus_system"]) gc.collect() ServiceLogging() - assert REGISTRY.get_sample_value("litellm_redis_circuit_breaker_state", {"state": "closed"}) == ( - redis_breaker_state_counts()["closed"] + assert ( + REGISTRY.get_sample_value("litellm_redis_circuit_breaker_state", {"state": "closed"}) + == (redis_breaker_state_counts()["closed"]) ) @@ -1758,6 +1759,34 @@ def test_log_redis_failure_redacts_the_line_it_writes(caplog): assert "WRONGTYPE" in caplog.text +@pytest.mark.parametrize("chained", [False, True]) +def test_log_redis_failure_redacts_traceback_and_exception_chain( + caplog: pytest.LogCaptureFixture, chained: bool +) -> None: + import logging + + from redis.exceptions import ResponseError + + from litellm.caching.redis_cache import log_redis_failure + + with caplog.at_level(logging.ERROR, logger="LiteLLM"): + try: + raise _annotated_pipeline_error() + except ResponseError as pipeline_error: + if chained: + try: + raise RuntimeError("Redis operation failed") from pipeline_error + except RuntimeError as wrapper: + log_redis_failure(verbose_logger, logging.ERROR, "Redis failed", wrapper, with_traceback=True) + else: + log_redis_failure(verbose_logger, logging.ERROR, "Redis failed", pipeline_error, with_traceback=True) + + assert SENSITIVE_CACHE_KEY not in caplog.text + assert "private customer message" not in caplog.text + assert "Traceback (most recent call last)" in caplog.text + assert "Redis failed" in caplog.text + + @pytest.mark.asyncio async def test_service_failure_hook_redacts_before_telemetry(): """Datadog/OTel must not receive the annotated command either.""" From c4fa1c5180f25bb063c4cd420da5cee0e5f4fe5b Mon Sep 17 00:00:00 2001 From: LeizhenZhang Date: Mon, 2 Nov 2026 18:34:19 -0600 Subject: [PATCH 6/6] fix(cache): retain exception names in redacted tracebacks Signed-off-by: LeizhenZhang --- litellm/caching/redis_cache.py | 4 +++- tests/unit/caching/test_redis_cache.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/litellm/caching/redis_cache.py b/litellm/caching/redis_cache.py index 920111bfb36..59a5131f7db 100644 --- a/litellm/caching/redis_cache.py +++ b/litellm/caching/redis_cache.py @@ -555,7 +555,9 @@ def log_redis_failure( if isinstance(exc, RedisCircuitBreakerOpenError): logger.debug("%s: %s", message, reason, stacklevel=2) return - exc_info: Final = (type(exc), Exception(reason), exc.__traceback__) if with_traceback else None + exc_info: Final = ( + (Exception, Exception(f"{type(exc).__name__}: {reason}"), exc.__traceback__) if with_traceback else None + ) if not is_redis_timeout_failure(exc): logger.log(level, "%s: %s", message, reason, exc_info=exc_info, stacklevel=2) return diff --git a/tests/unit/caching/test_redis_cache.py b/tests/unit/caching/test_redis_cache.py index 102e31ce49f..19813199487 100644 --- a/tests/unit/caching/test_redis_cache.py +++ b/tests/unit/caching/test_redis_cache.py @@ -1784,6 +1784,7 @@ def test_log_redis_failure_redacts_traceback_and_exception_chain( assert SENSITIVE_CACHE_KEY not in caplog.text assert "private customer message" not in caplog.text assert "Traceback (most recent call last)" in caplog.text + assert ("RuntimeError" if chained else "ResponseError") in caplog.text assert "Redis failed" in caplog.text