Skip to content

Commit 584f034

Browse files
committed
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 <miguel@chainloop.dev>
1 parent f27f6dd commit 584f034

2 files changed

Lines changed: 31 additions & 27 deletions

File tree

app/cli/internal/policydevel/eval.go

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ type EvalOptions struct {
4949
}
5050

5151
type EvalResult struct {
52-
Violations []json.RawMessage `json:"violations"`
52+
Violations []string `json:"violations"`
53+
Findings []json.RawMessage `json:"findings,omitempty"`
5354
SkipReasons []string `json:"skip_reasons"`
5455
Skipped bool `json:"skipped"`
5556
}
@@ -134,22 +135,29 @@ func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materi
134135
Result: &EvalResult{
135136
Skipped: policyEv.GetSkipped(),
136137
SkipReasons: policyEv.SkipReasons,
137-
Violations: make([]json.RawMessage, 0, len(policyEv.Violations)),
138+
Violations: make([]string, 0, len(policyEv.Violations)),
138139
},
139140
}
140141

141-
// Marshal violations using protojson to match the attestation storage format.
142-
// Subject is cleared since it's redundant in eval context (always the policy name).
142+
// Split violations into string messages and structured findings.
143+
// "violations" contains the message strings (what old CLIs see).
144+
// "findings" contains the full structured data when present.
143145
marshaler := protojson.MarshalOptions{UseProtoNames: true}
144146
for _, v := range policyEv.Violations {
145-
vc := proto.Clone(v).(*v12.PolicyEvaluation_Violation)
146-
vc.Subject = ""
147+
summary.Result.Violations = append(summary.Result.Violations, v.GetMessage())
147148

148-
b, err := marshaler.Marshal(vc)
149-
if err != nil {
150-
return nil, fmt.Errorf("marshaling violation: %w", err)
149+
if f := v.GetFinding(); f != nil {
150+
// Clone to clear subject before marshaling
151+
vc := proto.Clone(v).(*v12.PolicyEvaluation_Violation)
152+
vc.Subject = ""
153+
vc.Message = ""
154+
155+
b, err := marshaler.Marshal(vc)
156+
if err != nil {
157+
return nil, fmt.Errorf("marshaling finding: %w", err)
158+
}
159+
summary.Result.Findings = append(summary.Result.Findings, b)
151160
}
152-
summary.Result.Violations = append(summary.Result.Violations, b)
153161
}
154162

155163
// Include raw debug info if requested

app/cli/internal/policydevel/eval_test.go

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
149149
assert.Contains(t, string(result.Result.Violations[0]), "at least 2 components")
150150
})
151151

152-
t.Run("violations with finding_type use unified format matching attestation storage", func(t *testing.T) {
152+
t.Run("structured findings are returned separately from string violations", func(t *testing.T) {
153153
opts := &EvalOptions{
154154
PolicyPath: "testdata/sbom-structured-vuln-policy.yaml",
155155
MaterialPath: sbomPath,
@@ -160,23 +160,23 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
160160
require.NotNil(t, result)
161161
assert.False(t, result.Result.Skipped)
162162

163-
// Single unified violations field with full violation objects (same as attestation)
163+
// violations contains string messages
164164
require.Len(t, result.Result.Violations, 1)
165-
166-
var v map[string]any
167-
require.NoError(t, json.Unmarshal(result.Result.Violations[0], &v))
168-
assert.Nil(t, v["subject"], "subject should be excluded from eval output")
169-
assert.Contains(t, v["message"], "Vulnerability found in test-component@1.0.0")
170-
171-
vuln, ok := v["vulnerability"].(map[string]any)
172-
require.True(t, ok, "expected vulnerability finding in violation object")
165+
assert.Contains(t, result.Result.Violations[0], "Vulnerability found in test-component@1.0.0")
166+
167+
// findings contains the structured data
168+
require.Len(t, result.Result.Findings, 1)
169+
var f map[string]any
170+
require.NoError(t, json.Unmarshal(result.Result.Findings[0], &f))
171+
vuln, ok := f["vulnerability"].(map[string]any)
172+
require.True(t, ok, "expected vulnerability finding")
173173
assert.Equal(t, "CVE-2024-1234", vuln["external_id"])
174174
assert.Equal(t, "pkg:generic/test-component@1.0.0", vuln["package_purl"])
175175
assert.Equal(t, "HIGH", vuln["severity"])
176176
assert.InDelta(t, 7.5, vuln["cvss_v3_score"], 0.001)
177177
})
178178

179-
t.Run("violations without finding_type use same unified format", func(t *testing.T) {
179+
t.Run("plain string violations have no findings", func(t *testing.T) {
180180
opts := &EvalOptions{
181181
PolicyPath: "testdata/sbom-min-components-policy.yaml",
182182
MaterialPath: sbomPath,
@@ -186,12 +186,8 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
186186
require.NoError(t, err)
187187
require.NotNil(t, result)
188188
require.Len(t, result.Result.Violations, 1)
189-
190-
// Same structure as attestation: object with message (subject excluded in eval)
191-
var v map[string]any
192-
require.NoError(t, json.Unmarshal(result.Result.Violations[0], &v))
193-
assert.Nil(t, v["subject"], "subject should be excluded from eval output")
194-
assert.Contains(t, v["message"], "at least 2 components")
189+
assert.Contains(t, result.Result.Violations[0], "at least 2 components")
190+
assert.Empty(t, result.Result.Findings)
195191
})
196192

197193
t.Run("sbom metadata component policy", func(t *testing.T) {

0 commit comments

Comments
 (0)