From 16f1f93d6e6f3cf585378514fd5b09f8b77c5bb3 Mon Sep 17 00:00:00 2001 From: mubashir1osmani Date: Mon, 10 Aug 2026 17:19:39 -0700 Subject: [PATCH] fix(caching): redact secrets in Redis write-failure logs When a Redis write timed out, the error paths in async_set_cache and async_set_cache_sadd logged the raw value being written. The proxy caches its own general_settings row, whose value holds the master_key, so a Redis outage spilled the master key into logs in plaintext. Redact the value (and key) with redact_string at the call site, before the record is built. The handler-level SecretRedactionFilter already masks this under default config, but it is opt-out via LITELLM_DISABLE_REDACT_SECRETS and a sink that renders the record before litellm's handler runs bypasses it. Redacting at the source removes that dependence for this high-value path, matching the existing call-site redaction in the router fallback logs. The regression test captures records with a logger-level filter that runs before the handler filter, so it asserts on the source-level redaction and fails if the raw value is ever logged again. --- litellm/caching/redis_cache.py | 17 ++-- .../test_litellm/caching/test_redis_cache.py | 80 +++++++++++++++++++ 2 files changed, 89 insertions(+), 8 deletions(-) diff --git a/litellm/caching/redis_cache.py b/litellm/caching/redis_cache.py index 5fedfc5bcce..c82f8de9b12 100644 --- a/litellm/caching/redis_cache.py +++ b/litellm/caching/redis_cache.py @@ -30,6 +30,7 @@ from litellm.constants import ( ) from litellm.litellm_core_utils.core_helpers import _get_parent_otel_span_from_kwargs from litellm.litellm_core_utils.coroutine_checker import coroutine_checker +from litellm.litellm_core_utils.secret_redaction import redact_string from litellm.types.caching import ( RedisPipelineIncrementOperation, RedisPipelineLpopOperation, @@ -673,8 +674,8 @@ class RedisCache(BaseCache): if key is None: verbose_logger.debug( - "LiteLLM Redis Caching: async set() skipped — key is None, value=%r", - value, + "LiteLLM Redis Caching: async set() skipped — key is None, value=%s", + redact_string(str(value)), ) return None @@ -696,10 +697,10 @@ class RedisCache(BaseCache): ) ) verbose_logger.error( - "LiteLLM Redis Caching: async set() - Got exception from REDIS %s, key=%r, value=%r", + "LiteLLM Redis Caching: async set() - Got exception from REDIS %s, key=%s, value=%s", str(e), - key, - value, + redact_string(str(key)), + redact_string(str(value)), ) raise e @@ -750,7 +751,7 @@ class RedisCache(BaseCache): verbose_logger.error( "LiteLLM Redis Caching: async set() - Got exception from REDIS %s, Writing value=%s", str(e), - value, + redact_string(str(value)), ) _record_swallowed_redis_failure(self._circuit_breaker, e) @@ -881,7 +882,7 @@ class RedisCache(BaseCache): verbose_logger.error( "LiteLLM Redis Caching: async set() - Got exception from REDIS %s, Writing value=%s", str(e), - value, + redact_string(str(value)), ) raise e @@ -920,7 +921,7 @@ class RedisCache(BaseCache): verbose_logger.error( "LiteLLM Redis Caching: async set_cache_sadd() - Got exception from REDIS %s, Writing value=%s", str(e), - value, + redact_string(str(value)), ) _record_swallowed_redis_failure(self._circuit_breaker, e) diff --git a/tests/test_litellm/caching/test_redis_cache.py b/tests/test_litellm/caching/test_redis_cache.py index 59200719197..7bc4ac6cea8 100644 --- a/tests/test_litellm/caching/test_redis_cache.py +++ b/tests/test_litellm/caching/test_redis_cache.py @@ -1,6 +1,8 @@ import asyncio +import logging import os import sys +from typing import Final from unittest.mock import MagicMock, patch import pytest @@ -12,6 +14,8 @@ from unittest.mock import AsyncMock from litellm.caching.redis_cache import RedisCache +_MASTER_KEY: Final = "sk-1234567890abcdefghijklmnopqrstuvwxyz" + @pytest.fixture def redis_no_ping(): @@ -608,3 +612,79 @@ async def test_only_connectivity_failures_open_the_breaker(error, opens_breaker) await _run_under_circuit_breaker(breaker, "op", failing_call) assert breaker.is_open() is opens_breaker + + +class _RedisWritesTimeOut: + """Injected async client whose writes fail like a real 'Timeout connecting to server'.""" + + async def set(self, name: str, value: str, nx: bool = False, ex: object = None) -> None: + from redis.exceptions import TimeoutError as RedisTimeoutError + + raise RedisTimeoutError("Timeout connecting to server") + + async def sadd(self, key: str, *members: object) -> None: + from redis.exceptions import TimeoutError as RedisTimeoutError + + raise RedisTimeoutError("Timeout connecting to server") + + +class _CaptureRecords(logging.Filter): + """Logger-level filter that records each formatted message. + + Attached to the logger (not a handler), so it runs before the handler-level + SecretRedactionFilter can mutate the record. That isolates this test to the + redaction done at the log call site, independent of the logging config. + """ + + def __init__(self) -> None: + super().__init__() + self.messages: tuple[str, ...] = () + + def filter(self, record: logging.LogRecord) -> bool: + self.messages = (*self.messages, record.getMessage()) + return True + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "call_cache", + ( + pytest.param( + lambda c: c.async_set_cache( + key="litellm_config:param:general_settings", + value={"param_name": "general_settings", "param_value": {"master_key": _MASTER_KEY}}, + ), + id="async_set_cache", + ), + pytest.param( + lambda c: c.async_set_cache_sadd(key="k", value=[_MASTER_KEY], ttl=60), + id="async_set_cache_sadd", + ), + ), +) +async def test_redis_write_failure_does_not_log_master_key(redis_no_ping, call_cache): + """A Redis write timeout must not spill the cached value's secrets into logs. + + The proxy caches its own general_settings row, whose value holds the master key. When a + write to Redis fails, the error path logged that value verbatim, leaking the master key + in plaintext. Redacting at the call site keeps the leak out regardless of the logging + config, so this asserts on the record before any handler-level redaction filter runs. + """ + from litellm._logging import verbose_logger + + cache: Final = RedisCache(host="127.0.0.1", port=6379, socket_timeout=0.5) + capture: Final = _CaptureRecords() + verbose_logger.addFilter(capture) + try: + with patch.object(cache, "init_async_client", return_value=_RedisWritesTimeOut()): + try: + await call_cache(cache) + except Exception: + pass + finally: + verbose_logger.removeFilter(capture) + + joined: Final = "\n".join(capture.messages) + assert "Got exception from REDIS" in joined, "the write-failure path must have logged" + assert _MASTER_KEY not in joined, f"master key leaked in plaintext: {joined!r}" + assert "REDACTED" in joined, "the secret must be redacted, not silently dropped"