diff --git a/app/cli/internal/policydevel/eval.go b/app/cli/internal/policydevel/eval.go index 6cf6ad254..7d01faa83 100644 --- a/app/cli/internal/policydevel/eval.go +++ b/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 diff --git a/app/cli/internal/policydevel/eval_test.go b/app/cli/internal/policydevel/eval_test.go index c69f7a6f7..bbc758774 100644 --- a/app/cli/internal/policydevel/eval_test.go +++ b/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) { diff --git a/app/controlplane/api/gen/frontend/attestation/v1/crafting_state.ts b/app/controlplane/api/gen/frontend/attestation/v1/crafting_state.ts index 66d6287e2..e9f545b07 100644 --- a/app/controlplane/api/gen/frontend/attestation/v1/crafting_state.ts +++ b/app/controlplane/api/gen/frontend/attestation/v1/crafting_state.ts @@ -354,6 +354,8 @@ export interface PolicyVulnerabilityFinding { cwes: string[]; /** Suggested fix or upgrade path */ recommendation: string; + /** Optional longer description of the vulnerability */ + description: string; } /** @@ -3232,7 +3234,16 @@ export const PolicyEvaluationBundle = { }; function createBasePolicyVulnerabilityFinding(): PolicyVulnerabilityFinding { - return { message: "", externalId: "", packagePurl: "", severity: "", cvssV3Score: 0, cwes: [], recommendation: "" }; + return { + message: "", + externalId: "", + packagePurl: "", + severity: "", + cvssV3Score: 0, + cwes: [], + recommendation: "", + description: "", + }; } export const PolicyVulnerabilityFinding = { @@ -3258,6 +3269,9 @@ export const PolicyVulnerabilityFinding = { if (message.recommendation !== "") { writer.uint32(58).string(message.recommendation); } + if (message.description !== "") { + writer.uint32(66).string(message.description); + } return writer; }, @@ -3317,6 +3331,13 @@ export const PolicyVulnerabilityFinding = { message.recommendation = reader.string(); continue; + case 8: + if (tag !== 66) { + break; + } + + message.description = reader.string(); + continue; } if ((tag & 7) === 4 || tag === 0) { break; @@ -3335,6 +3356,7 @@ export const PolicyVulnerabilityFinding = { cvssV3Score: isSet(object.cvssV3Score) ? Number(object.cvssV3Score) : 0, cwes: Array.isArray(object?.cwes) ? object.cwes.map((e: any) => String(e)) : [], recommendation: isSet(object.recommendation) ? String(object.recommendation) : "", + description: isSet(object.description) ? String(object.description) : "", }; }, @@ -3351,6 +3373,7 @@ export const PolicyVulnerabilityFinding = { obj.cwes = []; } message.recommendation !== undefined && (obj.recommendation = message.recommendation); + message.description !== undefined && (obj.description = message.description); return obj; }, @@ -3367,6 +3390,7 @@ export const PolicyVulnerabilityFinding = { message.cvssV3Score = object.cvssV3Score ?? 0; message.cwes = object.cwes?.map((e) => e) || []; message.recommendation = object.recommendation ?? ""; + message.description = object.description ?? ""; return message; }, }; diff --git a/app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.jsonschema.json b/app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.jsonschema.json index 7b86551c1..99369bcab 100644 --- a/app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.jsonschema.json +++ b/app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.jsonschema.json @@ -49,6 +49,10 @@ }, "type": "array" }, + "description": { + "description": "Optional longer description of the vulnerability", + "type": "string" + }, "externalId": { "description": "Vulnerability identifier (e.g., CVE-2024-1234, GHSA-xxxx)", "type": "string" diff --git a/app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.schema.json b/app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.schema.json index 439bd26da..831387d86 100644 --- a/app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.schema.json +++ b/app/controlplane/api/gen/jsonschema/attestation.v1.PolicyVulnerabilityFinding.schema.json @@ -49,6 +49,10 @@ }, "type": "array" }, + "description": { + "description": "Optional longer description of the vulnerability", + "type": "string" + }, "external_id": { "description": "Vulnerability identifier (e.g., CVE-2024-1234, GHSA-xxxx)", "type": "string" diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state.pb.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state.pb.go index 292493eae..e532c29e8 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state.pb.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state.pb.go @@ -691,8 +691,10 @@ type PolicyVulnerabilityFinding struct { Cwes []string `protobuf:"bytes,6,rep,name=cwes,proto3" json:"cwes,omitempty"` // Suggested fix or upgrade path Recommendation string `protobuf:"bytes,7,opt,name=recommendation,proto3" json:"recommendation,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // Optional longer description of the vulnerability + Description string `protobuf:"bytes,8,opt,name=description,proto3" json:"description,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *PolicyVulnerabilityFinding) Reset() { @@ -774,6 +776,13 @@ func (x *PolicyVulnerabilityFinding) GetRecommendation() string { return "" } +func (x *PolicyVulnerabilityFinding) GetDescription() string { + if x != nil { + return x.Description + } + return "" +} + // Output schema for SAST findings from policy evaluation. // Used when a policy declares finding_type: SAST. type PolicySASTFinding struct { @@ -2749,7 +2758,7 @@ const file_attestation_v1_crafting_state_proto_rawDesc = "" + "\x05input\x18\x01 \x01(\fR\x05input\x12\x16\n" + "\x06output\x18\x02 \x01(\fR\x06output\"\\\n" + "\x16PolicyEvaluationBundle\x12B\n" + - "\vevaluations\x18\x01 \x03(\v2 .attestation.v1.PolicyEvaluationR\vevaluations\"\xaf\x02\n" + + "\vevaluations\x18\x01 \x03(\v2 .attestation.v1.PolicyEvaluationR\vevaluations\"\xd1\x02\n" + "\x1aPolicyVulnerabilityFinding\x12 \n" + "\amessage\x18\x01 \x01(\tB\x06\xbaH\x03\xc8\x01\x01R\amessage\x12'\n" + "\vexternal_id\x18\x02 \x01(\tB\x06\xbaH\x03\xc8\x01\x01R\n" + @@ -2758,7 +2767,8 @@ const file_attestation_v1_crafting_state_proto_rawDesc = "" + "\bseverity\x18\x04 \x01(\tB\x06\xbaH\x03\xc8\x01\x01R\bseverity\x12;\n" + "\rcvss_v3_score\x18\x05 \x01(\x01B\x17\xbaH\x14\x12\x12\x19\x00\x00\x00\x00\x00\x00$@)\x00\x00\x00\x00\x00\x00\x00\x00R\vcvssV3Score\x12\x12\n" + "\x04cwes\x18\x06 \x03(\tR\x04cwes\x12&\n" + - "\x0erecommendation\x18\a \x01(\tR\x0erecommendation\"\x8a\x02\n" + + "\x0erecommendation\x18\a \x01(\tR\x0erecommendation\x12 \n" + + "\vdescription\x18\b \x01(\tR\vdescription\"\x8a\x02\n" + "\x11PolicySASTFinding\x12 \n" + "\amessage\x18\x01 \x01(\tB\x06\xbaH\x03\xc8\x01\x01R\amessage\x12\x1f\n" + "\arule_id\x18\x02 \x01(\tB\x06\xbaH\x03\xc8\x01\x01R\x06ruleId\x12\"\n" + diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state.proto b/pkg/attestation/crafter/api/attestation/v1/crafting_state.proto index 9ede42ad1..c5148e14f 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state.proto +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state.proto @@ -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. diff --git a/pkg/policies/engine/rego/rego.go b/pkg/policies/engine/rego/rego.go index 796e10a58..12d4481bf 100644 --- a/pkg/policies/engine/rego/rego.go +++ b/pkg/policies/engine/rego/rego.go @@ -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. } } diff --git a/pkg/policies/engine/rego/rego_test.go b/pkg/policies/engine/rego/rego_test.go index 93b038ce4..f14f4daaf 100644 --- a/pkg/policies/engine/rego/rego_test.go +++ b/pkg/policies/engine/rego/rego_test.go @@ -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, diff --git a/pkg/policies/engine/rego/testfiles/deprecated_structured_violations.rego b/pkg/policies/engine/rego/testfiles/deprecated_structured_violations.rego new file mode 100644 index 000000000..66c836680 --- /dev/null +++ b/pkg/policies/engine/rego/testfiles/deprecated_structured_violations.rego @@ -0,0 +1,43 @@ +package main + +import rego.v1 + +################################ +# Common section do NOT change # +################################ + +result := { + "skipped": skipped, + "violations": violations, + "skip_reason": skip_reason, +} + +default skip_reason := "" + +skip_reason := m if { + not valid_input + m := "invalid input" +} + +default skipped := true + +skipped := false if valid_input + +######################################## +# EO Common section, custom code below # +######################################## + +valid_input := true + +# Deprecated: structured objects in violations. +# This format is supported temporarily for backward compatibility. +# Policies should migrate to use the "findings" field instead. +violations contains v if { + some vuln in input.vulnerabilities + v := { + "message": sprintf("Found vulnerability %s (%s)", [vuln.id, vuln.severity]), + "external_id": vuln.id, + "severity": vuln.severity, + "package_purl": vuln.purl, + } +} diff --git a/pkg/policies/engine/rego/testfiles/mixed_violations.rego b/pkg/policies/engine/rego/testfiles/mixed_violations.rego index 813c2a120..f90b8be7a 100644 --- a/pkg/policies/engine/rego/testfiles/mixed_violations.rego +++ b/pkg/policies/engine/rego/testfiles/mixed_violations.rego @@ -9,6 +9,7 @@ import rego.v1 result := { "skipped": skipped, "violations": violations, + "findings": findings, "skip_reason": skip_reason, } @@ -29,13 +30,14 @@ skipped := false if valid_input valid_input := true -# Returns a mix of string and structured violations +# Legacy string violations — should be ignored when findings is present violations contains msg if { input.has_string_violation msg := "simple string violation" } -violations contains v if { +# Structured findings take precedence over violations +findings contains v if { some vuln in input.vulnerabilities v := { "message": sprintf("Found vulnerability %s", [vuln.id]), diff --git a/pkg/policies/engine/rego/testfiles/structured_license.rego b/pkg/policies/engine/rego/testfiles/structured_license.rego index 2a2f010ef..f87a3ce4c 100644 --- a/pkg/policies/engine/rego/testfiles/structured_license.rego +++ b/pkg/policies/engine/rego/testfiles/structured_license.rego @@ -8,7 +8,7 @@ import rego.v1 result := { "skipped": skipped, - "violations": violations, + "findings": findings, "skip_reason": skip_reason, } @@ -29,8 +29,8 @@ skipped := false if valid_input valid_input := true -# Returns structured license violation objects -violations contains v if { +# Returns structured license finding objects +findings contains v if { some comp in input.components some license in comp.licenses license in input.banned_licenses diff --git a/pkg/policies/engine/rego/testfiles/structured_no_message.rego b/pkg/policies/engine/rego/testfiles/structured_no_message.rego index a4a958150..5650a6783 100644 --- a/pkg/policies/engine/rego/testfiles/structured_no_message.rego +++ b/pkg/policies/engine/rego/testfiles/structured_no_message.rego @@ -8,7 +8,7 @@ import rego.v1 result := { "skipped": skipped, - "violations": violations, + "findings": findings, "skip_reason": skip_reason, } @@ -29,8 +29,8 @@ skipped := false if valid_input valid_input := true -# Returns structured violation objects WITHOUT a "message" field -violations contains v if { +# Returns structured finding objects WITHOUT a "message" field +findings contains v if { some vuln in input.vulnerabilities v := { "external_id": vuln.id, diff --git a/pkg/policies/engine/rego/testfiles/structured_sast.rego b/pkg/policies/engine/rego/testfiles/structured_sast.rego index e14d28f29..15f3a2c34 100644 --- a/pkg/policies/engine/rego/testfiles/structured_sast.rego +++ b/pkg/policies/engine/rego/testfiles/structured_sast.rego @@ -8,7 +8,7 @@ import rego.v1 result := { "skipped": skipped, - "violations": violations, + "findings": findings, "skip_reason": skip_reason, } @@ -29,8 +29,8 @@ skipped := false if valid_input valid_input := true -# Returns structured SAST violation objects -violations contains v if { +# Returns structured SAST finding objects +findings contains v if { some finding in input.findings v := { "message": sprintf("SAST finding %s in %s", [finding.rule_id, finding.location]), diff --git a/pkg/policies/engine/rego/testfiles/structured_violations.rego b/pkg/policies/engine/rego/testfiles/structured_violations.rego index f39c43839..367c621d7 100644 --- a/pkg/policies/engine/rego/testfiles/structured_violations.rego +++ b/pkg/policies/engine/rego/testfiles/structured_violations.rego @@ -8,7 +8,7 @@ import rego.v1 result := { "skipped": skipped, - "violations": violations, + "findings": findings, "skip_reason": skip_reason, } @@ -29,8 +29,8 @@ skipped := false if valid_input valid_input := true -# Returns structured violation objects with a required "message" field -violations contains v if { +# Returns structured finding objects with a required "message" field +findings contains v if { some vuln in input.vulnerabilities v := { "message": sprintf("Found vulnerability %s (%s)", [vuln.id, vuln.severity]), diff --git a/pkg/policies/engine/rego/testfiles/structured_vulnerability_builtin.rego b/pkg/policies/engine/rego/testfiles/structured_vulnerability_builtin.rego index 735c6890f..6512d41e7 100644 --- a/pkg/policies/engine/rego/testfiles/structured_vulnerability_builtin.rego +++ b/pkg/policies/engine/rego/testfiles/structured_vulnerability_builtin.rego @@ -8,7 +8,7 @@ import rego.v1 result := { "skipped": skipped, - "violations": violations, + "findings": findings, "skip_reason": skip_reason, } @@ -30,7 +30,7 @@ skipped := false if valid_input valid_input := true # Returns structured vulnerability findings using the chainloop.vulnerability builtin -violations contains v if { +findings contains v if { some vuln in input.vulnerabilities v := chainloop.vulnerability( sprintf("Found vulnerability %s (%s)", [vuln.id, vuln.severity]), diff --git a/pkg/policies/engine/wasm/engine.go b/pkg/policies/engine/wasm/engine.go index e37b5160d..9e46fc33b 100644 --- a/pkg/policies/engine/wasm/engine.go +++ b/pkg/policies/engine/wasm/engine.go @@ -191,6 +191,7 @@ func (e *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte var result struct { Skipped bool `json:"skipped"` Violations []json.RawMessage `json:"violations"` + Findings []json.RawMessage `json:"findings"` SkipReason string `json:"skip_reason"` Ignore bool `json:"ignore"` } @@ -205,15 +206,28 @@ func (e *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte Skipped: result.Skipped, SkipReason: result.SkipReason, Ignore: result.Ignore, - Violations: make([]*engine.PolicyViolation, 0, len(result.Violations)), } - for _, raw := range result.Violations { - pv, err := parseWasmViolation(policy.Name, raw) - if err != nil { - return nil, fmt.Errorf("violation in policy %q: %w", policy.Name, err) + // Prefer non-empty "findings" (structured objects) over "violations" for backward compatibility. + if len(result.Findings) > 0 { + evalResult.Violations = make([]*engine.PolicyViolation, 0, len(result.Findings)) + for _, raw := range result.Findings { + pv, err := parseWasmFinding(policy.Name, raw) + if err != nil { + return nil, fmt.Errorf("finding in policy %q: %w", policy.Name, err) + } + evalResult.Violations = append(evalResult.Violations, pv) + } + } else { + // Fallback: violations (strings or deprecated structured objects). + evalResult.Violations = make([]*engine.PolicyViolation, 0, len(result.Violations)) + for _, raw := range result.Violations { + pv, err := parseWasmViolation(policy.Name, raw) + if err != nil { + return nil, fmt.Errorf("violation in policy %q: %w", policy.Name, err) + } + evalResult.Violations = append(evalResult.Violations, pv) } - evalResult.Violations = append(evalResult.Violations, pv) } e.logger.Debug().Str("policy", policy.Name).Int("violations", len(evalResult.Violations)).Bool("skipped", evalResult.Skipped).Msg("WASM policy evaluation complete") @@ -229,8 +243,17 @@ func (e *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte return evalResult, nil } +// parseWasmFinding parses a single structured finding (object with "message" field). +func parseWasmFinding(policyName string, raw json.RawMessage) (*engine.PolicyViolation, error) { + var obj map[string]any + if err := json.Unmarshal(raw, &obj); err != nil { + return nil, fmt.Errorf("finding must be a JSON object: %w", err) + } + return engine.NewStructuredViolation(policyName, obj) +} + // parseWasmViolation parses a single violation from WASM output. -// The violation can be either a JSON string or a JSON object with a required "message" field. +// Accepts both strings (legacy) and objects (deprecated, use findings instead). func parseWasmViolation(policyName string, raw json.RawMessage) (*engine.PolicyViolation, error) { var v any if err := json.Unmarshal(raw, &v); err != nil { diff --git a/pkg/policies/findings/registry.go b/pkg/policies/findings/registry.go index 1ee67d71a..8aa68dbe0 100644 --- a/pkg/policies/findings/registry.go +++ b/pkg/policies/findings/registry.go @@ -60,7 +60,9 @@ func ValidateFinding(findingType string, raw map[string]any) (proto.Message, err } msg := factory() - if err := protojson.Unmarshal(data, msg); err != nil { + // DiscardUnknown allows policies to include fields added in newer proto + // versions without breaking older CLIs that haven't been updated yet. + if err := (protojson.UnmarshalOptions{DiscardUnknown: true}).Unmarshal(data, msg); err != nil { return nil, fmt.Errorf("finding does not match %s schema: %w", findingType, err) } diff --git a/pkg/policies/findings/registry_test.go b/pkg/policies/findings/registry_test.go index d46e6c6fe..0564ed274 100644 --- a/pkg/policies/findings/registry_test.go +++ b/pkg/policies/findings/registry_test.go @@ -74,6 +74,26 @@ func TestValidateFinding(t *testing.T) { assert.InDelta(t, 9.8, f.GetCvssV3Score(), 0.01) }, }, + { + name: "valid vulnerability finding with description", + findingType: "VULNERABILITY", + raw: map[string]any{ + "message": "Found CVE-2024-5678", + "external_id": "CVE-2024-5678", + "package_purl": "pkg:golang/example.com/lib@v2.0.0", + "severity": "HIGH", + "description": "A buffer overflow vulnerability in the parsing module allows remote code execution.", + }, + checkFn: func(t *testing.T, msg interface{}) { + t.Helper() + f, ok := msg.(*v1.PolicyVulnerabilityFinding) + require.True(t, ok) + assert.Equal(t, "Found CVE-2024-5678", f.GetMessage()) + assert.Equal(t, "CVE-2024-5678", f.GetExternalId()) + assert.Equal(t, "HIGH", f.GetSeverity()) + assert.Equal(t, "A buffer overflow vulnerability in the parsing module allows remote code execution.", f.GetDescription()) + }, + }, { name: "vulnerability finding missing required field", findingType: "VULNERABILITY", @@ -142,6 +162,23 @@ func TestValidateFinding(t *testing.T) { assert.Equal(t, "GPL-3.0", f.GetLicenseId()) }, }, + { + name: "vulnerability finding with unknown field is accepted", + findingType: "VULNERABILITY", + raw: map[string]any{ + "message": "Found CVE-2024-9999", + "external_id": "CVE-2024-9999", + "package_purl": "pkg:golang/example.com/lib@v3.0.0", + "severity": "LOW", + "future_field": "some value from a newer policy", + }, + checkFn: func(t *testing.T, msg interface{}) { + t.Helper() + f, ok := msg.(*v1.PolicyVulnerabilityFinding) + require.True(t, ok) + assert.Equal(t, "CVE-2024-9999", f.GetExternalId()) + }, + }, { name: "unknown finding type", findingType: "UNKNOWN",