diff --git a/checks/binary_artifact.go b/checks/binary_artifact.go index 354e9cbc1f6..63a7ec98794 100644 --- a/checks/binary_artifact.go +++ b/checks/binary_artifact.go @@ -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 diff --git a/checks/raw/binary_artifact.go b/checks/raw/binary_artifact.go index 13f9108eca6..bc27fa2031d 100644 --- a/checks/raw/binary_artifact.go +++ b/checks/raw/binary_artifact.go @@ -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 } diff --git a/checks/raw/binary_artifact_test.go b/checks/raw/binary_artifact_test.go index 92181c8a45d..ae9970bc500 100644 --- a/checks/raw/binary_artifact_test.go +++ b/checks/raw/binary_artifact_test.go @@ -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)) + } +} diff --git a/policy/policy_test.go b/policy/policy_test.go index 7557d1504d1..59ef89c78b4 100644 --- a/policy/policy_test.go +++ b/policy/policy_test.go @@ -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, + requiredRequestTypes: []checker.RequestType{checker.FileBased}, + expectedEnabledChecks: 1, + expectedError: false, }, { - name: "With ScorecardPolicy and unsupported check", - sp: &ScorecardPolicy{}, + 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: "Without ScorecardPolicy and argsChecks", - sp: nil, + name: "request types limit enabled checks", argsChecks: []string{}, requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased}, - expectedEnabledChecks: 4, // All checks are enabled by default + expectedEnabledChecks: 5, // All checks which are FileBased and CommitBased expectedError: false, }, { - name: "With ScorecardPolicy and missing policy", - sp: &ScorecardPolicy{}, - argsChecks: []string{"Binary-Artifacts"}, - requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased}, - expectedEnabledChecks: 0, - expectedError: true, + 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)