feat(policies): add description field and split findings for backward compat (#2988)

Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>

Author: migmartri

app/cli/internal/policydevel/eval.go

@@ -49,7 +49,8 @@ type EvalOptions struct {
 }
 
 type EvalResult struct {
-	Violations  []json.RawMessage `json:"violations"`
+	Violations  []string          `json:"violations"`
+	Findings    []json.RawMessage `json:"findings,omitempty"`
 	SkipReasons []string          `json:"skip_reasons"`
 	Skipped     bool              `json:"skipped"`
 }
@@ -134,22 +135,29 @@ func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materi
 		Result: &EvalResult{
 			Skipped:     policyEv.GetSkipped(),
 			SkipReasons: policyEv.SkipReasons,
-			Violations:  make([]json.RawMessage, 0, len(policyEv.Violations)),
+			Violations:  make([]string, 0, len(policyEv.Violations)),
 		},
 	}
 
-	// Marshal violations using protojson to match the attestation storage format.
-	// Subject is cleared since it's redundant in eval context (always the policy name).
+	// Split violations into string messages and structured findings.
+	// "violations" contains the message strings (what old CLIs see).
+	// "findings" contains the full structured data when present.
 	marshaler := protojson.MarshalOptions{UseProtoNames: true}
 	for _, v := range policyEv.Violations {
-		vc := proto.Clone(v).(*v12.PolicyEvaluation_Violation)
-		vc.Subject = ""
+		summary.Result.Violations = append(summary.Result.Violations, v.GetMessage())
 
-		b, err := marshaler.Marshal(vc)
-		if err != nil {
-			return nil, fmt.Errorf("marshaling violation: %w", err)
+		if f := v.GetFinding(); f != nil {
+			// Clone to clear subject before marshaling
+			vc := proto.Clone(v).(*v12.PolicyEvaluation_Violation)
+			vc.Subject = ""
+			vc.Message = ""
+
+			b, err := marshaler.Marshal(vc)
+			if err != nil {
+				return nil, fmt.Errorf("marshaling finding: %w", err)
+			}
+			summary.Result.Findings = append(summary.Result.Findings, b)
 		}
-		summary.Result.Violations = append(summary.Result.Violations, b)
 	}
 
 	// Include raw debug info if requested

app/cli/internal/policydevel/eval_test.go

@@ -149,7 +149,7 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
 		assert.Contains(t, string(result.Result.Violations[0]), "at least 2 components")
 	})
 
-	t.Run("violations with finding_type use unified format matching attestation storage", func(t *testing.T) {
+	t.Run("structured findings are returned separately from string violations", func(t *testing.T) {
 		opts := &EvalOptions{
 			PolicyPath:   "testdata/sbom-structured-vuln-policy.yaml",
 			MaterialPath: sbomPath,
@@ -160,23 +160,23 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
 		require.NotNil(t, result)
 		assert.False(t, result.Result.Skipped)
 
-		// Single unified violations field with full violation objects (same as attestation)
+		// violations contains string messages
 		require.Len(t, result.Result.Violations, 1)
-
-		var v map[string]any
-		require.NoError(t, json.Unmarshal(result.Result.Violations[0], &v))
-		assert.Nil(t, v["subject"], "subject should be excluded from eval output")
-		assert.Contains(t, v["message"], "Vulnerability found in test-component@1.0.0")
-
-		vuln, ok := v["vulnerability"].(map[string]any)
-		require.True(t, ok, "expected vulnerability finding in violation object")
+		assert.Contains(t, result.Result.Violations[0], "Vulnerability found in test-component@1.0.0")
+
+		// findings contains the structured data
+		require.Len(t, result.Result.Findings, 1)
+		var f map[string]any
+		require.NoError(t, json.Unmarshal(result.Result.Findings[0], &f))
+		vuln, ok := f["vulnerability"].(map[string]any)
+		require.True(t, ok, "expected vulnerability finding")
 		assert.Equal(t, "CVE-2024-1234", vuln["external_id"])
 		assert.Equal(t, "pkg:generic/test-component@1.0.0", vuln["package_purl"])
 		assert.Equal(t, "HIGH", vuln["severity"])
 		assert.InDelta(t, 7.5, vuln["cvss_v3_score"], 0.001)
 	})
 
-	t.Run("violations without finding_type use same unified format", func(t *testing.T) {
+	t.Run("plain string violations have no findings", func(t *testing.T) {
 		opts := &EvalOptions{
 			PolicyPath:   "testdata/sbom-min-components-policy.yaml",
 			MaterialPath: sbomPath,
@@ -186,12 +186,8 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
 		require.NoError(t, err)
 		require.NotNil(t, result)
 		require.Len(t, result.Result.Violations, 1)
-
-		// Same structure as attestation: object with message (subject excluded in eval)
-		var v map[string]any
-		require.NoError(t, json.Unmarshal(result.Result.Violations[0], &v))
-		assert.Nil(t, v["subject"], "subject should be excluded from eval output")
-		assert.Contains(t, v["message"], "at least 2 components")
+		assert.Contains(t, result.Result.Violations[0], "at least 2 components")
+		assert.Empty(t, result.Result.Findings)
 	})
 
 	t.Run("sbom metadata component policy", func(t *testing.T) {

app/controlplane/api/gen/frontend/attestation/v1/crafting_state.ts

@@ -332,6 +332,8 @@ message PolicyVulnerabilityFinding {
   repeated string cwes = 6;
   // Suggested fix or upgrade path
   string recommendation = 7;
+  // Optional longer description of the vulnerability
+  string description = 8;
 }
 
 // Output schema for SAST findings from policy evaluation.

app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.jsonschema.json

@@ -206,29 +206,43 @@ func parseResultRule(res rego.ResultSet, policy *engine.Policy, rawData *engine.
 				ignore = val
 			}
 
-			violations, ok := ruleResult["violations"].([]any)
-			if !ok {
-				return nil, engine.ResultFormatError{Field: "violations"}
-			}
-
 			result.Skipped = skipped
 			result.SkipReason = reason
 			result.Ignore = ignore
 
-			for _, violation := range violations {
-				switch v := violation.(type) {
-				case string:
-					result.Violations = append(result.Violations, &engine.PolicyViolation{Subject: policy.Name, Violation: v})
-				case map[string]any:
-					pv, err := engine.NewStructuredViolation(policy.Name, v)
+			// Prefer non-empty "findings" (structured objects) over "violations" for backward compatibility.
+			findingsRaw, _ := ruleResult["findings"].([]any)
+			if len(findingsRaw) > 0 {
+				for _, f := range findingsRaw {
+					obj, ok := f.(map[string]any)
+					if !ok {
+						return nil, fmt.Errorf("finding must be an object, got %T", f)
+					}
+					pv, err := engine.NewStructuredViolation(policy.Name, obj)
 					if err != nil {
-						return nil, fmt.Errorf("structured violation in policy %q: %w", policy.Name, err)
+						return nil, fmt.Errorf("structured finding in policy %q: %w", policy.Name, err)
 					}
 					result.Violations = append(result.Violations, pv)
-				default:
-					return nil, fmt.Errorf("violation must be a string or object, got %T", violation)
+				}
+			} else if violations, ok := ruleResult["violations"].([]any); ok {
+				// Fallback: violations (strings or deprecated structured objects).
+				// TODO: remove structured object support once policies are fully migrated to findings.
+				for _, violation := range violations {
+					switch v := violation.(type) {
+					case string:
+						result.Violations = append(result.Violations, &engine.PolicyViolation{Subject: policy.Name, Violation: v})
+					case map[string]any:
+						pv, err := engine.NewStructuredViolation(policy.Name, v)
+						if err != nil {
+							return nil, fmt.Errorf("structured violation in policy %q: %w", policy.Name, err)
+						}
+						result.Violations = append(result.Violations, pv)
+					default:
+						return nil, fmt.Errorf("violation must be a string or object, got %T", violation)
+					}
 				}
 			}
+			// If neither findings nor violations is present, result has zero violations.
 		}
 	}
 

app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.schema.json

@@ -544,7 +544,7 @@ func TestRego_StructuredViolations(t *testing.T) {
 		checkFn        func(t *testing.T, result *engine.EvaluationResult)
 	}{
 		{
-			name:           "structured violations with message field",
+			name:           "structured findings with message field",
 			fixture:        "testfiles/structured_violations.rego",
 			input:          `{"vulnerabilities": [{"id": "CVE-2024-1234", "severity": "CRITICAL", "purl": "pkg:golang/example.com/lib@v1.0.0"}]}`,
 			wantViolations: 1,
@@ -560,43 +560,45 @@ func TestRego_StructuredViolations(t *testing.T) {
 			},
 		},
 		{
-			name:    "structured violations without message field errors",
+			name:    "structured findings without message field errors",
 			fixture: "testfiles/structured_no_message.rego",
 			input:   `{"vulnerabilities": [{"id": "CVE-2024-1234", "severity": "HIGH"}]}`,
-			wantErr: `missing required "message" field`,
+			wantErr: `structured finding in policy`,
 		},
 		{
-			name:           "mixed string and structured violations",
+			name:           "findings takes precedence over violations",
 			fixture:        "testfiles/mixed_violations.rego",
 			input:          `{"has_string_violation": true, "vulnerabilities": [{"id": "CVE-2024-5678", "severity": "HIGH"}]}`,
-			wantViolations: 2,
+			wantViolations: 1, // only findings, string violations ignored
 			checkFn: func(t *testing.T, result *engine.EvaluationResult) {
 				t.Helper()
-				var stringViolation, structuredViolation *engine.PolicyViolation
-				for _, v := range result.Violations {
-					if v.RawFinding == nil {
-						stringViolation = v
-					} else {
-						structuredViolation = v
-					}
-				}
-				require.NotNil(t, stringViolation, "expected a string violation")
-				assert.Equal(t, "simple string violation", stringViolation.Violation)
-				assert.Nil(t, stringViolation.RawFinding)
-
-				require.NotNil(t, structuredViolation, "expected a structured violation")
-				assert.Equal(t, "Found vulnerability CVE-2024-5678", structuredViolation.Violation)
-				assert.Equal(t, "CVE-2024-5678", structuredViolation.RawFinding["external_id"])
+				v := result.Violations[0]
+				require.NotNil(t, v.RawFinding)
+				assert.Equal(t, "Found vulnerability CVE-2024-5678", v.Violation)
+				assert.Equal(t, "CVE-2024-5678", v.RawFinding["external_id"])
+			},
+		},
+		{
+			name:           "deprecated structured objects in violations still work",
+			fixture:        "testfiles/deprecated_structured_violations.rego",
+			input:          `{"vulnerabilities": [{"id": "CVE-2024-1234", "severity": "CRITICAL", "purl": "pkg:golang/example.com/lib@v1.0.0"}]}`,
+			wantViolations: 1,
+			checkFn: func(t *testing.T, result *engine.EvaluationResult) {
+				t.Helper()
+				v := result.Violations[0]
+				assert.Equal(t, "Found vulnerability CVE-2024-1234 (CRITICAL)", v.Violation)
+				assert.NotNil(t, v.RawFinding)
+				assert.Equal(t, "CVE-2024-1234", v.RawFinding["external_id"])
 			},
 		},
 		{
-			name:           "no violations with structured policy",
+			name:           "no findings with structured policy",
 			fixture:        "testfiles/structured_violations.rego",
 			input:          `{"vulnerabilities": []}`,
 			wantViolations: 0,
 		},
 		{
-			name:           "multiple structured violations",
+			name:           "multiple structured findings",
 			fixture:        "testfiles/structured_violations.rego",
 			input:          `{"vulnerabilities": [{"id": "CVE-2024-1", "severity": "HIGH", "purl": "pkg:npm/foo@1.0"}, {"id": "CVE-2024-2", "severity": "LOW", "purl": "pkg:npm/bar@2.0"}]}`,
 			wantViolations: 2,