From 503052a119f42757c5be46ffd377310c26ed40dc Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 6 Apr 2026 12:23:04 +0200 Subject: [PATCH 1/5] feat: add optional description field to PolicyVulnerabilityFinding Also fix forward compatibility by using DiscardUnknown in protojson unmarshal so older CLIs gracefully ignore fields from newer policies. Signed-off-by: Miguel Martinez Trivino --- .../frontend/attestation/v1/crafting_state.ts | 26 ++++++++++++- ...PolicyVulnerabilityFinding.jsonschema.json | 4 ++ ....v1.PolicyVulnerabilityFinding.schema.json | 4 ++ .../api/attestation/v1/crafting_state.pb.go | 18 +++++++-- .../api/attestation/v1/crafting_state.proto | 2 + pkg/policies/findings/registry.go | 4 +- pkg/policies/findings/registry_test.go | 37 +++++++++++++++++++ 7 files changed, 89 insertions(+), 6 deletions(-) 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/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", From b8726ca9b6e5e039e65a40d3540d63540815140d Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 6 Apr 2026 13:19:28 +0200 Subject: [PATCH 2/5] feat(policies): split structured data into findings for backward compatibility Move structured violation objects from violations to a new findings field in the policy result. When findings is present it fully replaces violations, which reverts to strings-only matching pre-structured behavior. Old CLIs ignore the unknown findings key and continue to parse string violations without errors. Signed-off-by: Miguel Martinez Trivino --- pkg/policies/engine/rego/rego.go | 39 ++++++++----- pkg/policies/engine/rego/rego_test.go | 33 ++++------- .../rego/testfiles/mixed_violations.rego | 6 +- .../rego/testfiles/structured_license.rego | 6 +- .../rego/testfiles/structured_no_message.rego | 6 +- .../rego/testfiles/structured_sast.rego | 6 +- .../rego/testfiles/structured_violations.rego | 6 +- .../structured_vulnerability_builtin.rego | 4 +- pkg/policies/engine/wasm/engine.go | 56 +++++++++++-------- 9 files changed, 88 insertions(+), 74 deletions(-) diff --git a/pkg/policies/engine/rego/rego.go b/pkg/policies/engine/rego/rego.go index 796e10a58..e2e855b61 100644 --- a/pkg/policies/engine/rego/rego.go +++ b/pkg/policies/engine/rego/rego.go @@ -206,27 +206,38 @@ 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) + // Try "findings" first (new format, structured objects). + // When present it fully replaces "violations", providing backward + // compatibility: old CLIs that only understand string violations + // ignore the unknown "findings" key. + if findingsRaw, ok := ruleResult["findings"].([]any); ok { + 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 { + // Legacy format: string-only violations + violations, ok := ruleResult["violations"].([]any) + if !ok { + return nil, engine.ResultFormatError{Field: "violations"} + } + for _, violation := range violations { + vs, ok := violation.(string) + if !ok { + return nil, fmt.Errorf("violation must be a string, got %T", violation) + } + result.Violations = append(result.Violations, &engine.PolicyViolation{Subject: policy.Name, Violation: vs}) } } } diff --git a/pkg/policies/engine/rego/rego_test.go b/pkg/policies/engine/rego/rego_test.go index 93b038ce4..09e7079e5 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,32 @@ 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: "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/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..4b5235aa0 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,26 @@ 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)), + Violations: make([]*engine.PolicyViolation, 0), } - 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) + // "findings" (structured objects) takes precedence over "violations" (legacy strings) + if len(result.Findings) > 0 { + 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 { + for _, raw := range result.Violations { + pv, err := parseWasmStringViolation(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,25 +241,25 @@ func (e *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte return evalResult, nil } -// 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. -func parseWasmViolation(policyName string, raw json.RawMessage) (*engine.PolicyViolation, error) { - var v any - if err := json.Unmarshal(raw, &v); err != nil { - return nil, fmt.Errorf("invalid violation JSON: %w", err) +// 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) +} - switch typed := v.(type) { - case string: - return &engine.PolicyViolation{ - Subject: policyName, - Violation: typed, - }, nil - case map[string]any: - return engine.NewStructuredViolation(policyName, typed) - default: - return nil, fmt.Errorf("violation must be a string or object, got %T", v) +// parseWasmStringViolation parses a legacy string violation. +func parseWasmStringViolation(policyName string, raw json.RawMessage) (*engine.PolicyViolation, error) { + var s string + if err := json.Unmarshal(raw, &s); err != nil { + return nil, fmt.Errorf("violation must be a JSON string: %w", err) } + return &engine.PolicyViolation{ + Subject: policyName, + Violation: s, + }, nil } // MatchesParameters is a stub implementation for WASM policies From af3b2333d3f2ed9e3daebd26781a982f8d0e6844 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 6 Apr 2026 13:27:16 +0200 Subject: [PATCH 3/5] fix(policies): keep backward compat for structured objects in violations Keep accepting structured objects (maps) in the violations field as a deprecated path while policies are migrated to use the new findings field. This avoids breaking existing policies that already return structured violation objects. Signed-off-by: Miguel Martinez Trivino --- pkg/policies/engine/rego/rego.go | 20 ++++++--- pkg/policies/engine/rego/rego_test.go | 13 ++++++ .../deprecated_structured_violations.rego | 43 +++++++++++++++++++ pkg/policies/engine/wasm/engine.go | 35 ++++++++++----- 4 files changed, 95 insertions(+), 16 deletions(-) create mode 100644 pkg/policies/engine/rego/testfiles/deprecated_structured_violations.rego diff --git a/pkg/policies/engine/rego/rego.go b/pkg/policies/engine/rego/rego.go index e2e855b61..97a6f6b8d 100644 --- a/pkg/policies/engine/rego/rego.go +++ b/pkg/policies/engine/rego/rego.go @@ -227,17 +227,27 @@ func parseResultRule(res rego.ResultSet, policy *engine.Policy, rawData *engine. result.Violations = append(result.Violations, pv) } } else { - // Legacy format: string-only violations + // Fallback: violations (strings or structured objects). + // Structured objects in violations are deprecated — use findings instead. violations, ok := ruleResult["violations"].([]any) if !ok { return nil, engine.ResultFormatError{Field: "violations"} } for _, violation := range violations { - vs, ok := violation.(string) - if !ok { - return nil, fmt.Errorf("violation must be a string, got %T", violation) + switch v := violation.(type) { + case string: + result.Violations = append(result.Violations, &engine.PolicyViolation{Subject: policy.Name, Violation: v}) + case map[string]any: + // Deprecated: structured objects in violations will be removed in a future release. + // Migrate to the "findings" field instead. + 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) } - result.Violations = append(result.Violations, &engine.PolicyViolation{Subject: policy.Name, Violation: vs}) } } } diff --git a/pkg/policies/engine/rego/rego_test.go b/pkg/policies/engine/rego/rego_test.go index 09e7079e5..f14f4daaf 100644 --- a/pkg/policies/engine/rego/rego_test.go +++ b/pkg/policies/engine/rego/rego_test.go @@ -578,6 +578,19 @@ func TestRego_StructuredViolations(t *testing.T) { 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 findings with structured policy", fixture: "testfiles/structured_violations.rego", 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/wasm/engine.go b/pkg/policies/engine/wasm/engine.go index 4b5235aa0..a1f3c45a8 100644 --- a/pkg/policies/engine/wasm/engine.go +++ b/pkg/policies/engine/wasm/engine.go @@ -209,7 +209,7 @@ func (e *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte Violations: make([]*engine.PolicyViolation, 0), } - // "findings" (structured objects) takes precedence over "violations" (legacy strings) + // "findings" (structured objects) takes precedence over "violations" if len(result.Findings) > 0 { for _, raw := range result.Findings { pv, err := parseWasmFinding(policy.Name, raw) @@ -219,8 +219,10 @@ func (e *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte evalResult.Violations = append(evalResult.Violations, pv) } } else { + // Fallback: violations (strings or structured objects). + // Structured objects in violations are deprecated — use findings instead. for _, raw := range result.Violations { - pv, err := parseWasmStringViolation(policy.Name, raw) + pv, err := parseWasmViolation(policy.Name, raw) if err != nil { return nil, fmt.Errorf("violation in policy %q: %w", policy.Name, err) } @@ -250,16 +252,27 @@ func parseWasmFinding(policyName string, raw json.RawMessage) (*engine.PolicyVio return engine.NewStructuredViolation(policyName, obj) } -// parseWasmStringViolation parses a legacy string violation. -func parseWasmStringViolation(policyName string, raw json.RawMessage) (*engine.PolicyViolation, error) { - var s string - if err := json.Unmarshal(raw, &s); err != nil { - return nil, fmt.Errorf("violation must be a JSON string: %w", err) +// parseWasmViolation parses a single violation from WASM output. +// 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 { + return nil, fmt.Errorf("invalid violation JSON: %w", err) + } + + switch typed := v.(type) { + case string: + return &engine.PolicyViolation{ + Subject: policyName, + Violation: typed, + }, nil + case map[string]any: + // Deprecated: structured objects in violations will be removed in a future release. + // Migrate to the "findings" field instead. + return engine.NewStructuredViolation(policyName, typed) + default: + return nil, fmt.Errorf("violation must be a string or object, got %T", v) } - return &engine.PolicyViolation{ - Subject: policyName, - Violation: s, - }, nil } // MatchesParameters is a stub implementation for WASM policies From f27f6dd366e44be58f770dc2450127810bf2a07f Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 6 Apr 2026 13:32:28 +0200 Subject: [PATCH 4/5] refactor(policies): simplify findings/violations handling after review - Fix behavioral divergence between Rego and WASM engines for empty findings: both now use len>0 check consistently - Gracefully handle missing violations when findings is absent (no default findings array required in policies) - Pre-allocate violations slice in WASM engine - Trim redundant deprecation comments Signed-off-by: Miguel Martinez Trivino --- pkg/policies/engine/rego/rego.go | 21 +++++++-------------- pkg/policies/engine/wasm/engine.go | 10 ++++------ 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/pkg/policies/engine/rego/rego.go b/pkg/policies/engine/rego/rego.go index 97a6f6b8d..12d4481bf 100644 --- a/pkg/policies/engine/rego/rego.go +++ b/pkg/policies/engine/rego/rego.go @@ -210,11 +210,9 @@ func parseResultRule(res rego.ResultSet, policy *engine.Policy, rawData *engine. result.SkipReason = reason result.Ignore = ignore - // Try "findings" first (new format, structured objects). - // When present it fully replaces "violations", providing backward - // compatibility: old CLIs that only understand string violations - // ignore the unknown "findings" key. - if findingsRaw, ok := ruleResult["findings"].([]any); ok { + // 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 { @@ -226,20 +224,14 @@ func parseResultRule(res rego.ResultSet, policy *engine.Policy, rawData *engine. } result.Violations = append(result.Violations, pv) } - } else { - // Fallback: violations (strings or structured objects). - // Structured objects in violations are deprecated — use findings instead. - violations, ok := ruleResult["violations"].([]any) - if !ok { - return nil, engine.ResultFormatError{Field: "violations"} - } + } 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: - // Deprecated: structured objects in violations will be removed in a future release. - // Migrate to the "findings" field instead. pv, err := engine.NewStructuredViolation(policy.Name, v) if err != nil { return nil, fmt.Errorf("structured violation in policy %q: %w", policy.Name, err) @@ -250,6 +242,7 @@ func parseResultRule(res rego.ResultSet, policy *engine.Policy, rawData *engine. } } } + // If neither findings nor violations is present, result has zero violations. } } diff --git a/pkg/policies/engine/wasm/engine.go b/pkg/policies/engine/wasm/engine.go index a1f3c45a8..9e46fc33b 100644 --- a/pkg/policies/engine/wasm/engine.go +++ b/pkg/policies/engine/wasm/engine.go @@ -206,11 +206,11 @@ 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), } - // "findings" (structured objects) takes precedence over "violations" + // 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 { @@ -219,8 +219,8 @@ func (e *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte evalResult.Violations = append(evalResult.Violations, pv) } } else { - // Fallback: violations (strings or structured objects). - // Structured objects in violations are deprecated — use findings instead. + // 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 { @@ -267,8 +267,6 @@ func parseWasmViolation(policyName string, raw json.RawMessage) (*engine.PolicyV Violation: typed, }, nil case map[string]any: - // Deprecated: structured objects in violations will be removed in a future release. - // Migrate to the "findings" field instead. return engine.NewStructuredViolation(policyName, typed) default: return nil, fmt.Errorf("violation must be a string or object, got %T", v) From 584f0340a6857d4976bbbd427adef914bbaa0851 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Mon, 6 Apr 2026 13:37:57 +0200 Subject: [PATCH 5/5] feat(cli): split policy eval output into violations and findings Return string violations (what old CLIs see) and structured findings separately in policy eval output, making it clear what each CLI version would receive. Signed-off-by: Miguel Martinez Trivino --- app/cli/internal/policydevel/eval.go | 28 +++++++++++++-------- app/cli/internal/policydevel/eval_test.go | 30 ++++++++++------------- 2 files changed, 31 insertions(+), 27 deletions(-) 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) {