From 1c95237e4af88faaa0948d1f7a3f3435ace541d0 Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Mon, 7 Feb 2022 16:49:49 -0800 Subject: [PATCH] Only run allowed checks in different modes (#1579) Co-authored-by: Azeem Shaikh --- checker/check_request.go | 31 +++- checker/check_runner.go | 22 ++- checks/all_checks.go | 7 +- checks/all_checks_test.go | 2 +- checks/binary_artifact.go | 5 +- checks/branch_protection.go | 8 +- checks/ci_tests.go | 2 +- checks/cii_best_practices.go | 2 +- checks/code_review.go | 2 +- checks/contributors.go | 2 +- checks/dangerous_workflow.go | 5 +- checks/dependency_update_tool.go | 5 +- checks/fuzzing.go | 2 +- checks/license.go | 5 +- checks/maintained.go | 2 +- checks/packaging.go | 2 +- checks/permissions.go | 5 +- checks/pinned_dependencies.go | 5 +- checks/sast.go | 2 +- checks/security_policy.go | 5 +- checks/signed_releases.go | 2 +- checks/vulnerabilities.go | 2 +- cmd/root.go | 59 ++----- docs/checks/internal/validate/main.go | 230 +------------------------- errors/public.go | 7 +- 25 files changed, 116 insertions(+), 305 deletions(-) diff --git a/checker/check_request.go b/checker/check_request.go index d52a4693..7266bb25 100644 --- a/checker/check_request.go +++ b/checker/check_request.go @@ -30,5 +30,34 @@ type CheckRequest struct { Repo clients.Repo VulnerabilitiesClient clients.VulnerabilitiesClient // UPGRADEv6: return raw results instead of scores. - RawResults *RawResults + RawResults *RawResults + RequiredTypes []RequestType +} + +// RequestType identifies special requirements/attributes that need to be supported by checks. +type RequestType int + +const ( + // FileBased request types require checks to run solely on file-content. + FileBased RequestType = iota +) + +// ListUnsupported returns []RequestType not in `supported` and are `required`. +func ListUnsupported(required, supported []RequestType) []RequestType { + var ret []RequestType + for _, t := range required { + if !contains(supported, t) { + ret = append(ret, t) + } + } + return ret +} + +func contains(in []RequestType, exists RequestType) bool { + for _, r := range in { + if r == exists { + return true + } + } + return false } diff --git a/checker/check_runner.go b/checker/check_runner.go index 1955dea2..e7099a44 100644 --- a/checker/check_runner.go +++ b/checker/check_runner.go @@ -31,16 +31,22 @@ const checkRetries = 3 // Runner runs a check with retries. type Runner struct { - CheckRequest CheckRequest CheckName string Repo string + CheckRequest CheckRequest } // CheckFn defined for convenience. type CheckFn func(*CheckRequest) CheckResult +// Check defines a Scorecard check fn and its supported request types. +type Check struct { + Fn CheckFn + SupportedRequestTypes []RequestType +} + // CheckNameToFnMap defined here for convenience. -type CheckNameToFnMap map[string]CheckFn +type CheckNameToFnMap map[string]Check func logStats(ctx context.Context, startTime time.Time, result *CheckResult) error { runTimeInSecs := time.Now().Unix() - startTime.Unix() @@ -57,7 +63,15 @@ func logStats(ctx context.Context, startTime time.Time, result *CheckResult) err } // Run runs a given check. -func (r *Runner) Run(ctx context.Context, f CheckFn) CheckResult { +func (r *Runner) Run(ctx context.Context, c Check) CheckResult { + // Sanity check. + unsupported := ListUnsupported(r.CheckRequest.RequiredTypes, c.SupportedRequestTypes) + if len(unsupported) != 0 { + return CreateRuntimeErrorResult(r.CheckName, + sce.WithMessage(sce.ErrorUnsupportedCheck, + fmt.Sprintf("requiredType: %s not supported by check %s", fmt.Sprint(unsupported), r.CheckName))) + } + ctx, err := tag.New(ctx, tag.Upsert(stats.CheckName, r.CheckName)) if err != nil { panic(err) @@ -71,7 +85,7 @@ func (r *Runner) Run(ctx context.Context, f CheckFn) CheckResult { checkRequest.Ctx = ctx l = logger{} checkRequest.Dlogger = &l - res = f(&checkRequest) + res = c.Fn(&checkRequest) if res.Error2 != nil && errors.Is(res.Error2, sce.ErrRepoUnreachable) { checkRequest.Dlogger.Warn("%v", res.Error2) continue diff --git a/checks/all_checks.go b/checks/all_checks.go index 3dae2784..bb4942f2 100644 --- a/checks/all_checks.go +++ b/checks/all_checks.go @@ -22,13 +22,16 @@ import ( // AllChecks is the list of all security checks that will be run. var AllChecks = checker.CheckNameToFnMap{} -func registerCheck(name string, fn checker.CheckFn) error { +func registerCheck(name string, fn checker.CheckFn, supportedRequestTypes []checker.RequestType) error { if name == "" { return errInternalNameCannotBeEmpty } if fn == nil { return errInternalCheckFuncCannotBeNil } - AllChecks[name] = fn + AllChecks[name] = checker.Check{ + Fn: fn, + SupportedRequestTypes: supportedRequestTypes, + } return nil } diff --git a/checks/all_checks_test.go b/checks/all_checks_test.go index 2fea7388..ab20303b 100644 --- a/checks/all_checks_test.go +++ b/checks/all_checks_test.go @@ -62,7 +62,7 @@ func Test_registerCheck(t *testing.T) { 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 { + if err := registerCheck(tt.args.name, tt.args.fn, nil /*supportedRequestTypes*/); (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 0962399e..8439aee7 100644 --- a/checks/binary_artifact.go +++ b/checks/binary_artifact.go @@ -26,7 +26,10 @@ const CheckBinaryArtifacts string = "Binary-Artifacts" //nolint func init() { - if err := registerCheck(CheckBinaryArtifacts, BinaryArtifacts); err != nil { + var supportedRequestTypes = []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckBinaryArtifacts, BinaryArtifacts, supportedRequestTypes); err != nil { // this should never happen panic(err) } diff --git a/checks/branch_protection.go b/checks/branch_protection.go index 4ca3711e..2ab0a057 100644 --- a/checks/branch_protection.go +++ b/checks/branch_protection.go @@ -21,14 +21,12 @@ import ( sce "github.com/ossf/scorecard/v4/errors" ) -const ( - // CheckBranchProtection is the exported name for Branch-Protected check. - CheckBranchProtection = "Branch-Protection" -) +// CheckBranchProtection is the exported name for Branch-Protected check. +const CheckBranchProtection = "Branch-Protection" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckBranchProtection, BranchProtection); err != nil { + if err := registerCheck(CheckBranchProtection, BranchProtection, nil); err != nil { // this should never happen panic(err) } diff --git a/checks/ci_tests.go b/checks/ci_tests.go index 396f609d..8da124c1 100644 --- a/checks/ci_tests.go +++ b/checks/ci_tests.go @@ -31,7 +31,7 @@ const ( //nolint:gochecknoinits func init() { - if err := registerCheck(CheckCITests, CITests); err != nil { + if err := registerCheck(CheckCITests, CITests, nil); err != nil { // this should never happen panic(err) } diff --git a/checks/cii_best_practices.go b/checks/cii_best_practices.go index 50f82ffb..ecc8d5be 100644 --- a/checks/cii_best_practices.go +++ b/checks/cii_best_practices.go @@ -32,7 +32,7 @@ const ( //nolint:gochecknoinits func init() { - if err := registerCheck(CheckCIIBestPractices, CIIBestPractices); err != nil { + if err := registerCheck(CheckCIIBestPractices, CIIBestPractices, nil); err != nil { // this should never happen panic(err) } diff --git a/checks/code_review.go b/checks/code_review.go index c4491b27..73365d7d 100644 --- a/checks/code_review.go +++ b/checks/code_review.go @@ -26,7 +26,7 @@ const CheckCodeReview = "Code-Review" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckCodeReview, CodeReview); err != nil { + if err := registerCheck(CheckCodeReview, CodeReview, nil); err != nil { // this should never happen panic(err) } diff --git a/checks/contributors.go b/checks/contributors.go index b4d5787a..6cc16b13 100644 --- a/checks/contributors.go +++ b/checks/contributors.go @@ -31,7 +31,7 @@ const ( //nolint:gochecknoinits func init() { - if err := registerCheck(CheckContributors, Contributors); err != nil { + if err := registerCheck(CheckContributors, Contributors, nil); err != nil { // this should never happen panic(err) } diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go index 3d57a4bd..e418cb80 100644 --- a/checks/dangerous_workflow.go +++ b/checks/dangerous_workflow.go @@ -59,7 +59,10 @@ func containsUntrustedContextPattern(variable string) bool { //nolint:gochecknoinits func init() { - if err := registerCheck(CheckDangerousWorkflow, DangerousWorkflow); err != nil { + supportedRequestTypes := []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckDangerousWorkflow, DangerousWorkflow, supportedRequestTypes); err != nil { // this should never happen panic(err) } diff --git a/checks/dependency_update_tool.go b/checks/dependency_update_tool.go index c0c6e140..286dd6d5 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() { - if err := registerCheck(CheckDependencyUpdateTool, DependencyUpdateTool); err != nil { + var supportedRequestTypes = []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckDependencyUpdateTool, DependencyUpdateTool, supportedRequestTypes); err != nil { // this should never happen panic(err) } diff --git a/checks/fuzzing.go b/checks/fuzzing.go index f98cba9c..ac9c2b7a 100644 --- a/checks/fuzzing.go +++ b/checks/fuzzing.go @@ -28,7 +28,7 @@ const CheckFuzzing = "Fuzzing" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckFuzzing, Fuzzing); err != nil { + if err := registerCheck(CheckFuzzing, Fuzzing, nil); err != nil { // this should never happen panic(err) } diff --git a/checks/license.go b/checks/license.go index 4fdb19a6..3259f102 100644 --- a/checks/license.go +++ b/checks/license.go @@ -35,7 +35,10 @@ const CheckLicense = "License" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckLicense, LicenseCheck); err != nil { + supportedRequestTypes := []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckLicense, LicenseCheck, supportedRequestTypes); err != nil { // this should never happen panic(err) } diff --git a/checks/maintained.go b/checks/maintained.go index 7e33a962..5a4142e0 100644 --- a/checks/maintained.go +++ b/checks/maintained.go @@ -33,7 +33,7 @@ const ( //nolint:gochecknoinits func init() { - if err := registerCheck(CheckMaintained, IsMaintained); err != nil { + if err := registerCheck(CheckMaintained, IsMaintained, nil); err != nil { // this should never happen panic(err) } diff --git a/checks/packaging.go b/checks/packaging.go index 69e6569e..73ca3612 100644 --- a/checks/packaging.go +++ b/checks/packaging.go @@ -30,7 +30,7 @@ const CheckPackaging = "Packaging" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckPackaging, Packaging); err != nil { + if err := registerCheck(CheckPackaging, Packaging, nil); err != nil { // this should never happen panic(err) } diff --git a/checks/permissions.go b/checks/permissions.go index 0ca4cf2f..7b4fdf88 100644 --- a/checks/permissions.go +++ b/checks/permissions.go @@ -53,7 +53,10 @@ var permissionsOfInterest = []permission{ //nolint:gochecknoinits func init() { - if err := registerCheck(CheckTokenPermissions, TokenPermissions); err != nil { + supportedRequestTypes := []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckTokenPermissions, TokenPermissions, supportedRequestTypes); err != nil { // This should never happen. panic(err) } diff --git a/checks/pinned_dependencies.go b/checks/pinned_dependencies.go index fedcc88b..b74a00be 100644 --- a/checks/pinned_dependencies.go +++ b/checks/pinned_dependencies.go @@ -39,7 +39,10 @@ type worklowPinningResult struct { //nolint:gochecknoinits func init() { - if err := registerCheck(CheckPinnedDependencies, PinnedDependencies); err != nil { + supportedRequestTypes := []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckPinnedDependencies, PinnedDependencies, supportedRequestTypes); err != nil { // This should never happen. panic(err) } diff --git a/checks/sast.go b/checks/sast.go index b9f311eb..7953b0ac 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -31,7 +31,7 @@ var allowedConclusions = map[string]bool{"success": true, "neutral": true} //nolint:gochecknoinits func init() { - if err := registerCheck(CheckSAST, SAST); err != nil { + if err := registerCheck(CheckSAST, SAST, nil); err != nil { // This should never happen. panic(err) } diff --git a/checks/security_policy.go b/checks/security_policy.go index 59a1e5bb..7aa2b588 100644 --- a/checks/security_policy.go +++ b/checks/security_policy.go @@ -26,7 +26,10 @@ const CheckSecurityPolicy = "Security-Policy" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckSecurityPolicy, SecurityPolicy); err != nil { + supportedRequestTypes := []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckSecurityPolicy, SecurityPolicy, supportedRequestTypes); err != nil { // This should never happen. panic(err) } diff --git a/checks/signed_releases.go b/checks/signed_releases.go index 842e73b8..4d31ee7a 100644 --- a/checks/signed_releases.go +++ b/checks/signed_releases.go @@ -32,7 +32,7 @@ var artifactExtensions = []string{".asc", ".minisig", ".sig", ".sign"} //nolint:gochecknoinits func init() { - if err := registerCheck(CheckSignedReleases, SignedReleases); err != nil { + if err := registerCheck(CheckSignedReleases, SignedReleases, nil); err != nil { // this should never happen panic(err) } diff --git a/checks/vulnerabilities.go b/checks/vulnerabilities.go index 9a16e414..afffa6ec 100644 --- a/checks/vulnerabilities.go +++ b/checks/vulnerabilities.go @@ -26,7 +26,7 @@ const CheckVulnerabilities = "Vulnerabilities" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckVulnerabilities, Vulnerabilities); err != nil { + if err := registerCheck(CheckVulnerabilities, Vulnerabilities, nil); err != nil { // this should never happen panic(err) } diff --git a/cmd/root.go b/cmd/root.go index 4c3f4ace..0896e273 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -45,11 +45,6 @@ const ( cliEnableSarif = "ENABLE_SARIF" - // These strings must be the same as the ones used in - // checks.yaml for the "repos" field. - repoTypeLocal = "local" - repoTypeGitHub = "GitHub" - scorecardLong = "A program that shows security scorecard for an open source software." scorecardUse = `./scorecard [--repo=] [--local=folder] [--checks=check1,...] [--show-details] or ./scorecard --{npm,pypi,rubygems}= @@ -148,16 +143,11 @@ func scorecardCmd(cmd *cobra.Command, args []string) { log.Panicf("cannot read yaml file: %v", err) } - repoType := repoTypeGitHub + var requiredRequestTypes []checker.RequestType if flagLocal != "" { - repoType = repoTypeLocal + requiredRequestTypes = append(requiredRequestTypes, checker.FileBased) } - supportedChecks, err := getSupportedChecks(repoType, checkDocs) - if err != nil { - log.Panicf("cannot read supported checks: %v", err) - } - - enabledChecks, err := getEnabledChecks(policy, flagChecksToRun, supportedChecks, repoType) + enabledChecks, err := getEnabledChecks(policy, flagChecksToRun, requiredRequestTypes) if err != nil { log.Panic(err) } @@ -299,31 +289,11 @@ func checksHavePolicies(sp *spol.ScorecardPolicy, enabledChecks checker.CheckNam return true } -func getSupportedChecks(r string, checkDocs docs.Doc) ([]string, error) { - allChecks := checks.AllChecks - supportedChecks := []string{} - for check := range allChecks { - c, e := checkDocs.GetCheck(check) - if e != nil { - return nil, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("checkDocs.GetCheck: %v", e)) - } - types := c.GetSupportedRepoTypes() - for _, t := range types { - if r == t { - supportedChecks = append(supportedChecks, c.GetName()) - } - } - } - return supportedChecks, nil -} - -func isSupportedCheck(names []string, name string) bool { - for _, n := range names { - if n == name { - return true - } - } - return false +func isSupportedCheck(checkName string, requiredRequestTypes []checker.RequestType) bool { + unsupported := checker.ListUnsupported( + requiredRequestTypes, + checks.AllChecks[checkName].SupportedRequestTypes) + return len(unsupported) == 0 } func getAllChecks() checker.CheckNameToFnMap { @@ -333,17 +303,18 @@ func getAllChecks() checker.CheckNameToFnMap { } func getEnabledChecks(sp *spol.ScorecardPolicy, argsChecks []string, - supportedChecks []string, repoType string) (checker.CheckNameToFnMap, error) { + requiredRequestTypes []checker.RequestType) (checker.CheckNameToFnMap, error) { enabledChecks := checker.CheckNameToFnMap{} switch { case len(argsChecks) != 0: - // Populate checks to run with the CLI arguments. + // Populate checks to run with the `--repo` CLI argument. for _, checkName := range argsChecks { - if !isSupportedCheck(supportedChecks, checkName) { + if !isSupportedCheck(checkName, requiredRequestTypes) { return enabledChecks, sce.WithMessage(sce.ErrScorecardInternal, - fmt.Sprintf("repo type %s: unsupported check: %s", repoType, checkName)) + fmt.Sprintf("Unsupported RequestType %s by check: %s", + fmt.Sprint(requiredRequestTypes), checkName)) } if !enableCheck(checkName, &enabledChecks) { return enabledChecks, @@ -353,7 +324,7 @@ func getEnabledChecks(sp *spol.ScorecardPolicy, argsChecks []string, case sp != nil: // Populate checks to run with policy file. for checkName := range sp.GetPolicies() { - if !isSupportedCheck(supportedChecks, checkName) { + if !isSupportedCheck(checkName, requiredRequestTypes) { // We silently ignore the check, like we do // for the default case when no argsChecks // or policy are present. @@ -368,7 +339,7 @@ func getEnabledChecks(sp *spol.ScorecardPolicy, argsChecks []string, default: // Enable all checks that are supported. for checkName := range getAllChecks() { - if !isSupportedCheck(supportedChecks, checkName) { + if !isSupportedCheck(checkName, requiredRequestTypes) { continue } if !enableCheck(checkName, &enabledChecks) { diff --git a/docs/checks/internal/validate/main.go b/docs/checks/internal/validate/main.go index 2fd38d33..e0295147 100644 --- a/docs/checks/internal/validate/main.go +++ b/docs/checks/internal/validate/main.go @@ -14,219 +14,14 @@ package main import ( - "errors" "fmt" - "os" - "path" - "regexp" "strings" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - "github.com/ossf/scorecard/v4/checks" docs "github.com/ossf/scorecard/v4/docs/checks" ) -var ( - allowedRisks = map[string]bool{"Critical": true, "High": true, "Medium": true, "Low": true} - allowedRepoTypes = map[string]bool{"GitHub": true, "local": true} - supportedAPIs = map[string][]string{ - // InitRepo is supported for local repos in general. However, in the context of checks, - // this is only used to look up remote data, e.g. in Fuzzing check. - // So we only have "GitHub" supported. - "InitRepo": {"GitHub"}, - "URI": {"GitHub", "local"}, - "IsArchived": {"GitHub"}, - "ListFiles": {"GitHub", "local"}, - "GetFileContent": {"GitHub", "local"}, - "ListBranches": {"GitHub"}, - "GetDefaultBranch": {"GitHub"}, - "ListCommits": {"GitHub"}, - "ListIssues": {"GitHub"}, - "ListReleases": {"GitHub"}, - "ListContributors": {"GitHub"}, - "ListSuccessfulWorkflowRuns": {"GitHub"}, - "ListCheckRunsForRef": {"GitHub"}, - "ListStatuses": {"GitHub"}, - "Search": {"GitHub", "local"}, - "Close": {"GitHub", "local"}, - } -) - -// Identify the source file that declares each check. -func listCheckFiles() (map[string]string, error) { - checkFiles := make(map[string]string) - // Use regex to determine the file that contains the entry point. - // We're looking for `const someVarName = "CheckName"`. - regex := regexp.MustCompile(`const\s+[^"]*=\s+"(.*)"`) - files, err := os.ReadDir("checks/") - if err != nil { - return nil, fmt.Errorf("os.ReadDir: %w", err) - } - - for _, file := range files { - if !strings.HasSuffix(file.Name(), ".go") || - strings.HasSuffix(file.Name(), "_test.go") || file.IsDir() { - continue - } - - // WARNING: assume the filename is the same during migration - // from `checks/`` to `checks/raw/`. - // TODO: UPGRADEv6: remove this temporary fix. - fullpathraw := path.Join("checks/raw", file.Name()) - fullpath := path.Join("checks/", file.Name()) - content, err := os.ReadFile(fullpath) - if err != nil { - return nil, fmt.Errorf("os.ReadFile: %s: %w", fullpath, err) - } - - res := regex.FindStringSubmatch(string(content)) - if len(res) != 2 { - continue - } - - r := res[1] - if entry, exists := checkFiles[r]; exists { - //nolint:goerr113 - return nil, fmt.Errorf("check %s already exists: %v", r, entry) - } - // TODO: UPGRADEv6: remove this temporary fix. - if _, err := os.Stat(fullpathraw); errors.Is(err, os.ErrNotExist) { - checkFiles[r] = fullpath - } else { - checkFiles[r] = fullpathraw - } - } - - return checkFiles, nil -} - -// Extract API names for RepoClient interface. -func extractAPINames() ([]string, error) { - fns := []string{} - interfaceRe := regexp.MustCompile(`type\s+RepoClient\s+interface\s+{\s*`) - content, err := os.ReadFile("clients/repo_client.go") - if err != nil { - return nil, fmt.Errorf("os.ReadFile: %s: %w", "clients/repo_client.go", err) - } - - locs := interfaceRe.FindIndex(content) - if len(locs) != 2 || locs[1] == 0 { - //nolint:goerr113 - return nil, fmt.Errorf("FindIndex: cannot find Doc interface definition") - } - - nameRe := regexp.MustCompile(`[\s]+([A-Z]\S+)\s*\(.*\).+[\n]+`) - matches := nameRe.FindAllStringSubmatch(string(content[locs[1]-1:]), -1) - if len(matches) == 0 { - //nolint:goerr113 - return nil, fmt.Errorf("FindAllStringSubmatch: no match found") - } - - for _, v := range matches { - if len(v) != 2 { - //nolint:goerr113 - return nil, fmt.Errorf("invalid length: %d", len(v)) - } - fns = append(fns, v[1]) - } - return fns, nil -} - -func contains(l []string, elt string) bool { - for _, v := range l { - if v == elt { - return true - } - } - return false -} - -// Extract supported interfaces from a check implementation file. -func supportedInterfacesFromImplementation(checkName string, checkFiles map[string]string) ([]string, error) { - // Special case. No APIs are used, - // but we need the repo name for an online database lookup. - if checkName == checks.CheckCIIBestPractices || checkName == checks.CheckFuzzing { - return []string{"GitHub"}, nil - } - - // Create our map. - s := make(map[string]bool) - for k := range allowedRepoTypes { - s[k] = true - } - - // Read the source file for the check. - pathfn, exists := checkFiles[checkName] - if !exists { - //nolint:goerr113 - return nil, fmt.Errorf("check %s does not exists", checkName) - } - - content, err := os.ReadFile(pathfn) - if err != nil { - return nil, fmt.Errorf("os.ReadFile: %s: %w", pathfn, err) - } - - // For each API, check if it's used or not. - // Adjust supported repo types accordingly. - for api, supportedInterfaces := range supportedAPIs { - regex := fmt.Sprintf(`\.%s`, api) - re := regexp.MustCompile(regex) - r := re.Match(content) - if r { - for k := range allowedRepoTypes { - if !contains(supportedInterfaces, k) { - delete(s, k) - } - } - } - } - - r := []string{} - for k := range s { - r = append(r, k) - } - return r, nil -} - -func validateRepoTypeAPIs(checkName string, repoTypes []string, checkFiles map[string]string) error { - // For now, we only list APIs in a check's implementation. - // Long-term, we should use the callgraph feature using - // https://github.com/golang/tools/blob/master/cmd/callgraph/main.go - l, err := supportedInterfacesFromImplementation(checkName, checkFiles) - if err != nil { - return fmt.Errorf("supportedInterfacesFromImplementation: %w", err) - } - - if !cmp.Equal(l, repoTypes, cmpopts.SortSlices(func(x, y string) bool { return x < y })) { - //nolint: goerr113 - return fmt.Errorf("%s: got diff: %s", checkName, cmp.Diff(l, repoTypes)) - } - return nil -} - -func validateAPINames() error { - // Extract API names. - fns, err := extractAPINames() - if err != nil { - return fmt.Errorf("invalid functions: %w", err) - } - - // Validate function names. - functions := []string{} - for v := range supportedAPIs { - functions = append(functions, v) - } - - if !cmp.Equal(functions, fns, cmpopts.SortSlices(func(x, y string) bool { return x < y })) { - //nolint:goerr113 - return fmt.Errorf("got diff: %s", cmp.Diff(functions, fns)) - } - - return nil -} +var allowedRisks = map[string]bool{"Critical": true, "High": true, "Medium": true, "Low": true} func main() { m, err := docs.Read() @@ -234,15 +29,6 @@ func main() { panic(fmt.Sprintf("docs.Read: %v", err)) } - if err := validateAPINames(); err != nil { - panic(fmt.Sprintf("cannot extract function names: %v", err)) - } - - checkFiles, err := listCheckFiles() - if err != nil { - panic(err) - } - allChecks := checks.AllChecks for check := range allChecks { c, e := m.GetCheck(check) @@ -269,20 +55,6 @@ func main() { if _, exists := allowedRisks[r]; !exists { panic(fmt.Sprintf("risk for checkName: %s is invalid: '%s'", check, r)) } - repoTypes := c.GetSupportedRepoTypes() - if len(repoTypes) == 0 { - panic(fmt.Sprintf("repos for checkName: %s is empty", check)) - } - for _, rt := range repoTypes { - if _, exists := allowedRepoTypes[rt]; !exists { - panic(fmt.Sprintf("repo type for checkName: %s is invalid: '%s'", check, rt)) - } - } - - // Validate that the check only calls API the interface supports. - if err := validateRepoTypeAPIs(check, repoTypes, checkFiles); err != nil { - panic(fmt.Sprintf("validateRepoTypeAPIs: %v", err)) - } } for _, check := range m.GetChecks() { if _, exists := allChecks[check.GetName()]; !exists { diff --git a/errors/public.go b/errors/public.go index 203afda3..7ba9f534 100644 --- a/errors/public.go +++ b/errors/public.go @@ -19,16 +19,19 @@ import ( "fmt" ) -//nolint var ( + // ErrScorecardInternal indicates a runtime error in Scorecard code. ErrScorecardInternal = errors.New("internal error") - ErrRepoUnreachable = errors.New("repo unreachable") + // ErrRepoUnreachable indicates Scorecard is unable to establish connection with the repository. + ErrRepoUnreachable = errors.New("repo unreachable") // ErrorUnsupportedHost indicates the repo's host is unsupported. ErrorUnsupportedHost = errors.New("unsupported host") // ErrorInvalidURL indicates the repo's full URL was not passed. ErrorInvalidURL = errors.New("invalid repo flag") // ErrorShellParsing indicates there was an error when parsing shell code. ErrorShellParsing = errors.New("error parsing shell code") + // ErrorUnsupportedCheck indicates check caanot be run for given request. + ErrorUnsupportedCheck = errors.New("check is not supported for this request") ) // WithMessage wraps any of the errors listed above.