feat: surface caveat evaluation diagnostics for denied checks

internal/caveats/run.go

@@ -381,6 +381,12 @@ type ExpressionResult interface {
 
 	// MissingVarNames returns the names of the parameters missing from the context.
 	MissingVarNames() ([]string, error)
+
+	// LeafCaveatResults returns all leaf caveat evaluation results from this
+	// expression tree. For a leaf CaveatResult, this returns a single-element
+	// slice containing itself. For a synthetic (AND/OR/NOT) result, this
+	// recursively collects results from all children.
+	LeafCaveatResults() []*caveats.CaveatResult
 }
 
 type syntheticResult struct {
@@ -424,6 +430,17 @@ func (sr syntheticResult) MissingVarNames() ([]string, error) {
 	return nil, errors.New("not a partial value")
 }
 
+// LeafCaveatResults collects all leaf CaveatResult values from the children
+// of this synthetic result. This is used to build per-caveat diagnostics
+// when a permission check is denied due to caveat evaluation.
+func (sr syntheticResult) LeafCaveatResults() []*caveats.CaveatResult {
+	var results []*caveats.CaveatResult
+	for _, child := range sr.exprResultsForDebug {
+		results = append(results, child.LeafCaveatResults()...)
+	}
+	return results
+}
+
 func isFalseResult(result ExpressionResult) bool {
 	return !result.Value() && !result.IsPartial()
 }

internal/graph/computed/computecheck.go

@@ -9,6 +9,7 @@ import (
 	"github.com/authzed/spicedb/pkg/datalayer"
 	"github.com/authzed/spicedb/pkg/datastore"
 	"github.com/authzed/spicedb/pkg/genutil/slicez"
+	core "github.com/authzed/spicedb/pkg/proto/core/v1"
 	v1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1"
 	"github.com/authzed/spicedb/pkg/spiceerrors"
 	"github.com/authzed/spicedb/pkg/tuple"
@@ -153,7 +154,7 @@ func computeCheck(ctx context.Context,
 		}
 
 		for _, resourceID := range resourceIDsToCheck {
-			computed, err := computeCaveatedCheckResult(ctx, caveatRunner, params, resourceID, checkResult)
+			computed, err := computeCaveatedCheckResult(ctx, caveatRunner, params, resourceID, checkResult, params.DebugOption)
 			if err != nil {
 				return false, err
 			}
@@ -165,7 +166,7 @@ func computeCheck(ctx context.Context,
 	return results, metadata, debugInfo, err
 }
 
-func computeCaveatedCheckResult(ctx context.Context, runner *cexpr.CaveatRunner, params CheckParameters, resourceID string, checkResult *v1.DispatchCheckResponse) (*v1.ResourceCheckResult, error) {
+func computeCaveatedCheckResult(ctx context.Context, runner *cexpr.CaveatRunner, params CheckParameters, resourceID string, checkResult *v1.DispatchCheckResponse, debugOption DebugOption) (*v1.ResourceCheckResult, error) {
 	result, ok := checkResult.ResultsByResourceId[resourceID]
 	if !ok {
 		return &v1.ResourceCheckResult{
@@ -204,7 +205,87 @@ func computeCaveatedCheckResult(ctx context.Context, runner *cexpr.CaveatRunner,
 		}, nil
 	}
 
+	// The caveat evaluated to false, resulting in NOT_MEMBER.
+	// Only collect per-caveat diagnostics when debugging is enabled, to avoid
+	// leaking caveat expression internals and context values in production
+	// responses, and to avoid the cost of re-evaluation on the hot path.
+	if debugOption != NoDebugging {
+		caveatEvalInfo, err := collectCaveatEvalInfo(ctx, runner, result.Expression, params.CaveatContext, sr)
+		if err != nil {
+			// If we fail to collect diagnostics, still return the NOT_MEMBER result
+			// without diagnostics rather than failing the entire check.
+			return &v1.ResourceCheckResult{
+				Membership: v1.ResourceCheckResult_NOT_MEMBER,
+			}, nil
+		}
+
+		return &v1.ResourceCheckResult{
+			Membership:     v1.ResourceCheckResult_NOT_MEMBER,
+			CaveatEvalInfo: caveatEvalInfo,
+		}, nil
+	}
+
 	return &v1.ResourceCheckResult{
 		Membership: v1.ResourceCheckResult_NOT_MEMBER,
 	}, nil
 }
+
+// collectCaveatEvalInfo re-evaluates a caveat expression with debug information
+// enabled to collect per-leaf caveat evaluation results. This is called only when
+// the initial evaluation returned false (NOT_MEMBER), so the re-evaluation cost
+// is acceptable since the request is already denied.
+//
+// This addresses https://github.com/authzed/spicedb/issues/2802 by providing
+// applications with information about which specific caveat(s) caused a denial.
+func collectCaveatEvalInfo(
+	ctx context.Context,
+	runner *cexpr.CaveatRunner,
+	expr *core.CaveatExpression,
+	caveatContext map[string]any,
+	reader cexpr.CaveatDefinitionLookup,
+) ([]*v1.CaveatEvalResult, error) {
+	// Re-run with debug information to collect leaf results.
+	debugResult, err := runner.RunCaveatExpression(ctx, expr, caveatContext, reader, cexpr.RunCaveatExpressionWithDebugInformation)
+	if err != nil {
+		return nil, err
+	}
+
+	leafResults := debugResult.LeafCaveatResults()
+	if len(leafResults) == 0 {
+		return nil, nil
+	}
+
+	evalResults := make([]*v1.CaveatEvalResult, 0, len(leafResults))
+	for _, leaf := range leafResults {
+		evalResult := &v1.CaveatEvalResult{
+			CaveatName: leaf.ParentCaveat().Name(),
+		}
+
+		// Determine the result.
+		if leaf.IsPartial() {
+			evalResult.Result = v1.CaveatEvalResult_RESULT_MISSING_SOME_CONTEXT
+			missingNames, _ := leaf.MissingVarNames()
+			evalResult.MissingContextParams = missingNames
+		} else if leaf.Value() {
+			evalResult.Result = v1.CaveatEvalResult_RESULT_TRUE
+		} else {
+			evalResult.Result = v1.CaveatEvalResult_RESULT_FALSE
+		}
+
+		// Collect the expression string.
+		exprString, err := leaf.ExpressionString()
+		if err == nil {
+			evalResult.ExpressionString = exprString
+		}
+
+		// Collect the context values used during evaluation.
+		contextStruct, err := leaf.ContextStruct()
+		if err == nil {
+			evalResult.Context = contextStruct
+		}
+
+		evalResults = append(evalResults, evalResult)
+	}
+
+	return evalResults, nil
+}

internal/graph/computed/computecheck_test.go

@@ -854,6 +854,206 @@ func TestComputeCheckWithCaveats(t *testing.T) {
 	}
 }
 
+func TestComputeCheckCaveatEvalDiagnostics(t *testing.T) {
+	testCases := []struct {
+		name                string
+		schema              string
+		updates             []caveatedUpdate
+		check               string
+		context             map[string]any
+		expectedMembership  v1.ResourceCheckResult_Membership
+		expectedCaveatNames []string // expected caveat names in CaveatEvalInfo
+		expectedFalseCount  int      // number of caveats expected to be FALSE
+	}{
+		{
+			"single caveat evaluates to false",
+			`definition user {}
+
+			definition document {
+				relation viewer: user | user with testcaveat
+				permission view = viewer
+			}
+
+			caveat testcaveat(somecondition int) {
+				somecondition == 42
+			}`,
+			[]caveatedUpdate{
+				{tuple.UpdateOperationCreate, "document:foo#viewer@user:tom", "testcaveat", nil},
+			},
+			"document:foo#view@user:tom",
+			map[string]any{"somecondition": "41"},
+			v1.ResourceCheckResult_NOT_MEMBER,
+			[]string{"testcaveat"},
+			1,
+		},
+		{
+			"caveat false due to written context taking precedence",
+			`definition user {}
+
+			definition document {
+				relation viewer: user | user with testcaveat
+				permission view = viewer
+			}
+
+			caveat testcaveat(somecondition int) {
+				somecondition == 42
+			}`,
+			[]caveatedUpdate{
+				{tuple.UpdateOperationCreate, "document:foo#viewer@user:tom", "testcaveat", map[string]any{
+					"somecondition": 41, // written context says 41 (not 42)
+				}},
+			},
+			"document:foo#view@user:tom",
+			map[string]any{"somecondition": 42}, // request context says 42, but written takes precedence
+			v1.ResourceCheckResult_NOT_MEMBER,
+			[]string{"testcaveat"},
+			1,
+		},
+		{
+			"intersection with one false caveat",
+			`definition user {}
+
+			definition document {
+				relation viewer: user | user with viewcaveat
+				relation editor: user | user with editcaveat
+				permission view_and_edit = viewer & editor
+			}
+
+			caveat viewcaveat(somecondition int) {
+				somecondition == 42
+			}
+
+			caveat editcaveat(today string) {
+				today == 'tuesday'
+			}`,
+			[]caveatedUpdate{
+				{tuple.UpdateOperationCreate, "document:foo#viewer@user:tom", "viewcaveat", nil},
+				{tuple.UpdateOperationCreate, "document:foo#editor@user:tom", "editcaveat", nil},
+			},
+			"document:foo#view_and_edit@user:tom",
+			map[string]any{"somecondition": "42", "today": "wednesday"},
+			v1.ResourceCheckResult_NOT_MEMBER,
+			// Both leaf caveats are returned: viewcaveat (TRUE) and editcaveat (FALSE).
+			// This lets applications see the full evaluation picture.
+			[]string{"editcaveat", "viewcaveat"},
+			1, // only editcaveat is FALSE
+		},
+		{
+			"no caveat eval info for non-caveated NOT_MEMBER",
+			`definition user {}
+
+			definition document {
+				relation viewer: user
+				permission view = viewer
+			}`,
+			[]caveatedUpdate{
+				{tuple.UpdateOperationCreate, "document:foo#viewer@user:alice", "", nil},
+			},
+			"document:foo#view@user:tom",
+			nil,
+			v1.ResourceCheckResult_NOT_MEMBER,
+			nil, // no caveat eval info expected
+			0,
+		},
+		{
+			"no caveat eval info for MEMBER result",
+			`definition user {}
+
+			definition document {
+				relation viewer: user | user with testcaveat
+				permission view = viewer
+			}
+
+			caveat testcaveat(somecondition int) {
+				somecondition == 42
+			}`,
+			[]caveatedUpdate{
+				{tuple.UpdateOperationCreate, "document:foo#viewer@user:tom", "testcaveat", nil},
+			},
+			"document:foo#view@user:tom",
+			map[string]any{"somecondition": "42"},
+			v1.ResourceCheckResult_MEMBER,
+			nil, // no caveat eval info expected for allowed results
+			0,
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			ds, err := dsfortesting.NewMemDBDatastoreForTesting(t, 0, 0, memdb.DisableGC)
+			require.NoError(t, err)
+
+			dispatch, err := graph.NewLocalOnlyDispatcher(graph.MustNewDefaultDispatcherParametersForTesting())
+			require.NoError(t, err)
+			ctx := log.Logger.WithContext(datalayer.ContextWithHandle(t.Context()))
+			require.NoError(t, datalayer.SetInContext(ctx, datalayer.NewDataLayer(ds)))
+
+			revision, err := writeCaveatedTuples(ctx, t, ds, tt.schema, tt.updates)
+			require.NoError(t, err)
+
+			rel := tuple.MustParse(tt.check)
+
+			// With debugging enabled, diagnostics should be populated on denial.
+			result, _, err := computed.ComputeCheck(ctx, dispatch,
+				caveattypes.Default.TypeSet,
+				computed.CheckParameters{
+					ResourceType:  rel.Resource.RelationReference(),
+					Subject:       rel.Subject,
+					CaveatContext: tt.context,
+					AtRevision:    revision,
+					MaximumDepth:  50,
+					DebugOption:   computed.BasicDebuggingEnabled,
+				},
+				rel.Resource.ObjectID,
+				100,
+			)
+			require.NoError(t, err)
+			require.Equal(t, tt.expectedMembership, result.Membership, "unexpected membership for %s", tt.check)
+
+			// Verify that without debugging, diagnostics are NOT populated
+			// (to avoid leaking caveat internals in production).
+			resultNoDebug, _, err := computed.ComputeCheck(ctx, dispatch,
+				caveattypes.Default.TypeSet,
+				computed.CheckParameters{
+					ResourceType:  rel.Resource.RelationReference(),
+					Subject:       rel.Subject,
+					CaveatContext: tt.context,
+					AtRevision:    revision,
+					MaximumDepth:  50,
+					DebugOption:   computed.NoDebugging,
+				},
+				rel.Resource.ObjectID,
+				100,
+			)
+			require.NoError(t, err)
+			require.Empty(t, resultNoDebug.CaveatEvalInfo, "diagnostics must not be populated without debug mode")
+
+			if tt.expectedCaveatNames == nil {
+				require.Empty(t, result.CaveatEvalInfo, "expected no caveat eval info")
+			} else {
+				require.NotEmpty(t, result.CaveatEvalInfo, "expected caveat eval info to be populated")
+
+				// Collect caveat names from results
+				var caveatNames []string
+				falseCount := 0
+				for _, evalInfo := range result.CaveatEvalInfo {
+					caveatNames = append(caveatNames, evalInfo.CaveatName)
+					if evalInfo.Result == v1.CaveatEvalResult_RESULT_FALSE {
+						falseCount++
+					}
+					// Verify expression string is populated
+					require.NotEmpty(t, evalInfo.ExpressionString, "expected expression string for caveat %s", evalInfo.CaveatName)
+				}
+
+				sort.Strings(caveatNames)
+				sort.Strings(tt.expectedCaveatNames)
+				require.Equal(t, tt.expectedCaveatNames, caveatNames, "unexpected caveat names in eval info")
+				require.Equal(t, tt.expectedFalseCount, falseCount, "unexpected number of FALSE results")
+			}
+		})
+	}
+}
+
 func TestComputeCheckError(t *testing.T) {
 	ds, err := dsfortesting.NewMemDBDatastoreForTesting(t, 0, 0, memdb.DisableGC)
 	require.NoError(t, err)