diff --git a/checks/evaluation/signed_releases.go b/checks/evaluation/signed_releases.go index 35c705622ae..97b07b51ac8 100644 --- a/checks/evaluation/signed_releases.go +++ b/checks/evaluation/signed_releases.go @@ -106,8 +106,11 @@ func SignedReleases(name string, totalReleases := len(uniqueReleaseTags) + // TODO, the evaluation code should be the one limiting to 5, not assuming the probes have done it already + // however there are some ordering issues to consider, so going with the easy way for now if totalReleases > 5 { - totalReleases = 5 + err := sce.CreateInternal(sce.ErrScorecardInternal, "too many releases, please report this") + return checker.CreateRuntimeErrorResult(name, err) } if totalReleases == 0 { // This should not happen in production, but it is useful to have diff --git a/checks/evaluation/signed_releases_test.go b/checks/evaluation/signed_releases_test.go index cfbff61771f..478154497c0 100644 --- a/checks/evaluation/signed_releases_test.go +++ b/checks/evaluation/signed_releases_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/ossf/scorecard/v4/checker" + sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/finding" "github.com/ossf/scorecard/v4/probes/releasesAreSigned" "github.com/ossf/scorecard/v4/probes/releasesHaveProvenance" @@ -31,6 +32,7 @@ const ( release2 = 2 release3 = 3 release4 = 4 + release5 = 5 ) const ( @@ -262,6 +264,37 @@ func TestSignedReleases(t *testing.T) { NumberOfDebug: 5, }, }, + { + name: "too many releases (6 when lookback is 5)", + findings: []finding.Finding{ + // Release 1: + // Release 1, Asset 1: + signedProbe(release0, asset0, finding.OutcomePositive), + provenanceProbe(release0, asset0, finding.OutcomePositive), + // Release 2: + // Release 2, Asset 1: + signedProbe(release1, asset0, finding.OutcomePositive), + provenanceProbe(release1, asset0, finding.OutcomePositive), + // Release 3, Asset 1: + signedProbe(release2, asset0, finding.OutcomePositive), + provenanceProbe(release2, asset0, finding.OutcomePositive), + // Release 4, Asset 1: + signedProbe(release3, asset0, finding.OutcomePositive), + provenanceProbe(release3, asset0, finding.OutcomePositive), + // Release 5, Asset 1: + signedProbe(release4, asset0, finding.OutcomePositive), + provenanceProbe(release4, asset0, finding.OutcomePositive), + // Release 6, Asset 1: + signedProbe(release5, asset0, finding.OutcomePositive), + provenanceProbe(release5, asset0, finding.OutcomePositive), + }, + result: scut.TestReturn{ + Score: checker.InconclusiveResultScore, + Error: sce.ErrScorecardInternal, + NumberOfInfo: 12, // 2 (signed + provenance) for each release + NumberOfDebug: 6, // 1 for each release + }, + }, } for _, tt := range tests { diff --git a/probes/releasesHaveProvenance/impl.go b/probes/releasesHaveProvenance/impl.go index 02ba874e091..1433ab1f761 100644 --- a/probes/releasesHaveProvenance/impl.go +++ b/probes/releasesHaveProvenance/impl.go @@ -45,6 +45,7 @@ const ( var provenanceExtensions = []string{".intoto.jsonl"} +//nolint:gocognit // bug hotfix func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { if raw == nil { return nil, "", fmt.Errorf("%w: raw", uerror.ErrNil) @@ -61,6 +62,9 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { 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 bc8e148c590..1667b7f2c62 100644 --- a/probes/releasesHaveProvenance/impl_test.go +++ b/probes/releasesHaveProvenance/impl_test.go @@ -152,6 +152,70 @@ func Test_Run(t *testing.T) { finding.OutcomeNegative, }, }, + { + name: "enforece lookback limit of 5 releases", + raw: &checker.RawResults{ + SignedReleasesResults: checker.SignedReleasesData{ + Releases: []clients.Release{ + { + 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{ + {Name: "binary.tar.gz"}, + {Name: "binary.tar.gz.sig"}, + {Name: "binary.tar.gz.intoto.jsonl"}, + }, + }, + { + 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.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + finding.OutcomePositive, + }, + }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below