From d043b138fe1c5707143b7ab91b183801fbaeae25 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Fri, 8 Jul 2022 12:19:15 -0700 Subject: [PATCH 01/49] temp --- cmd/depdiff/dependencies.go | 64 ++++++++++++++++++++++++++ cmd/depdiff/errors.go | 13 ++++++ cmd/depdiff/scorecard_results.go | 7 +++ cmd/depdiff/vulnerabilities.go | 78 ++++++++++++++++++++++++++++++++ 4 files changed, 162 insertions(+) create mode 100644 cmd/depdiff/dependencies.go create mode 100644 cmd/depdiff/errors.go create mode 100644 cmd/depdiff/scorecard_results.go create mode 100644 cmd/depdiff/vulnerabilities.go diff --git a/cmd/depdiff/dependencies.go b/cmd/depdiff/dependencies.go new file mode 100644 index 00000000000..ab2940cafba --- /dev/null +++ b/cmd/depdiff/dependencies.go @@ -0,0 +1,64 @@ +package depdiff + +// ChangeType is the change type (added, updated, removed) of a dependency. +type ChangeType string + +const ( + Added ChangeType = "added" + Updated ChangeType = "updated" + Removed ChangeType = "removed" +) + +// IsValid determines if a ChangeType is valid. +func (ct *ChangeType) IsValid() bool { + switch *ct { + case Added, Updated, Removed: + return true + default: + return false + } +} + +// Dependency is a dependency. +type Dependency struct { + // // IsDirect suggests if the dependency is a direct dependency of a code commit. + // TODO: IsDirect remains a future feature since the current GitHub Dependency Review API + // mixes up direct and indirect dependencies in manifest files of different ecosystems. + IsDirect bool + + // ChangeType indicates whether the dependency is added or removed. + ChangeType ChangeType `json:"change_type"` + + // ManifestFileName is the name of the manifest file of the dependency, such as go.mod for Go. + ManifestFileName string `json:"manifest"` + + // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. + Ecosystem string `json:"ecosystem" bigquery:"System"` + + // Name is the name of the dependency. + Name string `json:"name" bigquery:"Name"` + + // Version is the package version of the dependency. + Version string `json:"version" bigquery:"Version"` + + // Package URL is a short link for a package. + PackageURL string `json:"package_url"` + + // License is the license of the dependency. + License string `json:"license"` + + // SrcRepoURL is the source repository URL of the dependency. + SrcRepoURL string `json:"source_repository_url"` + + // ScResults is the Scorecard scanning result of the dependency package repository. + ScResults ScorecardResult `json:"scorecard_results"` + + // Vulnerabilities is a list of Vulnerability. + Vulnerabilities []Vulnerability `json:"vulnerabilities"` + + // Dependencies is the list of dependencies of the current dependency, + // e.g. indirect (transitive) dependencies. + // TODO: this is not a version-zero feature, and will be used to analyze transitive + // dependencies in future versions. + Dependencies []Dependency `json:"dependencies"` +} diff --git a/cmd/depdiff/errors.go b/cmd/depdiff/errors.go new file mode 100644 index 00000000000..7a80d9d7229 --- /dev/null +++ b/cmd/depdiff/errors.go @@ -0,0 +1,13 @@ +package depdiff + +import ( + "errors" +) + +var ( + // ErrInvalidDepDiffFormat indicates the specified dependency diff output format is not valid. + ErrInvalidDepDiffFormat = errors.New("invalid depdiff format") + + // ErrInvalidDepDiffFormat indicates the specified dependency diff output format is not valid. + ErrMarshalDepDiffToJSON = errors.New("error marshal results to JSON") +) diff --git a/cmd/depdiff/scorecard_results.go b/cmd/depdiff/scorecard_results.go new file mode 100644 index 00000000000..7761b2564bc --- /dev/null +++ b/cmd/depdiff/scorecard_results.go @@ -0,0 +1,7 @@ +package depdiff + +// ScorecardResult is the Scorecard scanning result of a repository. +type ScorecardResult struct { + // AggregateScore is the Scorecard aggregate score (0-10) of the dependency. + AggregateScore float32 `json:"score"` +} diff --git a/cmd/depdiff/vulnerabilities.go b/cmd/depdiff/vulnerabilities.go new file mode 100644 index 00000000000..a45ba10f008 --- /dev/null +++ b/cmd/depdiff/vulnerabilities.go @@ -0,0 +1,78 @@ +package depdiff + +import ( + "time" +) + +// SeverityLevel is the level of severity of a vulnerability. +type SeverityLevel string + +const ( + Critical SeverityLevel = "CRITICAL" + High SeverityLevel = "HIGH" + Medium SeverityLevel = "MEDIUM" + Moderate SeverityLevel = "MODERATE" + Low SeverityLevel = "LOW" + None SeverityLevel = "NONE" + Unknown SeverityLevel = "UNKNOWN" +) + +// IsValid determines if a SeverityLevel is valid. +func (sl *SeverityLevel) IsValid() bool { + switch *sl { + case Critical, High, Medium, Moderate, Low, None, Unknown: + return true + default: + return false + } +} + +// Source is an authoritative source of a vulnerability. +type Source string + +const ( + GHSA Source = "GHSA" + NSWG Source = "NSWG" + OSV Source = "OSV" +) + +// IsValid determines if a Source is valid. +func (src *Source) IsValid() bool { + switch *src { + case GHSA, NSWG, OSV: + return true + default: + return false + } +} + +// Vulnerability is a security vulnerability of a dependency. +type Vulnerability struct { + // Source is the source of a vulnerability. + Source string `bigquery:"Source"` + + // ID is the identifier of a vulnerability. + ID string `json:"advisory_ghsa_id" bigquery:"SourceID"` + + // SourceURL is the source URL of a vulnerability. + SourceURL string `json:"advisory_url" bigquery:"SourceURL"` + + // Title is the text summary of a vulnerability. + Title string `json:"advisory_summary" bigquery:"Title"` + + // Description is a long text paragraph of a vulnerability. + Description string `json:"description" bigquery:"Description"` + + // Score is the score of a vulnerability given by an authoritative src. + // TODO: this is not a version-zero property and will be included in future versions. + // Score bigquery.NullFloat64 `bigquery:"Score"` + + // GitHubSeverity is the severity level of a vulnerability determined by GitHub. + GitHubSeverity SeverityLevel `json:"github_severity" bigquery:"GitHubSeverity"` + + // ReferenceURLs include all URLs that are related to a vulnerability. + ReferenceURLs []string `json:"reference_urls" bigquery:"ReferenceURLs"` + + // DisclosedTime is the time when a vulenrability is publicly disclosed. + DisclosedTime time.Time `json:"disclosed_time" bigquery:"Disclosed"` +} From 5f95398260bedfa473581753c0d8096935153032 Mon Sep 17 00:00:00 2001 From: Aiden Wang <54022336+aidenwang9867@users.noreply.github.com> Date: Fri, 8 Jul 2022 12:36:38 -0700 Subject: [PATCH 02/49] Update dependencies.go --- cmd/depdiff/dependencies.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cmd/depdiff/dependencies.go b/cmd/depdiff/dependencies.go index ab2940cafba..12ea643d3b5 100644 --- a/cmd/depdiff/dependencies.go +++ b/cmd/depdiff/dependencies.go @@ -1,3 +1,17 @@ +// 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 depdiff // ChangeType is the change type (added, updated, removed) of a dependency. From c29a8411987294a467f33acc1bad2f3e49a7f298 Mon Sep 17 00:00:00 2001 From: Aiden Wang <54022336+aidenwang9867@users.noreply.github.com> Date: Fri, 8 Jul 2022 12:36:49 -0700 Subject: [PATCH 03/49] Update errors.go --- cmd/depdiff/errors.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cmd/depdiff/errors.go b/cmd/depdiff/errors.go index 7a80d9d7229..daec146ef73 100644 --- a/cmd/depdiff/errors.go +++ b/cmd/depdiff/errors.go @@ -1,3 +1,17 @@ +// 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 depdiff import ( From 572bef56b313a2ee0508b92244192375a068043a Mon Sep 17 00:00:00 2001 From: Aiden Wang <54022336+aidenwang9867@users.noreply.github.com> Date: Fri, 8 Jul 2022 12:36:58 -0700 Subject: [PATCH 04/49] Update scorecard_results.go --- cmd/depdiff/scorecard_results.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cmd/depdiff/scorecard_results.go b/cmd/depdiff/scorecard_results.go index 7761b2564bc..d30713263c4 100644 --- a/cmd/depdiff/scorecard_results.go +++ b/cmd/depdiff/scorecard_results.go @@ -1,3 +1,17 @@ +// 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 depdiff // ScorecardResult is the Scorecard scanning result of a repository. From dc50937a3003e9084410b9ce28a10a25e28859b1 Mon Sep 17 00:00:00 2001 From: Aiden Wang <54022336+aidenwang9867@users.noreply.github.com> Date: Fri, 8 Jul 2022 12:37:07 -0700 Subject: [PATCH 05/49] Update vulnerabilities.go --- cmd/depdiff/vulnerabilities.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cmd/depdiff/vulnerabilities.go b/cmd/depdiff/vulnerabilities.go index a45ba10f008..646c63aeea0 100644 --- a/cmd/depdiff/vulnerabilities.go +++ b/cmd/depdiff/vulnerabilities.go @@ -1,3 +1,17 @@ +// 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 depdiff import ( From 4e902065c2886d78aea1e473414ccd2ae469b4f6 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Fri, 8 Jul 2022 15:38:26 -0700 Subject: [PATCH 06/49] save --- cmd/depdiff/errors.go | 27 ------------------- cmd/depdiff/scorecard_results.go | 21 --------------- .../check-depdiff}/dependencies.go | 27 +++++-------------- .../check-depdiff}/vulnerabilities.go | 0 4 files changed, 6 insertions(+), 69 deletions(-) delete mode 100644 cmd/depdiff/errors.go delete mode 100644 cmd/depdiff/scorecard_results.go rename {cmd/depdiff => pkg/check-depdiff}/dependencies.go (66%) rename {cmd/depdiff => pkg/check-depdiff}/vulnerabilities.go (100%) diff --git a/cmd/depdiff/errors.go b/cmd/depdiff/errors.go deleted file mode 100644 index daec146ef73..00000000000 --- a/cmd/depdiff/errors.go +++ /dev/null @@ -1,27 +0,0 @@ -// 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 depdiff - -import ( - "errors" -) - -var ( - // ErrInvalidDepDiffFormat indicates the specified dependency diff output format is not valid. - ErrInvalidDepDiffFormat = errors.New("invalid depdiff format") - - // ErrInvalidDepDiffFormat indicates the specified dependency diff output format is not valid. - ErrMarshalDepDiffToJSON = errors.New("error marshal results to JSON") -) diff --git a/cmd/depdiff/scorecard_results.go b/cmd/depdiff/scorecard_results.go deleted file mode 100644 index d30713263c4..00000000000 --- a/cmd/depdiff/scorecard_results.go +++ /dev/null @@ -1,21 +0,0 @@ -// 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 depdiff - -// ScorecardResult is the Scorecard scanning result of a repository. -type ScorecardResult struct { - // AggregateScore is the Scorecard aggregate score (0-10) of the dependency. - AggregateScore float32 `json:"score"` -} diff --git a/cmd/depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go similarity index 66% rename from cmd/depdiff/dependencies.go rename to pkg/check-depdiff/dependencies.go index 12ea643d3b5..2ec62820dc1 100644 --- a/cmd/depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -40,39 +40,24 @@ type Dependency struct { // mixes up direct and indirect dependencies in manifest files of different ecosystems. IsDirect bool - // ChangeType indicates whether the dependency is added or removed. + // ChangeType indicates whether the dependency is added, updated, or removed. ChangeType ChangeType `json:"change_type"` // ManifestFileName is the name of the manifest file of the dependency, such as go.mod for Go. ManifestFileName string `json:"manifest"` // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. - Ecosystem string `json:"ecosystem" bigquery:"System"` + Ecosystem string `json:"ecosystem"` // Name is the name of the dependency. - Name string `json:"name" bigquery:"Name"` + Name string `json:"name"` // Version is the package version of the dependency. - Version string `json:"version" bigquery:"Version"` + Version string `json:"version"` // Package URL is a short link for a package. - PackageURL string `json:"package_url"` - - // License is the license of the dependency. - License string `json:"license"` + PackageURL *string `json:"package_url"` // SrcRepoURL is the source repository URL of the dependency. - SrcRepoURL string `json:"source_repository_url"` - - // ScResults is the Scorecard scanning result of the dependency package repository. - ScResults ScorecardResult `json:"scorecard_results"` - - // Vulnerabilities is a list of Vulnerability. - Vulnerabilities []Vulnerability `json:"vulnerabilities"` - - // Dependencies is the list of dependencies of the current dependency, - // e.g. indirect (transitive) dependencies. - // TODO: this is not a version-zero feature, and will be used to analyze transitive - // dependencies in future versions. - Dependencies []Dependency `json:"dependencies"` + SrcRepoURL *string `json:"source_repository_url"` } diff --git a/cmd/depdiff/vulnerabilities.go b/pkg/check-depdiff/vulnerabilities.go similarity index 100% rename from cmd/depdiff/vulnerabilities.go rename to pkg/check-depdiff/vulnerabilities.go From 1fee5201f2d127efca9602f1691f290a1df2e05d Mon Sep 17 00:00:00 2001 From: aidenwang Date: Fri, 8 Jul 2022 17:17:44 -0700 Subject: [PATCH 07/49] temp --- pkg/check-depdiff/dependencies.go | 5 -- pkg/check-depdiff/vulnerabilities.go | 92 ---------------------------- 2 files changed, 97 deletions(-) delete mode 100644 pkg/check-depdiff/vulnerabilities.go diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index 2ec62820dc1..bfdb060897f 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -35,11 +35,6 @@ func (ct *ChangeType) IsValid() bool { // Dependency is a dependency. type Dependency struct { - // // IsDirect suggests if the dependency is a direct dependency of a code commit. - // TODO: IsDirect remains a future feature since the current GitHub Dependency Review API - // mixes up direct and indirect dependencies in manifest files of different ecosystems. - IsDirect bool - // ChangeType indicates whether the dependency is added, updated, or removed. ChangeType ChangeType `json:"change_type"` diff --git a/pkg/check-depdiff/vulnerabilities.go b/pkg/check-depdiff/vulnerabilities.go deleted file mode 100644 index 646c63aeea0..00000000000 --- a/pkg/check-depdiff/vulnerabilities.go +++ /dev/null @@ -1,92 +0,0 @@ -// 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 depdiff - -import ( - "time" -) - -// SeverityLevel is the level of severity of a vulnerability. -type SeverityLevel string - -const ( - Critical SeverityLevel = "CRITICAL" - High SeverityLevel = "HIGH" - Medium SeverityLevel = "MEDIUM" - Moderate SeverityLevel = "MODERATE" - Low SeverityLevel = "LOW" - None SeverityLevel = "NONE" - Unknown SeverityLevel = "UNKNOWN" -) - -// IsValid determines if a SeverityLevel is valid. -func (sl *SeverityLevel) IsValid() bool { - switch *sl { - case Critical, High, Medium, Moderate, Low, None, Unknown: - return true - default: - return false - } -} - -// Source is an authoritative source of a vulnerability. -type Source string - -const ( - GHSA Source = "GHSA" - NSWG Source = "NSWG" - OSV Source = "OSV" -) - -// IsValid determines if a Source is valid. -func (src *Source) IsValid() bool { - switch *src { - case GHSA, NSWG, OSV: - return true - default: - return false - } -} - -// Vulnerability is a security vulnerability of a dependency. -type Vulnerability struct { - // Source is the source of a vulnerability. - Source string `bigquery:"Source"` - - // ID is the identifier of a vulnerability. - ID string `json:"advisory_ghsa_id" bigquery:"SourceID"` - - // SourceURL is the source URL of a vulnerability. - SourceURL string `json:"advisory_url" bigquery:"SourceURL"` - - // Title is the text summary of a vulnerability. - Title string `json:"advisory_summary" bigquery:"Title"` - - // Description is a long text paragraph of a vulnerability. - Description string `json:"description" bigquery:"Description"` - - // Score is the score of a vulnerability given by an authoritative src. - // TODO: this is not a version-zero property and will be included in future versions. - // Score bigquery.NullFloat64 `bigquery:"Score"` - - // GitHubSeverity is the severity level of a vulnerability determined by GitHub. - GitHubSeverity SeverityLevel `json:"github_severity" bigquery:"GitHubSeverity"` - - // ReferenceURLs include all URLs that are related to a vulnerability. - ReferenceURLs []string `json:"reference_urls" bigquery:"ReferenceURLs"` - - // DisclosedTime is the time when a vulenrability is publicly disclosed. - DisclosedTime time.Time `json:"disclosed_time" bigquery:"Disclosed"` -} From 0595cdf08b5af6eab3ae3cfd15064b94a7b94ffc Mon Sep 17 00:00:00 2001 From: aidenwang Date: Fri, 8 Jul 2022 17:22:56 -0700 Subject: [PATCH 08/49] temp --- pkg/check-depdiff/depdiff.go | 101 +++++++++++++++++++++++++++++++++++ pkg/check-depdiff/errors.go | 16 ++++++ 2 files changed, 117 insertions(+) create mode 100644 pkg/check-depdiff/depdiff.go create mode 100644 pkg/check-depdiff/errors.go diff --git a/pkg/check-depdiff/depdiff.go b/pkg/check-depdiff/depdiff.go new file mode 100644 index 00000000000..6c3d8354701 --- /dev/null +++ b/pkg/check-depdiff/depdiff.go @@ -0,0 +1,101 @@ +package depdiff + +import ( + "fmt" + "net/http" + "path" + "strings" + "time" + + gogh "github.com/google/go-github/v38/github" +) + +type DepDiffContext struct { + OwnerName string + RepoName string + BaseSHA string + HeadSHA string + AccessToken string +} + +type DepDiffFormat string + +const ( + JSON DepDiffFormat = "json" + Plaintext DepDiffFormat = "plaintext" + Markdown DepDiffFormat = "md" +) + +func (f *DepDiffFormat) IsValid() bool { + switch *f { + case JSON, Plaintext, Markdown: + return true + default: + return false + } +} + +func GetDependencyDiff( + ownerName, repoName, baseSHA, headSHA, accessToken string, + format DepDiffFormat, +) (interface{}, error) { + format = DepDiffFormat(strings.ToLower(string(format))) + if !format.IsValid() { + return nil, ErrInvalidDepDiffFormat + } + ctx := DepDiffContext{ + OwnerName: ownerName, + RepoName: repoName, + BaseSHA: baseSHA, + HeadSHA: headSHA, + AccessToken: accessToken, + } + + // Fetch dependency diffs using the GitHub Dependency Review API. + deps, err := FetchDepDiffDataFromGitHub(ctx) + if err != nil { + return nil, err + } + fmt.Println(deps) + + switch format { + case JSON: + return nil, nil + case Markdown: + return nil, ErrDepDiffFormatNotSupported + case Plaintext: + return nil, ErrDepDiffFormatNotSupported + default: + return nil, ErrDepDiffFormatNotSupported + } +} + +// Get the depednency-diff using the GitHub Dependency Review +// (https://docs.github.com/en/rest/dependency-graph/dependency-review) API +func FetchDepDiffDataFromGitHub(ctx DepDiffContext) ([]Dependency, error) { + // Set a ten-seconds timeout to make sure the client can be created correctly. + client := gogh.NewClient(&http.Client{Timeout: 10 * time.Second}) + reqURL := path.Join( + "repos", ctx.OwnerName, ctx.RepoName, "dependency-graph", "compare", + ctx.BaseSHA+"..."+ctx.HeadSHA, + ) + req, err := client.NewRequest("GET", reqURL, nil) + if err != nil { + return nil, fmt.Errorf("request for dependency-diff failed with %w", err) + } + // To specify the return type to be JSON. + req.Header.Set("Accept", "application/vnd.github.v3+json") + // An access token is required in the request header to be able to use this API. + req.Header.Set("Authorization", "token "+ctx.AccessToken) + + depDiff := []Dependency{} + _, err = client.Do(req.Context(), req, &depDiff) + if err != nil { + return nil, fmt.Errorf("get response error: %w", err) + } + return depDiff, nil +} + +func GetAggregateScore(d Dependency) (float32, error) { + return 0, nil +} diff --git a/pkg/check-depdiff/errors.go b/pkg/check-depdiff/errors.go new file mode 100644 index 00000000000..d0806befa4b --- /dev/null +++ b/pkg/check-depdiff/errors.go @@ -0,0 +1,16 @@ +package depdiff + +import ( + "errors" +) + +var ( + // ErrInvalidDepDiffFormat indicates the specified dependency diff output format is not valid. + ErrInvalidDepDiffFormat = errors.New("invalid depdiff format") + + // ErrDepDiffFormatNotSupported indicates the specified dependency diff output format is not supported. + ErrDepDiffFormatNotSupported = errors.New("depdiff format not supported") + + // ErrInvalidDepDiffFormat indicates the specified dependency diff output format is not valid. + ErrMarshalDepDiffToJSON = errors.New("error marshal results to JSON") +) From c5cb697008c151ce65d39c13a98859ee89b14023 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Fri, 8 Jul 2022 17:43:39 -0700 Subject: [PATCH 09/49] temp --- pkg/check-depdiff/dependencies.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index bfdb060897f..79c79233d4a 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -18,8 +18,11 @@ package depdiff type ChangeType string const ( - Added ChangeType = "added" + // Added suggests the dependency is a new one. + Added ChangeType = "added" + // Updated suggests the dependency is bumped from an old version. Updated ChangeType = "updated" + // Removed suggests the dependency is removed. Removed ChangeType = "removed" ) @@ -35,6 +38,12 @@ func (ct *ChangeType) IsValid() bool { // Dependency is a dependency. type Dependency struct { + // Package URL is a short link for a package. + PackageURL *string `json:"package_url"` + + // SrcRepoURL is the source repository URL of the dependency. + SrcRepoURL *string `json:"source_repository_url"` + // ChangeType indicates whether the dependency is added, updated, or removed. ChangeType ChangeType `json:"change_type"` @@ -49,10 +58,4 @@ type Dependency struct { // Version is the package version of the dependency. Version string `json:"version"` - - // Package URL is a short link for a package. - PackageURL *string `json:"package_url"` - - // SrcRepoURL is the source repository URL of the dependency. - SrcRepoURL *string `json:"source_repository_url"` } From 7bc69116e7dd0c98b912f665d8dea1ba24cde00d Mon Sep 17 00:00:00 2001 From: aidenwang Date: Sun, 10 Jul 2022 22:39:42 -0700 Subject: [PATCH 10/49] temp --- pkg/check-depdiff/depdiff.go | 54 ++++++++++--------------------- pkg/check-depdiff/dependencies.go | 10 +++--- pkg/check-depdiff/errors.go | 14 ++++++++ 3 files changed, 36 insertions(+), 42 deletions(-) diff --git a/pkg/check-depdiff/depdiff.go b/pkg/check-depdiff/depdiff.go index 6c3d8354701..33ecd8ffe14 100644 --- a/pkg/check-depdiff/depdiff.go +++ b/pkg/check-depdiff/depdiff.go @@ -1,10 +1,23 @@ +// 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 depdiff import ( "fmt" "net/http" "path" - "strings" "time" gogh "github.com/google/go-github/v38/github" @@ -18,31 +31,7 @@ type DepDiffContext struct { AccessToken string } -type DepDiffFormat string - -const ( - JSON DepDiffFormat = "json" - Plaintext DepDiffFormat = "plaintext" - Markdown DepDiffFormat = "md" -) - -func (f *DepDiffFormat) IsValid() bool { - switch *f { - case JSON, Plaintext, Markdown: - return true - default: - return false - } -} - -func GetDependencyDiff( - ownerName, repoName, baseSHA, headSHA, accessToken string, - format DepDiffFormat, -) (interface{}, error) { - format = DepDiffFormat(strings.ToLower(string(format))) - if !format.IsValid() { - return nil, ErrInvalidDepDiffFormat - } +func GetDependencyDiff(ownerName, repoName, baseSHA, headSHA, accessToken string) (string, error) { ctx := DepDiffContext{ OwnerName: ownerName, RepoName: repoName, @@ -54,20 +43,11 @@ func GetDependencyDiff( // Fetch dependency diffs using the GitHub Dependency Review API. deps, err := FetchDepDiffDataFromGitHub(ctx) if err != nil { - return nil, err + return "", err } fmt.Println(deps) - switch format { - case JSON: - return nil, nil - case Markdown: - return nil, ErrDepDiffFormatNotSupported - case Plaintext: - return nil, ErrDepDiffFormatNotSupported - default: - return nil, ErrDepDiffFormatNotSupported - } + return "", nil } // Get the depednency-diff using the GitHub Dependency Review diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index 79c79233d4a..490ef3bafb6 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -45,17 +45,17 @@ type Dependency struct { SrcRepoURL *string `json:"source_repository_url"` // ChangeType indicates whether the dependency is added, updated, or removed. - ChangeType ChangeType `json:"change_type"` + ChangeType *ChangeType `json:"change_type"` // ManifestFileName is the name of the manifest file of the dependency, such as go.mod for Go. - ManifestFileName string `json:"manifest"` + ManifestFileName *string `json:"manifest"` // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. - Ecosystem string `json:"ecosystem"` + Ecosystem *string `json:"ecosystem"` // Name is the name of the dependency. - Name string `json:"name"` + Name *string `json:"name"` // Version is the package version of the dependency. - Version string `json:"version"` + Version *string `json:"version"` } diff --git a/pkg/check-depdiff/errors.go b/pkg/check-depdiff/errors.go index d0806befa4b..9f62a9b6385 100644 --- a/pkg/check-depdiff/errors.go +++ b/pkg/check-depdiff/errors.go @@ -1,3 +1,17 @@ +// 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 depdiff import ( From 62fb4940f6c7b6c8b81cbe3f158512ebae901c48 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 11 Jul 2022 09:38:02 -0700 Subject: [PATCH 11/49] temp --- pkg/check-depdiff/depdiff.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/check-depdiff/depdiff.go b/pkg/check-depdiff/depdiff.go index 33ecd8ffe14..0892678dbb9 100644 --- a/pkg/check-depdiff/depdiff.go +++ b/pkg/check-depdiff/depdiff.go @@ -41,7 +41,7 @@ func GetDependencyDiff(ownerName, repoName, baseSHA, headSHA, accessToken string } // Fetch dependency diffs using the GitHub Dependency Review API. - deps, err := FetchDepDiffDataFromGitHub(ctx) + deps, err := FetchDependencyDiffData(ctx) if err != nil { return "", err } @@ -50,9 +50,10 @@ func GetDependencyDiff(ownerName, repoName, baseSHA, headSHA, accessToken string return "", nil } -// Get the depednency-diff using the GitHub Dependency Review -// (https://docs.github.com/en/rest/dependency-graph/dependency-review) API -func FetchDepDiffDataFromGitHub(ctx DepDiffContext) ([]Dependency, error) { +// Get the depednency-diffs between two specified code commits. +func FetchDependencyDiffData(ctx DepDiffContext) ([]Dependency, error) { + // Currently, the GitHub Dependency Review + // (https://docs.github.com/en/rest/dependency-graph/dependency-review) API is used. // Set a ten-seconds timeout to make sure the client can be created correctly. client := gogh.NewClient(&http.Client{Timeout: 10 * time.Second}) reqURL := path.Join( @@ -63,7 +64,6 @@ func FetchDepDiffDataFromGitHub(ctx DepDiffContext) ([]Dependency, error) { if err != nil { return nil, fmt.Errorf("request for dependency-diff failed with %w", err) } - // To specify the return type to be JSON. req.Header.Set("Accept", "application/vnd.github.v3+json") // An access token is required in the request header to be able to use this API. req.Header.Set("Authorization", "token "+ctx.AccessToken) From 92a117e2792f2a58c6f58e2db44e4e0fe3da5194 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 11 Jul 2022 11:31:06 -0700 Subject: [PATCH 12/49] temp --- pkg/check-depdiff/dependencies.go | 51 +++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index bfdb060897f..a3a4d5d7832 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -14,12 +14,17 @@ package depdiff +import "github.com/ossf/scorecard/v4/pkg" + // ChangeType is the change type (added, updated, removed) of a dependency. type ChangeType string const ( - Added ChangeType = "added" + // Added suggests the dependency is a new one. + Added ChangeType = "added" + // Updated suggests the dependency is bumped from an old version. Updated ChangeType = "updated" + // Removed suggests the dependency is removed. Removed ChangeType = "removed" ) @@ -33,26 +38,54 @@ func (ct *ChangeType) IsValid() bool { } } -// Dependency is a dependency. -type Dependency struct { +// rawDependency is the Dependency structure that is used to receive +// the raw results from the GitHub Dependency Review API. +type rawDependency struct { + // Package URL is a short link for a package. + PackageURL *string `json:"package_url"` + + // SrcRepoURL is the source repository URL of the dependency. + SrcRepoURL *string `json:"source_repository_url"` + // ChangeType indicates whether the dependency is added, updated, or removed. - ChangeType ChangeType `json:"change_type"` + ChangeType *ChangeType `json:"change_type"` // ManifestFileName is the name of the manifest file of the dependency, such as go.mod for Go. - ManifestFileName string `json:"manifest"` + ManifestPath *string `json:"manifest"` // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. - Ecosystem string `json:"ecosystem"` + Ecosystem *string `json:"ecosystem"` // Name is the name of the dependency. Name string `json:"name"` // Version is the package version of the dependency. - Version string `json:"version"` + Version *string `json:"version"` +} +// Dependency is the dependency structure used in the returned results. +type Dependency struct { // Package URL is a short link for a package. - PackageURL *string `json:"package_url"` + PackageURL *string `json:"packageUrl"` // SrcRepoURL is the source repository URL of the dependency. - SrcRepoURL *string `json:"source_repository_url"` + SrcRepoURL *string `json:"srcRepoUrl"` + + // ChangeType indicates whether the dependency is added, updated, or removed. + ChangeType *ChangeType `json:"changeType"` + + // ManifestFileName is the name of the manifest file of the dependency, such as go.mod for Go. + ManifestPath *string `json:"manifest"` + + // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. + Ecosystem *string `json:"ecosystem"` + + // Name is the name of the dependency. + Name string `json:"name"` + + // Version is the package version of the dependency. + Version *string `json:"version"` + + // ScReresults is the scorecard result for the dependency repo. + ScReresults pkg.ScorecardResult `json:"scorecardResults"` } From c49c7f4624f2d322f6d32afa67d7b0f074f9e097 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 11 Jul 2022 11:31:55 -0700 Subject: [PATCH 13/49] temp --- pkg/check-depdiff/dependencies.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index a3a4d5d7832..6fd4c7cec71 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -64,7 +64,7 @@ type rawDependency struct { } // Dependency is the dependency structure used in the returned results. -type Dependency struct { +type DependencyCheckResult struct { // Package URL is a short link for a package. PackageURL *string `json:"packageUrl"` From c733ba50239430c5b7b194574aa5889b36cbeb2d Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 11 Jul 2022 11:32:12 -0700 Subject: [PATCH 14/49] temp --- pkg/check-depdiff/dependencies.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index 6fd4c7cec71..402ab758c4e 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -63,7 +63,7 @@ type rawDependency struct { Version *string `json:"version"` } -// Dependency is the dependency structure used in the returned results. +// DependencyCheckResult is the dependency structure used in the returned results. type DependencyCheckResult struct { // Package URL is a short link for a package. PackageURL *string `json:"packageUrl"` From 137908272d20a451f79f212827d69d73a2d751a8 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 11 Jul 2022 11:32:35 -0700 Subject: [PATCH 15/49] temp --- pkg/check-depdiff/dependencies.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index 402ab758c4e..35b9526b540 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -87,5 +87,5 @@ type DependencyCheckResult struct { Version *string `json:"version"` // ScReresults is the scorecard result for the dependency repo. - ScReresults pkg.ScorecardResult `json:"scorecardResults"` + ScReresults *pkg.ScorecardResult `json:"scorecardResults"` } From cdd18406a4e8da090307371c46507323446a9d6a Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 11 Jul 2022 13:00:28 -0700 Subject: [PATCH 16/49] temp --- pkg/check-depdiff/dependencies.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index 35b9526b540..14c0a3a51cc 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -38,31 +38,6 @@ func (ct *ChangeType) IsValid() bool { } } -// rawDependency is the Dependency structure that is used to receive -// the raw results from the GitHub Dependency Review API. -type rawDependency struct { - // Package URL is a short link for a package. - PackageURL *string `json:"package_url"` - - // SrcRepoURL is the source repository URL of the dependency. - SrcRepoURL *string `json:"source_repository_url"` - - // ChangeType indicates whether the dependency is added, updated, or removed. - ChangeType *ChangeType `json:"change_type"` - - // ManifestFileName is the name of the manifest file of the dependency, such as go.mod for Go. - ManifestPath *string `json:"manifest"` - - // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. - Ecosystem *string `json:"ecosystem"` - - // Name is the name of the dependency. - Name string `json:"name"` - - // Version is the package version of the dependency. - Version *string `json:"version"` -} - // DependencyCheckResult is the dependency structure used in the returned results. type DependencyCheckResult struct { // Package URL is a short link for a package. From 0e1223dcf61c63f233d4d065a0a39d17876e24b5 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 11 Jul 2022 13:28:46 -0700 Subject: [PATCH 17/49] temp --- pkg/check-depdiff/dependencies.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index 14c0a3a51cc..502366902c7 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -44,13 +44,13 @@ type DependencyCheckResult struct { PackageURL *string `json:"packageUrl"` // SrcRepoURL is the source repository URL of the dependency. - SrcRepoURL *string `json:"srcRepoUrl"` + SrcRepoURL *string `json:"sourceRepositoryURL"` // ChangeType indicates whether the dependency is added, updated, or removed. ChangeType *ChangeType `json:"changeType"` // ManifestFileName is the name of the manifest file of the dependency, such as go.mod for Go. - ManifestPath *string `json:"manifest"` + ManifestPath *string `json:"manifestPath"` // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. Ecosystem *string `json:"ecosystem"` From 2ac26d7c09c2156c9247e13ea81050512f46213c Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 11 Jul 2022 13:32:13 -0700 Subject: [PATCH 18/49] temp --- pkg/check-depdiff/dependencies.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index 502366902c7..e05fb3da631 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -62,5 +62,5 @@ type DependencyCheckResult struct { Version *string `json:"version"` // ScReresults is the scorecard result for the dependency repo. - ScReresults *pkg.ScorecardResult `json:"scorecardResults"` + ScorecardResults *pkg.ScorecardResult `json:"scorecardResults"` } From 2b0ffed4138f98bc60b112c6570a060552d8efee Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 11 Jul 2022 14:10:54 -0700 Subject: [PATCH 19/49] temp --- pkg/check-depdiff/dependencies.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/check-depdiff/dependencies.go b/pkg/check-depdiff/dependencies.go index e05fb3da631..bf386d592c8 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/check-depdiff/dependencies.go @@ -43,24 +43,24 @@ type DependencyCheckResult struct { // Package URL is a short link for a package. PackageURL *string `json:"packageUrl"` - // SrcRepoURL is the source repository URL of the dependency. - SrcRepoURL *string `json:"sourceRepositoryURL"` + // SourceRepository is the source repository URL of the dependency. + SourceRepository *string `json:"sourceRepository"` // ChangeType indicates whether the dependency is added, updated, or removed. ChangeType *ChangeType `json:"changeType"` - // ManifestFileName is the name of the manifest file of the dependency, such as go.mod for Go. + // ManifestPath is the name of the manifest file of the dependency, such as go.mod for Go. ManifestPath *string `json:"manifestPath"` // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. Ecosystem *string `json:"ecosystem"` - // Name is the name of the dependency. - Name string `json:"name"` - // Version is the package version of the dependency. Version *string `json:"version"` - // ScReresults is the scorecard result for the dependency repo. + // ScorecardResults is the scorecard result for the dependency repo. ScorecardResults *pkg.ScorecardResult `json:"scorecardResults"` + + // Name is the name of the dependency. + Name string `json:"name"` } From 847b3e7a04fc470b0085554b7378f9d09696087c Mon Sep 17 00:00:00 2001 From: aidenwang Date: Tue, 12 Jul 2022 14:44:22 -0700 Subject: [PATCH 20/49] temp --- dependencydiff/dependencydiff.go | 123 ++++++++++++++++++ .../errors.go | 0 .../raw_dependencies.go | 28 +--- pkg/check-depdiff/check-depdiff.go | 46 ------- pkg/check-depdiff/dependency_results.go | 47 ------- pkg/check-depdiff/raw/fetch.go | 49 ------- ...endencies.go => dependencydiff_results.go} | 8 +- 7 files changed, 130 insertions(+), 171 deletions(-) create mode 100644 dependencydiff/dependencydiff.go rename {pkg/check-depdiff => dependencydiff}/errors.go (100%) rename pkg/check-depdiff/raw/dependencies.go => dependencydiff/raw_dependencies.go (70%) delete mode 100644 pkg/check-depdiff/check-depdiff.go delete mode 100644 pkg/check-depdiff/dependency_results.go delete mode 100644 pkg/check-depdiff/raw/fetch.go rename pkg/{check-depdiff/dependencies.go => dependencydiff_results.go} (91%) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go new file mode 100644 index 00000000000..004aa08bb01 --- /dev/null +++ b/dependencydiff/dependencydiff.go @@ -0,0 +1,123 @@ +// 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 ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + "path" + + "github.com/ossf/scorecard/v4/checks" + "github.com/ossf/scorecard/v4/clients" + "github.com/ossf/scorecard/v4/clients/githubrepo" + "github.com/ossf/scorecard/v4/clients/githubrepo/roundtripper" + "github.com/ossf/scorecard/v4/pkg" +) + +func GetDependencyDiff(owner, repo, base, head string) ([]pkg.DependencyCheckResult, error) { + // Fetch dependency diffs using the GitHub Dependency Review API. + deps, err := FetchDependencyDiffData(owner, repo, base, head) + if err != nil { + return nil, err + } + // PrintDependencies(deps) + + ctx := context.Background() + ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) + if err != nil { + return nil, err + } + ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, nil) + ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, nil) + vulnsClient := clients.DefaultVulnerabilitiesClient() + if err != nil { + panic(err) + } + results := []pkg.DependencyCheckResult{} + count := 0 + for _, d := range deps { + if d.SourceRepository != nil { + // Running scorecard on this dependency and fetch the result + fmt.Printf("Running Scorecard checks for %s\n", d.Name) + scResult, err := pkg.RunScorecards( + ctx, + ghRepo, + clients.HeadSHA, + checks.AllChecks, + ghRepoClient, + ossFuzzRepoClient, + nil, + vulnsClient, + ) + if err != nil { + return nil, err + } + dd := pkg.DependencyCheckResult{ + PackageURL: d.PackageURL, + SourceRepository: d.SourceRepository, + ChangeType: d.ChangeType, + ManifestPath: d.ManifestPath, + Ecosystem: d.Ecosystem, + Version: d.Version, + ScorecardResults: &scResult, + Name: d.Name, + } + results = append(results, dd) + count += 1 + if count == 3 { + break + } + } else { + fmt.Printf("Skipping %s, no source repo found\n", d.Name) + } + // Skip those without source repo urls. + // TODO: use BigQuery to supplement null source repo URLs and fetch the Scorecard results for them. + } + return results, nil +} + +// Get the depednency-diffs between two specified code commits. +func FetchDependencyDiffData(owner, repo, base, head string) ([]Dependency, error) { + reqURL, err := url.Parse( + path.Join( + "api.github.com", + "repos", owner, repo, "dependency-graph", "compare", base+"..."+head, + ), + ) + if err != nil { + return nil, err + } + reqURL.Scheme = "https" + req, err := http.NewRequest("GET", reqURL.String(), nil) + if err != nil { + return nil, fmt.Errorf("request for dependency-diff failed with %w", err) + } + ctx := context.Background() + ghrt := roundtripper.NewTransport(ctx, nil) + resp, err := ghrt.RoundTrip(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + depDiff := []Dependency{} + err = json.NewDecoder(resp.Body).Decode(&depDiff) + if err != nil { + return nil, fmt.Errorf("parse response error: %w", err) + } + return depDiff, nil +} diff --git a/pkg/check-depdiff/errors.go b/dependencydiff/errors.go similarity index 100% rename from pkg/check-depdiff/errors.go rename to dependencydiff/errors.go diff --git a/pkg/check-depdiff/raw/dependencies.go b/dependencydiff/raw_dependencies.go similarity index 70% rename from pkg/check-depdiff/raw/dependencies.go rename to dependencydiff/raw_dependencies.go index 81c51682897..346701f462c 100644 --- a/pkg/check-depdiff/raw/dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -12,29 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -package raw - -// ChangeType is the change type (added, updated, removed) of a dependency. -type ChangeType string - -const ( - // Added suggests the dependency is a new one. - Added ChangeType = "added" - // Updated suggests the dependency is bumped from an old version. - Updated ChangeType = "updated" - // Removed suggests the dependency is removed. - Removed ChangeType = "removed" -) - -// IsValid determines if a ChangeType is valid. -func (ct *ChangeType) IsValid() bool { - switch *ct { - case Added, Updated, Removed: - return true - default: - return false - } -} +package dependencydiff + +import "github.com/ossf/scorecard/v4/pkg" // Dependency is a raw dependency fetched from the GitHub Dependency Review API. type Dependency struct { @@ -45,7 +25,7 @@ type Dependency struct { SourceRepository *string `json:"source_repository_url"` // ChangeType indicates whether the dependency is added, updated, or removed. - ChangeType *ChangeType `json:"change_type"` + ChangeType *pkg.ChangeType `json:"change_type"` // ManifestPath is the path of the manifest file of the dependency, such as go.mod for Go. ManifestPath *string `json:"manifest"` diff --git a/pkg/check-depdiff/check-depdiff.go b/pkg/check-depdiff/check-depdiff.go deleted file mode 100644 index d4a4aa55ea7..00000000000 --- a/pkg/check-depdiff/check-depdiff.go +++ /dev/null @@ -1,46 +0,0 @@ -// 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 depdiff - -import ( - "fmt" -) - -type DepDiffContext struct { - OwnerName string - RepoName string - BaseSHA string - HeadSHA string - AccessToken string -} - -func GetDependencyDiff(ownerName, repoName, baseSHA, headSHA, accessToken string) (string, error) { - ctx := DepDiffContext{ - OwnerName: ownerName, - RepoName: repoName, - BaseSHA: baseSHA, - HeadSHA: headSHA, - AccessToken: accessToken, - } - - // Fetch dependency diffs using the GitHub Dependency Review API. - deps, err := raw.FetchDependencyDiffData(ctx) - if err != nil { - return "", err - } - fmt.Println(deps) - - return "", nil -} diff --git a/pkg/check-depdiff/dependency_results.go b/pkg/check-depdiff/dependency_results.go deleted file mode 100644 index aeefa56857f..00000000000 --- a/pkg/check-depdiff/dependency_results.go +++ /dev/null @@ -1,47 +0,0 @@ -// 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 depdiff - -import ( - "github.com/ossf/scorecard/v4/pkg" - "github.com/ossf/scorecard/v4/pkg/check-depdiff/raw" -) - -// DependencyCheckResult is the dependency structure used in the returned results. -type DependencyCheckResult struct { - // Package URL is a short link for a package. - PackageURL *string `json:"packageUrl"` - - // SourceRepository is the source repository URL of the dependency. - SourceRepository *string `json:"sourceRepository"` - - // ChangeType indicates whether the dependency is added, updated, or removed. - ChangeType *raw.ChangeType `json:"changeType"` - - // ManifestPath is the path of the manifest file of the dependency, such as go.mod for Go. - ManifestPath *string `json:"manifestPath"` - - // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. - Ecosystem *string `json:"ecosystem"` - - // Version is the package version of the dependency. - Version *string `json:"version"` - - // ScorecardResults is the scorecard result for the dependency repo. - ScorecardResults *pkg.ScorecardResult `json:"scorecardResults"` - - // Name is the name of the dependency. - Name string `json:"name"` -} diff --git a/pkg/check-depdiff/raw/fetch.go b/pkg/check-depdiff/raw/fetch.go deleted file mode 100644 index 668064cdd89..00000000000 --- a/pkg/check-depdiff/raw/fetch.go +++ /dev/null @@ -1,49 +0,0 @@ -// 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 raw - -import ( - "fmt" - "net/http" - "path" - "time" - - gogh "github.com/google/go-github/v38/github" -) - -// Get the depednency-diffs between two specified code commits. -func FetchDependencyDiffData(owner, repo, base, head string) ([]Dependency, error) { - // Currently, the GitHub Dependency Review - // (https://docs.github.com/en/rest/dependency-graph/dependency-review) API is used. - // Set a ten-seconds timeout to make sure the client can be created correctly. - client := gogh.NewClient(&http.Client{Timeout: 10 * time.Second}) - reqURL := path.Join( - "repos", owner, repo, "dependency-graph", "compare", base+"..."+head, - ) - req, err := client.NewRequest("GET", reqURL, nil) - if err != nil { - return nil, fmt.Errorf("request for dependency-diff failed with %w", err) - } - req.Header.Set("Accept", "application/vnd.github.v3+json") - // An access token is required in the request header to be able to use this API. - req.Header.Set("Authorization", "token "+ctx.AccessToken) - - depDiff := []Dependency{} - _, err = client.Do(req.Context(), req, &depDiff) - if err != nil { - return nil, fmt.Errorf("get response error: %w", err) - } - return depDiff, nil -} diff --git a/pkg/check-depdiff/dependencies.go b/pkg/dependencydiff_results.go similarity index 91% rename from pkg/check-depdiff/dependencies.go rename to pkg/dependencydiff_results.go index bf386d592c8..2cb742000eb 100644 --- a/pkg/check-depdiff/dependencies.go +++ b/pkg/dependencydiff_results.go @@ -12,9 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package depdiff - -import "github.com/ossf/scorecard/v4/pkg" +package pkg // ChangeType is the change type (added, updated, removed) of a dependency. type ChangeType string @@ -49,7 +47,7 @@ type DependencyCheckResult struct { // ChangeType indicates whether the dependency is added, updated, or removed. ChangeType *ChangeType `json:"changeType"` - // ManifestPath is the name of the manifest file of the dependency, such as go.mod for Go. + // ManifestPath is the path of the manifest file of the dependency, such as go.mod for Go. ManifestPath *string `json:"manifestPath"` // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. @@ -59,7 +57,7 @@ type DependencyCheckResult struct { Version *string `json:"version"` // ScorecardResults is the scorecard result for the dependency repo. - ScorecardResults *pkg.ScorecardResult `json:"scorecardResults"` + ScorecardResults *ScorecardResult `json:"scorecardResults"` // Name is the name of the dependency. Name string `json:"name"` From 5c645056b04739207e1fef2f234784b338059cd1 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Tue, 12 Jul 2022 16:45:32 -0700 Subject: [PATCH 21/49] temp --- dependencydiff/dependencydiff.go | 72 ++++++------ dependencydiff/dependencydiff_test.go | 106 ++++++++++++++++++ dependencydiff/errors.go | 30 ----- dependencydiff/raw_dependencies.go | 14 +-- ...ff_results.go => dependencydiff_result.go} | 20 +++- 5 files changed, 165 insertions(+), 77 deletions(-) create mode 100644 dependencydiff/dependencydiff_test.go delete mode 100644 dependencydiff/errors.go rename pkg/{dependencydiff_results.go => dependencydiff_result.go} (81%) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 004aa08bb01..eec794c8d1c 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -29,32 +29,36 @@ import ( "github.com/ossf/scorecard/v4/pkg" ) -func GetDependencyDiff(owner, repo, base, head string) ([]pkg.DependencyCheckResult, error) { +// 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. +func GetDependencyDiffResults(ownerName, repoName, baseSHA, headSHA string) ([]pkg.DependencyCheckResult, error) { + ctx := context.Background() // Fetch dependency diffs using the GitHub Dependency Review API. - deps, err := FetchDependencyDiffData(owner, repo, base, head) + deps, err := FetchDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA) if err != nil { - return nil, err + return nil, fmt.Errorf("error fetching dependency changes: %w", err) } - // PrintDependencies(deps) - ctx := context.Background() - ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) + ghRepo, err := githubrepo.MakeGithubRepo(path.Join(ownerName, repoName)) if err != nil { - return nil, err + return nil, fmt.Errorf("error creating the github repo: %w", err) } ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, nil) ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, nil) - vulnsClient := clients.DefaultVulnerabilitiesClient() if err != nil { - panic(err) + return nil, fmt.Errorf("error creating the oss fuzz repo client: %w", err) } + vulnsClient := clients.DefaultVulnerabilitiesClient() + results := []pkg.DependencyCheckResult{} - count := 0 for _, d := range deps { if d.SourceRepository != nil { - // Running scorecard on this dependency and fetch the result - fmt.Printf("Running Scorecard checks for %s\n", d.Name) - scResult, err := pkg.RunScorecards( + if *d.SourceRepository == "" { + continue + } + // Running scorecard on this dependency and fetch the result. + // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. + scorecardResult, err := pkg.RunScorecards( ctx, ghRepo, clients.HeadSHA, @@ -65,59 +69,51 @@ func GetDependencyDiff(owner, repo, base, head string) ([]pkg.DependencyCheckRes vulnsClient, ) if err != nil { - return nil, err + return nil, fmt.Errorf("error fetching the scorecard result: %w", err) } - dd := pkg.DependencyCheckResult{ + depCheckResult := pkg.DependencyCheckResult{ PackageURL: d.PackageURL, SourceRepository: d.SourceRepository, ChangeType: d.ChangeType, ManifestPath: d.ManifestPath, Ecosystem: d.Ecosystem, Version: d.Version, - ScorecardResults: &scResult, + ScorecardResults: &scorecardResult, Name: d.Name, } - results = append(results, dd) - count += 1 - if count == 3 { - break - } - } else { - fmt.Printf("Skipping %s, no source repo found\n", d.Name) + results = append(results, depCheckResult) } // Skip those without source repo urls. - // TODO: use BigQuery to supplement null source repo URLs and fetch the Scorecard results for them. + // TODO: use the BigQuery dataset to supplement null source repo URLs + // so that we can fetch the Scorecard results for them. } return results, nil } -// Get the depednency-diffs between two specified code commits. -func FetchDependencyDiffData(owner, repo, base, head string) ([]Dependency, error) { - reqURL, err := url.Parse( - path.Join( - "api.github.com", - "repos", owner, repo, "dependency-graph", "compare", base+"..."+head, - ), - ) +// FetchDependencyDiffData fetches the depednency-diffs between the two code commits +// using the GitHub Dependency Review API, and returns a slice of Dependency. +func FetchDependencyDiffData(ctx context.Context, owner, repo, base, head string) ([]Dependency, error) { + reqURL, err := url.Parse("https://api.github.com") if err != nil { - return nil, err + return nil, fmt.Errorf("error parsing the url: %w", err) } - reqURL.Scheme = "https" + reqURL.Path = path.Join( + "repos", owner, repo, "dependency-graph", "compare", base+"..."+head, + ) req, err := http.NewRequest("GET", reqURL.String(), nil) if err != nil { return nil, fmt.Errorf("request for dependency-diff failed with %w", err) } - ctx := context.Background() ghrt := roundtripper.NewTransport(ctx, nil) resp, err := ghrt.RoundTrip(req) - if err != nil { - return nil, err + if err != nil || resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("error receiving the http reponse: %w with resp status code %v", err, resp.StatusCode) } defer resp.Body.Close() depDiff := []Dependency{} err = json.NewDecoder(resp.Body).Decode(&depDiff) if err != nil { - return nil, fmt.Errorf("parse response error: %w", err) + return nil, fmt.Errorf("error parsing the http response: %w", err) } return depDiff, nil } diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go new file mode 100644 index 00000000000..cb23134741a --- /dev/null +++ b/dependencydiff/dependencydiff_test.go @@ -0,0 +1,106 @@ +// 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 ( + "context" + "testing" + + "github.com/ossf/scorecard/v4/clients" + scut "github.com/ossf/scorecard/v4/utests" +) + +// TestGetDependencyDiffResults is a test function for GetDependencyDiffResults +func TestGetDependencyDiffResults(t *testing.T) { + t.Parallel() + //nolint + //nolint + tests := []struct { + name string + owner string + repo string + base string + head string + wantEmpty bool + wantErr bool + }{ + { + name: "error response", + owner: "no_such_owner", + repo: "repo_not_exist", + base: clients.HeadSHA, + head: clients.HeadSHA, + wantEmpty: true, + wantErr: true, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := GetDependencyDiffResults(tt.owner, tt.repo, tt.base, tt.head) + if (err != nil) != tt.wantErr { + t.Errorf("GetDependencyDiffResults() error = %v, wantErr %v", err, tt.wantErr) + return + } + if (len(got) == 0) != tt.wantEmpty { + t.Errorf("FetchDependencyDiffData() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) + } + }) + } +} + +// TestTestFetchDependencyDiffData is a test function for TestFetchDependencyDiffData +func TestFetchDependencyDiffData(t *testing.T) { + t.Parallel() + //nolint + tests := []struct { + name string + ctx context.Context + owner string + repo string + base string + head string + wantEmpty bool + wantErr bool + + expected scut.TestReturn + }{ + { + name: "error reponse", + ctx: context.Background(), + owner: "no_such_owner", + repo: "repo_not_exist", + base: clients.HeadSHA, + head: clients.HeadSHA, + wantEmpty: true, + wantErr: true, + }, + } // End of test cases. + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := FetchDependencyDiffData(tt.ctx, tt.owner, tt.repo, tt.base, tt.head) + if (err != nil) != tt.wantErr { + t.Errorf("FetchDependencyDiffData() error = %v, wantErr %v", err, tt.wantErr) + return + } + if (len(got) == 0) != tt.wantEmpty { + t.Errorf("FetchDependencyDiffData() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) + } + }) + } +} diff --git a/dependencydiff/errors.go b/dependencydiff/errors.go deleted file mode 100644 index 9f62a9b6385..00000000000 --- a/dependencydiff/errors.go +++ /dev/null @@ -1,30 +0,0 @@ -// 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 depdiff - -import ( - "errors" -) - -var ( - // ErrInvalidDepDiffFormat indicates the specified dependency diff output format is not valid. - ErrInvalidDepDiffFormat = errors.New("invalid depdiff format") - - // ErrDepDiffFormatNotSupported indicates the specified dependency diff output format is not supported. - ErrDepDiffFormatNotSupported = errors.New("depdiff format not supported") - - // ErrInvalidDepDiffFormat indicates the specified dependency diff output format is not valid. - ErrMarshalDepDiffToJSON = errors.New("error marshal results to JSON") -) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 346701f462c..eb4c552419b 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -18,24 +18,24 @@ import "github.com/ossf/scorecard/v4/pkg" // Dependency is a raw dependency fetched from the GitHub Dependency Review API. type Dependency struct { - // Package URL is a short link for a package. + // package URL is a short link for a package. PackageURL *string `json:"package_url"` - // SourceRepository is the source repository URL of the dependency. + // sourceRepository is the source repository URL of the dependency. SourceRepository *string `json:"source_repository_url"` - // ChangeType indicates whether the dependency is added, updated, or removed. + // changeType indicates whether the dependency is added, updated, or removed. ChangeType *pkg.ChangeType `json:"change_type"` - // ManifestPath is the path of the manifest file of the dependency, such as go.mod for Go. + // manifestPath is the path of the manifest file of the dependency, such as go.mod for Go. ManifestPath *string `json:"manifest"` - // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. + // ecosystem is the name of the package management system, such as NPM, GO, PYPI. Ecosystem *string `json:"ecosystem"` - // Version is the package version of the dependency. + // version is the package version of the dependency. Version *string `json:"version"` - // Name is the name of the dependency. + // name is the name of the dependency. Name string `json:"name"` } diff --git a/pkg/dependencydiff_results.go b/pkg/dependencydiff_result.go similarity index 81% rename from pkg/dependencydiff_results.go rename to pkg/dependencydiff_result.go index 2cb742000eb..4a4beaaec9b 100644 --- a/pkg/dependencydiff_results.go +++ b/pkg/dependencydiff_result.go @@ -14,13 +14,21 @@ package pkg +import ( + "encoding/json" + "fmt" + "io" + + sce "github.com/ossf/scorecard/v4/errors" +) + // ChangeType is the change type (added, updated, removed) of a dependency. type ChangeType string const ( - // Added suggests the dependency is a new one. + // Added suggests the dependency is a newly added one. Added ChangeType = "added" - // Updated suggests the dependency is bumped from an old version. + // Updated suggests the dependency is updated from an old version. Updated ChangeType = "updated" // Removed suggests the dependency is removed. Removed ChangeType = "removed" @@ -62,3 +70,11 @@ type DependencyCheckResult struct { // Name is the name of the dependency. Name string `json:"name"` } + +func (dr *DependencyCheckResult) AsJSON(writer io.Writer) error { + encoder := json.NewEncoder(writer) + if err := encoder.Encode(*dr); err != nil { + return sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("encoder.Encode: %v", err)) + } + return nil +} From ad9c056b4d5a24a0051d3e74aecdd0cff26268af Mon Sep 17 00:00:00 2001 From: aidenwang Date: Tue, 12 Jul 2022 17:08:53 -0700 Subject: [PATCH 22/49] temp --- dependencydiff/raw_dependencies.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index eb4c552419b..346701f462c 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -18,24 +18,24 @@ import "github.com/ossf/scorecard/v4/pkg" // Dependency is a raw dependency fetched from the GitHub Dependency Review API. type Dependency struct { - // package URL is a short link for a package. + // Package URL is a short link for a package. PackageURL *string `json:"package_url"` - // sourceRepository is the source repository URL of the dependency. + // SourceRepository is the source repository URL of the dependency. SourceRepository *string `json:"source_repository_url"` - // changeType indicates whether the dependency is added, updated, or removed. + // ChangeType indicates whether the dependency is added, updated, or removed. ChangeType *pkg.ChangeType `json:"change_type"` - // manifestPath is the path of the manifest file of the dependency, such as go.mod for Go. + // ManifestPath is the path of the manifest file of the dependency, such as go.mod for Go. ManifestPath *string `json:"manifest"` - // ecosystem is the name of the package management system, such as NPM, GO, PYPI. + // Ecosystem is the name of the package management system, such as NPM, GO, PYPI. Ecosystem *string `json:"ecosystem"` - // version is the package version of the dependency. + // Version is the package version of the dependency. Version *string `json:"version"` - // name is the name of the dependency. + // Name is the name of the dependency. Name string `json:"name"` } From 23a1745c1ddd449939e728ceaf386a7937c7c5fa Mon Sep 17 00:00:00 2001 From: aidenwang Date: Tue, 12 Jul 2022 17:16:10 -0700 Subject: [PATCH 23/49] temp --- dependencydiff/dependencydiff.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index eec794c8d1c..f6a83757380 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -106,7 +106,7 @@ func FetchDependencyDiffData(ctx context.Context, owner, repo, base, head string } ghrt := roundtripper.NewTransport(ctx, nil) resp, err := ghrt.RoundTrip(req) - if err != nil || resp.StatusCode != http.StatusOK { + if err != nil { return nil, fmt.Errorf("error receiving the http reponse: %w with resp status code %v", err, resp.StatusCode) } defer resp.Body.Close() From 5fea8bda08bdf3ba6653a53edeaa4d1ebd238956 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 11:56:01 -0700 Subject: [PATCH 24/49] temp0713-1 --- dependencydiff/dependencydiff.go | 90 +------------------------- dependencydiff/dependencydiff_test.go | 14 ++-- dependencydiff/raw_dependencies.go | 93 ++++++++++++++++++++++++++- pkg/dependencydiff_result.go | 1 + 4 files changed, 104 insertions(+), 94 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index f6a83757380..62281b74577 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -16,104 +16,20 @@ package dependencydiff import ( "context" - "encoding/json" "fmt" - "net/http" - "net/url" - "path" - "github.com/ossf/scorecard/v4/checks" - "github.com/ossf/scorecard/v4/clients" - "github.com/ossf/scorecard/v4/clients/githubrepo" - "github.com/ossf/scorecard/v4/clients/githubrepo/roundtripper" "github.com/ossf/scorecard/v4/pkg" ) // 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. func GetDependencyDiffResults(ownerName, repoName, baseSHA, headSHA string) ([]pkg.DependencyCheckResult, error) { ctx := context.Background() // Fetch dependency diffs using the GitHub Dependency Review API. - deps, err := FetchDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA) + deps, err := fetchRawDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA) if err != nil { return nil, fmt.Errorf("error fetching dependency changes: %w", err) } - - ghRepo, err := githubrepo.MakeGithubRepo(path.Join(ownerName, repoName)) - if err != nil { - return nil, fmt.Errorf("error creating the github repo: %w", err) - } - ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, nil) - ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, nil) - if err != nil { - return nil, fmt.Errorf("error creating the oss fuzz repo client: %w", err) - } - vulnsClient := clients.DefaultVulnerabilitiesClient() - - results := []pkg.DependencyCheckResult{} - for _, d := range deps { - if d.SourceRepository != nil { - if *d.SourceRepository == "" { - continue - } - // Running scorecard on this dependency and fetch the result. - // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. - scorecardResult, err := pkg.RunScorecards( - ctx, - ghRepo, - clients.HeadSHA, - checks.AllChecks, - ghRepoClient, - ossFuzzRepoClient, - nil, - vulnsClient, - ) - if err != nil { - return nil, fmt.Errorf("error fetching the scorecard result: %w", err) - } - depCheckResult := pkg.DependencyCheckResult{ - PackageURL: d.PackageURL, - SourceRepository: d.SourceRepository, - ChangeType: d.ChangeType, - ManifestPath: d.ManifestPath, - Ecosystem: d.Ecosystem, - Version: d.Version, - ScorecardResults: &scorecardResult, - Name: d.Name, - } - results = append(results, depCheckResult) - } - // Skip those without source repo urls. - // TODO: use the BigQuery dataset to supplement null source repo URLs - // so that we can fetch the Scorecard results for them. - } - return results, nil -} - -// FetchDependencyDiffData fetches the depednency-diffs between the two code commits -// using the GitHub Dependency Review API, and returns a slice of Dependency. -func FetchDependencyDiffData(ctx context.Context, owner, repo, base, head string) ([]Dependency, error) { - reqURL, err := url.Parse("https://api.github.com") - if err != nil { - return nil, fmt.Errorf("error parsing the url: %w", err) - } - reqURL.Path = path.Join( - "repos", owner, repo, "dependency-graph", "compare", base+"..."+head, - ) - req, err := http.NewRequest("GET", reqURL.String(), nil) - if err != nil { - return nil, fmt.Errorf("request for dependency-diff failed with %w", err) - } - ghrt := roundtripper.NewTransport(ctx, nil) - resp, err := ghrt.RoundTrip(req) - if err != nil { - return nil, fmt.Errorf("error receiving the http reponse: %w with resp status code %v", err, resp.StatusCode) - } - defer resp.Body.Close() - depDiff := []Dependency{} - err = json.NewDecoder(resp.Body).Decode(&depDiff) - if err != nil { - return nil, fmt.Errorf("error parsing the http response: %w", err) - } - return depDiff, nil + return deps, nil } diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index cb23134741a..c44b723cc86 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -55,8 +55,10 @@ func TestGetDependencyDiffResults(t *testing.T) { t.Errorf("GetDependencyDiffResults() error = %v, wantErr %v", err, tt.wantErr) return } - if (len(got) == 0) != tt.wantEmpty { - t.Errorf("FetchDependencyDiffData() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) + if got != nil { + if (len(got) == 0) != tt.wantEmpty { + t.Errorf("GetDependencyDiffResults() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) + } } }) } @@ -93,13 +95,15 @@ func TestFetchDependencyDiffData(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := FetchDependencyDiffData(tt.ctx, tt.owner, tt.repo, tt.base, tt.head) + got, err := fetchRawDependencyDiffData(tt.ctx, tt.owner, tt.repo, tt.base, tt.head) if (err != nil) != tt.wantErr { t.Errorf("FetchDependencyDiffData() error = %v, wantErr %v", err, tt.wantErr) return } - if (len(got) == 0) != tt.wantEmpty { - t.Errorf("FetchDependencyDiffData() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) + if got != nil { + if (len(got) == 0) != tt.wantEmpty { + t.Errorf("FetchDependencyDiffData() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) + } } }) } diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 346701f462c..7eac96376d4 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -14,10 +14,23 @@ package dependencydiff -import "github.com/ossf/scorecard/v4/pkg" +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + "path" + + "github.com/ossf/scorecard/v4/checks" + "github.com/ossf/scorecard/v4/clients" + "github.com/ossf/scorecard/v4/clients/githubrepo" + "github.com/ossf/scorecard/v4/clients/githubrepo/roundtripper" + "github.com/ossf/scorecard/v4/pkg" +) // Dependency is a raw dependency fetched from the GitHub Dependency Review API. -type Dependency struct { +type dependency struct { // Package URL is a short link for a package. PackageURL *string `json:"package_url"` @@ -39,3 +52,79 @@ type Dependency struct { // Name is the name of the dependency. Name string `json:"name"` } + +// fetchRawDependencyDiffData fetches the dependency-diffs between the two code commits +// using the GitHub Dependency Review API, and returns a slice of Dependency. +func fetchRawDependencyDiffData(ctx context.Context, owner, repo, base, head string) ([]pkg.DependencyCheckResult, error) { + reqURL, err := url.Parse("https://api.github.com") + if err != nil { + return nil, fmt.Errorf("error parsing the url: %w", err) + } + reqURL.Path = url.PathEscape(path.Join( + "repos", owner, repo, "dependency-graph", "compare", base+"..."+head, + )) + req, err := http.NewRequestWithContext(ctx, "GET", reqURL.String(), nil) + if err != nil { + return nil, fmt.Errorf("request for dependency-diff failed with %w", err) + } + ghrt := roundtripper.NewTransport(ctx, nil) + resp, err := ghrt.RoundTrip(req) + if err != nil { + return nil, fmt.Errorf("error receiving the http reponse: %w with resp status code %v", err, resp.StatusCode) + } + defer resp.Body.Close() + deps := []dependency{} + err = json.NewDecoder(resp.Body).Decode(&deps) + if err != nil { + return nil, fmt.Errorf("error parsing the http response: %w", err) + } + + ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) + if err != nil { + return nil, fmt.Errorf("error creating the github repo: %w", err) + } + ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, nil) + ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, nil) + if err != nil { + return nil, fmt.Errorf("error creating the oss fuzz repo client: %w", err) + } + vulnsClient := clients.DefaultVulnerabilitiesClient() + + results := []pkg.DependencyCheckResult{} + for _, d := range deps { + depCheckResult := pkg.DependencyCheckResult{ + PackageURL: d.PackageURL, + SourceRepository: d.SourceRepository, + ChangeType: d.ChangeType, + ManifestPath: d.ManifestPath, + Ecosystem: d.Ecosystem, + Version: d.Version, + Name: d.Name, + } + // For now we skip those without source repo urls. + // TODO: use the BigQuery dataset to supplement null source repo URLs + // so that we can fetch the Scorecard results for them. + if d.SourceRepository != nil { + if *d.SourceRepository != "" { + // If the srcRepo is valid, run scorecard on this dependency and fetch the result. + // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. + scorecardResult, err := pkg.RunScorecards( + ctx, + ghRepo, + clients.HeadSHA, + checks.AllChecks, + ghRepoClient, + ossFuzzRepoClient, + nil, + vulnsClient, + ) + if err != nil { + return nil, fmt.Errorf("error fetching the scorecard result: %w", err) + } + depCheckResult.ScorecardResults = &scorecardResult + } // Append a DependencyCheckResult with an empty ScorecardResult if the srcRepo is an empty string. + } // Same as before, if the srcRepo field is null, we also return a DependencyCheckResult with an empty ScorecardResult. + results = append(results, depCheckResult) + } + return results, nil +} diff --git a/pkg/dependencydiff_result.go b/pkg/dependencydiff_result.go index 4a4beaaec9b..589a9b64216 100644 --- a/pkg/dependencydiff_result.go +++ b/pkg/dependencydiff_result.go @@ -71,6 +71,7 @@ type DependencyCheckResult struct { Name string `json:"name"` } +// AsJSON for DependencyCheckResult exports the DependencyCheckResult as a JSON object. func (dr *DependencyCheckResult) AsJSON(writer io.Writer) error { encoder := json.NewEncoder(writer) if err := encoder.Encode(*dr); err != nil { From 1c313bd75a13888c78018e85e60c34542d0b8ec9 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 11:56:16 -0700 Subject: [PATCH 25/49] temp0713-2 --- dependencydiff/dependencydiff_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index c44b723cc86..78bd101ce1f 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -22,7 +22,7 @@ import ( scut "github.com/ossf/scorecard/v4/utests" ) -// TestGetDependencyDiffResults is a test function for GetDependencyDiffResults +// TestGetDependencyDiffResults is a test function for GetDependencyDiffResults. func TestGetDependencyDiffResults(t *testing.T) { t.Parallel() //nolint @@ -64,7 +64,7 @@ func TestGetDependencyDiffResults(t *testing.T) { } } -// TestTestFetchDependencyDiffData is a test function for TestFetchDependencyDiffData +// TestTestFetchDependencyDiffData is a test function for TestFetchDependencyDiffData. func TestFetchDependencyDiffData(t *testing.T) { t.Parallel() //nolint From 5e9c33dbde0d3e5b5e2f369c766dcfb4bb08e51e Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 16:05:44 -0700 Subject: [PATCH 26/49] temp0713-3 --- dependencydiff/dependencydiff.go | 7 +- dependencydiff/dependencydiff_test.go | 16 ++--- dependencydiff/raw_dependencies.go | 93 +++++++++++++++------------ pkg/dependencydiff_result.go | 3 +- 4 files changed, 59 insertions(+), 60 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 62281b74577..51a86ae1aba 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -16,7 +16,6 @@ package dependencydiff import ( "context" - "fmt" "github.com/ossf/scorecard/v4/pkg" ) @@ -27,9 +26,5 @@ import ( func GetDependencyDiffResults(ownerName, repoName, baseSHA, headSHA string) ([]pkg.DependencyCheckResult, error) { ctx := context.Background() // Fetch dependency diffs using the GitHub Dependency Review API. - deps, err := fetchRawDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA) - if err != nil { - return nil, fmt.Errorf("error fetching dependency changes: %w", err) - } - return deps, nil + return fetchRawDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA) } diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index 78bd101ce1f..87da317a97b 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -40,7 +40,7 @@ func TestGetDependencyDiffResults(t *testing.T) { name: "error response", owner: "no_such_owner", repo: "repo_not_exist", - base: clients.HeadSHA, + base: "", head: clients.HeadSHA, wantEmpty: true, wantErr: true, @@ -55,10 +55,8 @@ func TestGetDependencyDiffResults(t *testing.T) { t.Errorf("GetDependencyDiffResults() error = %v, wantErr %v", err, tt.wantErr) return } - if got != nil { - if (len(got) == 0) != tt.wantEmpty { - t.Errorf("GetDependencyDiffResults() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) - } + if (len(got) == 0) != tt.wantEmpty { + t.Errorf("GetDependencyDiffResults() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) } }) } @@ -85,7 +83,7 @@ func TestFetchDependencyDiffData(t *testing.T) { ctx: context.Background(), owner: "no_such_owner", repo: "repo_not_exist", - base: clients.HeadSHA, + base: "", head: clients.HeadSHA, wantEmpty: true, wantErr: true, @@ -100,10 +98,8 @@ func TestFetchDependencyDiffData(t *testing.T) { t.Errorf("FetchDependencyDiffData() error = %v, wantErr %v", err, tt.wantErr) return } - if got != nil { - if (len(got) == 0) != tt.wantEmpty { - t.Errorf("FetchDependencyDiffData() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) - } + if (len(got) == 0) != tt.wantEmpty { + t.Errorf("FetchDependencyDiffData() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) } }) } diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 7eac96376d4..0f13c773af0 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -16,12 +16,13 @@ package dependencydiff import ( "context" - "encoding/json" "fmt" "net/http" - "net/url" "path" + "time" + "github.com/google/go-github/v38/github" + "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" @@ -29,6 +30,16 @@ import ( "github.com/ossf/scorecard/v4/pkg" ) +// checksToRun specifies a list of checks to run on every dependency, which is a subset of all the checks. +// This helps us reduce the GH token usage and scale the large time consumption of running all the checks. +var checksToRun = checker.CheckNameToFnMap{ + checks.CheckMaintained: checks.AllChecks[checks.CheckMaintained], + checks.CheckSecurityPolicy: checks.AllChecks[checks.CheckSecurityPolicy], + checks.CheckLicense: checks.AllChecks[checks.CheckLicense], + checks.CheckCodeReview: checks.AllChecks[checks.CheckCodeReview], + checks.CheckSAST: checks.AllChecks[checks.CheckSAST], +} + // Dependency is a raw dependency fetched from the GitHub Dependency Review API. type dependency struct { // Package URL is a short link for a package. @@ -56,42 +67,40 @@ type dependency struct { // fetchRawDependencyDiffData fetches the dependency-diffs between the two code commits // using the GitHub Dependency Review API, and returns a slice of Dependency. func fetchRawDependencyDiffData(ctx context.Context, owner, repo, base, head string) ([]pkg.DependencyCheckResult, error) { - reqURL, err := url.Parse("https://api.github.com") - if err != nil { - return nil, fmt.Errorf("error parsing the url: %w", err) - } - reqURL.Path = url.PathEscape(path.Join( - "repos", owner, repo, "dependency-graph", "compare", base+"..."+head, - )) - req, err := http.NewRequestWithContext(ctx, "GET", reqURL.String(), nil) + ghClient := github.NewClient( + &http.Client{ + Transport: roundtripper.NewTransport(ctx, nil), + Timeout: 10 * time.Second, + }, + ) + req, err := ghClient.NewRequest( + "GET", + path.Join("repos", owner, repo, "dependency-graph", "compare", base+"..."+head), + nil, + ) if err != nil { return nil, fmt.Errorf("request for dependency-diff failed with %w", err) } - ghrt := roundtripper.NewTransport(ctx, nil) - resp, err := ghrt.RoundTrip(req) - if err != nil { - return nil, fmt.Errorf("error receiving the http reponse: %w with resp status code %v", err, resp.StatusCode) - } - defer resp.Body.Close() deps := []dependency{} - err = json.NewDecoder(resp.Body).Decode(&deps) + _, err = ghClient.Do(ctx, req, &deps) if err != nil { - return nil, fmt.Errorf("error parsing the http response: %w", err) + return nil, fmt.Errorf("error receiving the http reponse: %w", err) } ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) if err != nil { return nil, fmt.Errorf("error creating the github repo: %w", err) } + + // Initialize the client(s) corresponding to the checks to run above. ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, nil) - ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, nil) - if err != nil { - return nil, fmt.Errorf("error creating the oss fuzz repo client: %w", err) - } - vulnsClient := clients.DefaultVulnerabilitiesClient() results := []pkg.DependencyCheckResult{} for _, d := range deps { + // Skip removed dependencies and don't run scorecard checks on them. + if *d.ChangeType == pkg.Removed { + continue + } depCheckResult := pkg.DependencyCheckResult{ PackageURL: d.PackageURL, SourceRepository: d.SourceRepository, @@ -104,26 +113,26 @@ func fetchRawDependencyDiffData(ctx context.Context, owner, repo, base, head str // For now we skip those without source repo urls. // TODO: use the BigQuery dataset to supplement null source repo URLs // so that we can fetch the Scorecard results for them. - if d.SourceRepository != nil { - if *d.SourceRepository != "" { - // If the srcRepo is valid, run scorecard on this dependency and fetch the result. - // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. - scorecardResult, err := pkg.RunScorecards( - ctx, - ghRepo, - clients.HeadSHA, - checks.AllChecks, - ghRepoClient, - ossFuzzRepoClient, - nil, - vulnsClient, - ) - if err != nil { - return nil, fmt.Errorf("error fetching the scorecard result: %w", err) - } + if d.SourceRepository != nil && *d.SourceRepository != "" { + // If the srcRepo is valid, run scorecard on this dependency and fetch the result. + // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. + scorecardResult, err := pkg.RunScorecards( + ctx, + ghRepo, + clients.HeadSHA, /* TODO: In future versions, ideally, this should be commitSHA corresponding to d.Version instead of HEAD. */ + checksToRun, + ghRepoClient, + nil, + nil, + nil, /* Initialize coresponding clients if checks are needed.*/ + ) + // "err==nil" suggests the run succeeds, so that we record the scorecard check results for this dependency. + // Otherwise, it indicates the run fails and we leave the current dependency scorecard result empty + // rather than letting the entire API return nil since we still expect results for other dependencies. + if err == nil { depCheckResult.ScorecardResults = &scorecardResult - } // Append a DependencyCheckResult with an empty ScorecardResult if the srcRepo is an empty string. - } // Same as before, if the srcRepo field is null, we also return a DependencyCheckResult with an empty ScorecardResult. + } + } results = append(results, depCheckResult) } return results, nil diff --git a/pkg/dependencydiff_result.go b/pkg/dependencydiff_result.go index 589a9b64216..307f39e6320 100644 --- a/pkg/dependencydiff_result.go +++ b/pkg/dependencydiff_result.go @@ -73,8 +73,7 @@ type DependencyCheckResult struct { // AsJSON for DependencyCheckResult exports the DependencyCheckResult as a JSON object. func (dr *DependencyCheckResult) AsJSON(writer io.Writer) error { - encoder := json.NewEncoder(writer) - if err := encoder.Encode(*dr); err != nil { + if err := json.NewEncoder(writer).Encode(*dr); err != nil { return sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("encoder.Encode: %v", err)) } return nil From 4da19cf1f19077e75b927d89ab0cd5e050989468 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 16:43:39 -0700 Subject: [PATCH 27/49] temp0713-4 --- dependencydiff/raw_dependencies.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 0f13c773af0..8c0f48679b7 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -22,11 +22,13 @@ import ( "time" "github.com/google/go-github/v38/github" + "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" "github.com/ossf/scorecard/v4/clients/githubrepo/roundtripper" + "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" ) @@ -66,10 +68,13 @@ type dependency struct { // fetchRawDependencyDiffData fetches the dependency-diffs between the two code commits // using the GitHub Dependency Review API, and returns a slice of Dependency. -func fetchRawDependencyDiffData(ctx context.Context, owner, repo, base, head string) ([]pkg.DependencyCheckResult, error) { +func fetchRawDependencyDiffData( + ctx context.Context, owner, repo, base, head string, +) ([]pkg.DependencyCheckResult, error) { + ghrt := roundtripper.NewTransport(ctx, log.NewLogger(log.InfoLevel)) ghClient := github.NewClient( &http.Client{ - Transport: roundtripper.NewTransport(ctx, nil), + Transport: ghrt, Timeout: 10 * time.Second, }, ) @@ -119,7 +124,9 @@ func fetchRawDependencyDiffData(ctx context.Context, owner, repo, base, head str scorecardResult, err := pkg.RunScorecards( ctx, ghRepo, - clients.HeadSHA, /* TODO: In future versions, ideally, this should be commitSHA corresponding to d.Version instead of HEAD. */ + // TODO: In future versions, ideally, this should be + // the commitSHA corresponding to d.Version instead of HEAD. + clients.HeadSHA, checksToRun, ghRepoClient, nil, From 26be711c323ee9af363aa465b9d59860ea3560b8 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 17:18:55 -0700 Subject: [PATCH 28/49] temp0713-4 --- dependencydiff/dependencydiff.go | 7 +++++-- dependencydiff/dependencydiff_test.go | 4 ++-- dependencydiff/raw_dependencies.go | 30 ++++++++++++++++++++------- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 51a86ae1aba..6f7af8b1969 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -23,8 +23,11 @@ import ( // 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. -func GetDependencyDiffResults(ownerName, repoName, baseSHA, headSHA string) ([]pkg.DependencyCheckResult, error) { +func GetDependencyDiffResults( + ownerName, repoName, baseSHA, headSHA string, + scorecardChecksNames []string, +) ([]pkg.DependencyCheckResult, error) { ctx := context.Background() // Fetch dependency diffs using the GitHub Dependency Review API. - return fetchRawDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA) + return fetchRawDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA, scorecardChecksNames) } diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index 87da317a97b..c1ddb294f7a 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -50,7 +50,7 @@ func TestGetDependencyDiffResults(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := GetDependencyDiffResults(tt.owner, tt.repo, tt.base, tt.head) + got, err := GetDependencyDiffResults(tt.owner, tt.repo, tt.base, tt.head, nil) if (err != nil) != tt.wantErr { t.Errorf("GetDependencyDiffResults() error = %v, wantErr %v", err, tt.wantErr) return @@ -93,7 +93,7 @@ func TestFetchDependencyDiffData(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := fetchRawDependencyDiffData(tt.ctx, tt.owner, tt.repo, tt.base, tt.head) + got, err := fetchRawDependencyDiffData(tt.ctx, tt.owner, tt.repo, tt.base, tt.head, nil) if (err != nil) != tt.wantErr { t.Errorf("FetchDependencyDiffData() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 8c0f48679b7..b45cabd4cf3 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -34,12 +34,12 @@ import ( // checksToRun specifies a list of checks to run on every dependency, which is a subset of all the checks. // This helps us reduce the GH token usage and scale the large time consumption of running all the checks. -var checksToRun = checker.CheckNameToFnMap{ - checks.CheckMaintained: checks.AllChecks[checks.CheckMaintained], - checks.CheckSecurityPolicy: checks.AllChecks[checks.CheckSecurityPolicy], - checks.CheckLicense: checks.AllChecks[checks.CheckLicense], - checks.CheckCodeReview: checks.AllChecks[checks.CheckCodeReview], - checks.CheckSAST: checks.AllChecks[checks.CheckSAST], +var defaultChecksToRun = []string{ + checks.CheckMaintained, + checks.CheckSecurityPolicy, + checks.CheckLicense, + checks.CheckCodeReview, + checks.CheckSAST, } // Dependency is a raw dependency fetched from the GitHub Dependency Review API. @@ -69,7 +69,9 @@ type dependency struct { // fetchRawDependencyDiffData fetches the dependency-diffs between the two code commits // using the GitHub Dependency Review API, and returns a slice of Dependency. func fetchRawDependencyDiffData( - ctx context.Context, owner, repo, base, head string, + ctx context.Context, + owner, repo, base, head string, + checkNamesToRun []string, ) ([]pkg.DependencyCheckResult, error) { ghrt := roundtripper.NewTransport(ctx, log.NewLogger(log.InfoLevel)) ghClient := github.NewClient( @@ -97,7 +99,19 @@ func fetchRawDependencyDiffData( return nil, fmt.Errorf("error creating the github repo: %w", err) } - // Initialize the client(s) corresponding to the checks to run above. + // Initialize the checks to run from the caller's input. + checksToRun := checker.CheckNameToFnMap{} + if checkNamesToRun == nil && len(checkNamesToRun) == 0 { + // If no check names are provided, we run the default checks for the caller. + for _, cn := range defaultChecksToRun { + checksToRun[cn] = checks.AllChecks[cn] + } + } else { + for _, cn := range checkNamesToRun { + checksToRun[cn] = checks.AllChecks[cn] + } + } + // Initialize the client(s) corresponding to the checks to run. ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, nil) results := []pkg.DependencyCheckResult{} From f3419b274e920944ce7006c35b03665a8cdbb565 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 17:41:15 -0700 Subject: [PATCH 29/49] temp0713-5 --- dependencydiff/raw_dependencies.go | 33 +++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index b45cabd4cf3..2074ee692bf 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -69,11 +69,10 @@ type dependency struct { // fetchRawDependencyDiffData fetches the dependency-diffs between the two code commits // using the GitHub Dependency Review API, and returns a slice of Dependency. func fetchRawDependencyDiffData( - ctx context.Context, - owner, repo, base, head string, - checkNamesToRun []string, + ctx context.Context, owner, repo, base, head string, checkNamesToRun []string, ) ([]pkg.DependencyCheckResult, error) { - ghrt := roundtripper.NewTransport(ctx, log.NewLogger(log.InfoLevel)) + logger := log.NewLogger(log.InfoLevel) + ghrt := roundtripper.NewTransport(ctx, logger) ghClient := github.NewClient( &http.Client{ Transport: ghrt, @@ -105,14 +104,34 @@ func fetchRawDependencyDiffData( // If no check names are provided, we run the default checks for the caller. for _, cn := range defaultChecksToRun { checksToRun[cn] = checks.AllChecks[cn] + // Use the check name to decide if we need to create the three special clients below. } } else { for _, cn := range checkNamesToRun { checksToRun[cn] = checks.AllChecks[cn] } } + // Initialize the client(s) corresponding to the checks to run. ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, nil) + // Initialize these three clients as nil at first. + var ossFuzzClient clients.RepoClient = nil + var vulnsClient clients.VulnerabilitiesClient = nil + var ciiClient clients.CIIBestPracticesClient = nil + // Create the corresponding client if the check needs to run. + for cn := range checksToRun { + switch cn { + case checks.CheckFuzzing: + ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(ctx, logger) + if err != nil { + return nil, err + } + case checks.CheckVulnerabilities: + vulnsClient = clients.DefaultVulnerabilitiesClient() + case checks.CheckCIIBestPractices: + ciiClient = clients.DefaultCIIBestPracticesClient() + } + } results := []pkg.DependencyCheckResult{} for _, d := range deps { @@ -143,9 +162,9 @@ func fetchRawDependencyDiffData( clients.HeadSHA, checksToRun, ghRepoClient, - nil, - nil, - nil, /* Initialize coresponding clients if checks are needed.*/ + ossFuzzClient, + ciiClient, + vulnsClient, ) // "err==nil" suggests the run succeeds, so that we record the scorecard check results for this dependency. // Otherwise, it indicates the run fails and we leave the current dependency scorecard result empty From 70f81c2beb9b380aafe76da7c6b0c2f1f4a7e6ef Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 17:45:33 -0700 Subject: [PATCH 30/49] temp0713-6 --- dependencydiff/raw_dependencies.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 2074ee692bf..02e78b79045 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -32,8 +32,9 @@ import ( "github.com/ossf/scorecard/v4/pkg" ) -// checksToRun specifies a list of checks to run on every dependency, which is a subset of all the checks. +// defaultChecksToRun specifies a list of checks to run on every dependency, which is a subset of all the checks. // This helps us reduce the GH token usage and scale the large time consumption of running all the checks. +// If the caller doesn't specify any checks, this one will be used. var defaultChecksToRun = []string{ checks.CheckMaintained, checks.CheckSecurityPolicy, From e3f4d87f0959856ba6d1ec677c21470648006abd Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 17:47:46 -0700 Subject: [PATCH 31/49] temp0713-7 --- dependencydiff/raw_dependencies.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 02e78b79045..f93c8120bce 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -32,7 +32,7 @@ import ( "github.com/ossf/scorecard/v4/pkg" ) -// defaultChecksToRun specifies a list of checks to run on every dependency, which is a subset of all the checks. +// defaultChecksToRun specifies a list of critical checks to run on every dependency. // This helps us reduce the GH token usage and scale the large time consumption of running all the checks. // If the caller doesn't specify any checks, this one will be used. var defaultChecksToRun = []string{ @@ -43,7 +43,7 @@ var defaultChecksToRun = []string{ checks.CheckSAST, } -// Dependency is a raw dependency fetched from the GitHub Dependency Review API. +// dependency is a raw dependency fetched from the GitHub Dependency Review API. type dependency struct { // Package URL is a short link for a package. PackageURL *string `json:"package_url"` @@ -114,7 +114,7 @@ func fetchRawDependencyDiffData( } // Initialize the client(s) corresponding to the checks to run. - ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, nil) + ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, logger) // Initialize these three clients as nil at first. var ossFuzzClient clients.RepoClient = nil var vulnsClient clients.VulnerabilitiesClient = nil From c9b8cc742727aea37554704625fdc1c43679b2d3 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 17:50:35 -0700 Subject: [PATCH 32/49] temp0713-8 --- dependencydiff/raw_dependencies.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index f93c8120bce..e8dec2b890a 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -68,7 +68,7 @@ type dependency struct { } // fetchRawDependencyDiffData fetches the dependency-diffs between the two code commits -// using the GitHub Dependency Review API, and returns a slice of Dependency. +// using the GitHub Dependency Review API, and returns a slice of DependencyCheckResult. func fetchRawDependencyDiffData( ctx context.Context, owner, repo, base, head string, checkNamesToRun []string, ) ([]pkg.DependencyCheckResult, error) { @@ -105,7 +105,6 @@ func fetchRawDependencyDiffData( // If no check names are provided, we run the default checks for the caller. for _, cn := range defaultChecksToRun { checksToRun[cn] = checks.AllChecks[cn] - // Use the check name to decide if we need to create the three special clients below. } } else { for _, cn := range checkNamesToRun { From aea729fb65d5767ea16e9cc16428bd44adc2d915 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 17:56:22 -0700 Subject: [PATCH 33/49] temp0713-9 --- dependencydiff/dependencydiff.go | 3 +-- dependencydiff/dependencydiff_test.go | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 6f7af8b1969..e84819eab30 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -24,8 +24,7 @@ import ( // 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. func GetDependencyDiffResults( - ownerName, repoName, baseSHA, headSHA string, - scorecardChecksNames []string, + ownerName, repoName, baseSHA, headSHA string, scorecardChecksNames []string, ) ([]pkg.DependencyCheckResult, error) { ctx := context.Background() // Fetch dependency diffs using the GitHub Dependency Review API. diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index c1ddb294f7a..fc422617243 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -26,7 +26,6 @@ import ( func TestGetDependencyDiffResults(t *testing.T) { t.Parallel() //nolint - //nolint tests := []struct { name string owner string From fd3d7b1942ca1c2f91ae9c9679e2bd14218c57fe Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 20:41:25 -0700 Subject: [PATCH 34/49] temp0713-10 --- dependencydiff/dependencydiff.go | 9 +-- dependencydiff/dependencydiff_test.go | 85 +++++++------------------ dependencydiff/raw_dependencies.go | 92 +++++++++++++-------------- 3 files changed, 73 insertions(+), 113 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index e84819eab30..808e54bb108 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -17,6 +17,7 @@ package dependencydiff import ( "context" + "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" ) @@ -24,9 +25,9 @@ import ( // 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. func GetDependencyDiffResults( - ownerName, repoName, baseSHA, headSHA string, scorecardChecksNames []string, + ctx context.Context, + ownerName, repoName, baseSHA, headSHA string, scorecardChecksNames []string, logger *log.Logger, ) ([]pkg.DependencyCheckResult, error) { - ctx := context.Background() - // Fetch dependency diffs using the GitHub Dependency Review API. - return fetchRawDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA, scorecardChecksNames) + // Fetch dependency diffs and get the dependency check results. + return fetchRawDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA, scorecardChecksNames, logger) } diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index fc422617243..690526f4d53 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -19,7 +19,7 @@ import ( "testing" "github.com/ossf/scorecard/v4/clients" - scut "github.com/ossf/scorecard/v4/utests" + "github.com/ossf/scorecard/v4/log" ) // TestGetDependencyDiffResults is a test function for GetDependencyDiffResults. @@ -27,78 +27,39 @@ func TestGetDependencyDiffResults(t *testing.T) { t.Parallel() //nolint tests := []struct { - name string - owner string - repo string - base string - head string - wantEmpty bool - wantErr bool + name string + owner string + repo string + base string + head string + ctx context.Context + logger *log.Logger + wantEmptyResults bool + wantErr bool }{ { - name: "error response", - owner: "no_such_owner", - repo: "repo_not_exist", - base: "", - head: clients.HeadSHA, - wantEmpty: true, - wantErr: true, + name: "error response", + owner: "no_such_owner", + repo: "repo_not_exist", + base: "", + head: clients.HeadSHA, + ctx: context.Background(), + logger: log.NewLogger(log.InfoLevel), + wantEmptyResults: true, + wantErr: true, }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := GetDependencyDiffResults(tt.owner, tt.repo, tt.base, tt.head, nil) + got, err := GetDependencyDiffResults(tt.ctx, tt.owner, tt.repo, tt.base, tt.head, nil, tt.logger) if (err != nil) != tt.wantErr { - t.Errorf("GetDependencyDiffResults() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("GetDependencyDiffResults() error = [%v], errWanted [%v]", err, tt.wantErr) return } - if (len(got) == 0) != tt.wantEmpty { - t.Errorf("GetDependencyDiffResults() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) - } - }) - } -} - -// TestTestFetchDependencyDiffData is a test function for TestFetchDependencyDiffData. -func TestFetchDependencyDiffData(t *testing.T) { - t.Parallel() - //nolint - tests := []struct { - name string - ctx context.Context - owner string - repo string - base string - head string - wantEmpty bool - wantErr bool - - expected scut.TestReturn - }{ - { - name: "error reponse", - ctx: context.Background(), - owner: "no_such_owner", - repo: "repo_not_exist", - base: "", - head: clients.HeadSHA, - wantEmpty: true, - wantErr: true, - }, - } // End of test cases. - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - got, err := fetchRawDependencyDiffData(tt.ctx, tt.owner, tt.repo, tt.base, tt.head, nil) - if (err != nil) != tt.wantErr { - t.Errorf("FetchDependencyDiffData() error = %v, wantErr %v", err, tt.wantErr) - return - } - if (len(got) == 0) != tt.wantEmpty { - t.Errorf("FetchDependencyDiffData() = %v, want empty %v for %v", got, tt.wantEmpty, tt.name) + if (len(got) == 0) != tt.wantEmptyResults { + t.Errorf("GetDependencyDiffResults() = %v, want empty [%v] for [%v]", got, tt.wantEmptyResults, tt.name) } }) } diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index e8dec2b890a..d367d6eaa01 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -32,18 +32,8 @@ import ( "github.com/ossf/scorecard/v4/pkg" ) -// defaultChecksToRun specifies a list of critical checks to run on every dependency. -// This helps us reduce the GH token usage and scale the large time consumption of running all the checks. -// If the caller doesn't specify any checks, this one will be used. -var defaultChecksToRun = []string{ - checks.CheckMaintained, - checks.CheckSecurityPolicy, - checks.CheckLicense, - checks.CheckCodeReview, - checks.CheckSAST, -} - // dependency is a raw dependency fetched from the GitHub Dependency Review API. +// Fields of a dependnecy correspondings to those of pkg.DependencyCheckResult. type dependency struct { // Package URL is a short link for a package. PackageURL *string `json:"package_url"` @@ -70,9 +60,8 @@ type dependency struct { // fetchRawDependencyDiffData fetches the dependency-diffs between the two code commits // using the GitHub Dependency Review API, and returns a slice of DependencyCheckResult. func fetchRawDependencyDiffData( - ctx context.Context, owner, repo, base, head string, checkNamesToRun []string, + ctx context.Context, owner, repo, base, head string, checkNamesToRun []string, logger *log.Logger, ) ([]pkg.DependencyCheckResult, error) { - logger := log.NewLogger(log.InfoLevel) ghrt := roundtripper.NewTransport(ctx, logger) ghClient := github.NewClient( &http.Client{ @@ -86,26 +75,30 @@ func fetchRawDependencyDiffData( nil, ) if err != nil { - return nil, fmt.Errorf("request for dependency-diff failed with %w", err) + wrapped := fmt.Errorf("request for dependency-diff failed with %w", err) + logger.Error(wrapped, "") + return nil, wrapped } deps := []dependency{} _, err = ghClient.Do(ctx, req, &deps) if err != nil { - return nil, fmt.Errorf("error receiving the http reponse: %w", err) + wrapped := fmt.Errorf("error receiving the http reponse: %w", err) + logger.Error(wrapped, "") + return nil, wrapped } ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) if err != nil { - return nil, fmt.Errorf("error creating the github repo: %w", err) + wrapped := fmt.Errorf("error creating the github repo: %w", err) + logger.Error(wrapped, "") + return nil, wrapped } // Initialize the checks to run from the caller's input. checksToRun := checker.CheckNameToFnMap{} if checkNamesToRun == nil && len(checkNamesToRun) == 0 { - // If no check names are provided, we run the default checks for the caller. - for _, cn := range defaultChecksToRun { - checksToRun[cn] = checks.AllChecks[cn] - } + // If no check names are provided, we run all the checks for the caller. + checksToRun = checks.AllChecks } else { for _, cn := range checkNamesToRun { checksToRun[cn] = checks.AllChecks[cn] @@ -115,16 +108,18 @@ func fetchRawDependencyDiffData( // Initialize the client(s) corresponding to the checks to run. ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, logger) // Initialize these three clients as nil at first. - var ossFuzzClient clients.RepoClient = nil - var vulnsClient clients.VulnerabilitiesClient = nil - var ciiClient clients.CIIBestPracticesClient = nil + var ossFuzzClient clients.RepoClient + var vulnsClient clients.VulnerabilitiesClient + var ciiClient clients.CIIBestPracticesClient // Create the corresponding client if the check needs to run. for cn := range checksToRun { switch cn { case checks.CheckFuzzing: ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(ctx, logger) if err != nil { - return nil, err + wrapped := fmt.Errorf("error initializing the oss fuzz repo client: %v", err) + logger.Error(wrapped, "") + return nil, wrapped } case checks.CheckVulnerabilities: vulnsClient = clients.DefaultVulnerabilitiesClient() @@ -135,10 +130,6 @@ func fetchRawDependencyDiffData( results := []pkg.DependencyCheckResult{} for _, d := range deps { - // Skip removed dependencies and don't run scorecard checks on them. - if *d.ChangeType == pkg.Removed { - continue - } depCheckResult := pkg.DependencyCheckResult{ PackageURL: d.PackageURL, SourceRepository: d.SourceRepository, @@ -152,25 +143,32 @@ func fetchRawDependencyDiffData( // TODO: use the BigQuery dataset to supplement null source repo URLs // so that we can fetch the Scorecard results for them. if d.SourceRepository != nil && *d.SourceRepository != "" { - // If the srcRepo is valid, run scorecard on this dependency and fetch the result. - // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. - scorecardResult, err := pkg.RunScorecards( - ctx, - ghRepo, - // TODO: In future versions, ideally, this should be - // the commitSHA corresponding to d.Version instead of HEAD. - clients.HeadSHA, - checksToRun, - ghRepoClient, - ossFuzzClient, - ciiClient, - vulnsClient, - ) - // "err==nil" suggests the run succeeds, so that we record the scorecard check results for this dependency. - // Otherwise, it indicates the run fails and we leave the current dependency scorecard result empty - // rather than letting the entire API return nil since we still expect results for other dependencies. - if err == nil { - depCheckResult.ScorecardResults = &scorecardResult + if d.ChangeType != nil && *d.ChangeType != pkg.Removed { + // Run scorecard on those added/updated dependencies with valid srcRepo URLs and fetch the result. + // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. + scorecardResult, err := pkg.RunScorecards( + ctx, + ghRepo, + // TODO: In future versions, ideally, this should be + // the commitSHA corresponding to d.Version instead of HEAD. + clients.HeadSHA, + checksToRun, + ghRepoClient, + ossFuzzClient, + ciiClient, + vulnsClient, + ) + // "err==nil" suggests the run succeeds, so that we record the scorecard check results for this dependency. + // Otherwise, it indicates the run fails and we leave the current dependency scorecard result empty + // rather than letting the entire API return nil since we still expect results for other dependencies. + if err != nil { + logger.Error( + fmt.Errorf("error running scorecard checks: %v", err), + fmt.Sprintf("The scorecard checks running for dependency %s failed.", d.Name), + ) + } else { + depCheckResult.ScorecardResults = &scorecardResult + } } } results = append(results, depCheckResult) From 0e3cb52ff23ce1db8a9849e74c48b6c4f07152a7 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 21:09:38 -0700 Subject: [PATCH 35/49] temp0713-11 --- dependencydiff/raw_dependencies.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index d367d6eaa01..ac9ded31d5e 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -60,7 +60,9 @@ type dependency struct { // fetchRawDependencyDiffData fetches the dependency-diffs between the two code commits // using the GitHub Dependency Review API, and returns a slice of DependencyCheckResult. func fetchRawDependencyDiffData( - ctx context.Context, owner, repo, base, head string, checkNamesToRun []string, logger *log.Logger, + ctx context.Context, + owner, repo, base, head string, checkNamesToRun []string, + logger *log.Logger, ) ([]pkg.DependencyCheckResult, error) { ghrt := roundtripper.NewTransport(ctx, logger) ghClient := github.NewClient( @@ -117,7 +119,7 @@ func fetchRawDependencyDiffData( case checks.CheckFuzzing: ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(ctx, logger) if err != nil { - wrapped := fmt.Errorf("error initializing the oss fuzz repo client: %v", err) + wrapped := fmt.Errorf("error initializing the oss fuzz repo client: %w", err) logger.Error(wrapped, "") return nil, wrapped } @@ -163,7 +165,7 @@ func fetchRawDependencyDiffData( // rather than letting the entire API return nil since we still expect results for other dependencies. if err != nil { logger.Error( - fmt.Errorf("error running scorecard checks: %v", err), + fmt.Errorf("error running scorecard checks: %w", err), fmt.Sprintf("The scorecard checks running for dependency %s failed.", d.Name), ) } else { From 70a3894e6f7b9f78ed867305334ddcd2ed6a7067 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 21:37:41 -0700 Subject: [PATCH 36/49] temp0713-12 --- dependencydiff/dependencydiff.go | 11 ++++++-- dependencydiff/raw_dependencies.go | 43 +++++++++++------------------- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 808e54bb108..09ba7078d67 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -26,8 +26,15 @@ import ( // TO use this API, an access token must be set following https://github.com/ossf/scorecard#authentication. func GetDependencyDiffResults( ctx context.Context, - ownerName, repoName, baseSHA, headSHA string, scorecardChecksNames []string, logger *log.Logger, + ownerName, repoName, baseSHA, headSHA string, + scorecardChecksNames []string, + logger *log.Logger, ) ([]pkg.DependencyCheckResult, error) { // Fetch dependency diffs and get the dependency check results. - return fetchRawDependencyDiffData(ctx, ownerName, repoName, baseSHA, headSHA, scorecardChecksNames, logger) + return fetchRawDependencyDiffData( + ctx, + ownerName, repoName, baseSHA, headSHA, + scorecardChecksNames, + logger, + ) } diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index ac9ded31d5e..77593809afe 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -19,7 +19,6 @@ import ( "fmt" "net/http" "path" - "time" "github.com/google/go-github/v38/github" @@ -61,14 +60,14 @@ type dependency struct { // using the GitHub Dependency Review API, and returns a slice of DependencyCheckResult. func fetchRawDependencyDiffData( ctx context.Context, - owner, repo, base, head string, checkNamesToRun []string, - logger *log.Logger, -) ([]pkg.DependencyCheckResult, error) { + owner, repo, base, head string, + checkNamesToRun []string, + logger *log.Logger) ([]pkg.DependencyCheckResult, error) { + ghrt := roundtripper.NewTransport(ctx, logger) ghClient := github.NewClient( &http.Client{ Transport: ghrt, - Timeout: 10 * time.Second, }, ) req, err := ghClient.NewRequest( @@ -77,25 +76,13 @@ func fetchRawDependencyDiffData( nil, ) if err != nil { - wrapped := fmt.Errorf("request for dependency-diff failed with %w", err) - logger.Error(wrapped, "") - return nil, wrapped + return nil, fmt.Errorf("request for dependency-diff failed with %w", err) } deps := []dependency{} _, err = ghClient.Do(ctx, req, &deps) if err != nil { - wrapped := fmt.Errorf("error receiving the http reponse: %w", err) - logger.Error(wrapped, "") - return nil, wrapped - } - - ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) - if err != nil { - wrapped := fmt.Errorf("error creating the github repo: %w", err) - logger.Error(wrapped, "") - return nil, wrapped + return nil, fmt.Errorf("error receiving the http reponse: %w", err) } - // Initialize the checks to run from the caller's input. checksToRun := checker.CheckNameToFnMap{} if checkNamesToRun == nil && len(checkNamesToRun) == 0 { @@ -106,22 +93,24 @@ func fetchRawDependencyDiffData( checksToRun[cn] = checks.AllChecks[cn] } } - - // Initialize the client(s) corresponding to the checks to run. + // Initialize the repo and client(s) corresponding to the checks to run. + ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) + if err != nil { + return nil, fmt.Errorf("error creating the github repo: %w", err) + } ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, logger) // Initialize these three clients as nil at first. var ossFuzzClient clients.RepoClient var vulnsClient clients.VulnerabilitiesClient var ciiClient clients.CIIBestPracticesClient + // Create the corresponding client if the check needs to run. for cn := range checksToRun { switch cn { case checks.CheckFuzzing: ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(ctx, logger) if err != nil { - wrapped := fmt.Errorf("error initializing the oss fuzz repo client: %w", err) - logger.Error(wrapped, "") - return nil, wrapped + return nil, fmt.Errorf("error initializing the oss fuzz repo client: %w", err) } case checks.CheckVulnerabilities: vulnsClient = clients.DefaultVulnerabilitiesClient() @@ -129,7 +118,6 @@ func fetchRawDependencyDiffData( ciiClient = clients.DefaultCIIBestPracticesClient() } } - results := []pkg.DependencyCheckResult{} for _, d := range deps { depCheckResult := pkg.DependencyCheckResult{ @@ -160,15 +148,14 @@ func fetchRawDependencyDiffData( ciiClient, vulnsClient, ) - // "err==nil" suggests the run succeeds, so that we record the scorecard check results for this dependency. - // Otherwise, it indicates the run fails and we leave the current dependency scorecard result empty + // If the run fails, we leave the current dependency scorecard result empty // rather than letting the entire API return nil since we still expect results for other dependencies. if err != nil { logger.Error( fmt.Errorf("error running scorecard checks: %w", err), fmt.Sprintf("The scorecard checks running for dependency %s failed.", d.Name), ) - } else { + } else { // Otherwise, we record the scorecard check results for this dependency. depCheckResult.ScorecardResults = &scorecardResult } } From a9658df1a84c9a1124351f8bf4807b501d16e8bf Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 22:12:15 -0700 Subject: [PATCH 37/49] 1 --- dependencydiff/dependencydiff.go | 116 +++++++++++++++++++++++++++-- dependencydiff/raw_dependencies.go | 89 +--------------------- 2 files changed, 112 insertions(+), 93 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 09ba7078d67..a569ae2a80f 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -16,7 +16,13 @@ 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" "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" ) @@ -25,16 +31,112 @@ import ( // 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. func GetDependencyDiffResults( - ctx context.Context, - ownerName, repoName, baseSHA, headSHA string, - scorecardChecksNames []string, - logger *log.Logger, -) ([]pkg.DependencyCheckResult, error) { - // Fetch dependency diffs and get the dependency check results. - return fetchRawDependencyDiffData( + ctx context.Context, ownerName, repoName, baseSHA, headSHA string, + scorecardChecksNames []string, logger *log.Logger) ([]pkg.DependencyCheckResult, error) { + // Fetch the raw dependency diffs. + deps, err := fetchRawDependencyDiffData( ctx, ownerName, repoName, baseSHA, headSHA, scorecardChecksNames, logger, ) + if err != nil { + return nil, fmt.Errorf("error fetching the dependency-diff: %w", err) + } + // Initialize the checks to run from the caller's input. + checksToRun := initScorecardChecks(scorecardChecksNames) + // Initialize the repo and client(s) corresponding to the checks to run. + ghRepo, ghRepoClient, ossFuzzClient, vulnsClient, ciiClient, err := initRepoAndClient( + ownerName, repoName, ctx, logger, checksToRun, + ) + if err != nil { + return nil, fmt.Errorf("error initializing repo and client: %w", err) + } + results := []pkg.DependencyCheckResult{} + for _, d := range deps { + depCheckResult := pkg.DependencyCheckResult{ + PackageURL: d.PackageURL, + SourceRepository: d.SourceRepository, + ChangeType: d.ChangeType, + ManifestPath: d.ManifestPath, + Ecosystem: d.Ecosystem, + Version: d.Version, + Name: d.Name, + } + // For now we skip those without source repo urls. + // TODO: use the BigQuery dataset to supplement null source repo URLs + // so that we can fetch the Scorecard results for them. + if d.SourceRepository != nil && *d.SourceRepository != "" { + if d.ChangeType != nil && *d.ChangeType != pkg.Removed { + // Run scorecard on those added/updated dependencies with valid srcRepo URLs and fetch the result. + // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. + scorecardResult, err := pkg.RunScorecards( + ctx, + ghRepo, + // TODO: In future versions, ideally, this should be + // the commitSHA corresponding to d.Version instead of HEAD. + clients.HeadSHA, + checksToRun, + ghRepoClient, + ossFuzzClient, + ciiClient, + vulnsClient, + ) + // If the run fails, we leave the current dependency scorecard result empty + // rather than letting the entire API return nil since we still expect results for other dependencies. + if err != nil { + logger.Error( + fmt.Errorf("error running scorecard checks: %w", err), + fmt.Sprintf("The scorecard checks running for dependency %s failed.", d.Name), + ) + } else { // Otherwise, we record the scorecard check results for this dependency. + depCheckResult.ScorecardResults = &scorecardResult + } + } + } + results = append(results, depCheckResult) + } + return results, nil +} + +func initRepoAndClient(owner, repo string, ctx context.Context, logger *log.Logger, c checker.CheckNameToFnMap) ( + clients.Repo, clients.RepoClient, clients.RepoClient, + clients.VulnerabilitiesClient, clients.CIIBestPracticesClient, error) { + // Create the repo and the corresponding client if its check needs to run. + ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) + if err != nil { + return nil, nil, nil, nil, nil, fmt.Errorf("error creating the github repo: %w", err) + } + ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, logger) + // Initialize these three clients as nil at first. + var ossFuzzClient clients.RepoClient + var vulnsClient clients.VulnerabilitiesClient + var ciiClient clients.CIIBestPracticesClient + for cn := range c { + switch cn { + case checks.CheckFuzzing: + ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(ctx, logger) + if err != nil { + return nil, nil, nil, nil, nil, fmt.Errorf("error initializing the oss fuzz repo client: %w", err) + } + case checks.CheckVulnerabilities: + vulnsClient = clients.DefaultVulnerabilitiesClient() + case checks.CheckCIIBestPractices: + ciiClient = clients.DefaultCIIBestPracticesClient() + } + } + return ghRepo, ghRepoClient, ossFuzzClient, vulnsClient, ciiClient, nil +} + +func initScorecardChecks(checkNames []string) checker.CheckNameToFnMap { + checksToRun := checker.CheckNameToFnMap{} + if checkNames == nil && len(checkNames) == 0 { + // If no check names are provided, we run all the checks for the caller. + checksToRun = checks.AllChecks + } else { + for _, cn := range checkNames { + checksToRun[cn] = checks.AllChecks[cn] + } + } + return checksToRun } diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 77593809afe..25f48697306 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -22,10 +22,6 @@ import ( "github.com/google/go-github/v38/github" - "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" "github.com/ossf/scorecard/v4/clients/githubrepo/roundtripper" "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" @@ -62,7 +58,7 @@ func fetchRawDependencyDiffData( ctx context.Context, owner, repo, base, head string, checkNamesToRun []string, - logger *log.Logger) ([]pkg.DependencyCheckResult, error) { + logger *log.Logger) ([]dependency, error) { ghrt := roundtripper.NewTransport(ctx, logger) ghClient := github.NewClient( @@ -81,86 +77,7 @@ func fetchRawDependencyDiffData( deps := []dependency{} _, err = ghClient.Do(ctx, req, &deps) if err != nil { - return nil, fmt.Errorf("error receiving the http reponse: %w", err) + return nil, fmt.Errorf("error receiving the dependency-diff reponse: %w", err) } - // Initialize the checks to run from the caller's input. - checksToRun := checker.CheckNameToFnMap{} - if checkNamesToRun == nil && len(checkNamesToRun) == 0 { - // If no check names are provided, we run all the checks for the caller. - checksToRun = checks.AllChecks - } else { - for _, cn := range checkNamesToRun { - checksToRun[cn] = checks.AllChecks[cn] - } - } - // Initialize the repo and client(s) corresponding to the checks to run. - ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) - if err != nil { - return nil, fmt.Errorf("error creating the github repo: %w", err) - } - ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, logger) - // Initialize these three clients as nil at first. - var ossFuzzClient clients.RepoClient - var vulnsClient clients.VulnerabilitiesClient - var ciiClient clients.CIIBestPracticesClient - - // Create the corresponding client if the check needs to run. - for cn := range checksToRun { - switch cn { - case checks.CheckFuzzing: - ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(ctx, logger) - if err != nil { - return nil, fmt.Errorf("error initializing the oss fuzz repo client: %w", err) - } - case checks.CheckVulnerabilities: - vulnsClient = clients.DefaultVulnerabilitiesClient() - case checks.CheckCIIBestPractices: - ciiClient = clients.DefaultCIIBestPracticesClient() - } - } - results := []pkg.DependencyCheckResult{} - for _, d := range deps { - depCheckResult := pkg.DependencyCheckResult{ - PackageURL: d.PackageURL, - SourceRepository: d.SourceRepository, - ChangeType: d.ChangeType, - ManifestPath: d.ManifestPath, - Ecosystem: d.Ecosystem, - Version: d.Version, - Name: d.Name, - } - // For now we skip those without source repo urls. - // TODO: use the BigQuery dataset to supplement null source repo URLs - // so that we can fetch the Scorecard results for them. - if d.SourceRepository != nil && *d.SourceRepository != "" { - if d.ChangeType != nil && *d.ChangeType != pkg.Removed { - // Run scorecard on those added/updated dependencies with valid srcRepo URLs and fetch the result. - // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. - scorecardResult, err := pkg.RunScorecards( - ctx, - ghRepo, - // TODO: In future versions, ideally, this should be - // the commitSHA corresponding to d.Version instead of HEAD. - clients.HeadSHA, - checksToRun, - ghRepoClient, - ossFuzzClient, - ciiClient, - vulnsClient, - ) - // If the run fails, we leave the current dependency scorecard result empty - // rather than letting the entire API return nil since we still expect results for other dependencies. - if err != nil { - logger.Error( - fmt.Errorf("error running scorecard checks: %w", err), - fmt.Sprintf("The scorecard checks running for dependency %s failed.", d.Name), - ) - } else { // Otherwise, we record the scorecard check results for this dependency. - depCheckResult.ScorecardResults = &scorecardResult - } - } - } - results = append(results, depCheckResult) - } - return results, nil + return deps, nil } From e1e06534f4733c2a38e7ce217130867ee2a22867 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 22:21:20 -0700 Subject: [PATCH 38/49] temp --- dependencydiff/dependencydiff.go | 4 ++-- dependencydiff/raw_dependencies.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index a569ae2a80f..2919a237ef5 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -47,7 +47,7 @@ func GetDependencyDiffResults( checksToRun := initScorecardChecks(scorecardChecksNames) // Initialize the repo and client(s) corresponding to the checks to run. ghRepo, ghRepoClient, ossFuzzClient, vulnsClient, ciiClient, err := initRepoAndClient( - ownerName, repoName, ctx, logger, checksToRun, + ctx, ownerName, repoName, logger, checksToRun, ) if err != nil { return nil, fmt.Errorf("error initializing repo and client: %w", err) @@ -99,7 +99,7 @@ func GetDependencyDiffResults( return results, nil } -func initRepoAndClient(owner, repo string, ctx context.Context, logger *log.Logger, c checker.CheckNameToFnMap) ( +func initRepoAndClient(ctx context.Context, owner, repo string, logger *log.Logger, c checker.CheckNameToFnMap) ( clients.Repo, clients.RepoClient, clients.RepoClient, clients.VulnerabilitiesClient, clients.CIIBestPracticesClient, error) { // Create the repo and the corresponding client if its check needs to run. diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 25f48697306..5a08b144688 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -57,7 +57,6 @@ type dependency struct { func fetchRawDependencyDiffData( ctx context.Context, owner, repo, base, head string, - checkNamesToRun []string, logger *log.Logger) ([]dependency, error) { ghrt := roundtripper.NewTransport(ctx, logger) From 751d67dfb2e3d21b84908a0910acfc128d2f9e45 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Wed, 13 Jul 2022 22:28:15 -0700 Subject: [PATCH 39/49] temp --- dependencydiff/dependencydiff.go | 1 - 1 file changed, 1 deletion(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 2919a237ef5..bf975c6cf72 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -37,7 +37,6 @@ func GetDependencyDiffResults( deps, err := fetchRawDependencyDiffData( ctx, ownerName, repoName, baseSHA, headSHA, - scorecardChecksNames, logger, ) if err != nil { From 459cf9799d9d7b46a44a6824df930a94f739b038 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Thu, 14 Jul 2022 20:32:57 -0700 Subject: [PATCH 40/49] temp --- dependencydiff/dependencydiff.go | 140 ++++----------- dependencydiff/dependencydiff_test.go | 249 +++++++++++++++++++++++--- dependencydiff/raw_dependencies.go | 118 ++++++++++-- e2e/dependencydiff_test.go | 51 ++++++ pkg/dependencydiff_result.go | 14 +- 5 files changed, 422 insertions(+), 150 deletions(-) create mode 100644 e2e/dependencydiff_test.go diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index bf975c6cf72..a52cc765724 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -17,125 +17,57 @@ 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" "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" ) +// Depdiff is the exported name for dependency-diff. +const Depdiff = "Dependency-diff" + +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 +} + // 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. func GetDependencyDiffResults( - ctx context.Context, ownerName, repoName, baseSHA, headSHA string, - scorecardChecksNames []string, logger *log.Logger) ([]pkg.DependencyCheckResult, error) { + ctx context.Context, ownerName, repoName, baseSHA, headSHA string, scorecardChecksNames []string, + changeTypesToCheck map[pkg.ChangeType]bool) ([]pkg.DependencyCheckResult, error) { // Fetch the raw dependency diffs. - deps, err := fetchRawDependencyDiffData( - ctx, - ownerName, repoName, baseSHA, headSHA, - logger, - ) - if err != nil { - return nil, fmt.Errorf("error fetching the dependency-diff: %w", err) + dCtx := dependencydiffContext{ + logger: log.NewLogger(log.InfoLevel), + ownerName: ownerName, + repoName: repoName, + baseSHA: baseSHA, + headSHA: headSHA, + ctx: ctx, + changeTypesToCheck: changeTypesToCheck, + checkNamesToRun: scorecardChecksNames, } - // Initialize the checks to run from the caller's input. - checksToRun := initScorecardChecks(scorecardChecksNames) - // Initialize the repo and client(s) corresponding to the checks to run. - ghRepo, ghRepoClient, ossFuzzClient, vulnsClient, ciiClient, err := initRepoAndClient( - ctx, ownerName, repoName, logger, checksToRun, - ) + err := fetchRawDependencyDiffData(&dCtx) if err != nil { - return nil, fmt.Errorf("error initializing repo and client: %w", err) - } - results := []pkg.DependencyCheckResult{} - for _, d := range deps { - depCheckResult := pkg.DependencyCheckResult{ - PackageURL: d.PackageURL, - SourceRepository: d.SourceRepository, - ChangeType: d.ChangeType, - ManifestPath: d.ManifestPath, - Ecosystem: d.Ecosystem, - Version: d.Version, - Name: d.Name, - } - // For now we skip those without source repo urls. - // TODO: use the BigQuery dataset to supplement null source repo URLs - // so that we can fetch the Scorecard results for them. - if d.SourceRepository != nil && *d.SourceRepository != "" { - if d.ChangeType != nil && *d.ChangeType != pkg.Removed { - // Run scorecard on those added/updated dependencies with valid srcRepo URLs and fetch the result. - // TODO: use the Scorecare REST API to retrieve the Scorecard result statelessly. - scorecardResult, err := pkg.RunScorecards( - ctx, - ghRepo, - // TODO: In future versions, ideally, this should be - // the commitSHA corresponding to d.Version instead of HEAD. - clients.HeadSHA, - checksToRun, - ghRepoClient, - ossFuzzClient, - ciiClient, - vulnsClient, - ) - // If the run fails, we leave the current dependency scorecard result empty - // rather than letting the entire API return nil since we still expect results for other dependencies. - if err != nil { - logger.Error( - fmt.Errorf("error running scorecard checks: %w", err), - fmt.Sprintf("The scorecard checks running for dependency %s failed.", d.Name), - ) - } else { // Otherwise, we record the scorecard check results for this dependency. - depCheckResult.ScorecardResults = &scorecardResult - } - } - } - results = append(results, depCheckResult) + return nil, fmt.Errorf("error in fetchRawDependencyDiffData: %w", err) } - return results, nil -} -func initRepoAndClient(ctx context.Context, owner, repo string, logger *log.Logger, c checker.CheckNameToFnMap) ( - clients.Repo, clients.RepoClient, clients.RepoClient, - clients.VulnerabilitiesClient, clients.CIIBestPracticesClient, error) { - // Create the repo and the corresponding client if its check needs to run. - ghRepo, err := githubrepo.MakeGithubRepo(path.Join(owner, repo)) + // Initialize the repo and client(s) corresponding to the checks to run. + err = initRepoAndClientByChecks(&dCtx) if err != nil { - return nil, nil, nil, nil, nil, fmt.Errorf("error creating the github repo: %w", err) - } - ghRepoClient := githubrepo.CreateGithubRepoClient(ctx, logger) - // Initialize these three clients as nil at first. - var ossFuzzClient clients.RepoClient - var vulnsClient clients.VulnerabilitiesClient - var ciiClient clients.CIIBestPracticesClient - for cn := range c { - switch cn { - case checks.CheckFuzzing: - ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(ctx, logger) - if err != nil { - return nil, nil, nil, nil, nil, fmt.Errorf("error initializing the oss fuzz repo client: %w", err) - } - case checks.CheckVulnerabilities: - vulnsClient = clients.DefaultVulnerabilitiesClient() - case checks.CheckCIIBestPractices: - ciiClient = clients.DefaultCIIBestPracticesClient() - } - } - return ghRepo, ghRepoClient, ossFuzzClient, vulnsClient, ciiClient, nil -} - -func initScorecardChecks(checkNames []string) checker.CheckNameToFnMap { - checksToRun := checker.CheckNameToFnMap{} - if checkNames == nil && len(checkNames) == 0 { - // If no check names are provided, we run all the checks for the caller. - checksToRun = checks.AllChecks - } else { - for _, cn := range checkNames { - checksToRun[cn] = checks.AllChecks[cn] - } + return nil, fmt.Errorf("error in initRepoAndClientByChecks: %w", err) } - return checksToRun + err = getScorecardCheckResults(&dCtx) + return dCtx.results, fmt.Errorf("error in getScorecardCheckResults: %w", err) } diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index 690526f4d53..2e10a43d276 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -16,51 +16,252 @@ package dependencydiff import ( "context" + "path" "testing" + "github.com/ossf/scorecard/v4/checks" "github.com/ossf/scorecard/v4/clients" "github.com/ossf/scorecard/v4/log" + "github.com/ossf/scorecard/v4/pkg" ) -// TestGetDependencyDiffResults is a test function for GetDependencyDiffResults. -func TestGetDependencyDiffResults(t *testing.T) { +// Test_fetchRawDependencyDiffData is a test function for fetchRawDependencyDiffData. +func Test_fetchRawDependencyDiffData(t *testing.T) { t.Parallel() //nolint tests := []struct { - name string - owner string - repo string - base string - head string - ctx context.Context - logger *log.Logger - wantEmptyResults bool - wantErr bool + name string + dCtx dependencydiffContext + wantEmpty bool + wantErr bool }{ { - name: "error response", - owner: "no_such_owner", - repo: "repo_not_exist", - base: "", - head: clients.HeadSHA, - ctx: context.Background(), - logger: log.NewLogger(log.InfoLevel), - wantEmptyResults: true, - wantErr: true, + name: "error response", + dCtx: dependencydiffContext{ + logger: log.NewLogger(log.InfoLevel), + ctx: context.Background(), + ownerName: "no_such_owner", + repoName: "repo_not_exist", + baseSHA: "base", + headSHA: clients.HeadSHA, + }, + wantEmpty: true, + wantErr: true, + }, + { + name: "normal response", + dCtx: dependencydiffContext{ + logger: log.NewLogger(log.InfoLevel), + ctx: context.Background(), + ownerName: "ossf-tests", + repoName: "scorecard-depdiff", + baseSHA: "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", + headSHA: "1989568f93e484f6a86f8b276b170e3d6962ce12", + }, + wantEmpty: false, + wantErr: false, + }, + { + name: "normal response with empty results", + dCtx: dependencydiffContext{ + logger: log.NewLogger(log.InfoLevel), + ctx: context.Background(), + ownerName: "ossf-tests", + repoName: "scorecard-depdiff", + baseSHA: "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", + headSHA: "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", + }, + wantEmpty: true, + wantErr: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := fetchRawDependencyDiffData(&tt.dCtx) + if (err != nil) != tt.wantErr { + t.Errorf("fetchRawDependencyDiffData() error = {%v}, want error: %v", err, tt.wantErr) + return + } + lenResults := len(tt.dCtx.dependencydiffs) + if (lenResults == 0) != tt.wantEmpty { + t.Errorf("want empty results: %v, got len of results:%d", tt.wantEmpty, lenResults) + return + } + + }) + } +} + +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: "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, + wantRepoClient: false, + wantFuzzClient: false, + wantVulnClient: false, + wantCIIClient: false, + wantErr: true, + }, + { + name: "normal response with all clients", + dCtx: dependencydiffContext{ + logger: log.NewLogger(log.InfoLevel), + ctx: context.Background(), + ownerName: "ossf-tests", + repoName: "scorecard-depdiff", + checkNamesToRun: []string{ + checks.CheckFuzzing, + checks.CheckVulnerabilities, + checks.CheckCIIBestPractices, + }, + }, + wantGhRepo: true, + wantRepoClient: true, + wantFuzzClient: true, + wantVulnClient: true, + wantCIIClient: true, + }, + { + name: "normal response with only repo and repo client", + dCtx: dependencydiffContext{ + logger: log.NewLogger(log.InfoLevel), + ctx: context.Background(), + ownerName: "ossf-tests", + repoName: "scorecard-depdiff", + checkNamesToRun: []string{ + checks.CheckSecurityPolicy, + }, + }, + wantGhRepo: true, + wantRepoClient: true, + wantFuzzClient: false, + wantVulnClient: false, + wantCIIClient: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := initRepoAndClientByChecks(&tt.dCtx) + 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) + return + } + if (tt.dCtx.ghRepoClient != nil) != tt.wantRepoClient { + t.Errorf("init repo error, wantRepoClient: %v, got %v", tt.wantRepoClient, tt.dCtx.ghRepoClient) + return + } + if (tt.dCtx.ossFuzzClient != nil) != tt.wantFuzzClient { + t.Errorf("init repo error, wantFuzzClient: %v, got %v", tt.wantFuzzClient, tt.dCtx.ossFuzzClient) + return + } + if (tt.dCtx.vulnsClient != nil) != tt.wantVulnClient { + t.Errorf("init repo error, wantVulnClient: %v, got %v", tt.wantVulnClient, tt.dCtx.vulnsClient) + return + } + if (tt.dCtx.ciiClient != nil) != tt.wantCIIClient { + t.Errorf("init repo error, wantCIIClient: %v, got %v", tt.wantCIIClient, tt.dCtx.ciiClient) + return + } + }) + } +} + +func Test_getScorecardCheckResults(t *testing.T) { + t.Parallel() + //nolint + tests := []struct { + name string + dCtx dependencydiffContext + wantResultsLen int + wantErr bool + }{ + { + name: "empty response", + dCtx: dependencydiffContext{ + ctx: context.Background(), + ownerName: "ossf-tests", + repoName: "scorecard-depdiff", + }, + wantResultsLen: 0, + }, + { + name: "normal response 1", + dCtx: dependencydiffContext{ + ctx: context.Background(), + ownerName: "ossf-tests", + repoName: "scorecard-depdiff", + checkNamesToRun: []string{ + checks.CheckBranchProtection, + }, + dependencydiffs: []dependency{ + { + ChangeType: (*pkg.ChangeType)(asPointer(string(pkg.Added))), + Name: "tensorflow", + SourceRepository: asPointer("https://github.com/tensorflow/tensorflow"), + }, + { + ChangeType: (*pkg.ChangeType)(asPointer(string(pkg.Updated))), + Name: "numpy", + SourceRepository: nil, // To make it explicit here, this one doesn't have a valid srcRepo URL. + }, + { + ChangeType: (*pkg.ChangeType)(asPointer(string(pkg.Removed))), + Name: "pytorch", + SourceRepository: asPointer("https://github.com/pytorch/pytorch"), + }, + }, + }, + wantResultsLen: 3, // we need 3 results since the second one without a srcRepo URL won't break down the entire func. + wantErr: false, }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := GetDependencyDiffResults(tt.ctx, tt.owner, tt.repo, tt.base, tt.head, nil, tt.logger) + err := initRepoAndClientByChecks(&tt.dCtx) + if err != nil { + t.Errorf("init repo and client error") + return + } + err = getScorecardCheckResults(&tt.dCtx) if (err != nil) != tt.wantErr { - t.Errorf("GetDependencyDiffResults() error = [%v], errWanted [%v]", err, tt.wantErr) + t.Errorf("initRepoAndClientByChecks() error = {%v}, want error: %v", err, tt.wantErr) return } - if (len(got) == 0) != tt.wantEmptyResults { - t.Errorf("GetDependencyDiffResults() = %v, want empty [%v] for [%v]", got, tt.wantEmptyResults, tt.name) + lenResults := len(tt.dCtx.results) + if lenResults != tt.wantResultsLen { + t.Errorf("want empty results: %v, got len of results:%d", tt.wantResultsLen, lenResults) + return } }) } } + +func asPointer(s string) *string { + return &s +} diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 5a08b144688..66c0744c8a8 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -15,15 +15,17 @@ package dependencydiff import ( - "context" "fmt" "net/http" "path" "github.com/google/go-github/v38/github" + "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" "github.com/ossf/scorecard/v4/clients/githubrepo/roundtripper" - "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" ) @@ -54,29 +56,107 @@ type dependency struct { // fetchRawDependencyDiffData fetches the dependency-diffs between the two code commits // using the GitHub Dependency Review API, and returns a slice of DependencyCheckResult. -func fetchRawDependencyDiffData( - ctx context.Context, - owner, repo, base, head string, - logger *log.Logger) ([]dependency, error) { - - ghrt := roundtripper.NewTransport(ctx, logger) - ghClient := github.NewClient( - &http.Client{ - Transport: ghrt, - }, - ) +func fetchRawDependencyDiffData(dCtx *dependencydiffContext) error { + ghrt := roundtripper.NewTransport(dCtx.ctx, dCtx.logger) + ghClient := github.NewClient(&http.Client{Transport: ghrt}) req, err := ghClient.NewRequest( "GET", - path.Join("repos", owner, repo, "dependency-graph", "compare", base+"..."+head), + path.Join("repos", dCtx.ownerName, dCtx.repoName, + "dependency-graph", "compare", dCtx.baseSHA+"..."+dCtx.headSHA), nil, ) if err != nil { - return nil, fmt.Errorf("request for dependency-diff failed with %w", err) + return fmt.Errorf("request for dependency-diff failed with %w", err) } - deps := []dependency{} - _, err = ghClient.Do(ctx, req, &deps) + _, err = ghClient.Do(dCtx.ctx, req, &dCtx.dependencydiffs) if err != nil { - return nil, fmt.Errorf("error receiving the dependency-diff reponse: %w", err) + return fmt.Errorf("error parsing the dependency-diff reponse: %w", err) + } + return nil +} + +func initRepoAndClientByChecks(dCtx *dependencydiffContext) error { + // Create the repo and the corresponding client if its check needs to run. + ghRepo, err := githubrepo.MakeGithubRepo(path.Join(dCtx.ownerName, dCtx.repoName)) + if err != nil { + return fmt.Errorf("error creating the github repo: %w", err) + } + dCtx.ghRepo = ghRepo + dCtx.ghRepoClient = githubrepo.CreateGithubRepoClient(dCtx.ctx, dCtx.logger) + // Initialize these three clients as nil at first. + var ossFuzzClient clients.RepoClient + for _, cn := range dCtx.checkNamesToRun { + switch cn { + case checks.CheckFuzzing: + ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(dCtx.ctx, dCtx.logger) + if err != nil { + return fmt.Errorf("error initializing the oss fuzz repo client: %w", err) + } + dCtx.ossFuzzClient = ossFuzzClient + case checks.CheckVulnerabilities: + dCtx.vulnsClient = clients.DefaultVulnerabilitiesClient() + case checks.CheckCIIBestPractices: + dCtx.ciiClient = clients.DefaultCIIBestPracticesClient() + } + } + return nil +} + +func initScorecardChecks(checkNames []string) checker.CheckNameToFnMap { + checksToRun := checker.CheckNameToFnMap{} + if checkNames == nil && len(checkNames) == 0 { + // If no check names are provided, we run all the checks for the caller. + checksToRun = checks.AllChecks + } else { + for _, cn := range checkNames { + checksToRun[cn] = checks.AllChecks[cn] + } + } + return checksToRun +} + +func getScorecardCheckResults(dCtx *dependencydiffContext) error { + // Initialize the checks to run from the caller's input. + checksToRun := initScorecardChecks(dCtx.checkNamesToRun) + for _, d := range dCtx.dependencydiffs { + depCheckResult := pkg.DependencyCheckResult{ + PackageURL: d.PackageURL, + SourceRepository: d.SourceRepository, + ChangeType: d.ChangeType, + ManifestPath: d.ManifestPath, + Ecosystem: d.Ecosystem, + Version: d.Version, + Name: d.Name, + } + // For now we skip those without source repo urls. + // TODO: 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: use the Scorecare REST API to retrieve the Scorecard result statelessly. + scorecardResult, err := pkg.RunScorecards( + dCtx.ctx, + dCtx.ghRepo, + // TODO: 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 = fmt.Errorf("error running the scorecard checks: %w", err) + } else { // Otherwise, we record the scorecard check results for this dependency. + depCheckResult.ScorecardResultsWithError.ScorecardResults = &scorecardResult + } + } + } + dCtx.results = append(dCtx.results, depCheckResult) } - return deps, nil + return nil } diff --git a/e2e/dependencydiff_test.go b/e2e/dependencydiff_test.go new file mode 100644 index 00000000000..e7ff6454484 --- /dev/null +++ b/e2e/dependencydiff_test.go @@ -0,0 +1,51 @@ +// Copyright 2021 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 e2e + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/ossf/scorecard/v4/checks" + "github.com/ossf/scorecard/v4/dependencydiff" + "github.com/ossf/scorecard/v4/pkg" +) + +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 := "ossf-tests", "scorecard-depdiff" + baseSHA, headSHA := "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", "1989568f93e484f6a86f8b276b170e3d6962ce12" + + scorecardChecksNames := []string{ + checks.CheckBranchProtection, + } + changeTypesToCheck := map[pkg.ChangeType]bool{ + pkg.Removed: true, // Only checking those removed ones will make this test faster. + } + results, err := dependencydiff.GetDependencyDiffResults( + ctx, + ownerName, repoName, baseSHA, headSHA, + scorecardChecksNames, + changeTypesToCheck, + ) + Expect(err).Should(BeNil()) + Expect(len(results) > 0).Should(BeTrue()) + }) + }) +}) diff --git a/pkg/dependencydiff_result.go b/pkg/dependencydiff_result.go index 307f39e6320..2b7d1dc05b4 100644 --- a/pkg/dependencydiff_result.go +++ b/pkg/dependencydiff_result.go @@ -44,6 +44,14 @@ func (ct *ChangeType) IsValid() bool { } } +type ScorecardResultsWithError struct { + // ScorecardResults is the scorecard result for the dependency repo. + ScorecardResults *ScorecardResult `json:"scorecardResults"` + + // Error is an error returned when running the scorecard checks. A nil Error indicates the run succeeded. + Error error `json:"scorecardRunTimeError"` +} + // DependencyCheckResult is the dependency structure used in the returned results. type DependencyCheckResult struct { // Package URL is a short link for a package. @@ -64,11 +72,11 @@ type DependencyCheckResult struct { // Version is the package version of the dependency. Version *string `json:"version"` - // ScorecardResults is the scorecard result for the dependency repo. - ScorecardResults *ScorecardResult `json:"scorecardResults"` - // Name is the name of the dependency. Name string `json:"name"` + + // ScorecardResultsWithError is the scorecard checking results of the dependency. + ScorecardResultsWithError ScorecardResultsWithError `json:"scorecardResultsWithError"` } // AsJSON for DependencyCheckResult exports the DependencyCheckResult as a JSON object. From f243586e7532a48692b3e91509c3ed0df6f1229f Mon Sep 17 00:00:00 2001 From: aidenwang Date: Thu, 14 Jul 2022 21:34:57 -0700 Subject: [PATCH 41/49] temp --- dependencydiff/dependencydiff.go | 93 ++++++++++++++++++++++++++- dependencydiff/dependencydiff_test.go | 8 +-- dependencydiff/raw_dependencies.go | 90 -------------------------- pkg/dependencydiff_result.go | 13 ++-- 4 files changed, 101 insertions(+), 103 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index a52cc765724..d0b715a4680 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -17,8 +17,12 @@ 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" "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" ) @@ -68,6 +72,91 @@ func GetDependencyDiffResults( if err != nil { return nil, fmt.Errorf("error in initRepoAndClientByChecks: %w", err) } - err = getScorecardCheckResults(&dCtx) - return dCtx.results, fmt.Errorf("error in getScorecardCheckResults: %w", err) + getScorecardCheckResults(&dCtx) + return dCtx.results, nil +} + +func initRepoAndClientByChecks(dCtx *dependencydiffContext) error { + // Create the repo and the corresponding client if its check needs to run. + ghRepo, err := githubrepo.MakeGithubRepo(path.Join(dCtx.ownerName, dCtx.repoName)) + if err != nil { + return fmt.Errorf("error creating the github repo: %w", err) + } + dCtx.ghRepo = ghRepo + dCtx.ghRepoClient = githubrepo.CreateGithubRepoClient(dCtx.ctx, dCtx.logger) + // Initialize these three clients as nil at first. + var ossFuzzClient clients.RepoClient + for _, cn := range dCtx.checkNamesToRun { + switch cn { + case checks.CheckFuzzing: + ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(dCtx.ctx, dCtx.logger) + if err != nil { + return fmt.Errorf("error initializing the oss fuzz repo client: %w", err) + } + dCtx.ossFuzzClient = ossFuzzClient + case checks.CheckVulnerabilities: + dCtx.vulnsClient = clients.DefaultVulnerabilitiesClient() + case checks.CheckCIIBestPractices: + dCtx.ciiClient = clients.DefaultCIIBestPracticesClient() + } + } + return nil +} + +func initScorecardChecks(checkNames []string) checker.CheckNameToFnMap { + checksToRun := checker.CheckNameToFnMap{} + if checkNames == nil && len(checkNames) == 0 { + // If no check names are provided, we run all the checks for the caller. + checksToRun = checks.AllChecks + } else { + for _, cn := range checkNames { + checksToRun[cn] = checks.AllChecks[cn] + } + } + return checksToRun +} + +func getScorecardCheckResults(dCtx *dependencydiffContext) { + // Initialize the checks to run from the caller's input. + checksToRun := initScorecardChecks(dCtx.checkNamesToRun) + for _, d := range dCtx.dependencydiffs { + depCheckResult := pkg.DependencyCheckResult{ + PackageURL: d.PackageURL, + SourceRepository: d.SourceRepository, + ChangeType: d.ChangeType, + ManifestPath: d.ManifestPath, + Ecosystem: d.Ecosystem, + Version: d.Version, + Name: d.Name, + } + // For now we skip those without source repo urls. + // TODO: 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: use the Scorecare REST API to retrieve the Scorecard result statelessly. + scorecardResult, err := pkg.RunScorecards( + dCtx.ctx, + dCtx.ghRepo, + // TODO: 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 = fmt.Errorf("error running the scorecard checks: %w", 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 2e10a43d276..9ef0fc3c363 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -203,6 +203,7 @@ func Test_getScorecardCheckResults(t *testing.T) { name: "empty response", dCtx: dependencydiffContext{ ctx: context.Background(), + logger: log.NewLogger(log.InfoLevel), ownerName: "ossf-tests", repoName: "scorecard-depdiff", }, @@ -212,6 +213,7 @@ func Test_getScorecardCheckResults(t *testing.T) { name: "normal response 1", dCtx: dependencydiffContext{ ctx: context.Background(), + logger: log.NewLogger(log.InfoLevel), ownerName: "ossf-tests", repoName: "scorecard-depdiff", checkNamesToRun: []string{ @@ -248,11 +250,7 @@ func Test_getScorecardCheckResults(t *testing.T) { t.Errorf("init repo and client error") return } - err = getScorecardCheckResults(&tt.dCtx) - if (err != nil) != tt.wantErr { - t.Errorf("initRepoAndClientByChecks() error = {%v}, want error: %v", err, tt.wantErr) - return - } + getScorecardCheckResults(&tt.dCtx) lenResults := len(tt.dCtx.results) if lenResults != tt.wantResultsLen { t.Errorf("want empty results: %v, got len of results:%d", tt.wantResultsLen, lenResults) diff --git a/dependencydiff/raw_dependencies.go b/dependencydiff/raw_dependencies.go index 66c0744c8a8..519e560217c 100644 --- a/dependencydiff/raw_dependencies.go +++ b/dependencydiff/raw_dependencies.go @@ -21,10 +21,6 @@ import ( "github.com/google/go-github/v38/github" - "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" "github.com/ossf/scorecard/v4/clients/githubrepo/roundtripper" "github.com/ossf/scorecard/v4/pkg" ) @@ -74,89 +70,3 @@ func fetchRawDependencyDiffData(dCtx *dependencydiffContext) error { } return nil } - -func initRepoAndClientByChecks(dCtx *dependencydiffContext) error { - // Create the repo and the corresponding client if its check needs to run. - ghRepo, err := githubrepo.MakeGithubRepo(path.Join(dCtx.ownerName, dCtx.repoName)) - if err != nil { - return fmt.Errorf("error creating the github repo: %w", err) - } - dCtx.ghRepo = ghRepo - dCtx.ghRepoClient = githubrepo.CreateGithubRepoClient(dCtx.ctx, dCtx.logger) - // Initialize these three clients as nil at first. - var ossFuzzClient clients.RepoClient - for _, cn := range dCtx.checkNamesToRun { - switch cn { - case checks.CheckFuzzing: - ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(dCtx.ctx, dCtx.logger) - if err != nil { - return fmt.Errorf("error initializing the oss fuzz repo client: %w", err) - } - dCtx.ossFuzzClient = ossFuzzClient - case checks.CheckVulnerabilities: - dCtx.vulnsClient = clients.DefaultVulnerabilitiesClient() - case checks.CheckCIIBestPractices: - dCtx.ciiClient = clients.DefaultCIIBestPracticesClient() - } - } - return nil -} - -func initScorecardChecks(checkNames []string) checker.CheckNameToFnMap { - checksToRun := checker.CheckNameToFnMap{} - if checkNames == nil && len(checkNames) == 0 { - // If no check names are provided, we run all the checks for the caller. - checksToRun = checks.AllChecks - } else { - for _, cn := range checkNames { - checksToRun[cn] = checks.AllChecks[cn] - } - } - return checksToRun -} - -func getScorecardCheckResults(dCtx *dependencydiffContext) error { - // Initialize the checks to run from the caller's input. - checksToRun := initScorecardChecks(dCtx.checkNamesToRun) - for _, d := range dCtx.dependencydiffs { - depCheckResult := pkg.DependencyCheckResult{ - PackageURL: d.PackageURL, - SourceRepository: d.SourceRepository, - ChangeType: d.ChangeType, - ManifestPath: d.ManifestPath, - Ecosystem: d.Ecosystem, - Version: d.Version, - Name: d.Name, - } - // For now we skip those without source repo urls. - // TODO: 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: use the Scorecare REST API to retrieve the Scorecard result statelessly. - scorecardResult, err := pkg.RunScorecards( - dCtx.ctx, - dCtx.ghRepo, - // TODO: 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 = fmt.Errorf("error running the scorecard checks: %w", err) - } else { // Otherwise, we record the scorecard check results for this dependency. - depCheckResult.ScorecardResultsWithError.ScorecardResults = &scorecardResult - } - } - } - dCtx.results = append(dCtx.results, depCheckResult) - } - return nil -} diff --git a/pkg/dependencydiff_result.go b/pkg/dependencydiff_result.go index 2b7d1dc05b4..5208d9812ce 100644 --- a/pkg/dependencydiff_result.go +++ b/pkg/dependencydiff_result.go @@ -44,6 +44,7 @@ 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"` @@ -54,15 +55,15 @@ type ScorecardResultsWithError struct { // 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"` + // Package URL is a short link for a package. PackageURL *string `json:"packageUrl"` // SourceRepository is the source repository URL of the dependency. SourceRepository *string `json:"sourceRepository"` - // ChangeType indicates whether the dependency is added, updated, or removed. - ChangeType *ChangeType `json:"changeType"` - // ManifestPath is the path of the manifest file of the dependency, such as go.mod for Go. ManifestPath *string `json:"manifestPath"` @@ -72,11 +73,11 @@ type DependencyCheckResult struct { // Version is the package version of the dependency. Version *string `json:"version"` - // Name is the name of the dependency. - Name string `json:"name"` - // ScorecardResultsWithError is the scorecard checking results of the dependency. ScorecardResultsWithError ScorecardResultsWithError `json:"scorecardResultsWithError"` + + // Name is the name of the dependency. + Name string `json:"name"` } // AsJSON for DependencyCheckResult exports the DependencyCheckResult as a JSON object. From 04933796c510a7282c6c0379ea58d17ba889259f Mon Sep 17 00:00:00 2001 From: aidenwang Date: Fri, 15 Jul 2022 14:52:05 -0700 Subject: [PATCH 42/49] temp --- dependencydiff/dependencydiff_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index 9ef0fc3c363..300de1ef9e8 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -49,7 +49,7 @@ func Test_fetchRawDependencyDiffData(t *testing.T) { wantErr: true, }, { - name: "normal response", + name: "normal response with non-empty results", dCtx: dependencydiffContext{ logger: log.NewLogger(log.InfoLevel), ctx: context.Background(), From baae011610c0eb697f57035a27c357b52be88a9d Mon Sep 17 00:00:00 2001 From: aidenwang Date: Fri, 15 Jul 2022 16:39:51 -0700 Subject: [PATCH 43/49] temp --- dependencydiff/dependencydiff.go | 34 ++++++------- dependencydiff/dependencydiff_test.go | 70 +++------------------------ 2 files changed, 21 insertions(+), 83 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index d0b715a4680..c5828e1762f 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -19,12 +19,12 @@ import ( "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" "github.com/ossf/scorecard/v4/log" "github.com/ossf/scorecard/v4/pkg" + "github.com/ossf/scorecard/v4/policy" ) // Depdiff is the exported name for dependency-diff. @@ -72,7 +72,10 @@ func GetDependencyDiffResults( if err != nil { return nil, fmt.Errorf("error in initRepoAndClientByChecks: %w", err) } - getScorecardCheckResults(&dCtx) + err = getScorecardCheckResults(&dCtx) + if err != nil { + return nil, fmt.Errorf("error getting scorecard check results") + } return dCtx.results, nil } @@ -103,22 +106,12 @@ func initRepoAndClientByChecks(dCtx *dependencydiffContext) error { return nil } -func initScorecardChecks(checkNames []string) checker.CheckNameToFnMap { - checksToRun := checker.CheckNameToFnMap{} - if checkNames == nil && len(checkNames) == 0 { - // If no check names are provided, we run all the checks for the caller. - checksToRun = checks.AllChecks - } else { - for _, cn := range checkNames { - checksToRun[cn] = checks.AllChecks[cn] - } - } - return checksToRun -} - -func getScorecardCheckResults(dCtx *dependencydiffContext) { +func getScorecardCheckResults(dCtx *dependencydiffContext) error { // Initialize the checks to run from the caller's input. - checksToRun := initScorecardChecks(dCtx.checkNamesToRun) + checksToRun, err := policy.GetEnabled(nil, dCtx.checkNamesToRun, nil) + if err != nil { + return fmt.Errorf("error init scorecard checks: %w", err) + } for _, d := range dCtx.dependencydiffs { depCheckResult := pkg.DependencyCheckResult{ PackageURL: d.PackageURL, @@ -130,16 +123,16 @@ func getScorecardCheckResults(dCtx *dependencydiffContext) { Name: d.Name, } // For now we skip those without source repo urls. - // TODO: use the BigQuery dataset to supplement null source repo URLs to fetch the Scorecard results for them. + // 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: use the Scorecare REST API to retrieve the Scorecard result statelessly. + // TODO (#2064): use the Scorecare REST API to retrieve the Scorecard result statelessly. scorecardResult, err := pkg.RunScorecards( dCtx.ctx, dCtx.ghRepo, - // TODO: In future versions, ideally, this should be + // TODO (#2065): In future versions, ideally, this should be // the commitSHA corresponding to d.Version instead of HEAD. clients.HeadSHA, checksToRun, @@ -159,4 +152,5 @@ func getScorecardCheckResults(dCtx *dependencydiffContext) { } dCtx.results = append(dCtx.results, depCheckResult) } + return nil } diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index 300de1ef9e8..dbc5587b0c0 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -48,32 +48,7 @@ func Test_fetchRawDependencyDiffData(t *testing.T) { wantEmpty: true, wantErr: true, }, - { - name: "normal response with non-empty results", - dCtx: dependencydiffContext{ - logger: log.NewLogger(log.InfoLevel), - ctx: context.Background(), - ownerName: "ossf-tests", - repoName: "scorecard-depdiff", - baseSHA: "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", - headSHA: "1989568f93e484f6a86f8b276b170e3d6962ce12", - }, - wantEmpty: false, - wantErr: false, - }, - { - name: "normal response with empty results", - dCtx: dependencydiffContext{ - logger: log.NewLogger(log.InfoLevel), - ctx: context.Background(), - ownerName: "ossf-tests", - repoName: "scorecard-depdiff", - baseSHA: "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", - headSHA: "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", - }, - wantEmpty: true, - wantErr: false, - }, + // Considering of the token usage, normal responses are tested in the e2e test. } for _, tt := range tests { tt := tt @@ -120,42 +95,7 @@ func Test_initRepoAndClientByChecks(t *testing.T) { wantCIIClient: false, wantErr: true, }, - { - name: "normal response with all clients", - dCtx: dependencydiffContext{ - logger: log.NewLogger(log.InfoLevel), - ctx: context.Background(), - ownerName: "ossf-tests", - repoName: "scorecard-depdiff", - checkNamesToRun: []string{ - checks.CheckFuzzing, - checks.CheckVulnerabilities, - checks.CheckCIIBestPractices, - }, - }, - wantGhRepo: true, - wantRepoClient: true, - wantFuzzClient: true, - wantVulnClient: true, - wantCIIClient: true, - }, - { - name: "normal response with only repo and repo client", - dCtx: dependencydiffContext{ - logger: log.NewLogger(log.InfoLevel), - ctx: context.Background(), - ownerName: "ossf-tests", - repoName: "scorecard-depdiff", - checkNamesToRun: []string{ - checks.CheckSecurityPolicy, - }, - }, - wantGhRepo: true, - wantRepoClient: true, - wantFuzzClient: false, - wantVulnClient: false, - wantCIIClient: false, - }, + // Same as the above, putting the normal responses to the e2e test. } for _, tt := range tests { tt := tt @@ -250,7 +190,11 @@ func Test_getScorecardCheckResults(t *testing.T) { t.Errorf("init repo and client error") return } - getScorecardCheckResults(&tt.dCtx) + err = getScorecardCheckResults(&tt.dCtx) + if (err != nil) != tt.wantErr { + t.Errorf("getScorecardCheckResults() error = {%v}, want error: %v", err, tt.wantErr) + return + } lenResults := len(tt.dCtx.results) if lenResults != tt.wantResultsLen { t.Errorf("want empty results: %v, got len of results:%d", tt.wantResultsLen, lenResults) From fc1e227706acd2c297746749dbdba440303b5b97 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Fri, 15 Jul 2022 19:51:52 -0700 Subject: [PATCH 44/49] temp --- dependencydiff/dependencydiff.go | 2 +- dependencydiff/dependencydiff_test.go | 51 ++++----------------------- 2 files changed, 7 insertions(+), 46 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index c5828e1762f..57413922bda 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -74,7 +74,7 @@ func GetDependencyDiffResults( } err = getScorecardCheckResults(&dCtx) if err != nil { - return nil, fmt.Errorf("error getting scorecard check results") + return nil, fmt.Errorf("error getting scorecard check results: %w", err) } return dCtx.results, nil } diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index dbc5587b0c0..ea331f88ea9 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -19,10 +19,8 @@ import ( "path" "testing" - "github.com/ossf/scorecard/v4/checks" "github.com/ossf/scorecard/v4/clients" "github.com/ossf/scorecard/v4/log" - "github.com/ossf/scorecard/v4/pkg" ) // Test_fetchRawDependencyDiffData is a test function for fetchRawDependencyDiffData. @@ -134,51 +132,19 @@ func Test_getScorecardCheckResults(t *testing.T) { t.Parallel() //nolint tests := []struct { - name string - dCtx dependencydiffContext - wantResultsLen int - wantErr bool + name string + dCtx dependencydiffContext + wantErr bool }{ { name: "empty response", dCtx: dependencydiffContext{ ctx: context.Background(), logger: log.NewLogger(log.InfoLevel), - ownerName: "ossf-tests", - repoName: "scorecard-depdiff", - }, - wantResultsLen: 0, - }, - { - name: "normal response 1", - dCtx: dependencydiffContext{ - ctx: context.Background(), - logger: log.NewLogger(log.InfoLevel), - ownerName: "ossf-tests", - repoName: "scorecard-depdiff", - checkNamesToRun: []string{ - checks.CheckBranchProtection, - }, - dependencydiffs: []dependency{ - { - ChangeType: (*pkg.ChangeType)(asPointer(string(pkg.Added))), - Name: "tensorflow", - SourceRepository: asPointer("https://github.com/tensorflow/tensorflow"), - }, - { - ChangeType: (*pkg.ChangeType)(asPointer(string(pkg.Updated))), - Name: "numpy", - SourceRepository: nil, // To make it explicit here, this one doesn't have a valid srcRepo URL. - }, - { - ChangeType: (*pkg.ChangeType)(asPointer(string(pkg.Removed))), - Name: "pytorch", - SourceRepository: asPointer("https://github.com/pytorch/pytorch"), - }, - }, + ownerName: "owner_not_exist", + repoName: "repo_not_exist", }, - wantResultsLen: 3, // we need 3 results since the second one without a srcRepo URL won't break down the entire func. - wantErr: false, + wantErr: false, }, } for _, tt := range tests { @@ -195,11 +161,6 @@ func Test_getScorecardCheckResults(t *testing.T) { t.Errorf("getScorecardCheckResults() error = {%v}, want error: %v", err, tt.wantErr) return } - lenResults := len(tt.dCtx.results) - if lenResults != tt.wantResultsLen { - t.Errorf("want empty results: %v, got len of results:%d", tt.wantResultsLen, lenResults) - return - } }) } } From a7547d435ff06a914e516022b482246b9cdb227d Mon Sep 17 00:00:00 2001 From: aidenwang Date: Fri, 15 Jul 2022 20:03:12 -0700 Subject: [PATCH 45/49] temp --- dependencydiff/dependencydiff_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dependencydiff/dependencydiff_test.go b/dependencydiff/dependencydiff_test.go index ea331f88ea9..c0447519d45 100644 --- a/dependencydiff/dependencydiff_test.go +++ b/dependencydiff/dependencydiff_test.go @@ -164,7 +164,3 @@ func Test_getScorecardCheckResults(t *testing.T) { }) } } - -func asPointer(s string) *string { - return &s -} From 347d74d0daa98c20c4da1127c3932b572be640ee Mon Sep 17 00:00:00 2001 From: aidenwang Date: Sun, 17 Jul 2022 23:28:32 -0700 Subject: [PATCH 46/49] save --- dependencydiff/dependencydiff.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 57413922bda..30b3440ab1d 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -19,6 +19,7 @@ import ( "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" @@ -80,27 +81,28 @@ func GetDependencyDiffResults( } func initRepoAndClientByChecks(dCtx *dependencydiffContext) error { - // Create the repo and the corresponding client if its check needs to run. - ghRepo, err := githubrepo.MakeGithubRepo(path.Join(dCtx.ownerName, dCtx.repoName)) + repo, repoClient, ossFuzzClient, ciiClient, vulnsClient, err := checker.GetClients( + dCtx.ctx, path.Join(dCtx.ownerName, dCtx.repoName), "", dCtx.logger, + ) if err != nil { return fmt.Errorf("error creating the github repo: %w", err) } - dCtx.ghRepo = ghRepo + // If the caller doesn't specify the checks to run, run all 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 + return nil + } + dCtx.ghRepo = repo dCtx.ghRepoClient = githubrepo.CreateGithubRepoClient(dCtx.ctx, dCtx.logger) - // Initialize these three clients as nil at first. - var ossFuzzClient clients.RepoClient for _, cn := range dCtx.checkNamesToRun { switch cn { case checks.CheckFuzzing: - ossFuzzClient, err = githubrepo.CreateOssFuzzRepoClient(dCtx.ctx, dCtx.logger) - if err != nil { - return fmt.Errorf("error initializing the oss fuzz repo client: %w", err) - } dCtx.ossFuzzClient = ossFuzzClient - case checks.CheckVulnerabilities: - dCtx.vulnsClient = clients.DefaultVulnerabilitiesClient() case checks.CheckCIIBestPractices: - dCtx.ciiClient = clients.DefaultCIIBestPracticesClient() + dCtx.ciiClient = ciiClient + case checks.CheckVulnerabilities: + dCtx.vulnsClient = vulnsClient } } return nil From e825159d667208047842ada9d5281827c1001f62 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 18 Jul 2022 10:40:29 -0700 Subject: [PATCH 47/49] save --- e2e/dependencydiff_test.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/e2e/dependencydiff_test.go b/e2e/dependencydiff_test.go index e7ff6454484..5c18630d256 100644 --- a/e2e/dependencydiff_test.go +++ b/e2e/dependencydiff_test.go @@ -47,5 +47,42 @@ var _ = Describe("E2E TEST:"+dependencydiff.Depdiff, func() { Expect(err).Should(BeNil()) Expect(len(results) > 0).Should(BeTrue()) }) + It("Should return a valid empty result", func() { + ctx := context.Background() + ownerName, repoName := "ossf-tests", "scorecard-depdiff" + baseSHA, headSHA := "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", "fd2a82b3b735fffbc2d782ed5f50301b879ecc51" + + scorecardChecksNames := []string{ + checks.CheckBranchProtection, + } + changeTypesToCheck := map[pkg.ChangeType]bool{ + pkg.Removed: true, + } + results, err := dependencydiff.GetDependencyDiffResults( + ctx, + ownerName, repoName, baseSHA, headSHA, + scorecardChecksNames, + changeTypesToCheck, + ) + Expect(err).Should(BeNil()) + Expect(len(results) == 0).Should(BeTrue()) + }) + It("Should initialize clients corresponding to the checks to run and do not crash", func() { + ctx := context.Background() + ownerName, repoName := "ossf-tests", "scorecard-depdiff" + baseSHA, headSHA := "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", "1989568f93e484f6a86f8b276b170e3d6962ce12" + + scorecardChecksNames := []string{} + changeTypesToCheck := map[pkg.ChangeType]bool{ + pkg.Removed: true, + } + _, err := dependencydiff.GetDependencyDiffResults( + ctx, + ownerName, repoName, baseSHA, headSHA, + scorecardChecksNames, + changeTypesToCheck, + ) + Expect(err).Should(BeNil()) + }) }) }) From 08fa625084926ad492464e76824f104aa0620361 Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 18 Jul 2022 11:01:39 -0700 Subject: [PATCH 48/49] save --- e2e/dependencydiff_test.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/e2e/dependencydiff_test.go b/e2e/dependencydiff_test.go index 5c18630d256..d0246293537 100644 --- a/e2e/dependencydiff_test.go +++ b/e2e/dependencydiff_test.go @@ -25,13 +25,19 @@ import ( "github.com/ossf/scorecard/v4/pkg" ) +const ( + OWNER = "ossf-tests" + REPO = "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 := "ossf-tests", "scorecard-depdiff" - baseSHA, headSHA := "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", "1989568f93e484f6a86f8b276b170e3d6962ce12" - + ownerName, repoName := OWNER, REPO + baseSHA, headSHA := BASE, HEAD scorecardChecksNames := []string{ checks.CheckBranchProtection, } @@ -49,8 +55,8 @@ var _ = Describe("E2E TEST:"+dependencydiff.Depdiff, func() { }) It("Should return a valid empty result", func() { ctx := context.Background() - ownerName, repoName := "ossf-tests", "scorecard-depdiff" - baseSHA, headSHA := "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", "fd2a82b3b735fffbc2d782ed5f50301b879ecc51" + ownerName, repoName := OWNER, REPO + baseSHA, headSHA := BASE, BASE scorecardChecksNames := []string{ checks.CheckBranchProtection, @@ -69,8 +75,8 @@ 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 := "ossf-tests", "scorecard-depdiff" - baseSHA, headSHA := "fd2a82b3b735fffbc2d782ed5f50301b879ecc51", "1989568f93e484f6a86f8b276b170e3d6962ce12" + ownerName, repoName := OWNER, REPO + baseSHA, headSHA := BASE, HEAD scorecardChecksNames := []string{} changeTypesToCheck := map[pkg.ChangeType]bool{ From 3cb16cc5e3b6f182592a429fdce97cd0db3dad7f Mon Sep 17 00:00:00 2001 From: aidenwang Date: Mon, 18 Jul 2022 11:34:22 -0700 Subject: [PATCH 49/49] final_commit_before_merge --- dependencydiff/dependencydiff.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dependencydiff/dependencydiff.go b/dependencydiff/dependencydiff.go index 30b3440ab1d..21459d8ec50 100644 --- a/dependencydiff/dependencydiff.go +++ b/dependencydiff/dependencydiff.go @@ -23,6 +23,7 @@ import ( "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" "github.com/ossf/scorecard/v4/policy" @@ -146,7 +147,8 @@ 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 = fmt.Errorf("error running the scorecard checks: %w", err) + 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 }