Merge pull request from GHSA-grjv-gjgr-66g2

Fix exclusion operation in Check dispatch to *always* request the full set of results for the first branch

internal/dispatch/graph/check_test.go

@@ -298,6 +298,267 @@ func addFrame(trace *v1.CheckDebugTrace, foundFrames *mapz.Set[string]) {
 	}
 }
 
+func TestCheckPermissionOverSchema(t *testing.T) {
+	testCases := []struct {
+		name                   string
+		schema                 string
+		relationships          []*core.RelationTuple
+		resource               *core.ObjectAndRelation
+		subject                *core.ObjectAndRelation
+		expectedPermissionship v1.ResourceCheckResult_Membership
+	}{
+		{
+			"basic union",
+			`definition user {}
+		
+		 	 definition document {
+				relation editor: user
+				relation viewer: user
+				permission view = viewer + editor
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#viewer@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_MEMBER,
+		},
+		{
+			"basic intersection",
+			`definition user {}
+		
+		 	 definition document {
+				relation editor: user
+				relation viewer: user
+				permission view = viewer & editor
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#viewer@user:tom"),
+				tuple.MustParse("document:first#editor@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_MEMBER,
+		},
+		{
+			"basic exclusion",
+			`definition user {}
+		
+		 	 definition document {
+				relation editor: user
+				relation viewer: user
+				permission view = viewer - editor
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#viewer@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_MEMBER,
+		},
+		{
+			"basic union, multiple branches",
+			`definition user {}
+		
+		 	 definition document {
+				relation editor: user
+				relation viewer: user
+				permission view = viewer + editor
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#viewer@user:tom"),
+				tuple.MustParse("document:first#editor@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_MEMBER,
+		},
+		{
+			"basic union no permission",
+			`definition user {}
+		
+		 	 definition document {
+				relation editor: user
+				relation viewer: user
+				permission view = viewer + editor
+  			 }`,
+			[]*core.RelationTuple{},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_NOT_MEMBER,
+		},
+		{
+			"basic intersection no permission",
+			`definition user {}
+		
+		 	 definition document {
+				relation editor: user
+				relation viewer: user
+				permission view = viewer & editor
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#viewer@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_NOT_MEMBER,
+		},
+		{
+			"basic exclusion no permission",
+			`definition user {}
+		
+		 	 definition document {
+				relation banned: user
+				relation viewer: user
+				permission view = viewer - banned
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#viewer@user:tom"),
+				tuple.MustParse("document:first#banned@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_NOT_MEMBER,
+		},
+		{
+			"exclusion with multiple branches",
+			`definition user {}
+		
+			 definition group {
+			 	relation member: user
+				relation banned: user
+			 	permission view = member - banned
+			 }
+
+		 	 definition document {
+				relation group: group
+				permission view = group->view
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#group@group:first"),
+				tuple.MustParse("document:first#group@group:second"),
+				tuple.MustParse("group:first#member@user:tom"),
+				tuple.MustParse("group:first#banned@user:tom"),
+				tuple.MustParse("group:second#member@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_MEMBER,
+		},
+		{
+			"intersection with multiple branches",
+			`definition user {}
+		
+			 definition group {
+			 	relation member: user
+				relation other: user
+			 	permission view = member & other
+			 }
+
+		 	 definition document {
+				relation group: group
+				permission view = group->view
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#group@group:first"),
+				tuple.MustParse("document:first#group@group:second"),
+				tuple.MustParse("group:first#member@user:tom"),
+				tuple.MustParse("group:first#other@user:tom"),
+				tuple.MustParse("group:second#member@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_MEMBER,
+		},
+		{
+			"exclusion with multiple branches no permission",
+			`definition user {}
+		
+			 definition group {
+			 	relation member: user
+				relation banned: user
+			 	permission view = member - banned
+			 }
+
+		 	 definition document {
+				relation group: group
+				permission view = group->view
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#group@group:first"),
+				tuple.MustParse("document:first#group@group:second"),
+				tuple.MustParse("group:first#member@user:tom"),
+				tuple.MustParse("group:first#banned@user:tom"),
+				tuple.MustParse("group:second#member@user:tom"),
+				tuple.MustParse("group:second#banned@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_NOT_MEMBER,
+		},
+		{
+			"intersection with multiple branches no permission",
+			`definition user {}
+		
+			 definition group {
+			 	relation member: user
+				relation other: user
+			 	permission view = member & other
+			 }
+
+		 	 definition document {
+				relation group: group
+				permission view = group->view
+  			 }`,
+			[]*core.RelationTuple{
+				tuple.MustParse("document:first#group@group:first"),
+				tuple.MustParse("document:first#group@group:second"),
+				tuple.MustParse("group:first#member@user:tom"),
+				tuple.MustParse("group:second#member@user:tom"),
+			},
+			ONR("document", "first", "view"),
+			ONR("user", "tom", "..."),
+			v1.ResourceCheckResult_NOT_MEMBER,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			require := require.New(t)
+
+			dispatcher := NewLocalOnlyDispatcher(10)
+
+			ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
+			require.NoError(err)
+
+			ds, revision := testfixtures.DatastoreFromSchemaAndTestRelationships(ds, tc.schema, tc.relationships, require)
+
+			ctx := datastoremw.ContextWithHandle(context.Background())
+			require.NoError(datastoremw.SetInContext(ctx, ds))
+
+			resp, err := dispatcher.DispatchCheck(ctx, &v1.DispatchCheckRequest{
+				ResourceRelation: RR(tc.resource.Namespace, tc.resource.Relation),
+				ResourceIds:      []string{tc.resource.ObjectId},
+				Subject:          tc.subject,
+				Metadata: &v1.ResolverMeta{
+					AtRevision:     revision.String(),
+					DepthRemaining: 50,
+				},
+				ResultsSetting: v1.DispatchCheckRequest_ALLOW_SINGLE_RESULT,
+			})
+			require.NoError(err)
+
+			membership := v1.ResourceCheckResult_NOT_MEMBER
+			if r, ok := resp.ResultsByResourceId[tc.resource.ObjectId]; ok {
+				membership = r.Membership
+			}
+
+			require.Equal(tc.expectedPermissionship, membership)
+		})
+	}
+}
+
 func TestCheckDebugging(t *testing.T) {
 	type expectedFrame struct {
 		resourceType *core.RelationReference

internal/graph/check.go

@@ -771,7 +771,12 @@ func difference[T any](
 	othersChan := make(chan CheckResult, len(children)-1)
 
 	go func() {
-		result := handler(childCtx, crc, children[0])
+		result := handler(childCtx, currentRequestContext{
+			parentReq:           crc.parentReq,
+			filteredResourceIDs: crc.filteredResourceIDs,
+			resultsSetting:      v1.DispatchCheckRequest_REQUIRE_ALL_RESULTS,
+			maxDispatchCount:    crc.maxDispatchCount,
+		}, children[0])
 		baseChan <- result
 	}()
 

internal/graph/membershipset_test.go

@@ -705,6 +705,34 @@ func TestMembershipSetSubtract(t *testing.T) {
 			false,
 			false,
 		},
+		{
+			"non overlapping",
+			map[string]*core.CaveatExpression{
+				"resource1": nil,
+				"resource2": nil,
+			},
+			map[string]*core.CaveatExpression{
+				"resource2": nil,
+			},
+			map[string]*core.CaveatExpression{
+				"resource1": nil,
+			},
+			true,
+			false,
+		},
+		{
+			"non overlapping reversed",
+			map[string]*core.CaveatExpression{
+				"resource2": nil,
+			},
+			map[string]*core.CaveatExpression{
+				"resource1": nil,
+				"resource2": nil,
+			},
+			map[string]*core.CaveatExpression{},
+			false,
+			true,
+		},
 	}
 
 	for _, tc := range tcs {

internal/services/integrationtesting/arrowovermultiexclusion.yaml

@@ -0,0 +1,32 @@
+---
+schema: |+
+  definition user {}
+
+  definition group {
+    relation direct_member: user
+    relation excluded: user:*
+    permission view = direct_member - excluded
+  }
+
+  definition resource {
+    relation group: group
+    permission view = group->view
+  }
+
+relationships: >-
+  resource:one#group@group:group1
+
+  resource:one#group@group:group2
+
+  group:group1#direct_member@user:fred
+
+  group:group1#excluded@user:*
+
+  group:group2#direct_member@user:fred
+assertions:
+  assertTrue:
+    - "group:group1#direct_member@user:fred"
+    - "group:group2#direct_member@user:fred"
+    - "group:group2#view@user:fred"
+  assertFalse:
+    - "group:group1#view@user:fred"