Add EvaluationResult as rule evaluation result. (#5144)

Ref #5120

Co-authored-by: Eleftheria Stein-Kousathana <[email protected]>

cmd/dev/app/rule_type/rttst.go

@@ -326,8 +326,9 @@ func selectAndEval(
 	}
 
 	var evalErr error
+	//var result *interfaces.EvaluationResult
 	if selected {
-		evalErr = eng.Eval(ctx, inf.Entity, evalStatus.GetRule().Def, evalStatus.GetRule().Params, evalStatus)
+		_, evalErr = eng.Eval(ctx, inf.Entity, evalStatus.GetRule().Def, evalStatus.GetRule().Params, evalStatus)
 	} else {
 		evalErr = errors.NewErrEvaluationSkipped("entity not selected by selector %s", matchedSelector)
 	}

internal/engine/actions/alert/pull_request_comment/pull_request_comment.go

@@ -48,6 +48,9 @@ type Alert struct {
 type PrCommentTemplateParams struct {
 	// EvalErrorDetails is the details of the error that occurred during evaluation, which may be empty
 	EvalErrorDetails string
+
+	// EvalResult is the output of the evaluation, which may be empty
+	EvalResultOutput any
 }
 
 type paramsPR struct {
@@ -273,6 +276,10 @@ func (alert *Alert) getParamsForPRComment(
 		EvalErrorDetails: enginerr.ErrorAsEvalDetails(params.GetEvalErr()),
 	}
 
+	if params.GetEvalResult() != nil {
+		tmplParams.EvalResultOutput = params.GetEvalResult().Output
+	}
+
 	comment, err := commentTmpl.Render(ctx, tmplParams, PrCommentMaxLength)
 	if err != nil {
 		return nil, fmt.Errorf("cannot execute title template: %w", err)

internal/engine/actions/alert/pull_request_comment/pull_request_comment_test.go

@@ -16,16 +16,20 @@ import (
 
 	"github.com/mindersec/minder/internal/db"
 	enginerr "github.com/mindersec/minder/internal/engine/errors"
-	"github.com/mindersec/minder/internal/engine/interfaces"
+	engif "github.com/mindersec/minder/internal/engine/interfaces"
 	pbinternal "github.com/mindersec/minder/internal/proto"
 	mockghclient "github.com/mindersec/minder/internal/providers/github/mock"
 	pb "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1"
+	"github.com/mindersec/minder/pkg/engine/v1/interfaces"
 	"github.com/mindersec/minder/pkg/profiles/models"
 )
 
-var TestActionTypeValid interfaces.ActionType = "alert-test"
+var TestActionTypeValid engif.ActionType = "alert-test"
 
-const evaluationFailureDetails = "evaluation failure reason"
+const (
+	evaluationFailureDetails = "evaluation failure reason"
+	violationMsg             = "violation message"
+)
 
 func TestPullRequestCommentAlert(t *testing.T) {
 	t.Parallel()
@@ -36,8 +40,8 @@ func TestPullRequestCommentAlert(t *testing.T) {
 
 	tests := []struct {
 		name             string
-		actionType       interfaces.ActionType
-		cmd              interfaces.ActionCmd
+		actionType       engif.ActionType
+		cmd              engif.ActionCmd
 		reviewMsg        string
 		inputMetadata    *json.RawMessage
 		mockSetup        func(*mockghclient.MockGitHub)
@@ -48,7 +52,7 @@ func TestPullRequestCommentAlert(t *testing.T) {
 			name:       "create a PR comment",
 			actionType: TestActionTypeValid,
 			reviewMsg:  "This is a constant review message",
-			cmd:        interfaces.ActionCmdOn,
+			cmd:        engif.ActionCmdOn,
 			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
 				mockGitHub.EXPECT().
 					CreateReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
@@ -57,27 +61,34 @@ func TestPullRequestCommentAlert(t *testing.T) {
 			expectedMetadata: json.RawMessage(fmt.Sprintf(`{"review_id":"%s"}`, reviewIDStr)),
 		},
 		{
-			name:       "create a PR comment with template",
+			name:       "create a PR comment with eval error details template",
 			actionType: TestActionTypeValid,
 			reviewMsg:  "{{ .EvalErrorDetails }}",
-			cmd:        interfaces.ActionCmdOn,
+			cmd:        engif.ActionCmdOn,
 			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
 				mockGitHub.EXPECT().
 					CreateReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&github.PullRequestReviewRequest{})).
-					DoAndReturn(func(_ context.Context, _, _ string, _ int, review *github.PullRequestReviewRequest) (*github.PullRequestReview, error) {
-						if review.GetBody() != evaluationFailureDetails {
-							return nil, fmt.Errorf("expected review body to be %s, got %s", evaluationFailureDetails, review.GetBody())
-						}
-						return &github.PullRequestReview{ID: &reviewID}, nil
-					})
+					DoAndReturn(validateReviewBodyAndReturn(evaluationFailureDetails, reviewID))
+			},
+			expectedMetadata: json.RawMessage(fmt.Sprintf(`{"review_id":"%s"}`, reviewIDStr)),
+		},
+		{
+			name:       "create a PR comment with eval result output template",
+			actionType: TestActionTypeValid,
+			reviewMsg:  "{{ .EvalResultOutput.ViolationMsg }}",
+			cmd:        engif.ActionCmdOn,
+			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
+				mockGitHub.EXPECT().
+					CreateReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&github.PullRequestReviewRequest{})).
+					DoAndReturn(validateReviewBodyAndReturn(violationMsg, reviewID))
 			},
 			expectedMetadata: json.RawMessage(fmt.Sprintf(`{"review_id":"%s"}`, reviewIDStr)),
 		},
 		{
 			name:       "error from provider creating PR comment",
 			actionType: TestActionTypeValid,
 			reviewMsg:  "This is a constant review message",
-			cmd:        interfaces.ActionCmdOn,
+			cmd:        engif.ActionCmdOn,
 			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
 				mockGitHub.EXPECT().
 					CreateReview(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
@@ -89,7 +100,7 @@ func TestPullRequestCommentAlert(t *testing.T) {
 			name:          "dismiss PR comment",
 			actionType:    TestActionTypeValid,
 			reviewMsg:     "This is a constant review message",
-			cmd:           interfaces.ActionCmdOff,
+			cmd:           engif.ActionCmdOff,
 			inputMetadata: &successfulRunMetadata,
 			mockSetup: func(mockGitHub *mockghclient.MockGitHub) {
 				mockGitHub.EXPECT().
@@ -123,12 +134,15 @@ func TestPullRequestCommentAlert(t *testing.T) {
 			require.NoError(t, err)
 			require.NotNil(t, prCommentAlert)
 
-			evalParams := &interfaces.EvalStatusParams{
+			evalParams := &engif.EvalStatusParams{
 				EvalStatusFromDb: &db.ListRuleEvaluationsByProfileIdRow{},
 				Profile:          &models.ProfileAggregate{},
 				Rule:             &models.RuleInstance{},
 			}
 			evalParams.SetEvalErr(enginerr.NewErrEvaluationFailed(evaluationFailureDetails))
+			evalParams.SetEvalResult(&interfaces.EvaluationResult{
+				Output: exampleOutput{ViolationMsg: violationMsg},
+			})
 
 			retMeta, err := prCommentAlert.Do(
 				context.Background(),
@@ -142,3 +156,16 @@ func TestPullRequestCommentAlert(t *testing.T) {
 		})
 	}
 }
+
+type exampleOutput struct {
+	ViolationMsg string
+}
+
+func validateReviewBodyAndReturn(expectedBody string, reviewID int64) func(_ context.Context, _, _ string, _ int, review *github.PullRequestReviewRequest) (*github.PullRequestReview, error) {
+	return func(_ context.Context, _, _ string, _ int, review *github.PullRequestReviewRequest) (*github.PullRequestReview, error) {
+		if review.GetBody() != expectedBody {
+			return nil, fmt.Errorf("expected review body to be %s, got %s", expectedBody, review.GetBody())
+		}
+		return &github.PullRequestReview{ID: &reviewID}, nil
+	}
+}

internal/engine/eval/homoglyphs/application/invisible_characters_eval.go

@@ -49,19 +49,19 @@ func (ice *InvisibleCharactersEvaluator) Eval(
 	_ map[string]any,
 	_ protoreflect.ProtoMessage,
 	res *interfaces.Result,
-) error {
+) (*interfaces.EvaluationResult, error) {
 	violations, err := evaluateHomoglyphs(ctx, ice.processor, res, ice.reviewHandler)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	if len(violations) > 0 {
-		return evalerrors.NewDetailedErrEvaluationFailed(
+		return nil, evalerrors.NewDetailedErrEvaluationFailed(
 			templates.InvisibleCharactersTemplate,
 			map[string]any{"violations": violations},
 			"found invisible characters violations",
 		)
 	}
 
-	return nil
+	return &interfaces.EvaluationResult{}, nil
 }

internal/engine/eval/homoglyphs/application/mixed_scripts_eval.go

@@ -55,19 +55,19 @@ func (mse *MixedScriptsEvaluator) Eval(
 	_ map[string]any,
 	_ protoreflect.ProtoMessage,
 	res *interfaces.Result,
-) error {
+) (*interfaces.EvaluationResult, error) {
 	violations, err := evaluateHomoglyphs(ctx, mse.processor, res, mse.reviewHandler)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	if len(violations) > 0 {
-		return evalerrors.NewDetailedErrEvaluationFailed(
+		return nil, evalerrors.NewDetailedErrEvaluationFailed(
 			templates.MixedScriptsTemplate,
 			map[string]any{"violations": violations},
 			"found mixed scripts violations",
 		)
 	}
 
-	return nil
+	return &interfaces.EvaluationResult{}, nil
 }

internal/engine/eval/jq/jq.go

@@ -56,9 +56,14 @@ func NewJQEvaluator(
 }
 
 // Eval calls the jq library to evaluate the rule
-func (jqe *Evaluator) Eval(ctx context.Context, pol map[string]any, _ protoreflect.ProtoMessage, res *interfaces.Result) error {
+func (jqe *Evaluator) Eval(
+	ctx context.Context,
+	pol map[string]any,
+	_ protoreflect.ProtoMessage,
+	res *interfaces.Result,
+) (*interfaces.EvaluationResult, error) {
 	if res.Object == nil {
-		return fmt.Errorf("missing object")
+		return nil, fmt.Errorf("missing object")
 	}
 	obj := res.Object
 
@@ -72,21 +77,21 @@ func (jqe *Evaluator) Eval(ctx context.Context, pol map[string]any, _ protorefle
 		if a.Profile == nil {
 			expectedVal, err = util.JQReadConstant[any](a.Constant.AsInterface())
 			if err != nil {
-				return fmt.Errorf("cannot get values from profile accessor: %w", err)
+				return nil, fmt.Errorf("cannot get values from profile accessor: %w", err)
 			}
 		} else {
 			// Get the expected value from the profile accessor
 			expectedVal, err = util.JQReadFrom[any](ctx, a.Profile.Def, pol)
 			// we ignore util.ErrNoValueFound because we want to allow the JQ accessor to return the default value
 			// which is fine for DeepEqual
 			if err != nil && !errors.Is(err, util.ErrNoValueFound) {
-				return fmt.Errorf("cannot get values from profile accessor: %w", err)
+				return nil, fmt.Errorf("cannot get values from profile accessor: %w", err)
 			}
 		}
 
 		dataVal, err = util.JQReadFrom[any](ctx, a.Ingested.Def, obj)
 		if err != nil && !errors.Is(err, util.ErrNoValueFound) {
-			return fmt.Errorf("cannot get values from data accessor: %w", err)
+			return nil, fmt.Errorf("cannot get values from data accessor: %w", err)
 		}
 
 		// Deep compare
@@ -99,7 +104,7 @@ func (jqe *Evaluator) Eval(ctx context.Context, pol map[string]any, _ protorefle
 				msg = fmt.Sprintf("%s\nassertion: %s", msg, string(marshalledAssertion))
 			}
 
-			return evalerrors.NewDetailedErrEvaluationFailed(
+			return nil, evalerrors.NewDetailedErrEvaluationFailed(
 				templates.JqTemplate,
 				map[string]any{
 					"path":     a.Ingested.Def,
@@ -112,7 +117,7 @@ func (jqe *Evaluator) Eval(ctx context.Context, pol map[string]any, _ protorefle
 		}
 	}
 
-	return nil
+	return &interfaces.EvaluationResult{}, nil
 }
 
 // Convert numeric types to float64

internal/engine/eval/jq/jq_test.go

@@ -366,7 +366,7 @@ func TestValidJQEvals(t *testing.T) {
 			assert.NoError(t, err, "Got unexpected error")
 			assert.NotNil(t, jqe, "Got unexpected nil")
 
-			err = jqe.Eval(context.Background(), tt.args.pol, nil, &interfaces.Result{Object: tt.args.obj})
+			_, err = jqe.Eval(context.Background(), tt.args.pol, nil, &interfaces.Result{Object: tt.args.obj})
 			assert.NoError(t, err, "Got unexpected error")
 		})
 	}
@@ -576,7 +576,7 @@ func TestValidJQEvalsFailed(t *testing.T) {
 			assert.NoError(t, err, "Got unexpected error")
 			assert.NotNil(t, jqe, "Got unexpected nil")
 
-			err = jqe.Eval(context.Background(), tt.args.pol, nil, &interfaces.Result{Object: tt.args.obj})
+			_, err = jqe.Eval(context.Background(), tt.args.pol, nil, &interfaces.Result{Object: tt.args.obj})
 			assert.ErrorIs(t, err, evalerrors.ErrEvaluationFailed, "Got unexpected error")
 		})
 	}

internal/engine/eval/rego/eval.go

@@ -109,7 +109,7 @@ func (e *Evaluator) newRegoFromOptions(opts ...func(*rego.Rego)) *rego.Rego {
 // Eval implements the Evaluator interface.
 func (e *Evaluator) Eval(
 	ctx context.Context, pol map[string]any, entity protoreflect.ProtoMessage, res *interfaces.Result,
-) error {
+) (*interfaces.EvaluationResult, error) {
 	// The rego engine is actually able to handle nil
 	// objects quite gracefully, so we don't need to check
 	// this explicitly.
@@ -131,7 +131,7 @@ func (e *Evaluator) Eval(
 
 	pq, err := r.PrepareForEval(ctx)
 	if err != nil {
-		return fmt.Errorf("could not prepare Rego: %w", err)
+		return nil, fmt.Errorf("could not prepare Rego: %w", err)
 	}
 
 	rs, err := pq.Eval(ctx, rego.EvalInput(&Input{
@@ -140,8 +140,13 @@ func (e *Evaluator) Eval(
 		OutputFormat: e.cfg.ViolationFormat,
 	}))
 	if err != nil {
-		return fmt.Errorf("error evaluating profile. Might be wrong input: %w", err)
+		return nil, fmt.Errorf("error evaluating profile. Might be wrong input: %w", err)
 	}
 
-	return e.reseval.parseResult(rs, entity)
+	err = e.reseval.parseResult(rs, entity)
+	if err != nil {
+		return nil, err
+	}
+
+	return &interfaces.EvaluationResult{}, nil
 }