From 32b59637667c9abb7c4d2ecd9c1293634de2d80f Mon Sep 17 00:00:00 2001 From: Raghav Kaul <8695110+raghavkaul@users.noreply.github.com> Date: Mon, 13 May 2024 11:59:46 -0400 Subject: [PATCH] =?UTF-8?q?=20=E2=9A=A0=EF=B8=8F=20Add=20projectclient=20t?= =?UTF-8?q?o=20cli=20and=20cron,=20update=20runscorecard=20(#4096)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Raghav Kaul --- attestor/command/check.go | 3 ++- checker/check_request.go | 2 ++ checker/client.go | 5 +++++ checker/client_test.go | 27 ++++++++++++++---------- cmd/internal/scdiff/app/runner/runner.go | 6 +++++- cmd/root.go | 3 ++- cmd/serve.go | 4 +++- cron/internal/worker/main.go | 8 +++++-- dependencydiff/dependencydiff.go | 7 +++++- e2e/attestor_policy_test.go | 4 ++-- pkg/scorecard.go | 6 ++++++ pkg/scorecard_e2e_test.go | 8 +++---- pkg/scorecard_test.go | 6 ++++-- 13 files changed, 63 insertions(+), 26 deletions(-) diff --git a/attestor/command/check.go b/attestor/command/check.go index 71650d06..9b715f14 100644 --- a/attestor/command/check.go +++ b/attestor/command/check.go @@ -77,7 +77,7 @@ func RunCheckWithParams(repoURL, commitSHA, policyPath string) (policy.PolicyRes } } - repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, err := checker.GetClients( + repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients( ctx, repoURL, "", logger) if err != nil { return policy.Fail, fmt.Errorf("couldn't set up clients: %w", err) @@ -117,6 +117,7 @@ func RunCheckWithParams(repoURL, commitSHA, policyPath string) (policy.PolicyRes ossFuzzRepoClient, ciiClient, vulnsClient, + projectClient, ) if err != nil { return policy.Fail, fmt.Errorf("RunScorecard: %w", err) diff --git a/checker/check_request.go b/checker/check_request.go index b04d4050..6bd8be59 100644 --- a/checker/check_request.go +++ b/checker/check_request.go @@ -18,6 +18,7 @@ import ( "context" "github.com/ossf/scorecard/v5/clients" + "github.com/ossf/scorecard/v5/internal/packageclient" ) // CheckRequest struct encapsulates all data to be passed into a CheckFn. @@ -29,6 +30,7 @@ type CheckRequest struct { Dlogger DetailLogger Repo clients.Repo VulnerabilitiesClient clients.VulnerabilitiesClient + ProjectClient packageclient.ProjectPackageClient // UPGRADEv6: return raw results instead of scores. RawResults *RawResults RequiredTypes []RequestType diff --git a/checker/client.go b/checker/client.go index 22c9aff3..6e1829d7 100644 --- a/checker/client.go +++ b/checker/client.go @@ -23,6 +23,7 @@ import ( glrepo "github.com/ossf/scorecard/v5/clients/gitlabrepo" "github.com/ossf/scorecard/v5/clients/localdir" "github.com/ossf/scorecard/v5/clients/ossfuzz" + "github.com/ossf/scorecard/v5/internal/packageclient" "github.com/ossf/scorecard/v5/log" ) @@ -34,6 +35,7 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge clients.RepoClient, // ossFuzzClient clients.CIIBestPracticesClient, // ciiClient clients.VulnerabilitiesClient, // vulnClient + packageclient.ProjectPackageClient, // projectClient error, ) { var repo clients.Repo @@ -50,6 +52,7 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge nil, /*ossFuzzClient*/ nil, /*ciiClient*/ clients.DefaultVulnerabilitiesClient(), /*vulnClient*/ + nil, retErr } @@ -68,6 +71,7 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge nil, nil, nil, + packageclient.CreateDepsDevClient(), fmt.Errorf("error making github repo: %w", makeRepoError) } repoClient = ghrepo.CreateGithubRepoClient(ctx, logger) @@ -78,5 +82,6 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge ossfuzz.CreateOSSFuzzClient(ossfuzz.StatusURL), /*ossFuzzClient*/ clients.DefaultCIIBestPracticesClient(), /*ciiClient*/ clients.DefaultVulnerabilitiesClient(), /*vulnClient*/ + packageclient.CreateDepsDevClient(), nil } diff --git a/checker/client_test.go b/checker/client_test.go index 168db20e..d4e4e49e 100644 --- a/checker/client_test.go +++ b/checker/client_test.go @@ -20,6 +20,7 @@ import ( "github.com/ossf/scorecard/v5/log" ) +//nolint:gocognit func TestGetClients(t *testing.T) { type args struct { //nolint:govet ctx context.Context @@ -28,16 +29,17 @@ func TestGetClients(t *testing.T) { logger *log.Logger } tests := []struct { //nolint:govet - name string - args args - shouldOSSFuzzBeNil bool - shouldRepoClientBeNil bool - shouldVulnClientBeNil bool - shouldRepoBeNil bool - shouldCIIBeNil bool - wantErr bool - experimental bool - isGhHost bool + name string + args args + shouldOSSFuzzBeNil bool + shouldRepoClientBeNil bool + shouldVulnClientBeNil bool + shouldRepoBeNil bool + shouldCIIBeNil bool + shouldProjectClientBeNil bool + wantErr bool + experimental bool + isGhHost bool }{ { name: "localURI is not empty", @@ -105,7 +107,7 @@ func TestGetClients(t *testing.T) { t.Setenv("GH_HOST", "github.corp.com") t.Setenv("GH_TOKEN", "PAT") } - got, repoClient, ossFuzzClient, ciiClient, vulnsClient, err := GetClients(tt.args.ctx, tt.args.repoURI, tt.args.localURI, tt.args.logger) + got, repoClient, ossFuzzClient, ciiClient, vulnsClient, projectClient, err := GetClients(tt.args.ctx, tt.args.repoURI, tt.args.localURI, tt.args.logger) if (err != nil) != tt.wantErr { t.Fatalf("GetClients() error = %v, wantErr %v", err, tt.wantErr) } @@ -124,6 +126,9 @@ func TestGetClients(t *testing.T) { if vulnsClient != nil && tt.shouldVulnClientBeNil { t.Errorf("GetClients() vulnsClient = %v", vulnsClient) } + if projectClient != nil && tt.shouldProjectClientBeNil { + t.Errorf("GetClients() projectClient = %v", projectClient) + } }) } } diff --git a/cmd/internal/scdiff/app/runner/runner.go b/cmd/internal/scdiff/app/runner/runner.go index 6c8bf3f9..644a89fd 100644 --- a/cmd/internal/scdiff/app/runner/runner.go +++ b/cmd/internal/scdiff/app/runner/runner.go @@ -26,6 +26,7 @@ import ( "github.com/ossf/scorecard/v5/clients/gitlabrepo" "github.com/ossf/scorecard/v5/clients/ossfuzz" sce "github.com/ossf/scorecard/v5/errors" + "github.com/ossf/scorecard/v5/internal/packageclient" "github.com/ossf/scorecard/v5/log" "github.com/ossf/scorecard/v5/pkg" ) @@ -45,6 +46,7 @@ type Runner struct { ossFuzz clients.RepoClient cii clients.CIIBestPracticesClient vuln clients.VulnerabilitiesClient + deps packageclient.ProjectPackageClient } // Creates a Runner which will run the listed checks. If no checks are provided, all will run. @@ -79,7 +81,9 @@ func (r *Runner) Run(repoURI string) (pkg.ScorecardResult, error) { if err != nil { return pkg.ScorecardResult{}, err } - return pkg.RunScorecard(r.ctx, repo, commit, commitDepth, r.enabledChecks, repoClient, r.ossFuzz, r.cii, r.vuln) + return pkg.RunScorecard( + r.ctx, repo, commit, commitDepth, r.enabledChecks, repoClient, r.ossFuzz, r.cii, r.vuln, r.deps, + ) } // logs only if logger is set. diff --git a/cmd/root.go b/cmd/root.go index 6314e0a5..8b01f216 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -93,7 +93,7 @@ func rootCmd(o *options.Options) error { ctx := context.Background() logger := sclog.NewLogger(sclog.ParseLevel(o.LogLevel)) - repoURI, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, err := checker.GetClients( + repoURI, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients( ctx, o.Repo, o.Local, logger) // MODIFIED if err != nil { return fmt.Errorf("GetClients: %w", err) @@ -142,6 +142,7 @@ func rootCmd(o *options.Options) error { ossFuzzRepoClient, ciiClient, vulnsClient, + projectClient, ) if err != nil { return fmt.Errorf("RunScorecard: %w", err) diff --git a/cmd/serve.go b/cmd/serve.go index 87b2a8e5..9830fd46 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -27,6 +27,7 @@ import ( "github.com/ossf/scorecard/v5/clients" "github.com/ossf/scorecard/v5/clients/githubrepo" "github.com/ossf/scorecard/v5/clients/ossfuzz" + "github.com/ossf/scorecard/v5/internal/packageclient" "github.com/ossf/scorecard/v5/log" "github.com/ossf/scorecard/v5/options" "github.com/ossf/scorecard/v5/pkg" @@ -69,10 +70,11 @@ func serveCmd(o *options.Options) *cobra.Command { } defer ossFuzzRepoClient.Close() ciiClient := clients.DefaultCIIBestPracticesClient() + projectClient := packageclient.CreateDepsDevClient() checksToRun := checks.GetAll() repoResult, err := pkg.RunScorecard( ctx, repo, clients.HeadSHA /*commitSHA*/, o.CommitDepth, checksToRun, repoClient, - ossFuzzRepoClient, ciiClient, vulnsClient) + ossFuzzRepoClient, ciiClient, vulnsClient, projectClient) if err != nil { logger.Error(err, "running enabled scorecard checks on repo") rw.WriteHeader(http.StatusInternalServerError) diff --git a/cron/internal/worker/main.go b/cron/internal/worker/main.go index d71aa4d6..8bda9152 100644 --- a/cron/internal/worker/main.go +++ b/cron/internal/worker/main.go @@ -40,6 +40,7 @@ import ( "github.com/ossf/scorecard/v5/cron/worker" docs "github.com/ossf/scorecard/v5/docs/checks" sce "github.com/ossf/scorecard/v5/errors" + "github.com/ossf/scorecard/v5/internal/packageclient" "github.com/ossf/scorecard/v5/log" "github.com/ossf/scorecard/v5/pkg" "github.com/ossf/scorecard/v5/policy" @@ -89,6 +90,7 @@ type ScorecardWorker struct { ciiClient clients.CIIBestPracticesClient ossFuzzRepoClient clients.RepoClient vulnsClient clients.VulnerabilitiesClient + projectClient packageclient.ProjectPackageClient apiBucketURL string rawBucketURL string blacklistedChecks []string @@ -156,7 +158,8 @@ func (sw *ScorecardWorker) Close() { func (sw *ScorecardWorker) Process(ctx context.Context, req *data.ScorecardBatchRequest, bucketURL string) error { return processRequest(ctx, req, sw.blacklistedChecks, bucketURL, sw.rawBucketURL, sw.apiBucketURL, - sw.checkDocs, sw.githubClient, sw.gitlabClient, sw.ossFuzzRepoClient, sw.ciiClient, sw.vulnsClient, sw.logger) + sw.checkDocs, sw.githubClient, sw.gitlabClient, sw.ossFuzzRepoClient, sw.ciiClient, + sw.vulnsClient, sw.projectClient, sw.logger) } func (sw *ScorecardWorker) PostProcess() { @@ -171,6 +174,7 @@ func processRequest(ctx context.Context, githubClient, gitlabClient clients.RepoClient, ossFuzzRepoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient, vulnsClient clients.VulnerabilitiesClient, + projectClient packageclient.ProjectPackageClient, logger *log.Logger, ) error { filename := worker.ResultFilename(batchRequest) @@ -210,7 +214,7 @@ func processRequest(ctx context.Context, } result, err := pkg.RunScorecard(ctx, repo, commitSHA, 0, checksToRun, - repoClient, ossFuzzRepoClient, ciiClient, vulnsClient) + repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient) if errors.Is(err, sce.ErrRepoUnreachable) { // Not accessible repo - continue. continue diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 99a74739..3eb28f83 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -24,6 +24,7 @@ import ( "github.com/ossf/scorecard/v5/checks" "github.com/ossf/scorecard/v5/clients" sce "github.com/ossf/scorecard/v5/errors" + "github.com/ossf/scorecard/v5/internal/packageclient" sclog "github.com/ossf/scorecard/v5/log" "github.com/ossf/scorecard/v5/pkg" "github.com/ossf/scorecard/v5/policy" @@ -44,6 +45,7 @@ type dependencydiffContext struct { ossFuzzClient clients.RepoClient vulnsClient clients.VulnerabilitiesClient ciiClient clients.CIIBestPracticesClient + projectClient packageclient.ProjectPackageClient changeTypesToCheck []string checkNamesToRun []string dependencydiffs []dependency @@ -95,7 +97,7 @@ func GetDependencyDiffResults( } func initRepoAndClientByChecks(dCtx *dependencydiffContext, dSrcRepo string) error { - repo, repoClient, ossFuzzClient, ciiClient, vulnsClient, err := checker.GetClients( + repo, repoClient, ossFuzzClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients( dCtx.ctx, dSrcRepo, "", dCtx.logger) if err != nil { return fmt.Errorf("error getting the github repo and clients: %w", err) @@ -115,6 +117,8 @@ func initRepoAndClientByChecks(dCtx *dependencydiffContext, dSrcRepo string) err dCtx.ciiClient = ciiClient case checks.CheckVulnerabilities: dCtx.vulnsClient = vulnsClient + case checks.CheckSignedReleases: + dCtx.projectClient = projectClient } } return nil @@ -171,6 +175,7 @@ func getScorecardCheckResults(dCtx *dependencydiffContext) error { dCtx.ossFuzzClient, dCtx.ciiClient, dCtx.vulnsClient, + dCtx.projectClient, ) // If the run fails, we leave the current dependency scorecard result empty and record the error // rather than letting the entire API return nil since we still expect results for other dependencies. diff --git a/e2e/attestor_policy_test.go b/e2e/attestor_policy_test.go index 6d8e1700..c3c0f2b1 100644 --- a/e2e/attestor_policy_test.go +++ b/e2e/attestor_policy_test.go @@ -242,11 +242,11 @@ func getScorecardResult(repoURL string) (pkg.ScorecardResult, error) { Fn: checks.PinningDependencies, }, } - repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, err := checker.GetClients( + repo, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient, err := checker.GetClients( ctx, repoURL, "", logger) if err != nil { return pkg.ScorecardResult{}, fmt.Errorf("couldn't set up clients: %w", err) } - return pkg.RunScorecard(ctx, repo, clients.HeadSHA, 0, enabledChecks, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient) + return pkg.RunScorecard(ctx, repo, clients.HeadSHA, 0, enabledChecks, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, projectClient) } diff --git a/pkg/scorecard.go b/pkg/scorecard.go index 03640349..7989ac6d 100644 --- a/pkg/scorecard.go +++ b/pkg/scorecard.go @@ -31,6 +31,7 @@ import ( "github.com/ossf/scorecard/v5/config" sce "github.com/ossf/scorecard/v5/errors" "github.com/ossf/scorecard/v5/finding" + "github.com/ossf/scorecard/v5/internal/packageclient" proberegistration "github.com/ossf/scorecard/v5/internal/probes" sclog "github.com/ossf/scorecard/v5/log" "github.com/ossf/scorecard/v5/options" @@ -91,6 +92,7 @@ func runScorecard(ctx context.Context, ossFuzzRepoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient, vulnsClient clients.VulnerabilitiesClient, + projectClient packageclient.ProjectPackageClient, ) (ScorecardResult, error) { if err := repoClient.InitRepo(repo, commitSHA, commitDepth); err != nil { // No need to call sce.WithMessage() since InitRepo will do that for us. @@ -232,6 +234,7 @@ func RunScorecard(ctx context.Context, ossFuzzRepoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient, vulnsClient clients.VulnerabilitiesClient, + projectClient packageclient.ProjectPackageClient, ) (ScorecardResult, error) { return runScorecard(ctx, repo, @@ -243,6 +246,7 @@ func RunScorecard(ctx context.Context, ossFuzzRepoClient, ciiClient, vulnsClient, + projectClient, ) } @@ -257,6 +261,7 @@ func ExperimentalRunProbes(ctx context.Context, ossFuzzRepoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient, vulnsClient clients.VulnerabilitiesClient, + projectClient packageclient.ProjectPackageClient, ) (ScorecardResult, error) { return runScorecard(ctx, repo, @@ -268,5 +273,6 @@ func ExperimentalRunProbes(ctx context.Context, ossFuzzRepoClient, ciiClient, vulnsClient, + projectClient, ) } diff --git a/pkg/scorecard_e2e_test.go b/pkg/scorecard_e2e_test.go index e480e583..f2357dc9 100644 --- a/pkg/scorecard_e2e_test.go +++ b/pkg/scorecard_e2e_test.go @@ -108,20 +108,20 @@ var _ = Describe("E2E TEST: RunScorecard with re-used repoClient", func() { isolatedLogger := sclog.NewLogger(sclog.DebugLevel) lastRepo := repos[len(repos)-1] - repo, rc, ofrc, cc, vc, err := checker.GetClients(ctx, lastRepo, "", isolatedLogger) + repo, rc, ofrc, cc, vc, dc, err := checker.GetClients(ctx, lastRepo, "", isolatedLogger) Expect(err).Should(BeNil()) - isolatedResult, err := RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc, ofrc, cc, vc) + isolatedResult, err := RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc, ofrc, cc, vc, dc) Expect(err).Should(BeNil()) logger := sclog.NewLogger(sclog.DebugLevel) - _, rc2, ofrc2, cc2, vc2, err := checker.GetClients(ctx, repos[0], "", logger) + _, rc2, ofrc2, cc2, vc2, dc2, err := checker.GetClients(ctx, repos[0], "", logger) Expect(err).Should(BeNil()) var sharedResult ScorecardResult for i := range repos { repo, err = githubrepo.MakeGithubRepo(repos[i]) Expect(err).Should(BeNil()) - sharedResult, err = RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc2, ofrc2, cc2, vc2) + sharedResult, err = RunScorecard(ctx, repo, clients.HeadSHA, 0, allChecks, rc2, ofrc2, cc2, vc2, dc2) Expect(err).Should(BeNil()) } diff --git a/pkg/scorecard_test.go b/pkg/scorecard_test.go index 254ea846..da3bb651 100644 --- a/pkg/scorecard_test.go +++ b/pkg/scorecard_test.go @@ -179,7 +179,7 @@ func TestRunScorecard(t *testing.T) { }, nil }) defer ctrl.Finish() - got, err := RunScorecard(context.Background(), repo, tt.args.commitSHA, 0, nil, mockRepoClient, nil, nil, nil) + got, err := RunScorecard(context.Background(), repo, tt.args.commitSHA, 0, nil, mockRepoClient, nil, nil, nil, nil) if (err != nil) != tt.wantErr { t.Errorf("RunScorecard() error = %v, wantErr %v", err, tt.wantErr) return @@ -315,7 +315,9 @@ func TestExperimentalRunProbes(t *testing.T) { mockRepoClient, nil, nil, - nil) + nil, + nil, + ) if (err != nil) != tt.wantErr { t.Errorf("RunScorecard() error = %v, wantErr %v", err, tt.wantErr) return