fix(cache): log a key digest, not the cache key, on backend failure - #314
Merged
Merged
Conversation
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.
allen0099
force-pushed
the
fix/cache-log-key-digest
branch
from
September 27, 2026 14:10
f02dfa5 to
5fc3ce1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@cache's fail-open warnings ("Cache backend read failed" / "Cache backend write failed") logged the full cache key at WARNING. The key holds the raw query string (?token=...,?code=..., e-mails),varyheader values (#268) and anybuild_cache_keycomponents (user IDs etc.).fastapi_cachex.types.log_ref(value): first 12 hex digits of SHA-256 (UTF-8,surrogatepass), which is exactly what_state_refinstate/manager.pydid._state_refnow delegates to it, so OAuth state refs and cache key refs use the same format.cache.py: both warnings go through_log_backend_failure(), which logsmethod=... path=%r key_ref=... error=%rat WARNING andkey_ref=... key=<full key>at DEBUG. The sharedkey_reflets you match a warning to its key.manager.py:CacheManager.get()'s "Failed to decode cached value" warning had the same problem, and developer keys often embed user IDs or e-mails. It now logskey_refat WARNING and the key at DEBUG.docs/HTTP_CACHING.mdand its zh-TW mirror.changelog.d/299.security.md.Path and log forging
The path is client input and is percent-decoded. It is logged with
%r, so control characters come out escaped. Starlette'srequest.urlalready strips CR/LF/tab (urllib'surlsplit), so newline forging was not possible through this field. ANSI escapes (\x1b) and U+2028 did get through, and%rnow escapes them. A test drives the ASGI app directly with such a path, because test clients normalise it. No other WARNING+ site logs the path; the existing DEBUG sites usepath=%sand were left alone.Audit of other log sites (INFO and above)
cache.py: the two sites above were the only WARNING+ calls.invalidate(),clear_path()and every other key log are DEBUG only.routes.py,lock.py,proxy.py, backends (memory/redis/memcached): DEBUG only, nothing to change.manager.py: fixed (above).state/manager.py: already uses a digest (StateManagerlogs raw OAuth state values at WARNING #107).session/manager.py: two WARNINGs about missing IP/UA binding, with no keys or values. Nothing to change.DEBUG logging still writes full keys, query strings, session IDs and client IPs throughout. That is intended for local troubleshooting, as the issue proposes.
Tests
tests/test_cache_backend_failure.py: read and write failures (fail_open=True) with?token=...&email=...and avary=["X-Tenant"]value. At WARNING the token, e-mail, tenant value and|||are absent, and method,path='/callback', error and the digest are present. At DEBUG the full key is present, and its SHA-256 matcheskey_ref. A second test covers the control-character path.tests/test_cache_manager.py: the decode-failure warning carrieskey_ref, not the key, and DEBUG has the key.ruff check,ruff format --check,mypy --strict: clean.pytest: 997 passed, 194 skipped (no live Redis/Memcached). Coverage is 93.77%;cache.py,manager.pyandtypes.pyare at 100%.Closes #299