From 16f1f93d6e6f3cf585378514fd5b09f8b77c5bb3 Mon Sep 17 00:00:00 2001 From: mubashir1osmani Date: Mon, 10 Aug 2026 17:19:39 -0700 Subject: [PATCH 1/3] 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" From 3e86bafbde1ba621ff2027373ba322393ee2ee98 Mon Sep 17 00:00:00 2001 From: mubashir1osmani Date: Mon, 10 Aug 2026 18:23:56 -0700 Subject: [PATCH 2/3] test(caching): bind the redaction test to a closed port, not 6379 Constructing RedisCache runs a synchronous startup ping, which the redis_no_ping fixture does not suppress. Using the default port 6379 meant that ping could reach a real local Redis, adding latency and letting local Redis availability sway an otherwise isolated unit test. Use _closed_port(), like the sibling circuit-breaker tests, so the ping refuses immediately and the test stays hermetic. --- tests/test_litellm/caching/test_redis_cache.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_litellm/caching/test_redis_cache.py b/tests/test_litellm/caching/test_redis_cache.py index 7bc4ac6cea8..ded1385851f 100644 --- a/tests/test_litellm/caching/test_redis_cache.py +++ b/tests/test_litellm/caching/test_redis_cache.py @@ -672,7 +672,7 @@ async def test_redis_write_failure_does_not_log_master_key(redis_no_ping, call_c """ from litellm._logging import verbose_logger - cache: Final = RedisCache(host="127.0.0.1", port=6379, socket_timeout=0.5) + cache: Final = RedisCache(host="127.0.0.1", port=_closed_port(), socket_timeout=0.5) capture: Final = _CaptureRecords() verbose_logger.addFilter(capture) try: From f4891a273ce67baa357a30acf23a80c49b51cfbc Mon Sep 17 00:00:00 2001 From: mubashir1osmani Date: Mon, 10 Aug 2026 18:36:28 -0700 Subject: [PATCH 3/3] test(caching): drop the redaction unit test Review found RedisCache.__init__ still reaches the network during construction (sync info() and ping() before init_async_client is patched), and the closed-port trick leaves a port-reuse race. Making construction fully hermetic needs factory patching that isn't worth the weight here, so the fix is covered by the live-proxy proof in the PR instead. --- .../test_litellm/caching/test_redis_cache.py | 80 ------------------- 1 file changed, 80 deletions(-) diff --git a/tests/test_litellm/caching/test_redis_cache.py b/tests/test_litellm/caching/test_redis_cache.py index ded1385851f..59200719197 100644 --- a/tests/test_litellm/caching/test_redis_cache.py +++ b/tests/test_litellm/caching/test_redis_cache.py @@ -1,8 +1,6 @@ import asyncio -import logging import os import sys -from typing import Final from unittest.mock import MagicMock, patch import pytest @@ -14,8 +12,6 @@ from unittest.mock import AsyncMock from litellm.caching.redis_cache import RedisCache -_MASTER_KEY: Final = "sk-1234567890abcdefghijklmnopqrstuvwxyz" - @pytest.fixture def redis_no_ping(): @@ -612,79 +608,3 @@ 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=_closed_port(), 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"