fix(cache): hash credential header values that vary puts into the key - #317
Merged
Merged
Conversation
Authorization, Proxy-Authorization, Cookie and X-Session-Token are keyed on sha256:<hex> of the value; missing or empty stays name=. vary=['Cookie'] emits a UserWarning at decoration, since every visitor gets an entry.
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.
@cache(vary=[...])(unreleased, first shipping in 0.3.9) put raw header values into the cache key, sovary=["Authorization"]withcache_authorized=True, orvary=["Cookie"], wrote bearer tokens and session cookies intoget_all_keys(), the/cached-records//cached-hitsmonitoring routes and the Redis/Memcached keyspace.Changes
Credential headers are hashed. For
authorization,proxy-authorization,cookieand the session subsystem's default header (matched case-insensitively), a non-empty value becomesname=sha256:<64 hex>: the full SHA-256 of the normalised value (trimmed, repeated lines joined with,, as before). It is not the 12-hexlog_ref, because a key component needs collision resistance. Other headers stay readable.authorization=, not a hash of"". Anonymous callers share one entry, and the key still shows it is the anonymous one.DEFAULT_SESSION_HEADER_NAMEconstant insession/config.py, which is also theSessionConfig.header_namedefault. There is no circular import: the session package never importscache.py. A session header configured under another name is not hashed, and the docs say so.invalidate(vary=...)goes through the same_vary_components, so it deletes the hashed variant (tested). The monitoring routes just show the hashed component.vary=["Cookie"]warns. AUserWarningis emitted at decoration time withstacklevel=2, so it points at the user's@cache(...)line (a test checkswarning.filename). It explains the per-visitor entry growth and suggestskey_builder+build_cache_key(request, <cookie or user id>)orprivate=True.warnings.filterwarnings("ignore", message="cache vary on Cookie"), documented and tested. Skipping the warning when akey_builderis given would be wrong: thevarycomponents are still appended to whatever the builder returns, so the per-visitor growth remains. No new parameter.Authorization/X-Session-Token. One entry per caller is the intended use ofvarywithcache_authorized=True, and a caller keeps one token across many requests, unlike an arbitrary cookie bundle.Interaction with the per-caller rules, documented.
vary=["Authorization"]does not lift theAuthorizationbypass. Withoutpublic/cache_authorized, only the anonymousauthorization=entry is stored (tested).private.Docs: "Varying on request headers" in
docs/HTTP_CACHING.mdgains "Credential headers are hashed" and "vary=["Cookie"]warns". The@cacheandinvalidatedocstrings anddocs/CACHE_FLOW.mdare updated, plus the zh-TW mirrors.Changelog:
changelog.d/268.added.mdis amended rather than adding a312.security.md.varyhas never been released, so no user was exposed, and the 0.3.9 notes should describe vary's final behaviour in one place.Tests
New tests in
tests/test_cache_vary.pycover:get_all_keys(),/cached-recordsor/cached-hits, and the digest is shown instead;Authorizationbypass still applies without the opt-in;Proxy-Authorization,X-Session-Token,AUTHORIZATIONcasing, and joinedCookielines;invalidate(vary=...)deletes the hashed entry;Accept-Languageor the credential headers.No existing test asserted raw credential values.
Full suite including the live Redis and Memcached tests: 1235 passed, 1 skipped, coverage 99.97%. ruff check/format and
mypy --strictare clean, andpytest -k changelogpasses.Closes #312