Skip to content

Commit 5fc3ce1

Browse files
committed
fix(cache): log a key digest, not the cache key, on backend failure
The read/write failure warnings wrote the full cache key, which holds the raw query string, vary header values and custom key components. They now log method, path (%r-escaped) and key_ref, a 12-hex SHA-256 digest shared with the OAuth state logs via types.log_ref; the full key goes to DEBUG under the same key_ref. CacheManager.get()'s decode warning gets the same treatment.
1 parent d1c41e0 commit 5fc3ce1

9 files changed

Lines changed: 192 additions & 11 deletions

File tree

‎changelog.d/299.security.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
**`@cache` backend-failure warnings no longer log the cache key.** The
2+
"Cache backend read failed" and "Cache backend write failed" warnings wrote
3+
the full key, which holds the raw query string (`?token=...`, `?code=...`,
4+
e-mail addresses), `vary` header values and any `build_cache_key` components,
5+
into application logs. They now log the method, the path (formatted with `%r`,
6+
so a control character in it is escaped) and `key_ref`, the first 12 hex
7+
digits of the key's SHA-256, the same digest format as the OAuth state logs;
8+
the full key is logged at `DEBUG` under the same `key_ref`.
9+
`CacheManager.get()`'s "Failed to decode cached value" warning likewise logs
10+
`key_ref` instead of the developer's key, which often embeds user IDs or
11+
e-mail addresses.

‎docs/HTTP_CACHING.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,13 @@ served unstored. Either way a warning is logged on the `fastapi_cachex.cache`
134134
logger, and a backend outage cannot turn cached routes into 500s. The load
135135
goes to your handlers instead, so watch for those warnings.
136136

137+
The warning names the request's method and path and a `key_ref`, a short
138+
SHA-256 digest of the cache key, but not the key itself: the key holds the
139+
raw query string, `vary` header values and any `build_cache_key` components,
140+
which may be tokens or personal data. The full key is logged at `DEBUG` with
141+
the same `key_ref`, so enabling `DEBUG` on `fastapi_cachex.cache` while
142+
troubleshooting ties a warning to its key.
143+
137144
Pass `fail_open=False` to let the backend error propagate and fail the request
138145
instead:
139146

‎fastapi_cachex/cache.py‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
from .types import CacheEntry
4444
from .types import CacheKeyBuilder
4545
from .types import escape_key_component
46+
from .types import log_ref
4647

4748
if TYPE_CHECKING:
4849
from fastapi.routing import APIRoute
@@ -109,6 +110,29 @@ def per_user_key(request: Request) -> str:
109110
return key
110111

111112

113+
def _log_backend_failure(
114+
what: str, request: Request, cache_key: str, error: Exception
115+
) -> None:
116+
"""Log a failed backend call without writing the cache key at WARNING.
117+
118+
The key holds the raw query string, ``vary`` header values and any custom
119+
key components, so the warning carries the method, the path and a digest
120+
of the key; the full key goes to ``DEBUG`` under the same digest. The path
121+
is formatted with ``%r`` because it is percent-decoded client input and
122+
could otherwise put a CR/LF into the log.
123+
"""
124+
key_ref = log_ref(cache_key)
125+
logger.warning(
126+
"Cache backend %s. method=%s path=%r key_ref=%s error=%r",
127+
what,
128+
request.method,
129+
request.url.path,
130+
key_ref,
131+
error,
132+
)
133+
logger.debug("Cache backend %s; key_ref=%s key=%s", what, key_ref, cache_key)
134+
135+
112136
def _append_key_components(key: str, components: Sequence[str | int]) -> str:
113137
"""Append each component to ``key``, escaped, after another separator."""
114138
parts = [key]
@@ -940,11 +964,7 @@ async def serve(*args: Any, **kwargs: Any) -> Response:
940964
except Exception as e:
941965
if not fail_open:
942966
raise
943-
logger.warning(
944-
"Cache backend read failed; serving uncached. key=%s error=%r",
945-
cache_key,
946-
e,
947-
)
967+
_log_backend_failure("read failed; serving uncached", req, cache_key, e)
948968
cached_data = None
949969

950970
current_response: Response | None = None
@@ -1060,10 +1080,8 @@ async def serve(*args: Any, **kwargs: Any) -> Response:
10601080
except Exception as e:
10611081
if not fail_open:
10621082
raise
1063-
logger.warning(
1064-
"Cache backend write failed; response not stored. key=%s error=%r",
1065-
cache_key,
1066-
e,
1083+
_log_backend_failure(
1084+
"write failed; response not stored", req, cache_key, e
10671085
)
10681086
else:
10691087
logger.debug("Updated cache entry; key=%s ttl=%s", cache_key, ttl)

‎fastapi_cachex/manager.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
from .backends.base import validate_ttl
1313
from .proxy import BackendProxy
1414
from .types import CacheEntry
15+
from .types import log_ref
1516

1617
logger = logging.getLogger(__name__)
1718

@@ -77,7 +78,9 @@ async def get(self, key: str, default: Any = None) -> Any:
7778
try:
7879
return json.loads(cached.content)
7980
except _DECODE_ERRORS:
80-
logger.warning("Failed to decode cached value; key=%s", key)
81+
# Keys often embed user IDs or e-mails: only a digest at WARNING.
82+
logger.warning("Failed to decode cached value; key_ref=%s", log_ref(key))
83+
logger.debug("Failed to decode cached value; key=%s", key)
8184
return default
8285

8386
async def set(self, key: str, value: Any, ttl: int | None = None) -> None:

‎fastapi_cachex/state/manager.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from fastapi_cachex.backends.base import validate_ttl
1515
from fastapi_cachex.proxy import BackendProxy
1616
from fastapi_cachex.types import CacheEntry
17+
from fastapi_cachex.types import log_ref
1718

1819
from .exceptions import InvalidStateError
1920
from .exceptions import StateDataError
@@ -36,7 +37,7 @@ def _state_ref(state: str) -> str:
3637
The state comes straight from the callback query string, so logging it raw
3738
would leak live tokens and let a caller forge log lines with CR/LF.
3839
"""
39-
return hashlib.sha256(state.encode("utf-8", "surrogatepass")).hexdigest()[:12]
40+
return log_ref(state)
4041

4142

4243
def _binding_hash(binding: str) -> str:

‎fastapi_cachex/types.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Type definitions and type aliases for FastAPI-CacheX."""
22

3+
import hashlib
34
import re
45
from collections.abc import Callable
56
from dataclasses import dataclass
@@ -35,6 +36,18 @@ def unescape_key_component(value: str) -> str:
3536
return _KEY_UNESCAPE_RE.sub(lambda match: _KEY_UNESCAPES[match.group()], value)
3637

3738

39+
def log_ref(value: str) -> str:
40+
"""Return a short digest that identifies ``value`` in logs without revealing it.
41+
42+
Cache keys carry the raw query string, ``vary`` header values and custom
43+
key components, and OAuth states come from the callback query string;
44+
logging them at ``WARNING`` would leak tokens, e-mail addresses or user
45+
IDs into application logs. The same value always gives the same digest,
46+
so a warning can still be matched to the full value logged at ``DEBUG``.
47+
"""
48+
return hashlib.sha256(value.encode("utf-8", "surrogatepass")).hexdigest()[:12]
49+
50+
3851
# Status replayed for entries stored before ``CacheEntry`` carried a status code.
3952
DEFAULT_STATUS_CODE = 200
4053

‎i18n/zh-TW/docs/HTTP_CACHING.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ handler 回傳一般資料而非 `Response` 時,得到的處理與沒有 `@cac
8686

8787
`@cache` 採取 fail open。讀取時後端拋出錯誤(例如 Redis 或 Memcached 無法連線),該請求會被當成快取未命中,照常執行 handler。儲存回應時拋出錯誤(例如回應超過 Memcached 的項目大小上限,預設為 1 MB),回應會照常送出,只是不會被儲存。兩種情況都會在 `fastapi_cachex.cache` logger 記錄一則警告,因此後端中斷不會讓有快取的路由變成 500;負載會轉到你的 handler 上,請留意這些警告。
8888

89+
警告會列出請求的 method、路徑與 `key_ref`(快取鍵的簡短 SHA-256 摘要),但不會列出快取鍵本身:快取鍵含有原始查詢字串、`vary` 標頭值以及任何 `build_cache_key` 元件,可能是 token 或個人資料。完整的快取鍵會以 `DEBUG` 等級連同相同的 `key_ref` 記錄,因此排查問題時在 `fastapi_cachex.cache` 開啟 `DEBUG`,即可將警告對應到其快取鍵。
90+
8991
傳入 `fail_open=False` 則會讓後端錯誤直接往外拋出,使該請求失敗:
9092

9193
```python

‎tests/test_cache_backend_failure.py‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44
uncached instead of turning every cached route into a 500.
55
"""
66

7+
import hashlib
78
import logging
89

910
import pytest
1011
from fastapi import FastAPI
1112
from fastapi.responses import PlainTextResponse
1213
from fastapi.testclient import TestClient
14+
from starlette.types import Message
15+
from starlette.types import Scope
1316

1417
from fastapi_cachex import BackendProxy
1518
from fastapi_cachex import cache
@@ -131,3 +134,103 @@ async def large() -> PlainTextResponse:
131134

132135
assert response.status_code == 200
133136
assert response.text == body
137+
138+
139+
@pytest.mark.parametrize(
140+
("fail_get", "fail_set", "message"),
141+
[(True, False, "read failed"), (False, True, "write failed")],
142+
ids=["get", "set"],
143+
)
144+
def test_failure_warning_does_not_log_the_cache_key(
145+
caplog: pytest.LogCaptureFixture, *, fail_get: bool, fail_set: bool, message: str
146+
) -> None:
147+
"""The key holds the query string and vary values: only a digest at WARNING (#299)."""
148+
BackendProxy.set(FailingBackend(fail_get=fail_get, fail_set=fail_set))
149+
app = FastAPI()
150+
151+
@app.get("/callback")
152+
@cache(ttl=60, vary=["X-Tenant"])
153+
async def callback() -> dict[str, str]:
154+
return {"ok": "yes"}
155+
156+
with caplog.at_level(logging.DEBUG, logger="fastapi_cachex.cache"):
157+
response = TestClient(app).get(
158+
"/callback",
159+
params={"token": "s3cr3t-token", "email": "alice@example.com"},
160+
headers={"X-Tenant": "tenant-secret"},
161+
)
162+
163+
assert response.status_code == 200
164+
[warning] = [
165+
r.getMessage()
166+
for r in caplog.records
167+
if r.levelno == logging.WARNING and message in r.getMessage()
168+
]
169+
for secret in ("s3cr3t-token", "alice", "tenant-secret", "|||"):
170+
assert secret not in warning
171+
assert "method=GET" in warning
172+
assert "path='/callback'" in warning
173+
assert "error=ConnectionError('backend unreachable')" in warning
174+
175+
# The full key is at DEBUG only, tagged with the same digest.
176+
[debug] = [
177+
r.getMessage()
178+
for r in caplog.records
179+
if r.levelno == logging.DEBUG and message in r.getMessage()
180+
]
181+
key = debug.split(" key=", 1)[1]
182+
assert "token=s3cr3t-token" in key
183+
assert "tenant-secret" in key
184+
digest = hashlib.sha256(key.encode()).hexdigest()[:12]
185+
assert f"key_ref={digest}" in warning
186+
assert f"key_ref={digest}" in debug
187+
188+
189+
async def test_failure_warning_escapes_control_characters_in_the_path(
190+
caplog: pytest.LogCaptureFixture,
191+
) -> None:
192+
"""Control characters in the decoded path are escaped, not written raw."""
193+
BackendProxy.set(FailingBackend(fail_get=True, fail_set=False))
194+
app = FastAPI()
195+
196+
@app.get("/{name}")
197+
@cache(ttl=60)
198+
async def item(name: str) -> dict[str, str]:
199+
return {"name": name}
200+
201+
# ASGI servers percent-decode the path, so ``%0D%0A`` arrives as a real
202+
# CR/LF; test clients normalise it away, so drive the app directly.
203+
# ``request.url`` already drops CR/LF/tab (urllib), but an ANSI escape or
204+
# a line separator gets through and must show up escaped.
205+
scope: Scope = {
206+
"type": "http",
207+
"asgi": {"version": "3.0"},
208+
"http_version": "1.1",
209+
"method": "GET",
210+
"scheme": "http",
211+
"path": "/a\r\n\x1b[2K\u2028FAKE entry",
212+
"raw_path": b"/a%0D%0A%1B%5B2K%E2%80%A8FAKE%20entry",
213+
"root_path": "",
214+
"query_string": b"",
215+
"headers": [(b"host", b"testserver")],
216+
"client": ("127.0.0.1", 1),
217+
"server": ("testserver", 80),
218+
}
219+
sent: list[Message] = []
220+
221+
async def receive() -> Message:
222+
return {"type": "http.request", "body": b"", "more_body": False}
223+
224+
async def send(message: Message) -> None:
225+
sent.append(message)
226+
227+
with caplog.at_level(logging.WARNING, logger="fastapi_cachex.cache"):
228+
await app(scope, receive, send)
229+
230+
assert sent[0]["status"] == 200
231+
[warning] = [
232+
r.getMessage() for r in caplog.records if "read failed" in r.getMessage()
233+
]
234+
for raw in ("\n", "\r", "\x1b", "\u2028"):
235+
assert raw not in warning
236+
assert "\\x1b[2K\\u2028FAKE entry'" in warning

‎tests/test_cache_manager.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Tests for CacheManager application-level caching."""
22

33
import asyncio
4+
import logging
45
import time
56
from collections.abc import AsyncGenerator
67
from functools import partial
@@ -17,6 +18,7 @@
1718
from fastapi_cachex.manager_proxy import CacheManagerProxy
1819
from fastapi_cachex.proxy import BackendProxy
1920
from fastapi_cachex.types import CacheEntry
21+
from fastapi_cachex.types import log_ref
2022
from tests.conftest import Clock
2123
from tests.live_servers import REDIS_HOST
2224
from tests.live_servers import REDIS_PORT
@@ -296,6 +298,27 @@ def factory() -> object:
296298
assert not await cache_manager.has("key")
297299

298300

301+
async def test_decode_failure_warning_logs_a_digest_not_the_key(
302+
memory_backend: MemoryBackend, caplog: pytest.LogCaptureFixture
303+
) -> None:
304+
"""Keys often embed e-mails or user IDs: the full key is logged at DEBUG only."""
305+
manager = CacheManager(backend=memory_backend)
306+
key = "user:alice@example.com"
307+
entry = CacheEntry(fingerprint="x", content=b"not valid json")
308+
await memory_backend.set(f"{manager.key_prefix}{key}", entry, ttl=60)
309+
310+
with caplog.at_level(logging.DEBUG, logger="fastapi_cachex.manager"):
311+
assert await manager.get(key) is None
312+
313+
[warning] = [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING]
314+
assert "alice" not in warning
315+
assert f"key_ref={log_ref(key)}" in warning
316+
assert any(
317+
r.levelno == logging.DEBUG and f"key={key}" in r.getMessage()
318+
for r in caplog.records
319+
)
320+
321+
299322
# --- add ------------------------------------------------------------------------
300323

301324

0 commit comments

Comments
 (0)