Only run allowed checks in different modes (#1579)

Co-authored-by: Azeem Shaikh <azeems@google.com>
This commit is contained in:
Azeem Shaikh 2022-02-07 16:49:49 -08:00 committed by GitHub
parent eac2aecce6
commit 1c95237e4a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
25 changed files with 116 additions and 305 deletions

View File

@ -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
}

View File

@ -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

View File

@ -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
}

View File

@ -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)
}
})

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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=<repo_url>] [--local=folder] [--checks=check1,...]
[--show-details] or ./scorecard --{npm,pypi,rubygems}=<package_name>
@ -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) {

View File

@ -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 {

View File

@ -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.