Add an optional violation_format to rego rules (#1728)

Adds an optional parameter for the rego evaluator that allows specifying
that if the constraints mode is used, then the constraints message
should be a valid JSON object with a key and a value so that it decodes
into `map[string]any`.

This is done by passing an outputFormat into the rule, the usage can be
seen in unit tests.

The default is still "text" to keep backwards compatibility.

If the evaluator asks for JSON, but back comes just a string, we can
assume that the policy doesn't support JSON output, so we marshall the
string ourvelves into `{ "msg": $response }`.

The main use-case is rules that print a list of items violating a
policy, those can then be summarized using jq like this:
```
./bin/minder profile_status list --provider=github -i actions-github-profile -d -ojson 2>/dev/null | jq '.ruleEvaluationStatus | map(select(.ruleName == "repo_acti
on_list" and .status == "failure")) | map({repo_name: .entityInfo.repo_name, details: .details | fromjson})'
[
  {
    "repo_name": "testrepo",
    "details": [
      {
        "actions_not_allowed": [
          "docker/build-push-action",
          "docker/login-action",
          "docker/metadata-action",
          "docker/setup-buildx-action"
        ]
      }
    ]
  },
  {
    "repo_name": "bad-go",
    "details": [
      {
        "actions_not_allowed": [
          "docker/build-push-action",
          "docker/login-action",
          "docker/metadata-action",
          "docker/setup-buildx-action"
        ]
      }
    ]
  }
]
```

internal/engine/eval/rego/config.go

@@ -30,15 +30,18 @@ type Config struct {
 	// Type is the type of evaluation to perform
 	Type EvaluationType `json:"type" mapstructure:"type" validate:"required"`
 	// Def is the definition of the profile
-	Def string `json:"def" mapstructure:"def" validate:"required"`
+	Def             string                      `json:"def" mapstructure:"def" validate:"required"`
+	ViolationFormat ConstraintsViolationsFormat `json:"violation_format" mapstructure:"violationFormat"`
 }
 
 func (c *Config) getEvalType() resultEvaluator {
 	switch c.Type {
 	case DenyByDefaultEvaluationType:
 		return &denyByDefaultEvaluator{}
 	case ConstraintsEvaluationType:
-		return &constraintsEvaluator{}
+		return &constraintsEvaluator{
+			format: c.ViolationFormat,
+		}
 	}
 
 	return nil
@@ -60,6 +63,10 @@ func parseConfig(cfg *minderv1.RuleType_Definition_Eval_Rego) (*Config, error) {
 		return nil, fmt.Errorf("config failed validation: %w", err)
 	}
 
+	if cfg.ViolationFormat == nil {
+		conf.ViolationFormat = ConstraintsViolationsOutputText
+	}
+
 	typ := conf.getEvalType()
 	if typ == nil {
 		return nil, fmt.Errorf("unknown evaluation type: %s", conf.Type)

internal/engine/eval/rego/eval.go

@@ -57,6 +57,8 @@ type Input struct {
 	Profile map[string]any `json:"profile"`
 	// Ingested is the values set for the ingested data
 	Ingested any `json:"ingested"`
+	// OutputFormat is the format to output violations in
+	OutputFormat ConstraintsViolationsFormat `json:"output_format"`
 }
 
 type hook struct {
@@ -120,8 +122,9 @@ func (e *Evaluator) Eval(ctx context.Context, pol map[string]any, res *engif.Res
 	}
 
 	rs, err := pq.Eval(ctx, rego.EvalInput(&Input{
-		Profile:  pol,
-		Ingested: obj,
+		Profile:      pol,
+		Ingested:     obj,
+		OutputFormat: e.cfg.ViolationFormat,
 	}))
 	if err != nil {
 		return fmt.Errorf("error evaluating profile. Might be wrong input: %w", err)

internal/engine/eval/rego/rego_test.go

@@ -17,6 +17,7 @@ package rego_test
 
 import (
 	"context"
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -173,8 +174,8 @@ violations[{"msg": msg}] {
 		},
 	})
 	require.ErrorIs(t, err, engerrors.ErrEvaluationFailed, "should have failed the evaluation")
-	require.ErrorContains(t, err, "- evaluation failure: data should not contain foo\n")
-	require.ErrorContains(t, err, "- evaluation failure: datum should not contain bar")
+	require.ErrorContains(t, err, "- data should not contain foo\n")
+	require.ErrorContains(t, err, "- datum should not contain bar")
 }
 
 // Evaluates a simple query against a simple profile
@@ -260,6 +261,138 @@ violations[{"msg": msg}] {
 	assert.ErrorContains(t, err, "data did not match profile: foo", "should have failed the evaluation")
 }
 
+const (
+	jsonPolicyDef = `
+package minder
+
+violations[{"msg": msg}] {
+  expected_set := {x | x := input.profile.data[_]}
+  input_set := {x | x := input.ingested.data[_]}
+
+  intersection := expected_set & input_set
+  not count(intersection) == count(input.ingested.data)
+
+  difference := [x | x := input.ingested.data[_]; not intersection[x]]
+  
+  msg = format_message(difference, input.output_format)
+}
+
+format_message(difference, format) = msg {
+    format == "json"
+	json_body := {"actions_not_allowed": difference}
+    msg := json.marshal(json_body)
+}
+
+format_message(difference, format) = msg {
+    not format == "json"
+	msg := sprintf("extra actions found in workflows but not allowed in the profile: %v", [difference])
+}
+`
+)
+
+func TestConstraintsJSONOutput(t *testing.T) {
+	t.Parallel()
+
+	violationFormat := rego.ConstraintsViolationsOutputJSON.String()
+	e, err := rego.NewRegoEvaluator(
+		&minderv1.RuleType_Definition_Eval_Rego{
+			Type:            rego.ConstraintsEvaluationType.String(),
+			ViolationFormat: &violationFormat,
+			Def:             jsonPolicyDef,
+		},
+	)
+	require.NoError(t, err, "could not create evaluator")
+
+	pol := map[string]any{
+		"data": []string{"foo", "bar"},
+	}
+
+	err = e.Eval(context.Background(), pol, &engif.Result{
+		Object: map[string]any{
+			"data": []string{"foo", "bar", "baz"},
+		},
+	})
+	require.ErrorIs(t, err, engerrors.ErrEvaluationFailed, "should have failed the evaluation")
+
+	// check that the error payload msg is JSON in the expected format
+	errmsg := engerrors.ErrorAsEvalDetails(err)
+	var result []struct {
+		ActionsNotAllowed []string `json:"actions_not_allowed"`
+	}
+	err = json.Unmarshal([]byte(errmsg), &result)
+	require.NoError(t, err, "could not unmarshal error JSON")
+	assert.Len(t, result, 1, "should have one result")
+	assert.Contains(t, result[0].ActionsNotAllowed, "baz", "should have baz in the result")
+}
+
+func TestConstraintsJSONFalback(t *testing.T) {
+	t.Parallel()
+
+	violationFormat := rego.ConstraintsViolationsOutputJSON.String()
+	e, err := rego.NewRegoEvaluator(
+		&minderv1.RuleType_Definition_Eval_Rego{
+			Type:            rego.ConstraintsEvaluationType.String(),
+			ViolationFormat: &violationFormat,
+			Def: `
+package minder
+
+violations[{"msg": msg}] {
+	input.profile.data != input.ingested.data
+	msg := sprintf("data did not match profile: %s", [input.profile.data])
+}`,
+		},
+	)
+	require.NoError(t, err, "could not create evaluator")
+
+	pol := map[string]any{
+		"data": "foo",
+	}
+
+	err = e.Eval(context.Background(), pol, &engif.Result{
+		Object: map[string]any{
+			"data": "bar",
+		},
+	})
+	require.ErrorIs(t, err, engerrors.ErrEvaluationFailed, "should have failed the evaluation")
+
+	// check that the error payload msg is JSON in the expected format
+	errmsg := engerrors.ErrorAsEvalDetails(err)
+	var result []struct {
+		Msg string `json:"msg"`
+	}
+	err = json.Unmarshal([]byte(errmsg), &result)
+	require.NoError(t, err, "could not unmarshal error JSON")
+	assert.Len(t, result, 1, "should have one result")
+}
+
+func TestOutputTypePassedIntoRule(t *testing.T) {
+	t.Parallel()
+
+	e, err := rego.NewRegoEvaluator(
+		&minderv1.RuleType_Definition_Eval_Rego{
+			Type: rego.ConstraintsEvaluationType.String(),
+			Def:  jsonPolicyDef,
+		},
+	)
+	require.NoError(t, err, "could not create evaluator")
+
+	pol := map[string]any{
+		"data": []string{"one", "two"},
+	}
+
+	err = e.Eval(context.Background(), pol, &engif.Result{
+		Object: map[string]any{
+			"data": []string{"two", "three"},
+		},
+	})
+	require.Error(t, err, "should have failed the evaluation")
+	require.ErrorIs(t, err, engerrors.ErrEvaluationFailed, "should have failed the evaluation")
+
+	errmsg := engerrors.ErrorAsEvalDetails(err)
+	assert.Contains(t, errmsg, "extra actions found in workflows but not allowed in the profile", "should have the expected error message")
+	assert.Contains(t, errmsg, "three", "should have the expected content")
+}
+
 func TestCantCreateEvaluatorWithInvalidConfig(t *testing.T) {
 	t.Parallel()
 

internal/engine/eval/rego/result.go

@@ -16,7 +16,7 @@
 package rego
 
 import (
-	"errors"
+	"encoding/json"
 	"fmt"
 	"strings"
 
@@ -45,6 +45,20 @@ func (e EvaluationType) String() string {
 	return string(e)
 }
 
+// ConstraintsViolationsFormat is the format to output violations in
+type ConstraintsViolationsFormat string
+
+const (
+	// ConstraintsViolationsOutputText specifies that the violations should be printed as human-readable text
+	ConstraintsViolationsOutputText ConstraintsViolationsFormat = "text"
+	// ConstraintsViolationsOutputJSON specifies that violations should be output as JSON
+	ConstraintsViolationsOutputJSON ConstraintsViolationsFormat = "json"
+)
+
+func (c ConstraintsViolationsFormat) String() string {
+	return string(c)
+}
+
 type resultEvaluator interface {
 	getQuery() func(r *rego.Rego)
 	parseResult(rs rego.ResultSet) error
@@ -105,52 +119,132 @@ func (*denyByDefaultEvaluator) parseResult(rs rego.ResultSet) error {
 }
 
 type constraintsEvaluator struct {
+	format ConstraintsViolationsFormat
 }
 
 func (*constraintsEvaluator) getQuery() func(r *rego.Rego) {
 	return rego.Query(fmt.Sprintf("%s.violations[details]", RegoQueryPrefix))
 }
 
-func (*constraintsEvaluator) parseResult(rs rego.ResultSet) error {
+func (c *constraintsEvaluator) parseResult(rs rego.ResultSet) error {
 	if len(rs) == 0 {
 		// There were no violations
 		return nil
 	}
 
 	// Gather violations into one
-	violations := make([]string, 0, len(rs))
+	resBuilder := c.resultsBuilder(rs)
+	if resBuilder == nil {
+		return fmt.Errorf("invalid format: %s", c.format)
+	}
 	for _, r := range rs {
-		v := resultToViolation(r)
-		if errors.Is(v, engerrors.ErrEvaluationFailed) {
-			violations = append(violations, v.Error())
-		} else {
-			return fmt.Errorf("unexpected error in rego violation: %w", v)
+		v, err := resultToViolation(r)
+		if err != nil {
+			return fmt.Errorf("unexpected error in rego violation: %w", err)
+		}
+
+		err = resBuilder.addResult(v)
+		if err != nil {
+			return fmt.Errorf("cannot add result: %w", err)
 		}
 	}
 
-	return engerrors.NewErrEvaluationFailed("Evaluation failures: \n - %s", strings.Join(violations, "\n - "))
+	return resBuilder.formatResults()
+}
+
+func (c *constraintsEvaluator) resultsBuilder(rs rego.ResultSet) resultBuilder {
+	switch c.format {
+	case ConstraintsViolationsOutputText:
+		return newStringResultBuilder(rs)
+	case ConstraintsViolationsOutputJSON:
+		return newJSONResultBuilder(rs)
+	default:
+		return nil
+	}
 }
 
-func resultToViolation(r rego.Result) error {
+func resultToViolation(r rego.Result) (any, error) {
 	det := r.Bindings["details"]
 	if det == nil {
-		return fmt.Errorf("missing details in result")
+		return nil, fmt.Errorf("missing details in result")
 	}
 
 	detmap, ok := det.(map[string]interface{})
 	if !ok {
-		return fmt.Errorf("details is not a map")
+		return nil, fmt.Errorf("details is not a map")
 	}
 
 	msg, ok := detmap["msg"]
 	if !ok {
-		return fmt.Errorf("missing msg in details")
+		return nil, fmt.Errorf("missing msg in details")
 	}
 
+	return msg, nil
+}
+
+type resultBuilder interface {
+	addResult(msg any) error
+	formatResults() error
+}
+
+type stringResultBuilder struct {
+	results []string
+}
+
+func newStringResultBuilder(rs rego.ResultSet) *stringResultBuilder {
+	return &stringResultBuilder{
+		results: make([]string, 0, len(rs)),
+	}
+}
+
+func (srb *stringResultBuilder) addResult(msg any) error {
 	msgstr, ok := msg.(string)
 	if !ok {
 		return fmt.Errorf("msg is not a string")
 	}
+	srb.results = append(srb.results, msgstr)
+	return nil
+}
+
+func (srb *stringResultBuilder) formatResults() error {
+	return engerrors.NewErrEvaluationFailed("Evaluation failures: \n - %s", strings.Join(srb.results, "\n - "))
+}
+
+type jsonResultBuilder struct {
+	results []map[string]interface{}
+}
+
+func newJSONResultBuilder(rs rego.ResultSet) *jsonResultBuilder {
+	return &jsonResultBuilder{
+		results: make([]map[string]interface{}, 0, len(rs)),
+	}
+}
+
+func (jrb *jsonResultBuilder) addResult(msg any) error {
+	var result map[string]interface{}
+
+	msgstr, ok := msg.(string)
+	if !ok {
+		return fmt.Errorf("msg is not a string")
+	}
+
+	err := json.NewDecoder(strings.NewReader(msgstr)).Decode(&result)
+	if err != nil {
+		// fallback
+		result = map[string]interface{}{
+			"msg": msgstr,
+		}
+	}
+
+	jrb.results = append(jrb.results, result)
+	return nil
+}
+
+func (jrb *jsonResultBuilder) formatResults() error {
+	jsonArray, err := json.Marshal(jrb.results)
+	if err != nil {
+		return fmt.Errorf("failed to marshal violations: %w", err)
+	}
 
-	return engerrors.NewErrEvaluationFailed(msgstr)
+	return engerrors.NewErrEvaluationFailed(string(jsonArray))
 }