diff --git a/checks/binary_artifact.go b/checks/binary_artifact.go index bb0312d458d..636a8f2dfce 100644 --- a/checks/binary_artifact.go +++ b/checks/binary_artifact.go @@ -27,7 +27,6 @@ const CheckBinaryArtifacts string = "Binary-Artifacts" //nolint func init() { supportedRequestTypes := []checker.RequestType{ - checker.FileBased, checker.CommitBased, } if err := registerCheck(CheckBinaryArtifacts, BinaryArtifacts, supportedRequestTypes); err != nil { diff --git a/checks/binary_artifact_test.go b/checks/binary_artifact_test.go index 2d129b7c753..d3697945115 100644 --- a/checks/binary_artifact_test.go +++ b/checks/binary_artifact_test.go @@ -41,7 +41,7 @@ func TestBinaryArtifacts(t *testing.T) { inputFolder: "testdata/binaryartifacts/jars", err: nil, expected: checker.CheckResult{ - Score: 9, + Score: 8, }, }, { diff --git a/checks/raw/binary_artifact.go b/checks/raw/binary_artifact.go index e0b17a762a5..1eb21cb880e 100644 --- a/checks/raw/binary_artifact.go +++ b/checks/raw/binary_artifact.go @@ -17,11 +17,14 @@ package raw import ( "fmt" "path/filepath" + "regexp" "strings" "unicode" + semver "github.com/Masterminds/semver/v3" "github.com/h2non/filetype" "github.com/h2non/filetype/types" + "github.com/rhysd/actionlint" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checks/fileparser" @@ -29,6 +32,20 @@ import ( sce "github.com/ossf/scorecard/v4/errors" ) +var ( + gradleWrapperValidationActionRegex = regexp.MustCompile(`^gradle\/wrapper-validation-action@v?(.+)$`) + gradleWrapperValidationActionVersionConstraint = mustParseConstraint(`>= 1.0.0`) +) + +// mustParseConstraint attempts parse of semver constraint, panics if fail. +func mustParseConstraint(c string) *semver.Constraints { + if c, err := semver.NewConstraint(c); err != nil { + panic(fmt.Errorf("failed to parse constraint: %w", err)) + } else { + return c + } +} + // BinaryArtifacts retrieves the raw data for the Binary-Artifacts check. func BinaryArtifacts(c clients.RepoClient) (checker.BinaryArtifactData, error) { files := []checker.File{} @@ -39,11 +56,45 @@ func BinaryArtifacts(c clients.RepoClient) (checker.BinaryArtifactData, error) { if err != nil { return checker.BinaryArtifactData{}, fmt.Errorf("%w", err) } + // Ignore validated gradle-wrapper.jar files if present + files, err = excludeValidatedGradleWrappers(c, files) + if err != nil { + return checker.BinaryArtifactData{}, fmt.Errorf("%w", err) + } // No error, return the files. return checker.BinaryArtifactData{Files: files}, nil } +// excludeValidatedGradleWrappers returns the subset of files not confirmed +// to be Action-validated gradle-wrapper.jar files. +func excludeValidatedGradleWrappers(c clients.RepoClient, files []checker.File) ([]checker.File, error) { + // Check if gradle-wrapper.jar present + if !fileExists(files, "gradle-wrapper.jar") { + return files, nil + } + // Gradle wrapper JARs present, so check that they are validated + ok, err := gradleWrapperValidated(c) + if err != nil { + return files, fmt.Errorf( + "failure checking for Gradle wrapper validating Action: %w", err) + } + if !ok { + // Gradle Wrappers not validated + return files, nil + } + // It has been confirmed that latest commit has validated JARs! + // Remove Gradle wrapper JARs from files. + filterFiles := []checker.File{} + for _, f := range files { + if filepath.Base(f.Path) != "gradle-wrapper.jar" { + filterFiles = append(filterFiles, f) + } + } + files = filterFiles + return files, nil +} + var checkBinaryFileContent fileparser.DoWhileTrueOnFileContent = func(path string, content []byte, args ...interface{}, ) (bool, error) { @@ -127,3 +178,91 @@ func isText(content []byte) bool { } return true } + +// gradleWrapperValidated checks for the gradle-wrapper-verify action being +// used in a non-failing workflow on the latest commit. +func gradleWrapperValidated(c clients.RepoClient) (bool, error) { + gradleWrapperValidatingWorkflowFile := "" + err := fileparser.OnMatchingFileContentDo(c, fileparser.PathMatcher{ + Pattern: ".github/workflows/*", + CaseSensitive: false, + }, checkWorkflowValidatesGradleWrapper, &gradleWrapperValidatingWorkflowFile) + 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 { + return false, nil + } + for _, r := range runs { + if *r.HeadSHA == commits[0].SHA { + // Commit has corresponding successful run! + return true, nil + } + } + } + return false, nil +} + +// checkWorkflowValidatesGradleWrapper checks that the current workflow file +// is indeed using the gradle/wrapper-validation-action action, else continues. +func checkWorkflowValidatesGradleWrapper(path string, content []byte, args ...interface{}) (bool, error) { + validatingWorkflowFile, ok := args[0].(*string) + if !ok { + return false, fmt.Errorf("checkWorkflowValidatesGradleWrapper expects arg[0] of type *string: %w", errInvalidArgType) + } + + action, errs := actionlint.Parse(content) + if len(errs) > 0 { + return true, errs[0] + } + + for _, j := range action.Jobs { + for _, s := range j.Steps { + ea, ok := s.Exec.(*actionlint.ExecAction) + if !ok { + continue + } + if ea.Uses == nil { + continue + } + sms := gradleWrapperValidationActionRegex.FindStringSubmatch(ea.Uses.Value) + if len(sms) > 1 { + v, err := semver.NewVersion(sms[1]) + if err != nil { + // Couldn't parse version, hopefully another step has + // a correct one. + continue + } + if !gradleWrapperValidationActionVersionConstraint.Check(v) { + // Version out of acceptable range. + continue + } + // OK! This is it. + *validatingWorkflowFile = filepath.Base(path) + return true, nil + } + } + } + return true, nil +} + +// fileExists checks if a file of name name exists, including within +// subdirectories. +func fileExists(files []checker.File, name string) bool { + for _, f := range files { + if filepath.Base(f.Path) == name { + return true + } + } + return false +} diff --git a/checks/raw/binary_artifact_test.go b/checks/raw/binary_artifact_test.go index 5cd4b318bd8..86bd93249dd 100644 --- a/checks/raw/binary_artifact_test.go +++ b/checks/raw/binary_artifact_test.go @@ -21,54 +21,149 @@ import ( "github.com/golang/mock/gomock" + "github.com/ossf/scorecard/v4/clients" mockrepo "github.com/ossf/scorecard/v4/clients/mockclients" ) +func strptr(s string) *string { + return &s +} + // TestBinaryArtifact tests the BinaryArtifact checker. func TestBinaryArtifacts(t *testing.T) { t.Parallel() tests := []struct { - name string - err error - files []string - expect int + name string + err error + files [][]string + successfulWorkflowRuns []clients.WorkflowRun + commits []clients.Commit + getFileContentCount int + expect int }{ { name: "Jar file", err: nil, - files: []string{ - "../testdata/binaryartifacts/jars/aws-java-sdk-core-1.11.571.jar", + files: [][]string{ + {"../testdata/binaryartifacts/jars/aws-java-sdk-core-1.11.571.jar"}, }, - expect: 1, + getFileContentCount: 1, + expect: 1, }, { name: "Mach-O ARM64 executable", err: nil, - files: []string{ - "../testdata/binaryartifacts/executables/darwin-arm64-bt", + files: [][]string{ + {"../testdata/binaryartifacts/executables/darwin-arm64-bt"}, }, - expect: 1, + getFileContentCount: 1, + expect: 1, }, { name: "non binary file", err: nil, - files: []string{ - "../testdata/licensedir/withlicense/LICENSE", + files: [][]string{ + {"../testdata/licensedir/withlicense/LICENSE"}, }, + getFileContentCount: 1, }, { name: "non binary file", err: nil, - files: []string{ - "../doesnotexist", + files: [][]string{ + {"../doesnotexist"}, }, + getFileContentCount: 1, }, { name: "printable character .lib", err: nil, - files: []string{ - "../testdata/binaryartifacts/printable.lib", + files: [][]string{ + {"../testdata/binaryartifacts/printable.lib"}, + }, + getFileContentCount: 1, + }, + { + name: "gradle-wrapper.jar without verification action", + err: nil, + files: [][]string{ + {"../testdata/binaryartifacts/jars/gradle-wrapper.jar"}, + {}, + }, + getFileContentCount: 1, + expect: 1, + }, + { + name: "gradle-wrapper.jar with verification action", + err: nil, + files: [][]string{ + {"../testdata/binaryartifacts/jars/gradle-wrapper.jar"}, + { + "../testdata/binaryartifacts/workflows/nonverify.yml", + "../testdata/binaryartifacts/workflows/verify.yml", + }, + }, + successfulWorkflowRuns: []clients.WorkflowRun{ + { + HeadSHA: strptr("sha-a"), + }, + }, + commits: []clients.Commit{ + { + SHA: "sha-a", + }, + { + SHA: "sha-old", + }, + }, + getFileContentCount: 3, + expect: 0, + }, + { + name: "gradle-wrapper.jar with non-verification action", + err: nil, + files: [][]string{ + {"../testdata/binaryartifacts/jars/gradle-wrapper.jar"}, + {"../testdata/binaryartifacts/workflows/nonverify.yml"}, }, + getFileContentCount: 2, + expect: 1, + }, + { + name: "gradle-wrapper.jar with verification-failing commit", + err: nil, + files: [][]string{ + {"../testdata/binaryartifacts/jars/gradle-wrapper.jar"}, + {"../testdata/binaryartifacts/workflows/verify.yml"}, + }, + successfulWorkflowRuns: []clients.WorkflowRun{ + { + HeadSHA: strptr("sha-old"), + }, + }, + commits: []clients.Commit{ + { + SHA: "sha-a", + }, + { + SHA: "sha-old", + }, + }, + getFileContentCount: 2, + expect: 1, + }, + { + name: "gradle-wrapper.jar with outdated verification action", + err: nil, + files: [][]string{ + {"../testdata/binaryartifacts/jars/gradle-wrapper.jar"}, + { + "../testdata/binaryartifacts/workflows/nonverify.yml", + "../testdata/binaryartifacts/workflows/verify-outdated-action.yml", + }, + }, + getFileContentCount: 3, + expect: 1, }, } for _, tt := range tests { @@ -78,15 +173,25 @@ func TestBinaryArtifacts(t *testing.T) { ctrl := gomock.NewController(t) mockRepoClient := mockrepo.NewMockRepoClient(ctrl) - mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil) - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile(file) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil - }) + for _, files := range tt.files { + mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(files, nil) + } + for i := 0; i < tt.getFileContentCount; i++ { + mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { + // This will read the file and return the content + content, err := os.ReadFile(file) + if err != nil { + return content, fmt.Errorf("%w", err) + } + return content, nil + }) + } + if tt.successfulWorkflowRuns != nil { + mockRepoClient.EXPECT().ListSuccessfulWorkflowRuns(gomock.Any()).Return(tt.successfulWorkflowRuns, nil) + } + if tt.commits != nil { + mockRepoClient.EXPECT().ListCommits().Return(tt.commits, nil) + } f, err := BinaryArtifacts(mockRepoClient) diff --git a/checks/testdata/binaryartifacts/jars/gradle-wrapper.jar b/checks/testdata/binaryartifacts/jars/gradle-wrapper.jar new file mode 100644 index 00000000000..5c2d1cf016b Binary files /dev/null and b/checks/testdata/binaryartifacts/jars/gradle-wrapper.jar differ diff --git a/checks/testdata/binaryartifacts/workflows/nonverify.yml b/checks/testdata/binaryartifacts/workflows/nonverify.yml new file mode 100644 index 00000000000..6075d4ad58a --- /dev/null +++ b/checks/testdata/binaryartifacts/workflows/nonverify.yml @@ -0,0 +1,13 @@ +name: "Test Workflow" +on: [push, pull_request] + +jobs: + validation: + name: "Build" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-go@v3 + with: + go-version: '1.18.3' # The Go version to download (if necessary) and use. + - run: go run main.go diff --git a/checks/testdata/binaryartifacts/workflows/verify-outdated-action.yml b/checks/testdata/binaryartifacts/workflows/verify-outdated-action.yml new file mode 100644 index 00000000000..41237b99dee --- /dev/null +++ b/checks/testdata/binaryartifacts/workflows/verify-outdated-action.yml @@ -0,0 +1,12 @@ +name: "GW Validate Workflow" +on: [push, pull_request] + +jobs: + gw_validat3: + name: "GW Validate Job" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: GW Validate Step + # this is a made-up outdated version of the action: + uses: gradle/wrapper-validation-action@v0.1.0 diff --git a/checks/testdata/binaryartifacts/workflows/verify.yml b/checks/testdata/binaryartifacts/workflows/verify.yml new file mode 100644 index 00000000000..bcc9f989eab --- /dev/null +++ b/checks/testdata/binaryartifacts/workflows/verify.yml @@ -0,0 +1,11 @@ +name: "GW Validate Workflow" +on: [push, pull_request] + +jobs: + gw_validat3: + name: "GW Validate Job" + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: GW Validate Step + uses: gradle/wrapper-validation-action@v1 diff --git a/clients/githubrepo/workflows.go b/clients/githubrepo/workflows.go index aa9884a6764..7c53a5668bb 100644 --- a/clients/githubrepo/workflows.go +++ b/clients/githubrepo/workflows.go @@ -55,7 +55,8 @@ func workflowsRunsFrom(data *github.WorkflowRuns) []clients.WorkflowRun { var workflowRuns []clients.WorkflowRun for _, workflowRun := range data.WorkflowRuns { workflowRuns = append(workflowRuns, clients.WorkflowRun{ - URL: workflowRun.GetURL(), + URL: workflowRun.GetURL(), + HeadSHA: workflowRun.HeadSHA, }) } return workflowRuns diff --git a/clients/workflows.go b/clients/workflows.go index 202809e9913..02e5cbe58de 100644 --- a/clients/workflows.go +++ b/clients/workflows.go @@ -16,5 +16,6 @@ package clients // WorkflowRun represents VCS WorkflowRun. type WorkflowRun struct { - URL string + HeadSHA *string `json:"head_sha"` + URL string } diff --git a/go.mod b/go.mod index 0a62d35847d..bf2a7a856b3 100644 --- a/go.mod +++ b/go.mod @@ -45,6 +45,7 @@ require ( ) require ( + github.com/Masterminds/semver/v3 v3.1.1 github.com/caarlos0/env/v6 v6.9.3 github.com/mcuadros/go-jsonschema-generator v0.0.0-20200330054847-ba7a369d4303 github.com/onsi/ginkgo/v2 v2.1.4 diff --git a/go.sum b/go.sum index b707b81354e..cc6300e9e41 100644 --- a/go.sum +++ b/go.sum @@ -188,9 +188,11 @@ github.com/Djarvur/go-err113 v0.1.0/go.mod h1:4UJr5HIiMZrwgkSPdsjy2uOQExX/WEILpI github.com/GoogleCloudPlatform/cloudsql-proxy v0.0.0-20191009163259-e802c2cb94ae/go.mod h1:mjwGPas4yKduTyubHvD1Atl9r1rUq8DfVy+gkVvZ+oo= github.com/GoogleCloudPlatform/cloudsql-proxy v1.29.0/go.mod h1:spvB9eLJH9dutlbPSRmHvSXXHOwGRyeXh1jVdquA2G8= github.com/GoogleCloudPlatform/k8s-cloud-provider v0.0.0-20190822182118-27a4ced34534/go.mod h1:iroGtC8B3tQiqtds1l+mgk/BBOrxbqjH+eUfFQYRc14= +github.com/Masterminds/semver v1.5.0 h1:H65muMkzWKEuNDnfl9d70GUjFniHKHRbFPGBuZ3QEww= github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= github.com/Masterminds/semver/v3 v3.0.3/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Masterminds/semver/v3 v3.1.0/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= +github.com/Masterminds/semver/v3 v3.1.1 h1:hLg3sBzpNErnxhQtUy/mmLR2I9foDujNK030IGemrRc= github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs= github.com/Microsoft/go-winio v0.4.11/go.mod h1:VhR8bwka0BXejwEJY73c50VrPtXAaKcyvVC4A4RozmA= github.com/Microsoft/go-winio v0.4.14/go.mod h1:qXqCSQ3Xa7+6tgxaGTIe4Kpcdsi+P8jBhyzoq1bpyYA=