Fix query result cache leaking across security contexts: include full SecurityClaims in cache key - #9704
Merged
Conversation
The resolver query cache key only hashed the claims' user attributes, so requests differing only in additional security rules (e.g. locked filters on magic auth tokens), permissions, or skipped security checks produced identical keys and could receive each other's cached results. Hash the full marshaled SecurityClaims instead, matching the security engine's own computeCacheKey.
SecurityClaims\ in cache keySecurityClaims in cache key
Collaborator
Author
|
@codex: review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
pjain1
reviewed
Jul 20, 2026
The user ID does not affect the resolved security policy: built-in rules derive user identity from the "id" and "email" user attributes, which remain part of the key. Excluding it lets requests that resolve to the same policy share cached results, which helps embed use cases where the user ID varies per session. Addresses review feedback from @pjain1.
pjain1
approved these changes
Jul 20, 2026
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.
runtime.Resolveonly hashedClaims.UserAttributes, so requests that differed only inAdditionalRules(e.g. locked filters on magic auth tokens),Permissions, orSkipChecksproduced identical cache keys and could receive each other's cached results.SecurityClaimsis hashed into the key, matching what the security engine'scomputeCacheKeyalready does.Resolvecovers all resolvers (metrics,metrics_sql, annotations, unions); the legacyruntime.Querypath already included the full claims in its key and was not affected.metrics_security.yamlthat run the same query back-to-back with identical (empty) user attributes but different security contexts; without the fix, three of the four cases fail by returning another context's cached rows.Steps to reproduce (before this fix):
country, and an explore dashboard on it.country = 'US', URL B locked tocountry = 'EU'. Both are anonymous, so both carry empty user attributes; the locked filters travel asAdditionalRules.skip_nested_security: trueand one without; call the skipping one first, then the enforcing one with identical user attributes — the second returns the unfiltered cached result.Checklist: