mirror of
https://github.com/ossf/scorecard.git
synced 2024-09-11 08:55:27 +03:00
✨ Support Binary-Artifacts check again for local repos (#3415)
* invert workflow check and explain early exit. Signed-off-by: Spencer Schrock <sschrock@google.com> * make workflow run validation optional. Signed-off-by: Spencer Schrock <sschrock@google.com> * mark binary artifacts as local file friendly. Signed-off-by: Spencer Schrock <sschrock@google.com> * add test for gradle wrapper without workflow run support Signed-off-by: Spencer Schrock <sschrock@google.com> * fix policy tests and make their names more clear. Signed-off-by: Spencer Schrock <sschrock@google.com> --------- Signed-off-by: Spencer Schrock <sschrock@google.com>
This commit is contained in:
parent
475d975cc2
commit
eba10dffd5
@ -24,10 +24,11 @@ import (
|
||||
// CheckBinaryArtifacts is the exported name for Binary-Artifacts check.
|
||||
const CheckBinaryArtifacts string = "Binary-Artifacts"
|
||||
|
||||
//nolint
|
||||
//nolint:gochecknoinits
|
||||
func init() {
|
||||
supportedRequestTypes := []checker.RequestType{
|
||||
checker.CommitBased,
|
||||
checker.FileBased,
|
||||
}
|
||||
if err := registerCheck(CheckBinaryArtifacts, BinaryArtifacts, supportedRequestTypes); err != nil {
|
||||
// this should never happen
|
||||
|
@ -15,6 +15,7 @@
|
||||
package raw
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
@ -201,26 +202,35 @@ func gradleWrapperValidated(c clients.RepoClient) (bool, error) {
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("%w", err)
|
||||
}
|
||||
if gradleWrapperValidatingWorkflowFile != "" {
|
||||
// If validated, check that latest commit has a relevant successful run
|
||||
runs, err := c.ListSuccessfulWorkflowRuns(gradleWrapperValidatingWorkflowFile)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failure listing workflow runs: %w", err)
|
||||
}
|
||||
commits, err := c.ListCommits()
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failure listing commits: %w", err)
|
||||
}
|
||||
if len(commits) < 1 || len(runs) < 1 {
|
||||
// no matching files, validation failed
|
||||
if gradleWrapperValidatingWorkflowFile == "" {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// If validated, check that latest commit has a relevant successful run
|
||||
runs, err := c.ListSuccessfulWorkflowRuns(gradleWrapperValidatingWorkflowFile)
|
||||
if err != nil {
|
||||
// some clients, such as the local file client, don't support this feature
|
||||
// claim unvalidated, so that other parts of the check can still be used.
|
||||
if errors.Is(err, clients.ErrUnsupportedFeature) {
|
||||
return false, nil
|
||||
}
|
||||
for _, r := range runs {
|
||||
if *r.HeadSHA == commits[0].SHA {
|
||||
// Commit has corresponding successful run!
|
||||
return true, nil
|
||||
}
|
||||
return false, fmt.Errorf("failure listing workflow runs: %w", err)
|
||||
}
|
||||
commits, err := c.ListCommits()
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failure listing commits: %w", err)
|
||||
}
|
||||
if len(commits) < 1 || len(runs) < 1 {
|
||||
return false, nil
|
||||
}
|
||||
for _, r := range runs {
|
||||
if *r.HeadSHA == commits[0].SHA {
|
||||
// Commit has corresponding successful run!
|
||||
return true, nil
|
||||
}
|
||||
}
|
||||
|
||||
return false, nil
|
||||
}
|
||||
|
||||
|
@ -256,3 +256,35 @@ func TestBinaryArtifacts(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBinaryArtifacts_workflow_runs_unsupported(t *testing.T) {
|
||||
ctrl := gomock.NewController(t)
|
||||
mockRepoClient := mockrepo.NewMockRepoClient(ctrl)
|
||||
const jarFile = "gradle-wrapper.jar"
|
||||
const verifyWorkflow = ".github/workflows/verify.yaml"
|
||||
files := []string{jarFile, verifyWorkflow}
|
||||
mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(files, nil).AnyTimes()
|
||||
mockRepoClient.EXPECT().GetFileContent(jarFile).DoAndReturn(func(file string) ([]byte, error) {
|
||||
content, err := os.ReadFile("../testdata/binaryartifacts/jars/gradle-wrapper.jar")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("%w", err)
|
||||
}
|
||||
return content, nil
|
||||
}).AnyTimes()
|
||||
mockRepoClient.EXPECT().GetFileContent(verifyWorkflow).DoAndReturn(func(file string) ([]byte, error) {
|
||||
content, err := os.ReadFile("../testdata/binaryartifacts/workflows/verify.yaml")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("%w", err)
|
||||
}
|
||||
return content, nil
|
||||
}).AnyTimes()
|
||||
|
||||
mockRepoClient.EXPECT().ListSuccessfulWorkflowRuns(gomock.Any()).Return(nil, clients.ErrUnsupportedFeature).AnyTimes()
|
||||
got, err := BinaryArtifacts(mockRepoClient)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(got.Files) != 1 {
|
||||
t.Errorf("expected 1 file, got %d", len(got.Files))
|
||||
}
|
||||
}
|
||||
|
@ -217,7 +217,7 @@ func TestIsSupportedCheck(t *testing.T) {
|
||||
result := isSupportedCheck(checkName, requiredRequestTypes)
|
||||
|
||||
// Assert the result
|
||||
expectedResult := false
|
||||
expectedResult := true
|
||||
if result != expectedResult {
|
||||
t.Errorf("Unexpected result: got %v, want %v", result, expectedResult)
|
||||
}
|
||||
@ -308,49 +308,59 @@ func TestGetEnabled(t *testing.T) {
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
sp *ScorecardPolicy
|
||||
policyFile string
|
||||
argsChecks []string
|
||||
requiredRequestTypes []checker.RequestType
|
||||
expectedEnabledChecks int
|
||||
expectedError bool
|
||||
}{
|
||||
{
|
||||
name: "With ScorecardPolicy and argsChecks",
|
||||
sp: &ScorecardPolicy{},
|
||||
name: "checks limited to those specified by checks arg",
|
||||
argsChecks: []string{"Binary-Artifacts"},
|
||||
requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased},
|
||||
expectedEnabledChecks: 0,
|
||||
expectedError: true,
|
||||
},
|
||||
{
|
||||
name: "With ScorecardPolicy and unsupported check",
|
||||
sp: &ScorecardPolicy{},
|
||||
argsChecks: []string{"Binary-Artifacts", "UnsupportedCheck"},
|
||||
requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased},
|
||||
expectedEnabledChecks: 0,
|
||||
expectedError: true,
|
||||
},
|
||||
{
|
||||
name: "Without ScorecardPolicy and argsChecks",
|
||||
sp: nil,
|
||||
argsChecks: []string{},
|
||||
requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased},
|
||||
expectedEnabledChecks: 4, // All checks are enabled by default
|
||||
requiredRequestTypes: []checker.RequestType{checker.FileBased},
|
||||
expectedEnabledChecks: 1,
|
||||
expectedError: false,
|
||||
},
|
||||
{
|
||||
name: "With ScorecardPolicy and missing policy",
|
||||
sp: &ScorecardPolicy{},
|
||||
argsChecks: []string{"Binary-Artifacts"},
|
||||
name: "mix of supported and unsupported checks",
|
||||
argsChecks: []string{"Binary-Artifacts", "UnsupportedCheck"},
|
||||
requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased},
|
||||
expectedEnabledChecks: 0,
|
||||
expectedEnabledChecks: 1,
|
||||
expectedError: true,
|
||||
},
|
||||
{
|
||||
name: "request types limit enabled checks",
|
||||
argsChecks: []string{},
|
||||
requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased},
|
||||
expectedEnabledChecks: 5, // All checks which are FileBased and CommitBased
|
||||
expectedError: false,
|
||||
},
|
||||
{
|
||||
name: "all checks in policy file enabled",
|
||||
policyFile: "testdata/policy-ok.yaml",
|
||||
argsChecks: []string{},
|
||||
requiredRequestTypes: []checker.RequestType{},
|
||||
expectedEnabledChecks: 3,
|
||||
expectedError: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
enabledChecks, err := GetEnabled(tt.sp, tt.argsChecks, tt.requiredRequestTypes)
|
||||
var sp *ScorecardPolicy
|
||||
if tt.policyFile != "" {
|
||||
policyBytes, err := os.ReadFile(tt.policyFile)
|
||||
if err != nil {
|
||||
t.Fatalf("reading policy file: %v", err)
|
||||
}
|
||||
pol, err := parseFromYAML(policyBytes)
|
||||
if err != nil {
|
||||
t.Fatalf("parsing policy file: %v", err)
|
||||
}
|
||||
sp = pol
|
||||
}
|
||||
|
||||
enabledChecks, err := GetEnabled(sp, tt.argsChecks, tt.requiredRequestTypes)
|
||||
|
||||
if len(enabledChecks) != tt.expectedEnabledChecks {
|
||||
t.Errorf("Unexpected number of enabled checks: got %v, want %v", len(enabledChecks), tt.expectedEnabledChecks)
|
||||
|
Loading…
Reference in New Issue
Block a user