From f7b329e8305d11178836bb35ae95a5f7e7cf2292 Mon Sep 17 00:00:00 2001 From: naveen <172697+naveensrinivasan@users.noreply.github.com> Date: Wed, 12 Jan 2022 22:14:18 +0000 Subject: [PATCH] :sparkles: Unit test for all_checks Addresses https://github.com/ossf/scorecard/issues/435 Signed-off-by: naveen <172697+naveensrinivasan@users.noreply.github.com> --- checks/all_checks.go | 13 +++++- checks/all_checks_test.go | 70 ++++++++++++++++++++++++++++++++ checks/binary_artifact.go | 5 ++- checks/branch_protection.go | 5 ++- checks/ci_tests.go | 5 ++- checks/cii_best_practices.go | 5 ++- checks/code_review.go | 5 ++- checks/contributors.go | 5 ++- checks/dangerous_workflow.go | 5 ++- checks/dependency_update_tool.go | 5 ++- checks/errors.go | 18 ++++---- checks/fuzzing.go | 5 ++- checks/license.go | 5 ++- checks/maintained.go | 5 ++- checks/packaging.go | 5 ++- checks/permissions.go | 5 ++- checks/pinned_dependencies.go | 5 ++- checks/sast.go | 5 ++- checks/security_policy.go | 5 ++- checks/signed_releases.go | 5 ++- checks/vulnerabilities.go | 5 ++- 21 files changed, 163 insertions(+), 28 deletions(-) create mode 100644 checks/all_checks_test.go diff --git a/checks/all_checks.go b/checks/all_checks.go index 63eb7de1..3dae2784 100644 --- a/checks/all_checks.go +++ b/checks/all_checks.go @@ -15,11 +15,20 @@ // Package checks defines all Scorecard checks. package checks -import "github.com/ossf/scorecard/v4/checker" +import ( + "github.com/ossf/scorecard/v4/checker" +) // AllChecks is the list of all security checks that will be run. var AllChecks = checker.CheckNameToFnMap{} -func registerCheck(name string, fn checker.CheckFn) { +func registerCheck(name string, fn checker.CheckFn) error { + if name == "" { + return errInternalNameCannotBeEmpty + } + if fn == nil { + return errInternalCheckFuncCannotBeNil + } AllChecks[name] = fn + return nil } diff --git a/checks/all_checks_test.go b/checks/all_checks_test.go new file mode 100644 index 00000000..2fea7388 --- /dev/null +++ b/checks/all_checks_test.go @@ -0,0 +1,70 @@ +// Copyright 2020 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package checks defines all Scorecard checks. +package checks + +import ( + "testing" + + "github.com/ossf/scorecard/v4/checker" +) + +func Test_registerCheck(t *testing.T) { + t.Parallel() + //nolint + type args struct { + name string + fn checker.CheckFn + } + //nolint + tests := []struct { + name string + args args + wanterr bool + }{ + { + name: "registerCheck", + args: args{ + name: "test", + fn: func(x *checker.CheckRequest) checker.CheckResult { return checker.CheckResult{} }, + }, + wanterr: false, + }, + { + name: "empty func", + args: args{ + name: "test", + }, + wanterr: true, + }, + { + name: "empty name", + args: args{ + name: "", + fn: func(x *checker.CheckRequest) checker.CheckResult { return checker.CheckResult{} }, + }, + wanterr: true, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if err := registerCheck(tt.args.name, tt.args.fn); (err != nil) != tt.wanterr { + t.Errorf("registerCheck() error = %v, wantErr %v", err, tt.wanterr) + } + }) + } +} diff --git a/checks/binary_artifact.go b/checks/binary_artifact.go index aa5ae2aa..0962399e 100644 --- a/checks/binary_artifact.go +++ b/checks/binary_artifact.go @@ -26,7 +26,10 @@ const CheckBinaryArtifacts string = "Binary-Artifacts" //nolint func init() { - registerCheck(CheckBinaryArtifacts, BinaryArtifacts) + if err := registerCheck(CheckBinaryArtifacts, BinaryArtifacts); err != nil { + // this should never happen + panic(err) + } } // BinaryArtifacts will check the repository contains binary artifacts. diff --git a/checks/branch_protection.go b/checks/branch_protection.go index 916870f4..4ca3711e 100644 --- a/checks/branch_protection.go +++ b/checks/branch_protection.go @@ -28,7 +28,10 @@ const ( //nolint:gochecknoinits func init() { - registerCheck(CheckBranchProtection, BranchProtection) + if err := registerCheck(CheckBranchProtection, BranchProtection); err != nil { + // this should never happen + panic(err) + } } // BranchProtection runs the Branch-Protection check. diff --git a/checks/ci_tests.go b/checks/ci_tests.go index 8468b205..c3de3da8 100644 --- a/checks/ci_tests.go +++ b/checks/ci_tests.go @@ -31,7 +31,10 @@ const ( //nolint:gochecknoinits func init() { - registerCheck(CheckCITests, CITests) + if err := registerCheck(CheckCITests, CITests); err != nil { + // this should never happen + panic(err) + } } // CITests runs CI-Tests check. diff --git a/checks/cii_best_practices.go b/checks/cii_best_practices.go index 657ad071..50f82ffb 100644 --- a/checks/cii_best_practices.go +++ b/checks/cii_best_practices.go @@ -32,7 +32,10 @@ const ( //nolint:gochecknoinits func init() { - registerCheck(CheckCIIBestPractices, CIIBestPractices) + if err := registerCheck(CheckCIIBestPractices, CIIBestPractices); err != nil { + // this should never happen + panic(err) + } } // CIIBestPractices runs CII-Best-Practices check. diff --git a/checks/code_review.go b/checks/code_review.go index e350d8e2..089fe7d6 100644 --- a/checks/code_review.go +++ b/checks/code_review.go @@ -27,7 +27,10 @@ const CheckCodeReview = "Code-Review" //nolint:gochecknoinits func init() { - registerCheck(CheckCodeReview, DoesCodeReview) + if err := registerCheck(CheckCodeReview, DoesCodeReview); err != nil { + // this should never happen + panic(err) + } } // DoesCodeReview attempts to determine whether a project requires review before code gets merged. diff --git a/checks/contributors.go b/checks/contributors.go index 3200057d..b4d5787a 100644 --- a/checks/contributors.go +++ b/checks/contributors.go @@ -31,7 +31,10 @@ const ( //nolint:gochecknoinits func init() { - registerCheck(CheckContributors, Contributors) + if err := registerCheck(CheckContributors, Contributors); err != nil { + // this should never happen + panic(err) + } } // Contributors run Contributors check. diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go index 2ea42f35..3d57a4bd 100644 --- a/checks/dangerous_workflow.go +++ b/checks/dangerous_workflow.go @@ -59,7 +59,10 @@ func containsUntrustedContextPattern(variable string) bool { //nolint:gochecknoinits func init() { - registerCheck(CheckDangerousWorkflow, DangerousWorkflow) + if err := registerCheck(CheckDangerousWorkflow, DangerousWorkflow); err != nil { + // this should never happen + panic(err) + } } // Holds stateful data to pass thru callbacks. diff --git a/checks/dependency_update_tool.go b/checks/dependency_update_tool.go index 6ecbde15..c0c6e140 100644 --- a/checks/dependency_update_tool.go +++ b/checks/dependency_update_tool.go @@ -26,7 +26,10 @@ const CheckDependencyUpdateTool = "Dependency-Update-Tool" //nolint func init() { - registerCheck(CheckDependencyUpdateTool, DependencyUpdateTool) + if err := registerCheck(CheckDependencyUpdateTool, DependencyUpdateTool); err != nil { + // this should never happen + panic(err) + } } // DependencyUpdateTool checks if the repository uses a dependency update tool. diff --git a/checks/errors.go b/checks/errors.go index 81586ca2..61abf511 100644 --- a/checks/errors.go +++ b/checks/errors.go @@ -20,12 +20,14 @@ import ( //nolint var ( - errInternalInvalidDockerFile = errors.New("invalid Dockerfile") - errInternalInvalidYamlFile = errors.New("invalid yaml file") - errInternalFilenameMatch = errors.New("filename match error") - errInternalEmptyFile = errors.New("empty file") - errInvalidGitHubWorkflow = errors.New("invalid GitHub workflow") - errInternalNoReviews = errors.New("no reviews found") - errInternalNoCommits = errors.New("no commits found") - errInternalInvalidPermissions = errors.New("invalid permissions") + errInternalInvalidDockerFile = errors.New("invalid Dockerfile") + errInternalInvalidYamlFile = errors.New("invalid yaml file") + errInternalFilenameMatch = errors.New("filename match error") + errInternalEmptyFile = errors.New("empty file") + errInvalidGitHubWorkflow = errors.New("invalid GitHub workflow") + errInternalNoReviews = errors.New("no reviews found") + errInternalNoCommits = errors.New("no commits found") + errInternalInvalidPermissions = errors.New("invalid permissions") + errInternalNameCannotBeEmpty = errors.New("name cannot be empty") + errInternalCheckFuncCannotBeNil = errors.New("checkFunc cannot be nil") ) diff --git a/checks/fuzzing.go b/checks/fuzzing.go index 6bd25bff..f98cba9c 100644 --- a/checks/fuzzing.go +++ b/checks/fuzzing.go @@ -28,7 +28,10 @@ const CheckFuzzing = "Fuzzing" //nolint:gochecknoinits func init() { - registerCheck(CheckFuzzing, Fuzzing) + if err := registerCheck(CheckFuzzing, Fuzzing); err != nil { + // this should never happen + panic(err) + } } func checkCFLite(c *checker.CheckRequest) (bool, error) { diff --git a/checks/license.go b/checks/license.go index 63559c9c..4fdb19a6 100644 --- a/checks/license.go +++ b/checks/license.go @@ -35,7 +35,10 @@ const CheckLicense = "License" //nolint:gochecknoinits func init() { - registerCheck(CheckLicense, LicenseCheck) + if err := registerCheck(CheckLicense, LicenseCheck); err != nil { + // this should never happen + panic(err) + } } const ( diff --git a/checks/maintained.go b/checks/maintained.go index eaa92c63..a0d2650f 100644 --- a/checks/maintained.go +++ b/checks/maintained.go @@ -32,7 +32,10 @@ const ( //nolint:gochecknoinits func init() { - registerCheck(CheckMaintained, IsMaintained) + if err := registerCheck(CheckMaintained, IsMaintained); err != nil { + // this should never happen + panic(err) + } } // IsMaintained runs Maintained check. diff --git a/checks/packaging.go b/checks/packaging.go index 8197ab49..56827e6c 100644 --- a/checks/packaging.go +++ b/checks/packaging.go @@ -31,7 +31,10 @@ const CheckPackaging = "Packaging" //nolint:gochecknoinits func init() { - registerCheck(CheckPackaging, Packaging) + if err := registerCheck(CheckPackaging, Packaging); err != nil { + // this should never happen + panic(err) + } } func isGithubWorkflowFile(filename string) (bool, error) { diff --git a/checks/permissions.go b/checks/permissions.go index 93118c90..0ca4cf2f 100644 --- a/checks/permissions.go +++ b/checks/permissions.go @@ -53,7 +53,10 @@ var permissionsOfInterest = []permission{ //nolint:gochecknoinits func init() { - registerCheck(CheckTokenPermissions, TokenPermissions) + if err := registerCheck(CheckTokenPermissions, TokenPermissions); err != nil { + // This should never happen. + panic(err) + } } // Holds stateful data to pass thru callbacks. diff --git a/checks/pinned_dependencies.go b/checks/pinned_dependencies.go index 23a64259..47b5349b 100644 --- a/checks/pinned_dependencies.go +++ b/checks/pinned_dependencies.go @@ -39,7 +39,10 @@ type worklowPinningResult struct { //nolint:gochecknoinits func init() { - registerCheck(CheckPinnedDependencies, PinnedDependencies) + if err := registerCheck(CheckPinnedDependencies, PinnedDependencies); err != nil { + // This should never happen. + panic(err) + } } // PinnedDependencies will check the repository if it contains frozen dependecies. diff --git a/checks/sast.go b/checks/sast.go index 1c0e2f00..30f39cc1 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -31,7 +31,10 @@ var allowedConclusions = map[string]bool{"success": true, "neutral": true} //nolint:gochecknoinits func init() { - registerCheck(CheckSAST, SAST) + if err := registerCheck(CheckSAST, SAST); err != nil { + // This should never happen. + panic(err) + } } // SAST runs SAST check. diff --git a/checks/security_policy.go b/checks/security_policy.go index f5922044..59a1e5bb 100644 --- a/checks/security_policy.go +++ b/checks/security_policy.go @@ -26,7 +26,10 @@ const CheckSecurityPolicy = "Security-Policy" //nolint:gochecknoinits func init() { - registerCheck(CheckSecurityPolicy, SecurityPolicy) + if err := registerCheck(CheckSecurityPolicy, SecurityPolicy); err != nil { + // This should never happen. + panic(err) + } } // SecurityPolicy runs Security-Policy check. diff --git a/checks/signed_releases.go b/checks/signed_releases.go index ca32ea9a..842e73b8 100644 --- a/checks/signed_releases.go +++ b/checks/signed_releases.go @@ -32,7 +32,10 @@ var artifactExtensions = []string{".asc", ".minisig", ".sig", ".sign"} //nolint:gochecknoinits func init() { - registerCheck(CheckSignedReleases, SignedReleases) + if err := registerCheck(CheckSignedReleases, SignedReleases); err != nil { + // this should never happen + panic(err) + } } // SignedReleases runs Signed-Releases check. diff --git a/checks/vulnerabilities.go b/checks/vulnerabilities.go index b2abc1ce..97222ef1 100644 --- a/checks/vulnerabilities.go +++ b/checks/vulnerabilities.go @@ -30,7 +30,10 @@ const ( //nolint:gochecknoinits func init() { - registerCheck(CheckVulnerabilities, HasUnfixedVulnerabilities) + if err := registerCheck(CheckVulnerabilities, HasUnfixedVulnerabilities); err != nil { + // this should never happen + panic(err) + } } func getVulnerabilities(resp *clients.VulnerabilitiesResponse) []string {