🌱 Refactor vulnerabilities client

This commit is contained in:
naveen 2022-01-03 01:43:38 +00:00 committed by Naveen
parent c8f15a495e
commit de39061cc5
10 changed files with 202 additions and 88 deletions

View File

@ -111,7 +111,7 @@ cron/data/metadata.pb.go: cron/data/metadata.proto | $(PROTOC)
protoc --go_out=../../../ cron/data/metadata.proto
generate-mocks: ## Compiles and generates all mocks using mockgen.
generate-mocks: clients/mockclients/repo_client.go clients/mockclients/repo.go clients/mockclients/cii_client.go
generate-mocks: clients/mockclients/repo_client.go clients/mockclients/repo.go clients/mockclients/cii_client.go checks/mockclients/vulnerabilities.go
clients/mockclients/repo_client.go: clients/repo_client.go
# Generating MockRepoClient
$(MOCKGEN) -source=clients/repo_client.go -destination=clients/mockclients/repo_client.go -package=mockrepo -copyright_file=clients/mockclients/license.txt
@ -121,6 +121,9 @@ clients/mockclients/repo.go: clients/repo.go
clients/mockclients/cii_client.go: clients/cii_client.go
# Generating MockCIIClient
$(MOCKGEN) -source=clients/cii_client.go -destination=clients/mockclients/cii_client.go -package=mockrepo -copyright_file=clients/mockclients/license.txt
checks/mockclients/vulnerabilities.go: clients/vulnerabilities.go
# Generating MockCIIClient
$(MOCKGEN) -source=clients/vulnerabilities.go -destination=clients/mockclients/vulnerabilities.go -package=mockrepo -copyright_file=clients/mockclients/license.txt
generate-docs: ## Generates docs
generate-docs: validate-docs docs/checks.md

View File

@ -22,12 +22,13 @@ import (
// CheckRequest struct encapsulates all data to be passed into a CheckFn.
type CheckRequest struct {
Ctx context.Context
RepoClient clients.RepoClient
CIIClient clients.CIIBestPracticesClient
OssFuzzRepo clients.RepoClient
Dlogger DetailLogger
Repo clients.Repo
Ctx context.Context
RepoClient clients.RepoClient
CIIClient clients.CIIBestPracticesClient
OssFuzzRepo clients.RepoClient
Dlogger DetailLogger
Repo clients.Repo
VulnerabilitiesClient clients.VulnerabilitiesClient
// UPGRADEv6: return raw results instead of scores.
RawResults *RawResults
}

View File

@ -15,45 +15,25 @@
package checks
import (
"bytes"
"encoding/json"
"fmt"
"net/http"
"strings"
"github.com/ossf/scorecard/v3/checker"
"github.com/ossf/scorecard/v3/clients"
sce "github.com/ossf/scorecard/v3/errors"
)
const (
// CheckVulnerabilities is the registered name for the OSV check.
CheckVulnerabilities = "Vulnerabilities"
osvQueryEndpoint = "https://api.osv.dev/v1/query"
)
type osvQuery struct {
Commit string `json:"commit"`
}
type osvResponse struct {
Vulns []struct {
ID string `json:"id"`
} `json:"vulns"`
}
// Vulnerabilities cheks for vulnerabilities in api.osv.dev.
type Vulnerabilities interface {
HasUnfixedVulnerabilities(c *checker.CheckRequest) checker.CheckResult
}
type vulns struct{}
//nolint:gochecknoinits
func init() {
v := &vulns{}
registerCheck(CheckVulnerabilities, v.HasUnfixedVulnerabilities)
registerCheck(CheckVulnerabilities, HasUnfixedVulnerabilities)
}
func (resp *osvResponse) getVulnerabilities() []string {
func getVulnerabilities(resp *clients.VulnerabilitiesResponse) []string {
ids := make([]string, 0, len(resp.Vulns))
for _, vuln := range resp.Vulns {
ids = append(ids, vuln.ID)
@ -61,13 +41,8 @@ func (resp *osvResponse) getVulnerabilities() []string {
return ids
}
// NewVulnerabilities creates a new Vulnerabilities check.
func NewVulnerabilities() Vulnerabilities {
return &vulns{}
}
// HasUnfixedVulnerabilities runs Vulnerabilities check.
func (v *vulns) HasUnfixedVulnerabilities(c *checker.CheckRequest) checker.CheckResult {
func HasUnfixedVulnerabilities(c *checker.CheckRequest) checker.CheckResult {
commits, err := c.RepoClient.ListCommits()
if err != nil {
e := sce.WithMessage(sce.ErrScorecardInternal, "Client.Repositories.ListCommits")
@ -78,38 +53,14 @@ func (v *vulns) HasUnfixedVulnerabilities(c *checker.CheckRequest) checker.Check
return checker.CreateInconclusiveResult(CheckVulnerabilities, "no commits found")
}
query, err := json.Marshal(&osvQuery{
Commit: commits[0].SHA,
})
resp, err := c.VulnerabilitiesClient.HasUnfixedVulnerabilities(c.Ctx, commits[0].SHA)
if err != nil {
e := sce.WithMessage(sce.ErrScorecardInternal, "json.Marshal")
return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e)
}
req, err := http.NewRequestWithContext(c.Ctx, http.MethodPost, osvQueryEndpoint, bytes.NewReader(query))
if err != nil {
e := sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("http.NewRequestWithContext: %v", err))
return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e)
}
// Use our own http client as the one from CheckRequest adds GitHub tokens to the headers.
httpClient := &http.Client{}
resp, err := httpClient.Do(req)
if err != nil {
e := sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("httpClient.Do: %v", err))
return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e)
}
defer resp.Body.Close()
var osvResp osvResponse
decoder := json.NewDecoder(resp.Body)
if err := decoder.Decode(&osvResp); err != nil {
e := sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("decoder.Decode: %v", err))
e := sce.WithMessage(sce.ErrScorecardInternal, "VulnerabilitiesClient.HasUnfixedVulnerabilities")
return checker.CreateRuntimeErrorResult(CheckVulnerabilities, e)
}
// TODO: take severity into account.
vulnIDs := osvResp.getVulnerabilities()
vulnIDs := getVulnerabilities(&resp)
if len(vulnIDs) > 0 {
c.Dlogger.Warn3(&checker.LogMessage{
Text: fmt.Sprintf("HEAD is vulnerable to %s", strings.Join(vulnIDs, ", ")),

View File

@ -0,0 +1,66 @@
// 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.
//
// Code generated by MockGen. DO NOT EDIT.
// Source: clients/vulnerabilities.go
// Package mockrepo is a generated GoMock package.
package mockrepo
import (
context "context"
reflect "reflect"
gomock "github.com/golang/mock/gomock"
clients "github.com/ossf/scorecard/v3/clients"
)
// MockVulnerabilitiesClient is a mock of VulnerabilitiesClient interface.
type MockVulnerabilitiesClient struct {
ctrl *gomock.Controller
recorder *MockVulnerabilitiesClientMockRecorder
}
// MockVulnerabilitiesClientMockRecorder is the mock recorder for MockVulnerabilitiesClient.
type MockVulnerabilitiesClientMockRecorder struct {
mock *MockVulnerabilitiesClient
}
// NewMockVulnerabilitiesClient creates a new mock instance.
func NewMockVulnerabilitiesClient(ctrl *gomock.Controller) *MockVulnerabilitiesClient {
mock := &MockVulnerabilitiesClient{ctrl: ctrl}
mock.recorder = &MockVulnerabilitiesClientMockRecorder{mock}
return mock
}
// EXPECT returns an object that allows the caller to indicate expected use.
func (m *MockVulnerabilitiesClient) EXPECT() *MockVulnerabilitiesClientMockRecorder {
return m.recorder
}
// HasUnfixedVulnerabilities mocks base method.
func (m *MockVulnerabilitiesClient) HasUnfixedVulnerabilities(context context.Context, commit string) (clients.OSVResponse, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "HasUnfixedVulnerabilities", context, commit)
ret0, _ := ret[0].(clients.OSVResponse)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// HasUnfixedVulnerabilities indicates an expected call of HasUnfixedVulnerabilities.
func (mr *MockVulnerabilitiesClientMockRecorder) HasUnfixedVulnerabilities(context, commit interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasUnfixedVulnerabilities", reflect.TypeOf((*MockVulnerabilitiesClient)(nil).HasUnfixedVulnerabilities), context, commit)
}

View File

@ -0,0 +1,80 @@
// 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 clients
import (
"bytes"
"context"
"encoding/json"
"net/http"
"github.com/ossf/scorecard/v3/errors"
)
const osvQueryEndpoint = "https://api.osv.dev/v1/query"
type osvQuery struct {
Commit string `json:"commit"`
}
// VulnerabilitiesClient cheks for vulnerabilities in api.osv.dev.
type VulnerabilitiesClient interface {
HasUnfixedVulnerabilities(context context.Context, commit string) (VulnerabilitiesResponse, error)
}
// VulnerabilitiesResponse is the response from the OSV API.
type VulnerabilitiesResponse struct {
Vulns []struct {
ID string `json:"id"`
} `json:"vulns"`
}
type vulns struct{}
// DefaultVulnerabilitiesClient is a new Vulnerabilities client.
func DefaultVulnerabilitiesClient() VulnerabilitiesClient {
return vulns{}
}
// HasUnfixedVulnerabilities runs Vulnerabilities check.
func (v vulns) HasUnfixedVulnerabilities(ctx context.Context, commit string) (VulnerabilitiesResponse, error) {
query, err := json.Marshal(&osvQuery{
Commit: commit,
})
if err != nil {
return VulnerabilitiesResponse{}, errors.WithMessage(err, "failed to marshal query")
}
req, err := http.NewRequestWithContext(ctx, http.MethodPost, osvQueryEndpoint, bytes.NewReader(query))
if err != nil {
return VulnerabilitiesResponse{}, errors.WithMessage(err, "failed to create request")
}
// Use our own http client as the one from CheckRequest adds GitHub tokens to the headers.
httpClient := &http.Client{}
resp, err := httpClient.Do(req)
if err != nil {
return VulnerabilitiesResponse{}, errors.WithMessage(err, "failed to send request")
}
defer resp.Body.Close()
var osvResp VulnerabilitiesResponse
decoder := json.NewDecoder(resp.Body)
if err := decoder.Decode(&osvResp); err != nil {
return VulnerabilitiesResponse{}, errors.WithMessage(err, "failed to decode response")
}
return osvResp, nil
}

View File

@ -193,7 +193,7 @@ func scorecardCmd(cmd *cobra.Command, args []string) {
// nolint: errcheck
defer logger.Sync() // Flushes buffer, if any.
repoURI, repoClient, ossFuzzRepoClient, ciiClient, repoType, err := getRepoAccessors(ctx, uri, logger)
repoURI, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, repoType, err := getRepoAccessors(ctx, uri, logger)
if err != nil {
log.Panic(err)
}
@ -228,7 +228,7 @@ func scorecardCmd(cmd *cobra.Command, args []string) {
log.Panicf("only json format is supported")
}
repoResult, err := pkg.RunScorecards(ctx, repoURI, raw, enabledChecks, repoClient, ossFuzzRepoClient, ciiClient)
repoResult, err := pkg.RunScorecards(ctx, repoURI, raw, enabledChecks, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient)
if err != nil {
log.Panic(err)
}
@ -425,6 +425,7 @@ func getRepoAccessors(ctx context.Context, uri string, logger *zap.Logger) (
repoClient clients.RepoClient,
ossFuzzRepoClient clients.RepoClient,
ciiClient clients.CIIBestPracticesClient,
vulnerabilityClient clients.VulnerabilitiesClient,
repoType string,
err error) {
var localRepo, githubRepo clients.Repo
@ -442,6 +443,7 @@ func getRepoAccessors(ctx context.Context, uri string, logger *zap.Logger) (
repo = githubRepo
repoClient = githubrepo.CreateGithubRepoClient(ctx, logger)
ciiClient = clients.DefaultCIIBestPracticesClient()
vulnerabilityClient = clients.DefaultVulnerabilitiesClient()
ossFuzzRepoClient, err = githubrepo.CreateOssFuzzRepoClient(ctx, logger)
return
}

View File

@ -66,13 +66,14 @@ var serveCmd = &cobra.Command{
ctx := r.Context()
repoClient := githubrepo.CreateGithubRepoClient(ctx, logger)
ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, logger)
vulnsClient := clients.DefaultVulnerabilitiesClient()
if err != nil {
sugar.Error(err)
rw.WriteHeader(http.StatusInternalServerError)
}
defer ossFuzzRepoClient.Close()
ciiClient := clients.DefaultCIIBestPracticesClient()
repoResult, err := pkg.RunScorecards(ctx, repo, false, checks.AllChecks, repoClient, ossFuzzRepoClient, ciiClient)
repoResult, err := pkg.RunScorecards(ctx, repo, false, checks.AllChecks, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient)
if err != nil {
sugar.Error(err)
rw.WriteHeader(http.StatusInternalServerError)

View File

@ -52,7 +52,9 @@ func processRequest(ctx context.Context,
batchRequest *data.ScorecardBatchRequest, checksToRun checker.CheckNameToFnMap,
bucketURL, bucketURL2 string, checkDocs docs.Doc,
repoClient clients.RepoClient, ossFuzzRepoClient clients.RepoClient,
ciiClient clients.CIIBestPracticesClient, logger *zap.Logger) error {
ciiClient clients.CIIBestPracticesClient,
vulnsClient clients.VulnerabilitiesClient,
logger *zap.Logger) error {
filename := data.GetBlobFilename(
fmt.Sprintf("shard-%07d", batchRequest.GetShardNum()),
batchRequest.GetJobTime().AsTime())
@ -83,7 +85,8 @@ func processRequest(ctx context.Context,
continue
}
repo.AppendMetadata(repo.Metadata()...)
result, err := pkg.RunScorecards(ctx, repo, false, checksToRun, repoClient, ossFuzzRepoClient, ciiClient)
result, err := pkg.RunScorecards(ctx, repo, false, checksToRun,
repoClient, ossFuzzRepoClient, ciiClient, vulnsClient)
if errors.Is(err, sce.ErrRepoUnreachable) {
// Not accessible repo - continue.
continue
@ -190,6 +193,7 @@ func main() {
repoClient := githubrepo.CreateGithubRepoClient(ctx, logger)
ciiClient := clients.BlobCIIBestPracticesClient(ciiDataBucketURL)
ossFuzzRepoClient, err := githubrepo.CreateOssFuzzRepoClient(ctx, logger)
vulnsClient := clients.DefaultVulnerabilitiesClient()
if err != nil {
panic(err)
}
@ -222,7 +226,7 @@ func main() {
}
if err := processRequest(ctx, req, checksToRun,
bucketURL, bucketURL2, checkDocs,
repoClient, ossFuzzRepoClient, ciiClient, logger); err != nil {
repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, logger); err != nil {
logger.Warn(fmt.Sprintf("error processing request: %v", err))
// Nack the message so that another worker can retry.
subscriber.Nack()

View File

@ -22,6 +22,7 @@ import (
"github.com/ossf/scorecard/v3/checker"
"github.com/ossf/scorecard/v3/checks"
"github.com/ossf/scorecard/v3/clients"
"github.com/ossf/scorecard/v3/clients/githubrepo"
scut "github.com/ossf/scorecard/v3/utests"
)
@ -37,10 +38,11 @@ var _ = Describe("E2E TEST:Vulnerabilities", func() {
dl := scut.TestDetailLogger{}
req := checker.CheckRequest{
Ctx: context.Background(),
RepoClient: repoClient,
Repo: repo,
Dlogger: &dl,
Ctx: context.Background(),
RepoClient: repoClient,
VulnerabilitiesClient: clients.DefaultVulnerabilitiesClient(),
Repo: repo,
Dlogger: &dl,
}
expected := scut.TestReturn{
Error: nil,
@ -50,7 +52,7 @@ var _ = Describe("E2E TEST:Vulnerabilities", func() {
NumberOfDebug: 0,
}
result := checks.NewVulnerabilities().HasUnfixedVulnerabilities(&req)
result := checks.HasUnfixedVulnerabilities(&req)
// UPGRADEv2: to remove.
// Old version.
Expect(result.Error).Should(BeNil())
@ -69,10 +71,11 @@ var _ = Describe("E2E TEST:Vulnerabilities", func() {
dl := scut.TestDetailLogger{}
checkRequest := checker.CheckRequest{
Ctx: context.Background(),
RepoClient: repoClient,
Repo: repo,
Dlogger: &dl,
Ctx: context.Background(),
RepoClient: repoClient,
VulnerabilitiesClient: clients.DefaultVulnerabilitiesClient(),
Repo: repo,
Dlogger: &dl,
}
expected := scut.TestReturn{
Error: nil,
@ -81,7 +84,7 @@ var _ = Describe("E2E TEST:Vulnerabilities", func() {
NumberOfInfo: 0,
NumberOfDebug: 0,
}
result := checks.NewVulnerabilities().HasUnfixedVulnerabilities(&checkRequest)
result := checks.HasUnfixedVulnerabilities(&checkRequest)
// UPGRADEv2: to remove.
// Old version.
Expect(result.Error).Should(BeNil())

View File

@ -30,14 +30,16 @@ import (
func runEnabledChecks(ctx context.Context,
repo clients.Repo, raw *checker.RawResults, checksToRun checker.CheckNameToFnMap,
repoClient clients.RepoClient, ossFuzzRepoClient clients.RepoClient, ciiClient clients.CIIBestPracticesClient,
vulnsClient clients.VulnerabilitiesClient,
resultsCh chan checker.CheckResult) {
request := checker.CheckRequest{
Ctx: ctx,
RepoClient: repoClient,
OssFuzzRepo: ossFuzzRepoClient,
CIIClient: ciiClient,
Repo: repo,
RawResults: raw,
Ctx: ctx,
RepoClient: repoClient,
OssFuzzRepo: ossFuzzRepoClient,
CIIClient: ciiClient,
VulnerabilitiesClient: vulnsClient,
Repo: repo,
RawResults: raw,
}
wg := sync.WaitGroup{}
for checkName, checkFn := range checksToRun {
@ -78,7 +80,8 @@ func RunScorecards(ctx context.Context,
checksToRun checker.CheckNameToFnMap,
repoClient clients.RepoClient,
ossFuzzRepoClient clients.RepoClient,
ciiClient clients.CIIBestPracticesClient) (ScorecardResult, error) {
ciiClient clients.CIIBestPracticesClient,
vulnsClient clients.VulnerabilitiesClient) (ScorecardResult, error) {
if err := repoClient.InitRepo(repo); err != nil {
// No need to call sce.WithMessage() since InitRepo will do that for us.
//nolint:wrapcheck
@ -104,9 +107,9 @@ func RunScorecards(ctx context.Context,
}
resultsCh := make(chan checker.CheckResult)
if raw {
go runEnabledChecks(ctx, repo, &ret.RawResults, checksToRun, repoClient, ossFuzzRepoClient, ciiClient, resultsCh)
go runEnabledChecks(ctx, repo, &ret.RawResults, checksToRun, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, resultsCh)
} else {
go runEnabledChecks(ctx, repo, nil, checksToRun, repoClient, ossFuzzRepoClient, ciiClient, resultsCh)
go runEnabledChecks(ctx, repo, nil, checksToRun, repoClient, ossFuzzRepoClient, ciiClient, vulnsClient, resultsCh)
}
for result := range resultsCh {