feat(policies): add suppressed_findings to policy evaluation result#3093
feat(policies): add suppressed_findings to policy evaluation result#3093migmartri wants to merge 3 commits intochainloop-dev:mainfrom
Conversation
Adds a new top-level key on the Rego/WASM policy result alongside findings and violations. Items use the same schema as findings, with two new optional correlation fields on the finding proto types: chainloop_finding_id and chainloop_assessment_ids. Refs: chainloop-dev#3091 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
There was a problem hiding this comment.
1 issue found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/policies/engine/rego/testfiles/suppressed_findings.rego">
<violation number="1" location="pkg/policies/engine/rego/testfiles/suppressed_findings.rego:53">
P2: Handle the optional correlation fields without direct dereferences; otherwise omitted fields make the suppressed finding undefined and it disappears.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
…d_findings fixture Use object.get with zero-value defaults for chainloop_finding_id and chainloop_assessment_ids. Direct dereferences make the entire entry undefined when either field is missing, which silently drops the suppressed finding. Identified by cubic. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
|
|
||
| // Parse the optional "suppressed_findings" array. Each entry must be a | ||
| // structured finding object using the same schema as `findings`. | ||
| if suppressedRaw, ok := ruleResult["suppressed_findings"].([]any); ok { |
There was a problem hiding this comment.
superssed findinfgs might be added even if ew don't have findings
There was a problem hiding this comment.
Right — fixed in 0bbe6de. Reworked the model so suppressed_findings are disjoint from findings (a vulnerability is either a finding or a suppressed finding, never both) and parsing now happens independently of the findings branch, so a policy can emit suppressed_findings without any findings. Also added a new suppressed_findings repeated field on the PolicyEvaluation proto so they survive into the CAS bundle.
…nd surface to CAS Reworks the model so suppressed_findings are findings the policy chose not to count as gating violations: they are disjoint from `findings` and may appear even when `findings` is empty. - Parse suppressed_findings independently of findings in both Rego and WASM engines. - Plumb engine.EvaluationResult.SuppressedFindings through engineEvaluationsToAPIViolations and into a new PolicyEvaluation.suppressed_findings repeated field, so they reach the CAS-stored PolicyEvaluationBundle. - Update fixture and tests to reflect the disjoint invariant. Refs: chainloop-dev#3091 Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/policies/engine/rego/rego_test.go">
<violation number="1" location="pkg/policies/engine/rego/rego_test.go:752">
P1: The test now enforces that `suppressed_findings` are disjoint from `findings`, which contradicts the feature contract that suppressed findings should also be present in findings.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic.
| for _, sv := range result.SuppressedFindings { | ||
| require.NotNil(t, sv.RawFinding) | ||
| id, _ := sv.RawFinding["external_id"].(string) | ||
| assert.False(t, findingIDs[id], "suppressed finding %q must NOT also appear in findings", id) |
There was a problem hiding this comment.
P1: The test now enforces that suppressed_findings are disjoint from findings, which contradicts the feature contract that suppressed findings should also be present in findings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At pkg/policies/engine/rego/rego_test.go, line 752:
<comment>The test now enforces that `suppressed_findings` are disjoint from `findings`, which contradicts the feature contract that suppressed findings should also be present in findings.</comment>
<file context>
@@ -741,7 +749,7 @@ func TestRego_SuppressedFindings(t *testing.T) {
require.NotNil(t, sv.RawFinding)
id, _ := sv.RawFinding["external_id"].(string)
- assert.True(t, findingIDs[id], "suppressed finding %q must also appear in findings", id)
+ assert.False(t, findingIDs[id], "suppressed finding %q must NOT also appear in findings", id)
}
</file context>
Tip: Review your code locally with the cubic CLI to iterate faster.
Summary
suppressed_findingskey on the policy evaluation result, alongside the existingfindingsandviolations.suppressed_findingsuse the same Rego object schema as items infindings(e.g.PolicyVulnerabilityFinding), and the invariant is that every entry insuppressed_findingsalso appears infindings.PolicyVulnerabilityFinding,PolicySASTFinding,PolicyLicenseViolationFinding):chainloop_finding_idandchainloop_assessment_ids, used by policies to reference the platform-side finding and the assessments that drove the suppression.EvaluationResult.SuppressedFindings.Closes #3091
This change does not yet wire
suppressed_findingsinto the ingestion pipeline; downstream consumers can ignore it until follow-up work lands.Assisted-by: Claude Code