Skip to content

Commit 90eaace

Browse files
committed
fix(cli): standardize policy eval output to match attestation violation format
Replace dual violations/structured_violations fields in EvalResult with a single violations array of protojson-marshaled PolicyEvaluation_Violation objects, matching the format stored in attestations. Subject field is excluded since it's redundant in the eval context (always the policy name). Signed-off-by: Miguel Martinez <migmartri@gmail.com> Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
1 parent b417baf commit 90eaace

2 files changed

Lines changed: 31 additions & 36 deletions

File tree

app/cli/internal/policydevel/eval.go

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/rs/zerolog"
2727
"google.golang.org/grpc"
2828
"google.golang.org/protobuf/encoding/protojson"
29+
"google.golang.org/protobuf/proto"
2930

3031
v12 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1"
3132
"github.com/chainloop-dev/chainloop/pkg/attestation/crafter/materials"
@@ -48,10 +49,9 @@ type EvalOptions struct {
4849
}
4950

5051
type EvalResult struct {
51-
Violations []string `json:"violations"`
52-
StructuredViolations []json.RawMessage `json:"structured_violations,omitempty"`
53-
SkipReasons []string `json:"skip_reasons"`
54-
Skipped bool `json:"skipped"`
52+
Violations []json.RawMessage `json:"violations"`
53+
SkipReasons []string `json:"skip_reasons"`
54+
Skipped bool `json:"skipped"`
5555
}
5656

5757
type EvalSummary struct {
@@ -134,29 +134,22 @@ func verifyMaterial(pol *v1.Policies, material *v12.Attestation_Material, materi
134134
Result: &EvalResult{
135135
Skipped: policyEv.GetSkipped(),
136136
SkipReasons: policyEv.SkipReasons,
137-
Violations: make([]string, 0, len(policyEv.Violations)),
137+
Violations: make([]json.RawMessage, 0, len(policyEv.Violations)),
138138
},
139139
}
140140

141-
hasStructuredFindings := false
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).
143+
marshaler := protojson.MarshalOptions{UseProtoNames: true}
142144
for _, v := range policyEv.Violations {
143-
summary.Result.Violations = append(summary.Result.Violations, v.Message)
144-
if v.GetFinding() != nil {
145-
hasStructuredFindings = true
146-
}
147-
}
145+
vc := proto.Clone(v).(*v12.PolicyEvaluation_Violation)
146+
vc.Subject = ""
148147

149-
// Include structured violations when any violation has finding data
150-
if hasStructuredFindings {
151-
marshaler := protojson.MarshalOptions{UseProtoNames: true}
152-
summary.Result.StructuredViolations = make([]json.RawMessage, 0, len(policyEv.Violations))
153-
for _, v := range policyEv.Violations {
154-
b, err := marshaler.Marshal(v)
155-
if err != nil {
156-
return nil, fmt.Errorf("marshaling structured violation: %w", err)
157-
}
158-
summary.Result.StructuredViolations = append(summary.Result.StructuredViolations, b)
148+
b, err := marshaler.Marshal(vc)
149+
if err != nil {
150+
return nil, fmt.Errorf("marshaling violation: %w", err)
159151
}
152+
summary.Result.Violations = append(summary.Result.Violations, b)
160153
}
161154

162155
// Include raw debug info if requested

app/cli/internal/policydevel/eval_test.go

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,10 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
146146
require.NotNil(t, result)
147147
assert.False(t, result.Result.Skipped)
148148
assert.Len(t, result.Result.Violations, 1)
149-
assert.Contains(t, result.Result.Violations[0], "at least 2 components")
149+
assert.Contains(t, string(result.Result.Violations[0]), "at least 2 components")
150150
})
151151

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

163-
// Both fields populated: violations (messages) and structured_violations (proto JSON)
163+
// Single unified violations field with full violation objects (same as attestation)
164164
require.Len(t, result.Result.Violations, 1)
165-
assert.Contains(t, result.Result.Violations[0], "Vulnerability found in test-component@1.0.0")
166165

167-
require.Len(t, result.Result.StructuredViolations, 1)
168-
var sv map[string]any
169-
require.NoError(t, json.Unmarshal(result.Result.StructuredViolations[0], &sv))
170-
assert.Contains(t, sv["message"], "Vulnerability found in test-component@1.0.0")
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")
171170

172-
vuln, ok := sv["vulnerability"].(map[string]any)
173-
require.True(t, ok, "expected vulnerability finding in structured violation")
171+
vuln, ok := v["vulnerability"].(map[string]any)
172+
require.True(t, ok, "expected vulnerability finding in violation object")
174173
assert.Equal(t, "CVE-2024-1234", vuln["external_id"])
175174
assert.Equal(t, "pkg:generic/test-component@1.0.0", vuln["package_purl"])
176175
assert.Equal(t, "HIGH", vuln["severity"])
177176
assert.InDelta(t, 7.5, vuln["cvss_v3_score"], 0.001)
178177
})
179178

180-
t.Run("no structured violations for plain string policies", func(t *testing.T) {
179+
t.Run("violations without finding_type use same unified format", func(t *testing.T) {
181180
opts := &EvalOptions{
182181
PolicyPath: "testdata/sbom-min-components-policy.yaml",
183182
MaterialPath: sbomPath,
@@ -187,9 +186,12 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
187186
require.NoError(t, err)
188187
require.NotNil(t, result)
189188
require.Len(t, result.Result.Violations, 1)
190-
assert.Contains(t, result.Result.Violations[0], "at least 2 components")
191-
// No structured_violations when policy returns plain strings
192-
assert.Empty(t, result.Result.StructuredViolations)
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")
193195
})
194196

195197
t.Run("sbom metadata component policy", func(t *testing.T) {
@@ -229,6 +231,6 @@ func TestEvaluateSimplifiedPolicies(t *testing.T) {
229231
require.NotNil(t, result)
230232
assert.False(t, result.Result.Skipped)
231233
assert.Len(t, result.Result.Violations, 1)
232-
assert.Contains(t, result.Result.Violations[0], "too few components")
234+
assert.Contains(t, string(result.Result.Violations[0]), "too few components")
233235
})
234236
}

0 commit comments

Comments
 (0)