diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 21459d8ec50..3c18f0cb87e 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -17,12 +17,10 @@ package dependencydiff import ( "context" "fmt" - "path" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checks" "github.com/ossf/scorecard/v4/clients" - "github.com/ossf/scorecard/v4/clients/githubrepo" sce "github.com/ossf/scorecard/v4/errors" "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" @@ -69,8 +67,6 @@ func GetDependencyDiffResults( return nil, fmt.Errorf("error in fetchRawDependencyDiffData: %w", err) } - // Initialize the repo and client(s) corresponding to the checks to run. - err = initRepoAndClientByChecks(&dCtx) if err != nil { return nil, fmt.Errorf("error in initRepoAndClientByChecks: %w", err) } @@ -81,21 +77,20 @@ func GetDependencyDiffResults( return dCtx.results, nil } -func initRepoAndClientByChecks(dCtx *dependencydiffContext) error { +func initRepoAndClientByChecks(dCtx *dependencydiffContext, dSrcRepo string) error { repo, repoClient, ossFuzzClient, ciiClient, vulnsClient, err := checker.GetClients( - dCtx.ctx, path.Join(dCtx.ownerName, dCtx.repoName), "", dCtx.logger, + dCtx.ctx, dSrcRepo, "", dCtx.logger, ) if err != nil { - return fmt.Errorf("error creating the github repo: %w", err) + return fmt.Errorf("error getting the github repo and clients: %w", err) } - // If the caller doesn't specify the checks to run, run all checks and return all the clients. + dCtx.ghRepo = repo + dCtx.ghRepoClient = repoClient + // If the caller doesn't specify the checks to run, run all the checks and return all the clients. if dCtx.checkNamesToRun == nil || len(dCtx.checkNamesToRun) == 0 { - dCtx.ghRepo, dCtx.ghRepoClient, dCtx.ossFuzzClient, dCtx.ciiClient, dCtx.vulnsClient = - repo, repoClient, ossFuzzClient, ciiClient, vulnsClient + dCtx.ossFuzzClient, dCtx.ciiClient, dCtx.vulnsClient = ossFuzzClient, ciiClient, vulnsClient return nil } - dCtx.ghRepo = repo - dCtx.ghRepoClient = githubrepo.CreateGithubRepoClient(dCtx.ctx, dCtx.logger) for _, cn := range dCtx.checkNamesToRun { switch cn { case checks.CheckFuzzing: @@ -125,33 +120,40 @@ func getScorecardCheckResults(dCtx *dependencydiffContext) error { Version: d.Version, Name: d.Name, } + // Run the checks on all types if (1) the type is found in changeTypesToCheck or (2) no types are specified. + TypeFoundOrNoneGiven := dCtx.changeTypesToCheck[*d.ChangeType] || + (dCtx.changeTypesToCheck == nil || len(dCtx.changeTypesToCheck) == 0) // For now we skip those without source repo urls. // TODO (#2063): use the BigQuery dataset to supplement null source repo URLs to fetch the Scorecard results for them. - if d.SourceRepository != nil && *d.SourceRepository != "" { - if d.ChangeType != nil && (dCtx.changeTypesToCheck[*d.ChangeType] || dCtx.changeTypesToCheck == nil) { - // Run scorecard on those types of dependencies that the caller would like to check. - // If the input map changeTypesToCheck is empty, by default, we run checks for all valid types. - // TODO (#2064): use the Scorecare REST API to retrieve the Scorecard result statelessly. - scorecardResult, err := pkg.RunScorecards( - dCtx.ctx, - dCtx.ghRepo, - // TODO (#2065): In future versions, ideally, this should be - // the commitSHA corresponding to d.Version instead of HEAD. - clients.HeadSHA, - checksToRun, - dCtx.ghRepoClient, - dCtx.ossFuzzClient, - dCtx.ciiClient, - dCtx.vulnsClient, - ) - // 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. - if err != nil { - depCheckResult.ScorecardResultsWithError.Error = sce.WithMessage(sce.ErrScorecardInternal, - fmt.Sprintf("error running the scorecard checks: %v", err)) - } else { // Otherwise, we record the scorecard check results for this dependency. - depCheckResult.ScorecardResultsWithError.ScorecardResults = &scorecardResult - } + if d.SourceRepository != nil && TypeFoundOrNoneGiven { + // Initialize the repo and client(s) corresponding to the checks to run. + err = initRepoAndClientByChecks(dCtx, *d.SourceRepository) + if err != nil { + return fmt.Errorf("error init repo and clients: %w", err) + } + + // Run scorecard on those types of dependencies that the caller would like to check. + // If the input map changeTypesToCheck is empty, by default, we run the checks for all valid types. + // TODO (#2064): use the Scorecare REST API to retrieve the Scorecard result statelessly. + scorecardResult, err := pkg.RunScorecards( + dCtx.ctx, + dCtx.ghRepo, + // TODO (#2065): In future versions, ideally, this should be + // the commitSHA corresponding to d.Version instead of HEAD. + clients.HeadSHA, + checksToRun, + dCtx.ghRepoClient, + dCtx.ossFuzzClient, + dCtx.ciiClient, + dCtx.vulnsClient, + ) + // 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. + if err != nil { + depCheckResult.ScorecardResultsWithError.Error = sce.WithMessage(sce.ErrScorecardInternal, + fmt.Sprintf("error running the scorecard checks: %v", err)) + } else { // Otherwise, we record the scorecard check results for this dependency. + depCheckResult.ScorecardResultsWithError.ScorecardResults = &scorecardResult } } dCtx.results = append(dCtx.results, depCheckResult) diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index c0447519d45..768d3dc46a1 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -71,22 +71,25 @@ func Test_initRepoAndClientByChecks(t *testing.T) { t.Parallel() //nolint tests := []struct { - name string - dCtx dependencydiffContext - wantGhRepo, wantRepoClient, wantFuzzClient bool - wantVulnClient, wantCIIClient bool - wantErr bool + name string + dCtx dependencydiffContext + srcRepo string + wantRepoClient, wantFuzzClient bool + wantVulnClient, wantCIIClient bool + wantErr bool }{ { name: "error creating repo", dCtx: dependencydiffContext{ logger: log.NewLogger(log.InfoLevel), ctx: context.Background(), - ownerName: path.Join("host_not_exist.com", "owner_not_exist"), - repoName: "repo_not_exist", checkNamesToRun: []string{}, }, - wantGhRepo: false, + srcRepo: path.Join( + "host_not_exist.com", + "owner_not_exist", + "repo_not_exist", + ), wantRepoClient: false, wantFuzzClient: false, wantVulnClient: false, @@ -99,13 +102,9 @@ func Test_initRepoAndClientByChecks(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := initRepoAndClientByChecks(&tt.dCtx) + err := initRepoAndClientByChecks(&tt.dCtx, tt.srcRepo) if (err != nil) != tt.wantErr { - t.Errorf("initRepoAndClientByChecks() error = {%v}, want error: %v", err, tt.wantErr) - return - } - if (tt.dCtx.ghRepo != nil) != tt.wantGhRepo { - t.Errorf("init repo error, wantGhRepo: %v, got %v", tt.wantGhRepo, tt.dCtx.ghRepo) + t.Errorf("initClientByChecks() error = {%v}, want error: %v", err, tt.wantErr) return } if (tt.dCtx.ghRepoClient != nil) != tt.wantRepoClient { @@ -151,12 +150,7 @@ func Test_getScorecardCheckResults(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - err := initRepoAndClientByChecks(&tt.dCtx) - if err != nil { - t.Errorf("init repo and client error") - return - } - err = getScorecardCheckResults(&tt.dCtx) + err := getScorecardCheckResults(&tt.dCtx) if (err != nil) != tt.wantErr { t.Errorf("getScorecardCheckResults() error = {%v}, want error: %v", err, tt.wantErr) return