Skip to content

Commit e1590b7

Browse files
committed
fix(renderer): backfill skipped/passed counts for pre-existing attestations
Attestations signed before the skipped/passed counters were added to the predicate decode them as zero. When inline evaluations are available, recompute at describe time so DerivePolicyStatusSummary doesn't misclassify skipped-only runs as PASSED. Identified by cubic. Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
1 parent c1b5757 commit e1590b7

2 files changed

Lines changed: 77 additions & 2 deletions

File tree

pkg/attestation/renderer/chainloop/v02.go

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,22 @@ func (p *ProvenancePredicateV02) GetPolicyEvaluationsRef() *intoto.ResourceDescr
476476
}
477477

478478
func (p *ProvenancePredicateV02) GetPolicyEvaluationStatus() *PolicyEvaluationStatus {
479+
skipped, passed := p.PolicySkippedCount, p.PolicyPassedCount
480+
// Attestations signed before the skipped/passed counters were added to
481+
// the predicate decode them as zero. When the inline evaluations are
482+
// available, recompute to keep the canonical PolicyStatus derivation
483+
// correct for historic envelopes (e.g. skipped-only runs would otherwise
484+
// be misclassified as PASSED).
485+
if skipped == 0 && passed == 0 && p.PolicyEvaluationsCount > 0 {
486+
evals := p.PolicyEvaluations
487+
if len(evals) == 0 {
488+
evals = p.PolicyEvaluationsFallback
489+
}
490+
if len(evals) > 0 {
491+
skipped, passed = countSkippedAndPassed(evals)
492+
}
493+
}
494+
479495
return &PolicyEvaluationStatus{
480496
Strategy: p.PolicyCheckBlockingStrategy,
481497
Bypassed: p.PolicyBlockBypassEnabled,
@@ -484,9 +500,25 @@ func (p *ProvenancePredicateV02) GetPolicyEvaluationStatus() *PolicyEvaluationSt
484500
HasGatedViolations: p.PolicyHasGatedViolations,
485501
EvaluationsCount: p.PolicyEvaluationsCount,
486502
ViolationsCount: p.PolicyViolationsCount,
487-
SkippedCount: p.PolicySkippedCount,
488-
PassedCount: p.PolicyPassedCount,
503+
SkippedCount: skipped,
504+
PassedCount: passed,
505+
}
506+
}
507+
508+
func countSkippedAndPassed(evals map[string][]*PolicyEvaluation) (skipped, passed int) {
509+
for _, list := range evals {
510+
for _, ev := range list {
511+
switch {
512+
case len(ev.Violations) > 0:
513+
// violation-bearing evaluations are neither passed nor skipped
514+
case ev.Skipped:
515+
skipped++
516+
default:
517+
passed++
518+
}
519+
}
489520
}
521+
return
490522
}
491523

492524
// Translate a ResourceDescriptor to a NormalizedMaterial

pkg/attestation/renderer/chainloop/v02_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,49 @@ func TestPolicyEvaluationStatusCounts(t *testing.T) {
710710
assert.True(t, status.HasViolations)
711711
}
712712

713+
// Attestations predating the skipped/passed counters must still surface them
714+
// correctly at describe time — otherwise a run where every evaluation was
715+
// skipped would be misclassified as PASSED by DerivePolicyStatusSummary.
716+
func TestPolicyEvaluationStatusBackfill(t *testing.T) {
717+
evals := map[string][]*PolicyEvaluation{
718+
"material-a": {
719+
{Skipped: true},
720+
{Skipped: true},
721+
},
722+
"material-b": {
723+
{}, // passed
724+
{Violations: []*PolicyViolation{{Message: "boom"}}},
725+
},
726+
}
727+
p := &ProvenancePredicateV02{
728+
PolicyEvaluations: evals,
729+
PolicyEvaluationsCount: 4,
730+
PolicyViolationsCount: 1,
731+
PolicyHasViolations: true,
732+
}
733+
734+
status := p.GetPolicyEvaluationStatus()
735+
assert.Equal(t, 2, status.SkippedCount)
736+
assert.Equal(t, 1, status.PassedCount)
737+
}
738+
739+
// When the predicate already carries counters, the inline evaluations are
740+
// ignored — the stored values are authoritative for new attestations.
741+
func TestPolicyEvaluationStatusDoesNotOverrideStoredCounters(t *testing.T) {
742+
p := &ProvenancePredicateV02{
743+
PolicyEvaluations: map[string][]*PolicyEvaluation{
744+
"m": {{Skipped: true}},
745+
},
746+
PolicyEvaluationsCount: 3,
747+
PolicySkippedCount: 0,
748+
PolicyPassedCount: 3,
749+
}
750+
751+
status := p.GetPolicyEvaluationStatus()
752+
assert.Equal(t, 0, status.SkippedCount)
753+
assert.Equal(t, 3, status.PassedCount)
754+
}
755+
713756
func TestPolicyEvaluationsField(t *testing.T) {
714757
raw, err := os.ReadFile("testdata/attestation-pe-snake.json")
715758
require.NoError(t, err)

0 commit comments

Comments
 (0)