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.
This commit is contained in:
mubashir1osmani 2026-08-10 17:19:39 -07:00
parent fd66d87e46
commit 16f1f93d6e
2 changed files with 89 additions and 8 deletions

View file

@ -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)

View file

@ -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"