🐛 Signed-Releases: dont warn about signatures if provenance present (#…

…4024) * reduce number of findings to 1 per probe per release having different findings for different release artifacts isnt how the probe works and it makes the whole thing very noisy Signed-off-by: Spencer Schrock <sschrock@google.com> * dont log lack of signature if we have provenance reduce test warn counts for cases where there is provenance but no signature Signed-off-by: Spencer Schrock <sschrock@google.com> --------- Signed-off-by: Spencer Schrock <sschrock@google.com>
ossf · Apr 12, 2024 · d8b26d9 · d8b26d9
1 parent 21d53ce
commit d8b26d9
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 99 deletions.
diff --git a/checks/evaluation/signed_releases.go b/checks/evaluation/signed_releases.go
@@ -42,6 +42,10 @@ func SignedReleases(name string,
 		return checker.CreateRuntimeErrorResult(name, e)
 	}
 
+	// keep track of releases which have provenance so we don't log about signatures
+	// on our second pass through below
+	hasProvenance := make(map[string]bool)
+
 	// Debug all releases and check for OutcomeNotApplicable
 	// All probes have OutcomeNotApplicable in case the project has no
 	// releases. Therefore, check for any finding with OutcomeNotApplicable.
@@ -67,7 +71,9 @@ func SignedReleases(name string,
 			loggedReleases = append(loggedReleases, releaseName)
 		}
 
-		// Check if outcome is NotApplicable
+		if f.Probe == releasesHaveProvenance.Probe && f.Outcome == finding.OutcomeTrue {
+			hasProvenance[releaseName] = true
+		}
 	}
 
 	totalTrue := 0
@@ -100,6 +106,9 @@ func SignedReleases(name string,
 			}
 		case finding.OutcomeFalse:
 			logLevel = checker.DetailWarn
+			if f.Probe == releasesAreSigned.Probe && hasProvenance[releaseName] {
+				continue
+			}
 		default:
 			logLevel = checker.DetailDebug
 		}

diff --git a/checks/evaluation/signed_releases_test.go b/checks/evaluation/signed_releases_test.go
@@ -105,167 +105,85 @@ func TestSignedReleases(t *testing.T) {
 			result: scut.TestReturn{
 				Score:         checker.MaxResultScore,
 				NumberOfInfo:  1,
-				NumberOfWarn:  1,
+				NumberOfWarn:  0,
 				NumberOfDebug: 1,
 			},
 		},
 
 		{
-			name: "3 releases. One release has one signed, and one release has two provenance.",
+			name: "3 releases. One release has one signed, and one release has provenance.",
 			findings: []finding.Finding{
 				// Release 1:
-				//     Asset 1:
-				signedProbe(release0, asset0, finding.OutcomeFalse),
-				provenanceProbe(release0, asset0, finding.OutcomeFalse),
-				//     Asset 2:
 				signedProbe(release0, asset1, finding.OutcomeTrue),
-				provenanceProbe(release0, asset1, finding.OutcomeFalse),
+				provenanceProbe(release0, asset0, finding.OutcomeFalse),
 				// Release 2
-				//     Asset 1:
 				signedProbe(release1, asset0, finding.OutcomeFalse),
 				provenanceProbe(release1, asset0, finding.OutcomeFalse),
-				// Release 2
-				//     Asset 2:
-				signedProbe(release1, asset1, finding.OutcomeFalse),
-				provenanceProbe(release1, asset1, finding.OutcomeFalse),
-				// Release 2
-				//     Asset 3:
-				signedProbe(release1, asset2, finding.OutcomeFalse),
-				provenanceProbe(release1, asset2, finding.OutcomeFalse),
 				// Release 3
-				//     Asset 1:
 				signedProbe(release2, asset0, finding.OutcomeFalse),
-				provenanceProbe(release2, asset0, finding.OutcomeTrue),
-				//     Asset 2:
-				signedProbe(release2, asset1, finding.OutcomeFalse),
 				provenanceProbe(release2, asset1, finding.OutcomeTrue),
-				//     Asset 3:
-				signedProbe(release2, asset2, finding.OutcomeFalse),
-				provenanceProbe(release2, asset2, finding.OutcomeFalse),
 			},
 			result: scut.TestReturn{
 				Score:         6,
-				NumberOfInfo:  3,
-				NumberOfWarn:  13,
+				NumberOfInfo:  2,
+				NumberOfWarn:  3,
 				NumberOfDebug: 3,
 			},
 		},
 		{
 			name: "5 releases. Two releases have one signed each, and two releases have one provenance each.",
 			findings: []finding.Finding{
 				// Release 1:
-				// Release 1, Asset 1:
-				signedProbe(release0, asset0, finding.OutcomeFalse),
-				provenanceProbe(release0, asset0, finding.OutcomeFalse),
 				signedProbe(release0, asset1, finding.OutcomeTrue),
 				provenanceProbe(release0, asset1, finding.OutcomeFalse),
 				// Release 2:
-				// Release 2, Asset 1:
-				signedProbe(release1, asset1, finding.OutcomeTrue),
+				signedProbe(release1, asset0, finding.OutcomeTrue),
 				provenanceProbe(release1, asset0, finding.OutcomeFalse),
-				// Release 2, Asset 2:
-				signedProbe(release1, asset1, finding.OutcomeFalse),
-				provenanceProbe(release1, asset1, finding.OutcomeFalse),
-				// Release 2, Asset 3:
-				signedProbe(release1, asset2, finding.OutcomeFalse),
-				provenanceProbe(release1, asset2, finding.OutcomeFalse),
-				// Release 3, Asset 1:
+				// Release 3:
 				signedProbe(release2, asset0, finding.OutcomeFalse),
 				provenanceProbe(release2, asset0, finding.OutcomeTrue),
-				// Release 3, Asset 2:
-				signedProbe(release2, asset1, finding.OutcomeFalse),
-				provenanceProbe(release2, asset1, finding.OutcomeFalse),
-				// Release 3, Asset 3:
-				signedProbe(release2, asset2, finding.OutcomeFalse),
-				provenanceProbe(release2, asset2, finding.OutcomeFalse),
 				// Release 4, Asset 1:
 				signedProbe(release3, asset0, finding.OutcomeFalse),
 				provenanceProbe(release3, asset0, finding.OutcomeTrue),
-				// Release 4, Asset 2:
-				signedProbe(release3, asset1, finding.OutcomeFalse),
-				provenanceProbe(release3, asset1, finding.OutcomeFalse),
-				// Release 4, Asset 3:
-				signedProbe(release3, asset2, finding.OutcomeFalse),
-				provenanceProbe(release3, asset2, finding.OutcomeFalse),
 				// Release 5, Asset 1:
 				signedProbe(release4, asset0, finding.OutcomeFalse),
 				provenanceProbe(release4, asset0, finding.OutcomeFalse),
-				// Release 5, Asset 2:
-				signedProbe(release4, asset1, finding.OutcomeFalse),
-				provenanceProbe(release4, asset1, finding.OutcomeFalse),
-				// Release 5, Asset 3:
-				signedProbe(release4, asset2, finding.OutcomeFalse),
-				provenanceProbe(release4, asset2, finding.OutcomeFalse),
-				// Release 5, Asset 4:
-				signedProbe(release4, asset3, finding.OutcomeFalse),
-				provenanceProbe(release4, asset3, finding.OutcomeFalse),
 			},
 			result: scut.TestReturn{
 				Score:         7,
 				NumberOfInfo:  4,
-				NumberOfWarn:  26,
+				NumberOfWarn:  4,
 				NumberOfDebug: 5,
 			},
 		},
 		{
 			name: "5 releases. All have one signed artifact.",
 			findings: []finding.Finding{
 				// Release 1:
-				// Release 1, Asset 1:
-				signedProbe(release0, asset0, finding.OutcomeFalse),
-				provenanceProbe(release0, asset0, finding.OutcomeFalse),
 				signedProbe(release0, asset1, finding.OutcomeTrue),
 				provenanceProbe(release0, asset1, finding.OutcomeFalse),
 				// Release 2:
-				// Release 2, Asset 1:
 				signedProbe(release1, asset0, finding.OutcomeTrue),
 				provenanceProbe(release1, asset0, finding.OutcomeFalse),
-				// Release 2, Asset 2:
-				signedProbe(release1, asset1, finding.OutcomeFalse),
-				provenanceProbe(release1, asset1, finding.OutcomeFalse),
-				// Release 2, Asset 3:
-				signedProbe(release1, asset2, finding.OutcomeFalse),
-				provenanceProbe(release1, asset2, finding.OutcomeFalse),
-				// Release 3, Asset 1:
+				// Release 3:
 				signedProbe(release2, asset0, finding.OutcomeTrue),
-				provenanceProbe(release2, asset0, finding.OutcomeTrue),
-				// Release 3, Asset 2:
-				signedProbe(release2, asset1, finding.OutcomeFalse),
-				provenanceProbe(release2, asset1, finding.OutcomeFalse),
-				// Release 3, Asset 3:
-				signedProbe(release2, asset2, finding.OutcomeFalse),
-				provenanceProbe(release2, asset2, finding.OutcomeFalse),
-				// Release 4, Asset 1:
+				provenanceProbe(release2, asset0, finding.OutcomeFalse),
+				// Release 4:
 				signedProbe(release3, asset0, finding.OutcomeTrue),
-				provenanceProbe(release3, asset0, finding.OutcomeTrue),
-				// Release 4, Asset 2:
-				signedProbe(release3, asset1, finding.OutcomeFalse),
-				provenanceProbe(release3, asset1, finding.OutcomeFalse),
-				// Release 4, Asset 3:
-				signedProbe(release3, asset2, finding.OutcomeFalse),
-				provenanceProbe(release3, asset2, finding.OutcomeFalse),
-				// Release 5, Asset 1:
+				provenanceProbe(release3, asset0, finding.OutcomeFalse),
+				// Release 5:
 				signedProbe(release4, asset0, finding.OutcomeTrue),
 				provenanceProbe(release4, asset0, finding.OutcomeFalse),
-				// Release 5, Asset 2:
-				signedProbe(release4, asset1, finding.OutcomeFalse),
-				provenanceProbe(release4, asset1, finding.OutcomeFalse),
-				// Release 5, Asset 3:
-				signedProbe(release4, asset2, finding.OutcomeFalse),
-				provenanceProbe(release4, asset2, finding.OutcomeFalse),
-				// Release 5, Asset 4:
-				signedProbe(release4, asset3, finding.OutcomeFalse),
-				provenanceProbe(release4, asset3, finding.OutcomeFalse),
 			},
 			result: scut.TestReturn{
 				Score:         8,
-				NumberOfInfo:  7,
-				NumberOfWarn:  23,
+				NumberOfInfo:  5,
+				NumberOfWarn:  5,
 				NumberOfDebug: 5,
 			},
 		},
 		{
-			name: "too many releases (6 when lookback is 5)",
+			name: "too many releases is an error (6 when lookback is 5)",
 			findings: []finding.Finding{
 				// Release 1:
 				// Release 1, Asset 1: