Skip to content

Commit ec1f870

Browse files
committed
refactor(policies): simplify suppress renderer per /simplify review
- Drop renderEvaluation's (active, suppressed int) returns and count from p.Violations in groupEvaluations. Single-purpose return signature. - assessmentScopes: replace map dedup with slices.Contains (typical N=1-2). - Trim narrative/spec comments on the new Suppressed/SuppressedCount fields and on groupEvaluations/renderEvaluation — keep only the WHY (gate-counter invariance across modes); proto field names speak for themselves. - msg += idiom in policiesTable. No behavior change; all 2408 unit tests still pass. Assisted-by: Claude Code Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev> Chainloop-Trace-Sessions: 5bd2a917-fb7b-400c-9772-60ba6af6c9af, b66717f5-626e-4c20-8d33-59c129b5885d
1 parent ad9c230 commit ec1f870

13 files changed

Lines changed: 68 additions & 126 deletions

app/cli/cmd/workflow_workflow_run_describe.go

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"io"
2323
"os"
24+
"slices"
2425
"strings"
2526
"time"
2627

@@ -254,9 +255,6 @@ func policiesTable(evs []*action.PolicyEvaluation, mt table.Writer, debugMode bo
254255
for _, ev := range evs {
255256
msg := ""
256257

257-
// Partition: active violations count toward the gate; suppressed
258-
// entries are kept in the CAS bundle for audit and shown separately
259-
// so operators can see policy decisions without losing context.
260258
var active []string
261259
var suppressed []*action.PolicyViolation
262260
for _, v := range ev.Violations {
@@ -297,7 +295,7 @@ func policiesTable(evs []*action.PolicyEvaluation, mt table.Writer, debugMode bo
297295
}
298296

299297
if s := renderSuppressed(suppressed); s != "" {
300-
msg = msg + "\n" + s
298+
msg += "\n" + s
301299
}
302300

303301
name := ev.Name
@@ -308,11 +306,6 @@ func policiesTable(evs []*action.PolicyEvaluation, mt table.Writer, debugMode bo
308306
}
309307
}
310308

311-
// renderSuppressed formats a "Suppressed (N)" sub-section listing entries the
312-
// policy excluded from the gate. Each line shows the violation message plus,
313-
// when a structured finding with an assessment is available, the
314-
// precedence-resolved status and scope (e.g. "NOT_AFFECTED, PROJECT") so
315-
// operators can audit suppression decisions without downloading the bundle.
316309
func renderSuppressed(suppressed []*action.PolicyViolation) string {
317310
if len(suppressed) == 0 {
318311
return ""
@@ -330,10 +323,6 @@ func renderSuppressed(suppressed []*action.PolicyViolation) string {
330323
return header + "\n" + dim.Sprint(strings.Join(lines, "\n"))
331324
}
332325

333-
// suppressedAssessment extracts the effective assessment status and scope
334-
// from whichever structured finding is attached to the violation, if any.
335-
// Returns the empty string when no assessment is available (unstructured
336-
// policy, or finding without an assessment annotation).
337326
func suppressedAssessment(v *action.PolicyViolation) string {
338327
switch {
339328
case v.Vulnerability != nil && v.Vulnerability.Assessment != nil:
@@ -359,16 +348,11 @@ func prettyAssessment(status string, scopes []string) string {
359348

360349
func assessmentScopes(in []*attv1.PolicyAssessment) []string {
361350
scopes := make([]string, 0, len(in))
362-
seen := make(map[string]struct{}, len(in))
363351
for _, a := range in {
364352
scope := strings.TrimPrefix(a.GetScope(), "ASSESSMENT_SCOPE_")
365-
if scope == "" {
353+
if scope == "" || slices.Contains(scopes, scope) {
366354
continue
367355
}
368-
if _, dup := seen[scope]; dup {
369-
continue
370-
}
371-
seen[scope] = struct{}{}
372356
scopes = append(scopes, scope)
373357
}
374358
return scopes

app/cli/pkg/action/workflow_run_describe.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,15 +114,11 @@ type PolicyEvaluation struct {
114114
}
115115

116116
type PolicyViolation struct {
117-
Subject string `json:"subject"`
118-
Message string `json:"message"`
119-
// Suppress, when true, excludes this entry from the gate count while
120-
// keeping it in the CAS-stored bundle (audit trail preserved). Set by
121-
// the policy author via the rego "suppress" key.
122-
Suppress bool `json:"suppress,omitempty"`
123-
// Structured finding data from the policy, present when the policy
124-
// declared finding_type. Mirrors the oneof on the wire — exactly one
125-
// pointer is set per violation, or none for unstructured policies.
117+
Subject string `json:"subject"`
118+
Message string `json:"message"`
119+
Suppress bool `json:"suppress,omitempty"`
120+
// Mirrors the oneof on the wire — exactly one pointer is set per
121+
// violation, or none for unstructured policies.
126122
Vulnerability *attv1.PolicyVulnerabilityFinding `json:"vulnerability,omitempty"`
127123
Sast *attv1.PolicySASTFinding `json:"sast,omitempty"`
128124
LicenseViolation *attv1.PolicyLicenseViolationFinding `json:"license_violation,omitempty"`

app/controlplane/api/controlplane/v1/response_messages.pb.go

Lines changed: 6 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/controlplane/api/controlplane/v1/response_messages.proto

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,8 @@ message PolicyStatusSummary {
148148
// the contract using the ENFORCED blocking strategy. Independent of status:
149149
// a PASSED run can still have has_gates=true.
150150
bool has_gates = 6;
151-
// Total number of violations across all evaluations that were suppressed
152-
// by the policy. Suppressed entries are excluded from violated/has_gates
153-
// accounting but kept in the CAS audit trail. UI can show a "Suppressed (N)"
154-
// badge without partitioning the violations client-side.
151+
// Number of suppressed violations across all evaluations. Excluded
152+
// from violated; kept in the CAS audit trail.
155153
int32 suppressed = 7;
156154
}
157155

@@ -260,14 +258,11 @@ message PolicyEvaluation {
260258
message PolicyViolation {
261259
string subject = 1;
262260
string message = 2;
263-
// Whether this violation was excluded from the policy gate. Suppressed
264-
// entries are part of the audit trail but did not contribute to
265-
// PolicyEvaluationStatus.summary.violated or has_gated_violations.
261+
// Whether this violation was excluded from the policy gate (still kept
262+
// in the CAS audit trail).
266263
bool suppress = 3;
267-
// Structured finding data from the policy. Populated when the policy
268-
// declared finding_type and emitted a structured rather than string
269-
// violation. Mirrors the oneof on
270-
// attestation.v1.PolicyEvaluation.Violation in the CAS-stored bundle.
264+
// Structured finding mirroring attestation.v1.PolicyEvaluation.Violation.
265+
// Populated when the policy declares finding_type.
271266
oneof finding {
272267
attestation.v1.PolicyVulnerabilityFinding vulnerability = 4;
273268
attestation.v1.PolicySASTFinding sast = 5;

app/controlplane/api/gen/frontend/controlplane/v1/response_messages.ts

Lines changed: 4 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyStatusSummary.jsonschema.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyStatusSummary.schema.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyViolation.jsonschema.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/controlplane/api/gen/jsonschema/controlplane.v1.PolicyViolation.schema.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/attestation/renderer/chainloop/chainloop.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,7 @@ type PolicyEvaluationStatus struct {
6868
SkippedCount int
6969
// Number of evaluations that passed: neither skipped nor with violations.
7070
PassedCount int
71-
// Total number of violations across all evaluations that were suppressed
72-
// by the policy. Excluded from ViolationsCount / HasViolations accounting
73-
// but surfaced separately so the UI can render a "Suppressed (N)" badge.
71+
// Number of suppressed violations across all evaluations.
7472
SuppressedCount int
7573
}
7674

0 commit comments

Comments
 (0)