Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🐛 ensure Signed-Releases only scores 5 releases #3768

Merged
merged 2 commits into from
Jan 3, 2024
Merged

🐛 ensure Signed-Releases only scores 5 releases #3768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d0ac2b
limit provenance probe to 5 releases and check in evaluation code too
spencerschrock Jan 3, 2024
7aed0fc
add tests
spencerschrock Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion checks/evaluation/signed_releases.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 33 additions & 0 deletions checks/evaluation/signed_releases_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -31,6 +32,7 @@ const (
release2 = 2
release3 = 3
release4 = 4
release5 = 5
)

const (
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions probes/releasesHaveProvenance/impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
64 changes: 64 additions & 0 deletions probes/releasesHaveProvenance/impl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down