🐛 fix signed-releases lookback limit precedence (#4060)

* switch signed-releases lookback limit precedence

if the 6th release had no assets, the lookback limit exit condition was
being skipped. This led to scenarios where too many releases were being
considered by the Signed-Releases check.

#4059

Signed-off-by: Spencer Schrock <sschrock@google.com>

* make exit condition stronger

any release after the lookback should be skipped

Signed-off-by: Spencer Schrock <sschrock@google.com>

---------

Signed-off-by: Spencer Schrock <sschrock@google.com>

probes/releasesAreSigned/impl.go

@@ -53,12 +53,12 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 
 	totalReleases := 0
 	for releaseIndex, release := range releases {
-		if len(release.Assets) == 0 {
-			continue
+		if releaseIndex >= releaseLookBack {
+			break
 		}
 
-		if releaseIndex == releaseLookBack {
-			break
+		if len(release.Assets) == 0 {
+			continue
 		}
 
 		totalReleases++

probes/releasesAreSigned/impl_test.go

@@ -189,6 +189,30 @@ func Test_Run(t *testing.T) {
 				finding.OutcomeTrue,
 			},
 		},
+		{
+			// https://github.com/ossf/scorecard/issues/4059
+			name: "lookback cutoff not skipped if 6th release has no assets",
+			raw: &checker.RawResults{
+				SignedReleasesResults: checker.SignedReleasesData{
+					Releases: []clients.Release{
+						release("v0.8.5"),
+						release("v0.8.4"),
+						release("v0.8.3"),
+						release("v0.8.2"),
+						release("v0.8.1"),
+						releaseNoAssets("v0.8.0"),
+						release("v0.7.0"),
+					},
+				},
+			},
+			outcomes: []finding.Outcome{
+				finding.OutcomeTrue,
+				finding.OutcomeTrue,
+				finding.OutcomeTrue,
+				finding.OutcomeTrue,
+				finding.OutcomeTrue,
+			},
+		},
 	}
 	for _, tt := range tests {
 		tt := tt // Re-initializing variable so it is not changed while executing the closure below
@@ -251,3 +275,12 @@ func release(version string) clients.Release {
 		},
 	}
 }
+
+func releaseNoAssets(version string) clients.Release {
+	return clients.Release{
+		TagName:         version,
+		URL:             fmt.Sprintf("https://github.com/test/test_artifact/releases/tag/%s", version),
+		TargetCommitish: "master",
+		Assets:          []clients.ReleaseAsset{},
+	}
+}

probes/releasesHaveProvenance/impl.go

@@ -56,12 +56,12 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) {
 
 	for i := range releases {
 		release := releases[i]
+		if i >= releaseLookBack {
+			break
+		}
 		if len(release.Assets) == 0 {
 			continue
 		}
-		if i == releaseLookBack {
-			break
-		}
 		totalReleases++
 		hasProvenance := false
 		for j := range release.Assets {

probes/releasesHaveProvenance/impl_test.go

@@ -217,6 +217,75 @@ func Test_Run(t *testing.T) {
 				finding.OutcomeTrue,
 			},
 		},
+		{
+			// https://github.com/ossf/scorecard/issues/4059
+			name: "lookback cutoff not skipped if 6th release has no assets",
+			raw: &checker.RawResults{
+				SignedReleasesResults: checker.SignedReleasesData{
+					Releases: []clients.Release{
+						{
+							TagName: "v7.0",
+							Assets: []clients.ReleaseAsset{
+								{Name: "binary.tar.gz"},
+								{Name: "binary.tar.gz.sig"},
+								{Name: "binary.tar.gz.intoto.jsonl"},
+							},
+						},
+						{
+							TagName: "v6.0",
+							Assets: []clients.ReleaseAsset{
+								{Name: "binary.tar.gz"},
+								{Name: "binary.tar.gz.sig"},
+								{Name: "binary.tar.gz.intoto.jsonl"},
+							},
+						},
+						{
+							TagName: "v5.0",
+							Assets: []clients.ReleaseAsset{
+								{Name: "binary.tar.gz"},
+								{Name: "binary.tar.gz.sig"},
+								{Name: "binary.tar.gz.intoto.jsonl"},
+							},
+						},
+						{
+							TagName: "v4.0",
+							Assets: []clients.ReleaseAsset{
+								{Name: "binary.tar.gz"},
+								{Name: "binary.tar.gz.sig"},
+								{Name: "binary.tar.gz.intoto.jsonl"},
+							},
+						},
+						{
+							TagName: "v3.0",
+							Assets: []clients.ReleaseAsset{
+								{Name: "binary.tar.gz"},
+								{Name: "binary.tar.gz.sig"},
+								{Name: "binary.tar.gz.intoto.jsonl"},
+							},
+						},
+						{
+							TagName: "v2.0",
+							Assets:  []clients.ReleaseAsset{},
+						},
+						{
+							TagName: "v1.0",
+							Assets: []clients.ReleaseAsset{
+								{Name: "binary.tar.gz"},
+								{Name: "binary.tar.gz.sig"},
+								{Name: "binary.tar.gz.intoto.jsonl"},
+							},
+						},
+					},
+				},
+			},
+			outcomes: []finding.Outcome{
+				finding.OutcomeTrue,
+				finding.OutcomeTrue,
+				finding.OutcomeTrue,
+				finding.OutcomeTrue,
+				finding.OutcomeTrue,
+			},
+		},
 	}
 	for _, tt := range tests {
 		tt := tt // Re-initializing variable so it is not changed while executing the closure below