Fix query result cache leaking across security contexts: include full SecurityClaims in cache key

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,19 @@ 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 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.
+	// 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
+	}
+	if _, err := hash.Write(claimsJSON); err != nil {
+		return nil, nil, err
 	}
 	for _, ref := range resolver.Refs() {
 		res, err := ctrl.Get(ctx, ref, false)

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