From cdfb58b4930d17b82b04d3f000d6901ddcf1d621 Mon Sep 17 00:00:00 2001 From: Luke Harrison Date: Mon, 2 Dec 2024 16:12:51 -0500 Subject: [PATCH] :sparkles: Allow incomplete local checks (#4423) * :sparkles: allow incomplete localdir checks (#3832) Signed-off-by: Luke Harrison * :sparkles: fixes as per @spencerschrock (#3832) Signed-off-by: Luke Harrison * :sparkles: fixed linting issues (#3832) Signed-off-by: Luke Harrison --------- Signed-off-by: Luke Harrison Signed-off-by: Luke Harrison --- checks/dangerous_workflow.go | 2 +- checks/fuzzing.go | 5 ++- checks/license.go | 1 + checks/packaging.go | 23 ++++++++++++-- checks/permissions.go | 2 +- checks/pinned_dependencies.go | 2 +- checks/raw/fuzzing.go | 10 ++++++ checks/raw/github/packaging.go | 9 +++++- checks/raw/sast.go | 5 +++ checks/sast.go | 5 ++- checks/security_policy.go | 1 + clients/localdir/client.go | 57 +++++++++++++++++----------------- cmd/root.go | 2 ++ policy/policy_test.go | 2 +- 14 files changed, 88 insertions(+), 38 deletions(-) diff --git a/checks/dangerous_workflow.go b/checks/dangerous_workflow.go index 61e7b337aff..7dae1cce40e 100644 --- a/checks/dangerous_workflow.go +++ b/checks/dangerous_workflow.go @@ -29,8 +29,8 @@ const CheckDangerousWorkflow = "Dangerous-Workflow" //nolint:gochecknoinits func init() { supportedRequestTypes := []checker.RequestType{ - checker.FileBased, checker.CommitBased, + checker.FileBased, } if err := registerCheck(CheckDangerousWorkflow, DangerousWorkflow, supportedRequestTypes); err != nil { // this should never happen diff --git a/checks/fuzzing.go b/checks/fuzzing.go index 9f727fc6570..0a55e0253c1 100644 --- a/checks/fuzzing.go +++ b/checks/fuzzing.go @@ -28,7 +28,10 @@ const CheckFuzzing = "Fuzzing" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckFuzzing, Fuzzing, nil); err != nil { + supportedRequestTypes := []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckFuzzing, Fuzzing, supportedRequestTypes); err != nil { // this should never happen panic(err) } diff --git a/checks/license.go b/checks/license.go index 79609a1e28f..ec9e0d3ac77 100644 --- a/checks/license.go +++ b/checks/license.go @@ -30,6 +30,7 @@ const CheckLicense = "License" func init() { supportedRequestTypes := []checker.RequestType{ checker.CommitBased, + checker.FileBased, } if err := registerCheck(CheckLicense, License, supportedRequestTypes); err != nil { // this should never happen diff --git a/checks/packaging.go b/checks/packaging.go index 59c2762e288..8b935e9e45f 100644 --- a/checks/packaging.go +++ b/checks/packaging.go @@ -21,6 +21,7 @@ import ( "github.com/ossf/scorecard/v5/checks/raw/gitlab" "github.com/ossf/scorecard/v5/clients/githubrepo" "github.com/ossf/scorecard/v5/clients/gitlabrepo" + "github.com/ossf/scorecard/v5/clients/localdir" sce "github.com/ossf/scorecard/v5/errors" "github.com/ossf/scorecard/v5/probes" "github.com/ossf/scorecard/v5/probes/zrunner" @@ -31,7 +32,10 @@ const CheckPackaging = "Packaging" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckPackaging, Packaging, nil); err != nil { + supportedRequestTypes := []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckPackaging, Packaging, supportedRequestTypes); err != nil { // this should never happen panic(err) } @@ -39,10 +43,23 @@ func init() { // Packaging runs Packaging check. func Packaging(c *checker.CheckRequest) checker.CheckResult { - var rawData checker.PackagingData - var err error + var rawData, rawDataGithub, rawDataGitlab checker.PackagingData + var err, errGithub, errGitlab error switch v := c.RepoClient.(type) { + case *localdir.Client: + // Performing both packaging checks since we dont know when local + rawDataGithub, errGithub = github.Packaging(c) + rawDataGitlab, errGitlab = gitlab.Packaging(c) + // Appending results of checks + rawData.Packages = append(rawData.Packages, rawDataGithub.Packages...) + rawData.Packages = append(rawData.Packages, rawDataGitlab.Packages...) + // checking for errors + if errGithub != nil { + err = errGithub + } else if errGitlab != nil { + err = errGitlab + } case *githubrepo.Client: rawData, err = github.Packaging(c) case *gitlabrepo.Client: diff --git a/checks/permissions.go b/checks/permissions.go index 3014d8997b8..e9fa1b15082 100644 --- a/checks/permissions.go +++ b/checks/permissions.go @@ -29,8 +29,8 @@ const CheckTokenPermissions = "Token-Permissions" //nolint:gochecknoinits func init() { supportedRequestTypes := []checker.RequestType{ - checker.FileBased, checker.CommitBased, + checker.FileBased, } if err := registerCheck(CheckTokenPermissions, TokenPermissions, supportedRequestTypes); err != nil { // This should never happen. diff --git a/checks/pinned_dependencies.go b/checks/pinned_dependencies.go index a82cd6e9373..8a7ac0cdc8b 100644 --- a/checks/pinned_dependencies.go +++ b/checks/pinned_dependencies.go @@ -29,8 +29,8 @@ const CheckPinnedDependencies = "Pinned-Dependencies" //nolint:gochecknoinits func init() { supportedRequestTypes := []checker.RequestType{ - checker.FileBased, checker.CommitBased, + checker.FileBased, } if err := registerCheck(CheckPinnedDependencies, PinningDependencies, supportedRequestTypes); err != nil { // This should never happen. diff --git a/checks/raw/fuzzing.go b/checks/raw/fuzzing.go index f4475e729d4..5d30e5da1c9 100644 --- a/checks/raw/fuzzing.go +++ b/checks/raw/fuzzing.go @@ -338,6 +338,8 @@ func getProminentLanguages(langs []clients.Language) []clients.LanguageName { numLangs := len(langs) if numLangs == 0 { return nil + } else if len(langs) == 1 && langs[0].Name == clients.All { + return getAllLanguages() } totalLoC := 0 // Use a map to record languages and their lines of code to drop potential duplicates. @@ -361,6 +363,14 @@ func getProminentLanguages(langs []clients.Language) []clients.LanguageName { return ret } +func getAllLanguages() []clients.LanguageName { + allLanguages := make([]clients.LanguageName, 0, len(languageFuzzSpecs)) + for l := range languageFuzzSpecs { + allLanguages = append(allLanguages, l) + } + return allLanguages +} + func propertyBasedDescription(language string) *string { s := fmt.Sprintf("Property-based testing in %s generates test instances randomly or exhaustively "+ "and test that specific properties are satisfied.", language) diff --git a/checks/raw/github/packaging.go b/checks/raw/github/packaging.go index 4deb99563a9..5aa02bd8bd9 100644 --- a/checks/raw/github/packaging.go +++ b/checks/raw/github/packaging.go @@ -15,6 +15,7 @@ package github import ( + "errors" "fmt" "io" "path/filepath" @@ -23,6 +24,7 @@ import ( "github.com/ossf/scorecard/v5/checker" "github.com/ossf/scorecard/v5/checks/fileparser" + "github.com/ossf/scorecard/v5/clients" "github.com/ossf/scorecard/v5/finding" ) @@ -73,7 +75,12 @@ func Packaging(c *checker.CheckRequest) (checker.PackagingData, error) { runs, err := c.RepoClient.ListSuccessfulWorkflowRuns(filepath.Base(fp)) if err != nil { - return data, fmt.Errorf("Client.Actions.ListWorkflowRunsByFileName: %w", err) + // assume the workflow will have run for localdir client + if errors.Is(err, clients.ErrUnsupportedFeature) { + runs = append(runs, clients.WorkflowRun{}) + } else { + return data, fmt.Errorf("Client.Actions.ListWorkflowRunsByFileName: %w", err) + } } if len(runs) > 0 { diff --git a/checks/raw/sast.go b/checks/raw/sast.go index 1806adcaeb9..0d654c2c118 100644 --- a/checks/raw/sast.go +++ b/checks/raw/sast.go @@ -28,6 +28,7 @@ import ( "github.com/ossf/scorecard/v5/checker" "github.com/ossf/scorecard/v5/checks/fileparser" + "github.com/ossf/scorecard/v5/clients" sce "github.com/ossf/scorecard/v5/errors" "github.com/ossf/scorecard/v5/finding" ) @@ -92,6 +93,10 @@ func sastToolInCheckRuns(c *checker.CheckRequest) ([]checker.SASTCommit, error) var sastCommits []checker.SASTCommit commits, err := c.RepoClient.ListCommits() if err != nil { + // ignoring check for local dir + if errors.Is(err, clients.ErrUnsupportedFeature) { + return sastCommits, nil + } return sastCommits, sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("RepoClient.ListCommits: %v", err)) } diff --git a/checks/sast.go b/checks/sast.go index f09a6ba4d7a..05f20846c1d 100644 --- a/checks/sast.go +++ b/checks/sast.go @@ -28,7 +28,10 @@ const CheckSAST = "SAST" //nolint:gochecknoinits func init() { - if err := registerCheck(CheckSAST, SAST, nil); err != nil { + supportedRequestTypes := []checker.RequestType{ + checker.FileBased, + } + if err := registerCheck(CheckSAST, SAST, supportedRequestTypes); err != nil { // This should never happen. panic(err) } diff --git a/checks/security_policy.go b/checks/security_policy.go index dd292494930..00118725d60 100644 --- a/checks/security_policy.go +++ b/checks/security_policy.go @@ -30,6 +30,7 @@ const CheckSecurityPolicy = "Security-Policy" func init() { supportedRequestTypes := []checker.RequestType{ checker.CommitBased, + checker.FileBased, } if err := registerCheck(CheckSecurityPolicy, SecurityPolicy, supportedRequestTypes); err != nil { // This should never happen. diff --git a/clients/localdir/client.go b/clients/localdir/client.go index eb270f10b56..e3c18eb4c15 100644 --- a/clients/localdir/client.go +++ b/clients/localdir/client.go @@ -34,12 +34,12 @@ import ( ) var ( - _ clients.RepoClient = &localDirClient{} + _ clients.RepoClient = &Client{} errInputRepoType = errors.New("input repo should be of type repoLocal") ) //nolint:govet -type localDirClient struct { +type Client struct { logger *log.Logger ctx context.Context path string @@ -50,7 +50,7 @@ type localDirClient struct { } // InitRepo sets up the local repo. -func (client *localDirClient) InitRepo(inputRepo clients.Repo, commitSHA string, commitDepth int) error { +func (client *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitDepth int) error { localRepo, ok := inputRepo.(*Repo) if !ok { return fmt.Errorf("%w: %v", errInputRepoType, inputRepo) @@ -66,12 +66,12 @@ func (client *localDirClient) InitRepo(inputRepo clients.Repo, commitSHA string, } // URI implements RepoClient.URI. -func (client *localDirClient) URI() string { +func (client *Client) URI() string { return fmt.Sprintf("file://%s", client.path) } // IsArchived implements RepoClient.IsArchived. -func (client *localDirClient) IsArchived() (bool, error) { +func (client *Client) IsArchived() (bool, error) { return false, fmt.Errorf("IsArchived: %w", clients.ErrUnsupportedFeature) } @@ -148,7 +148,7 @@ func applyPredicate( } // LocalPath implements RepoClient.LocalPath. -func (client *localDirClient) LocalPath() (string, error) { +func (client *Client) LocalPath() (string, error) { clientPath, err := filepath.Abs(client.path) if err != nil { return "", fmt.Errorf("error during filepath.Abs: %w", err) @@ -157,7 +157,7 @@ func (client *localDirClient) LocalPath() (string, error) { } // ListFiles implements RepoClient.ListFiles. -func (client *localDirClient) ListFiles(predicate func(string) (bool, error)) ([]string, error) { +func (client *Client) ListFiles(predicate func(string) (bool, error)) ([]string, error) { client.once.Do(func() { client.files, client.errFiles = listFiles(client.path) }) @@ -175,102 +175,103 @@ func getFile(clientpath, filename string) (*os.File, error) { } // GetFileReader implements RepoClient.GetFileReader. -func (client *localDirClient) GetFileReader(filename string) (io.ReadCloser, error) { +func (client *Client) GetFileReader(filename string) (io.ReadCloser, error) { return getFile(client.path, filename) } // GetBranch implements RepoClient.GetBranch. -func (client *localDirClient) GetBranch(branch string) (*clients.BranchRef, error) { +func (client *Client) GetBranch(branch string) (*clients.BranchRef, error) { return nil, fmt.Errorf("ListBranches: %w", clients.ErrUnsupportedFeature) } // GetDefaultBranch implements RepoClient.GetDefaultBranch. -func (client *localDirClient) GetDefaultBranch() (*clients.BranchRef, error) { +func (client *Client) GetDefaultBranch() (*clients.BranchRef, error) { return nil, fmt.Errorf("GetDefaultBranch: %w", clients.ErrUnsupportedFeature) } // GetDefaultBranchName implements RepoClient.GetDefaultBranchName. -func (client *localDirClient) GetDefaultBranchName() (string, error) { +func (client *Client) GetDefaultBranchName() (string, error) { return "", fmt.Errorf("GetDefaultBranchName: %w", clients.ErrUnsupportedFeature) } // ListCommits implements RepoClient.ListCommits. -func (client *localDirClient) ListCommits() ([]clients.Commit, error) { +func (client *Client) ListCommits() ([]clients.Commit, error) { return nil, fmt.Errorf("ListCommits: %w", clients.ErrUnsupportedFeature) } // ListIssues implements RepoClient.ListIssues. -func (client *localDirClient) ListIssues() ([]clients.Issue, error) { +func (client *Client) ListIssues() ([]clients.Issue, error) { return nil, fmt.Errorf("ListIssues: %w", clients.ErrUnsupportedFeature) } // ListReleases implements RepoClient.ListReleases. -func (client *localDirClient) ListReleases() ([]clients.Release, error) { +func (client *Client) ListReleases() ([]clients.Release, error) { return nil, fmt.Errorf("ListReleases: %w", clients.ErrUnsupportedFeature) } // ListContributors implements RepoClient.ListContributors. -func (client *localDirClient) ListContributors() ([]clients.User, error) { +func (client *Client) ListContributors() ([]clients.User, error) { return nil, fmt.Errorf("ListContributors: %w", clients.ErrUnsupportedFeature) } // ListSuccessfulWorkflowRuns implements RepoClient.WorkflowRunsByFilename. -func (client *localDirClient) ListSuccessfulWorkflowRuns(filename string) ([]clients.WorkflowRun, error) { +func (client *Client) ListSuccessfulWorkflowRuns(filename string) ([]clients.WorkflowRun, error) { return nil, fmt.Errorf("ListSuccessfulWorkflowRuns: %w", clients.ErrUnsupportedFeature) } // ListCheckRunsForRef implements RepoClient.ListCheckRunsForRef. -func (client *localDirClient) ListCheckRunsForRef(ref string) ([]clients.CheckRun, error) { +func (client *Client) ListCheckRunsForRef(ref string) ([]clients.CheckRun, error) { return nil, fmt.Errorf("ListCheckRunsForRef: %w", clients.ErrUnsupportedFeature) } // ListStatuses implements RepoClient.ListStatuses. -func (client *localDirClient) ListStatuses(ref string) ([]clients.Status, error) { +func (client *Client) ListStatuses(ref string) ([]clients.Status, error) { return nil, fmt.Errorf("ListStatuses: %w", clients.ErrUnsupportedFeature) } // ListWebhooks implements RepoClient.ListWebhooks. -func (client *localDirClient) ListWebhooks() ([]clients.Webhook, error) { +func (client *Client) ListWebhooks() ([]clients.Webhook, error) { return nil, fmt.Errorf("ListWebhooks: %w", clients.ErrUnsupportedFeature) } // Search implements RepoClient.Search. -func (client *localDirClient) Search(request clients.SearchRequest) (clients.SearchResponse, error) { +func (client *Client) Search(request clients.SearchRequest) (clients.SearchResponse, error) { return clients.SearchResponse{}, fmt.Errorf("Search: %w", clients.ErrUnsupportedFeature) } // SearchCommits implements RepoClient.SearchCommits. -func (client *localDirClient) SearchCommits(request clients.SearchCommitsOptions) ([]clients.Commit, error) { +func (client *Client) SearchCommits(request clients.SearchCommitsOptions) ([]clients.Commit, error) { return nil, fmt.Errorf("Search: %w", clients.ErrUnsupportedFeature) } -func (client *localDirClient) Close() error { +func (client *Client) Close() error { return nil } // ListProgrammingLanguages implements RepoClient.ListProgrammingLanguages. // TODO: add ListProgrammingLanguages support for local directories. -func (client *localDirClient) ListProgrammingLanguages() ([]clients.Language, error) { - return nil, fmt.Errorf("ListProgrammingLanguages: %w", clients.ErrUnsupportedFeature) +func (client *Client) ListProgrammingLanguages() ([]clients.Language, error) { + // for now just return all programming languages + return []clients.Language{{Name: clients.All, NumLines: 1}}, nil } // ListLicenses implements RepoClient.ListLicenses. // TODO: add ListLicenses support for local directories. -func (client *localDirClient) ListLicenses() ([]clients.License, error) { +func (client *Client) ListLicenses() ([]clients.License, error) { return nil, fmt.Errorf("ListLicenses: %w", clients.ErrUnsupportedFeature) } -func (client *localDirClient) GetCreatedAt() (time.Time, error) { +func (client *Client) GetCreatedAt() (time.Time, error) { return time.Time{}, fmt.Errorf("GetCreatedAt: %w", clients.ErrUnsupportedFeature) } -func (client *localDirClient) GetOrgRepoClient(ctx context.Context) (clients.RepoClient, error) { +func (client *Client) GetOrgRepoClient(ctx context.Context) (clients.RepoClient, error) { return nil, fmt.Errorf("GetOrgRepoClient: %w", clients.ErrUnsupportedFeature) } // CreateLocalDirClient returns a client which implements RepoClient interface. func CreateLocalDirClient(ctx context.Context, logger *log.Logger) clients.RepoClient { - return &localDirClient{ + return &Client{ ctx: ctx, logger: logger, } diff --git a/cmd/root.go b/cmd/root.go index 39d2881518d..23626a634bb 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -118,9 +118,11 @@ func rootCmd(o *options.Options) error { } var requiredRequestTypes []checker.RequestType + // if local option not set add file based if o.Local != "" { requiredRequestTypes = append(requiredRequestTypes, checker.FileBased) } + // if commit option set to anything other than HEAD add commit based if !strings.EqualFold(o.Commit, clients.HeadSHA) { requiredRequestTypes = append(requiredRequestTypes, checker.CommitBased) } diff --git a/policy/policy_test.go b/policy/policy_test.go index 8d762d9bc61..bf2050943f4 100644 --- a/policy/policy_test.go +++ b/policy/policy_test.go @@ -333,7 +333,7 @@ func TestGetEnabled(t *testing.T) { name: "request types limit enabled checks", argsChecks: []string{}, requiredRequestTypes: []checker.RequestType{checker.FileBased, checker.CommitBased}, - expectedEnabledChecks: 5, // All checks which are FileBased and CommitBased + expectedEnabledChecks: 7, // All checks which are FileBased and CommitBased expectedError: false, }, {