Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions app/cli/internal/policydevel/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down Expand Up @@ -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
Expand Down
30 changes: 13 additions & 17 deletions app/cli/internal/policydevel/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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) {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 14 additions & 4 deletions pkg/attestation/crafter/api/attestation/v1/crafting_state.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
42 changes: 28 additions & 14 deletions pkg/policies/engine/rego/rego.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Comment thread
migmartri marked this conversation as resolved.
// 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.
}
}

Expand Down
46 changes: 24 additions & 22 deletions pkg/policies/engine/rego/rego_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Loading
Loading