diff --git a/probes/releasesAreSigned/impl.go b/probes/releasesAreSigned/impl.go index 4f86c96c3a4..217f50af633 100644 --- a/probes/releasesAreSigned/impl.go +++ b/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++ diff --git a/probes/releasesAreSigned/impl_test.go b/probes/releasesAreSigned/impl_test.go index 525ba71e6b8..ac2057876e0 100644 --- a/probes/releasesAreSigned/impl_test.go +++ b/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{}, + } +} diff --git a/probes/releasesHaveProvenance/impl.go b/probes/releasesHaveProvenance/impl.go index 74fdc27199f..8a87696ff0c 100644 --- a/probes/releasesHaveProvenance/impl.go +++ b/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 { diff --git a/probes/releasesHaveProvenance/impl_test.go b/probes/releasesHaveProvenance/impl_test.go index 5b94964016e..d5dbd9d90af 100644 --- a/probes/releasesHaveProvenance/impl_test.go +++ b/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