diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 3c18f0cb..c4012035 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -17,12 +17,13 @@ package dependencydiff import ( "context" "fmt" + "strings" "github.com/ossf/scorecard/v4/checker" "github.com/ossf/scorecard/v4/checks" "github.com/ossf/scorecard/v4/clients" sce "github.com/ossf/scorecard/v4/errors" - "github.com/ossf/scorecard/v4/log" + sclog "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" "github.com/ossf/scorecard/v4/policy" ) @@ -30,46 +31,54 @@ import ( // Depdiff is the exported name for dependency-diff. const Depdiff = "Dependency-diff" +// A private context struct used for GetDependencyCheckResults. type dependencydiffContext struct { - logger *log.Logger - ownerName, repoName, baseSHA, headSHA string - ctx context.Context - ghRepo clients.Repo - ghRepoClient clients.RepoClient - ossFuzzClient clients.RepoClient - vulnsClient clients.VulnerabilitiesClient - ciiClient clients.CIIBestPracticesClient - changeTypesToCheck map[pkg.ChangeType]bool - checkNamesToRun []string - dependencydiffs []dependency - results []pkg.DependencyCheckResult + logger *sclog.Logger + ownerName, repoName, base, head string + ctx context.Context + ghRepo clients.Repo + ghRepoClient clients.RepoClient + ossFuzzClient clients.RepoClient + vulnsClient clients.VulnerabilitiesClient + ciiClient clients.CIIBestPracticesClient + changeTypesToCheck map[pkg.ChangeType]bool + checkNamesToRun []string + dependencydiffs []dependency + results []pkg.DependencyCheckResult } // GetDependencyDiffResults gets dependency changes between two given code commits BASE and HEAD // along with the Scorecard check results of the dependencies, and returns a slice of DependencyCheckResult. -// TO use this API, an access token must be set following https://github.com/ossf/scorecard#authentication. +// TO use this API, an access token must be set. See https://github.com/ossf/scorecard#authentication. func GetDependencyDiffResults( - ctx context.Context, ownerName, repoName, baseSHA, headSHA string, scorecardChecksNames []string, - changeTypesToCheck map[pkg.ChangeType]bool) ([]pkg.DependencyCheckResult, error) { - // Fetch the raw dependency diffs. + ctx context.Context, + repoURI string, /* Use the format "ownerName/repoName" as the repo URI, such as "ossf/scorecard". */ + base, head string, /* Two code commits base and head, can use either SHAs or branch names. */ + checksToRun []string, /* A list of enabled check names to run. */ + changeTypesToCheck map[pkg.ChangeType]bool, /* A list of change types for which to surface scorecard results. */ +) ([]pkg.DependencyCheckResult, error) { + + logger := sclog.NewLogger(sclog.DefaultLevel) + ownerAndRepo := strings.Split(repoURI, "/") + if len(ownerAndRepo) != 2 { + return nil, fmt.Errorf("%w: repo uri input", errInvalid) + } + owner, repo := ownerAndRepo[0], ownerAndRepo[1] dCtx := dependencydiffContext{ - logger: log.NewLogger(log.InfoLevel), - ownerName: ownerName, - repoName: repoName, - baseSHA: baseSHA, - headSHA: headSHA, + logger: logger, + ownerName: owner, + repoName: repo, + base: base, + head: head, ctx: ctx, changeTypesToCheck: changeTypesToCheck, - checkNamesToRun: scorecardChecksNames, + checkNamesToRun: checksToRun, } + // Fetch the raw dependency diffs. This API will also handle error cases such as invalid base or head. err := fetchRawDependencyDiffData(&dCtx) if err != nil { return nil, fmt.Errorf("error in fetchRawDependencyDiffData: %w", err) } - - if err != nil { - return nil, fmt.Errorf("error in initRepoAndClientByChecks: %w", err) - } err = getScorecardCheckResults(&dCtx) if err != nil { return nil, fmt.Errorf("error getting scorecard check results: %w", err) @@ -150,10 +159,12 @@ func getScorecardCheckResults(dCtx *dependencydiffContext) error { // 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)) + wrappedErr := sce.WithMessage(sce.ErrScorecardInternal, + fmt.Sprintf("scorecard running failed for %s: %v", d.Name, err)) + dCtx.logger.Error(wrappedErr, "") + depCheckResult.ScorecardResultWithError.Error = wrappedErr } else { // Otherwise, we record the scorecard check results for this dependency. - depCheckResult.ScorecardResultsWithError.ScorecardResults = &scorecardResult + depCheckResult.ScorecardResultWithError.ScorecardResult = &scorecardResult } } dCtx.results = append(dCtx.results, depCheckResult) diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index 768d3dc4..4767ff21 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -40,8 +40,8 @@ func Test_fetchRawDependencyDiffData(t *testing.T) { ctx: context.Background(), ownerName: "no_such_owner", repoName: "repo_not_exist", - baseSHA: "base", - headSHA: clients.HeadSHA, + base: "main", + head: clients.HeadSHA, }, wantEmpty: true, wantErr: true, diff --git a/dependencydiff/errors.go b/dependencydiff/errors.go new file mode 100644 index 00000000..0bad31b0 --- /dev/null +++ b/dependencydiff/errors.go @@ -0,0 +1,22 @@ +// Copyright 2022 Security Scorecard Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dependencydiff + +import "errors" + +// static Errors for mapping +var ( + errInvalid = errors.New("invalid") +) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 519e5602..de313375 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -58,7 +58,7 @@ func fetchRawDependencyDiffData(dCtx *dependencydiffContext) error { req, err := ghClient.NewRequest( "GET", path.Join("repos", dCtx.ownerName, dCtx.repoName, - "dependency-graph", "compare", dCtx.baseSHA+"..."+dCtx.headSHA), + "dependency-graph", "compare", dCtx.base+"..."+dCtx.head), nil, ) if err != nil { diff --git a/e2e/dependencydiff_test.go b/e2e/dependencydiff_test.go index d0246293..fa388ba8 100644 --- a/e2e/dependencydiff_test.go +++ b/e2e/dependencydiff_test.go @@ -26,19 +26,16 @@ import ( ) const ( - OWNER = "ossf-tests" - REPO = "scorecard-depdiff" - BASE = "fd2a82b3b735fffbc2d782ed5f50301b879ecc51" - HEAD = "1989568f93e484f6a86f8b276b170e3d6962ce12" + repoURI = "ossf-tests/scorecard-depdiff" + base = "fd2a82b3b735fffbc2d782ed5f50301b879ecc51" + head = "1989568f93e484f6a86f8b276b170e3d6962ce12" ) var _ = Describe("E2E TEST:"+dependencydiff.Depdiff, func() { Context("E2E TEST:Validating use of the dependency-diff API", func() { It("Should return a slice of dependency-diff checking results", func() { ctx := context.Background() - ownerName, repoName := OWNER, REPO - baseSHA, headSHA := BASE, HEAD - scorecardChecksNames := []string{ + checksToRun := []string{ checks.CheckBranchProtection, } changeTypesToCheck := map[pkg.ChangeType]bool{ @@ -46,8 +43,9 @@ var _ = Describe("E2E TEST:"+dependencydiff.Depdiff, func() { } results, err := dependencydiff.GetDependencyDiffResults( ctx, - ownerName, repoName, baseSHA, headSHA, - scorecardChecksNames, + repoURI, + base, head, + checksToRun, changeTypesToCheck, ) Expect(err).Should(BeNil()) @@ -55,10 +53,7 @@ var _ = Describe("E2E TEST:"+dependencydiff.Depdiff, func() { }) It("Should return a valid empty result", func() { ctx := context.Background() - ownerName, repoName := OWNER, REPO - baseSHA, headSHA := BASE, BASE - - scorecardChecksNames := []string{ + checksToRun := []string{ checks.CheckBranchProtection, } changeTypesToCheck := map[pkg.ChangeType]bool{ @@ -66,8 +61,9 @@ var _ = Describe("E2E TEST:"+dependencydiff.Depdiff, func() { } results, err := dependencydiff.GetDependencyDiffResults( ctx, - ownerName, repoName, baseSHA, headSHA, - scorecardChecksNames, + repoURI, + base, base, + checksToRun, changeTypesToCheck, ) Expect(err).Should(BeNil()) @@ -75,17 +71,17 @@ var _ = Describe("E2E TEST:"+dependencydiff.Depdiff, func() { }) It("Should initialize clients corresponding to the checks to run and do not crash", func() { ctx := context.Background() - ownerName, repoName := OWNER, REPO - baseSHA, headSHA := BASE, HEAD - - scorecardChecksNames := []string{} + checksToRun := []string{ + checks.CheckFuzzing, + } changeTypesToCheck := map[pkg.ChangeType]bool{ pkg.Removed: true, } _, err := dependencydiff.GetDependencyDiffResults( ctx, - ownerName, repoName, baseSHA, headSHA, - scorecardChecksNames, + repoURI, + base, head, + checksToRun, changeTypesToCheck, ) Expect(err).Should(BeNil()) diff --git a/pkg/dependencydiff_result.go b/pkg/dependencydiff_result.go index 5208d981..74b5b834 100644 --- a/pkg/dependencydiff_result.go +++ b/pkg/dependencydiff_result.go @@ -44,40 +44,41 @@ func (ct *ChangeType) IsValid() bool { } } -// ScorecardResultsWithError is used for the dependency-diff module to record scorecard results and their errors. -type ScorecardResultsWithError struct { - // ScorecardResults is the scorecard result for the dependency repo. - ScorecardResults *ScorecardResult `json:"scorecardResults"` +// ScorecardResultWithError is used for the dependency-diff module to record the scorecard result +// and a potential error field if the Scorecard run fails. +type ScorecardResultWithError struct { + // ScorecardResult is the scorecard result for the dependency repo. + ScorecardResult *ScorecardResult // Error is an error returned when running the scorecard checks. A nil Error indicates the run succeeded. - Error error `json:"scorecardRunTimeError"` + Error error } // DependencyCheckResult is the dependency structure used in the returned results. type DependencyCheckResult struct { // ChangeType indicates whether the dependency is added, updated, or removed. - ChangeType *ChangeType `json:"changeType"` + ChangeType *ChangeType // Package URL is a short link for a package. - PackageURL *string `json:"packageUrl"` + PackageURL *string // SourceRepository is the source repository URL of the dependency. - SourceRepository *string `json:"sourceRepository"` + SourceRepository *string // ManifestPath is the path of the manifest file of the dependency, such as go.mod for Go. - ManifestPath *string `json:"manifestPath"` + ManifestPath *string // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. - Ecosystem *string `json:"ecosystem"` + Ecosystem *string // Version is the package version of the dependency. - Version *string `json:"version"` + Version *string - // ScorecardResultsWithError is the scorecard checking results of the dependency. - ScorecardResultsWithError ScorecardResultsWithError `json:"scorecardResultsWithError"` + // ScorecardResultWithError is the scorecard checking results of the dependency. + ScorecardResultWithError ScorecardResultWithError // Name is the name of the dependency. - Name string `json:"name"` + Name string } // AsJSON for DependencyCheckResult exports the DependencyCheckResult as a JSON object.