Skip to content

Commit

Permalink
🐛 Bug fixing: Using the wrong URI to initialize the repo in Dependenc…
Browse files Browse the repository at this point in the history 
…ydiff (#2072)

* temp

* temp

* temp

* temp

* temp

* temp

* temp
  • Loading branch information
@aidenwang9867
aidenwang9867 authored Jul 19, 2022
1 parent 10681da commit 4bd1692
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 57 deletions.
76 changes: 39 additions & 37 deletions dependencydiff/dependencydiff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
34 changes: 14 additions & 20 deletions dependencydiff/dependencydiff_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 4bd1692

Please sign in to comment.