From 52748ff86b8d81aa6996853a70151cbf45bbfe9a Mon Sep 17 00:00:00 2001 From: Nishant Bangarwa Date: Sun, 19 Jul 2026 13:25:09 +0530 Subject: [PATCH 1/2] Fix query result cache leaking across security contexts 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. --- runtime/resolver.go | 19 +++--- .../resolvers/testdata/metrics_security.yaml | 64 +++++++++++++++++++ 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/runtime/resolver.go b/runtime/resolver.go index 6925aea2238b..e4ce0d4b4e11 100644 --- a/runtime/resolver.go +++ b/runtime/resolver.go @@ -9,9 +9,7 @@ import ( "errors" "fmt" "io" - "strconv" - "github.com/mitchellh/hashstructure/v2" runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1" "github.com/rilldata/rill/runtime/drivers" "github.com/rilldata/rill/runtime/pkg/jsonval" @@ -207,14 +205,15 @@ func (r *Runtime) Resolve(ctx context.Context, opts *ResolveOptions) (res Resolv if _, err := hash.Write(cacheKey); err != nil { return nil, nil, err } - if opts.Claims.UserAttributes != nil { - h, err := hashstructure.Hash(opts.Claims.UserAttributes, hashstructure.FormatV2, nil) - if err != nil { - return nil, nil, err - } - if _, err = hash.Write([]byte(strconv.FormatUint(h, 16))); err != nil { - return nil, nil, err - } + // Hash the full security claims, not just the user attributes: + // permissions, additional rules (e.g. locked filters on magic auth tokens) and skipped checks + // all change the resolved security policy, and results must not be shared across them. + claimsJSON, err := json.Marshal(opts.Claims) + if err != nil { + return nil, nil, err + } + if _, err := hash.Write(claimsJSON); err != nil { + return nil, nil, err } for _, ref := range resolver.Refs() { res, err := ctrl.Get(ctx, ref, false) diff --git a/runtime/resolvers/testdata/metrics_security.yaml b/runtime/resolvers/testdata/metrics_security.yaml index d314a8c287b9..a303de3e77c9 100644 --- a/runtime/resolvers/testdata/metrics_security.yaml +++ b/runtime/resolvers/testdata/metrics_security.yaml @@ -33,3 +33,67 @@ tests: result: - country: US sum: 9 + # The following tests run the same query back-to-back with identical (empty) user attributes, + # varying only the additional rules and skipped security checks. + # They protect against cached results leaking across different security contexts. + - name: cache_isolation_row_filter_dk + resolver: metrics + properties: + metrics_view: metrics_no_security + dimensions: + - name: country + measures: + - name: sum + sort: + - name: country + additional_rules: + - row_filter: country = 'DK' + result: + - country: DK + sum: 6 + - name: cache_isolation_row_filter_us + resolver: metrics + properties: + metrics_view: metrics_no_security + dimensions: + - name: country + measures: + - name: sum + sort: + - name: country + additional_rules: + - row_filter: country = 'US' + result: + - country: US + sum: 9 + - name: cache_isolation_no_rules + resolver: metrics + properties: + metrics_view: metrics_no_security + dimensions: + - name: country + measures: + - name: sum + sort: + - name: country + result: + - country: DK + sum: 6 + - country: US + sum: 9 + - name: cache_isolation_skip_checks + resolver: metrics + properties: + metrics_view: metrics_no_security + dimensions: + - name: country + measures: + - name: sum + sort: + - name: country + skip_security_checks: true + result: + - country: DK + sum: 6 + - country: US + sum: 9 From c56db6b098344b6e2a7cf65dc74945d844cb17b7 Mon Sep 17 00:00:00 2001 From: Nishant Bangarwa Date: Mon, 20 Jul 2026 12:38:29 +0530 Subject: [PATCH 2/2] Exclude UserID from the resolver cache key 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. --- runtime/resolver.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/runtime/resolver.go b/runtime/resolver.go index e4ce0d4b4e11..9670d606dad0 100644 --- a/runtime/resolver.go +++ b/runtime/resolver.go @@ -205,10 +205,14 @@ func (r *Runtime) Resolve(ctx context.Context, opts *ResolveOptions) (res Resolv if _, err := hash.Write(cacheKey); err != nil { return nil, nil, err } - // Hash the full security claims, not just the user attributes: + // Hash the security claims, not just the user attributes: // permissions, additional rules (e.g. locked filters on magic auth tokens) and skipped checks // all change the resolved security policy, and results must not be shared across them. - claimsJSON, err := json.Marshal(opts.Claims) + // The user ID is excluded since it does not affect the resolved policy, + // which enables sharing results between users that resolve to the same policy (common for embeds). + claimsForKey := *opts.Claims + claimsForKey.UserID = "" + claimsJSON, err := json.Marshal(&claimsForKey) if err != nil { return nil, nil, err }