⚠️ OSV scanner integration (#2509)

* Improve OSV scanning integration (squashed)

Signed-off-by: Rex P <rexpan@google.com>

* Add support for grouping vulnerabilities and aliases

Signed-off-by: Rex P <rexpan@google.com>

* Updated documentation, spit vulnerability output to multiple warnings

Signed-off-by: Rex P <rexpan@google.com>

* Updated documentation, spit vulnerability output to multiple warnings

Signed-off-by: Rex P <rexpan@google.com>

* Add its own codebase into docs

Signed-off-by: Rex P <rexpan@google.com>

* Update scorecard test to not prevent known vulns

Signed-off-by: Rex P <rexpan@google.com>

Signed-off-by: Rex P <rexpan@google.com>
Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>

.gitignore

@@ -42,6 +42,7 @@ e2e-coverage.out
 .vscode/
 *.iml
 .idea
+.history
 
 # tools
 bin

checker/client.go

@@ -45,7 +45,7 @@ func GetClients(ctx context.Context, repoURI, localURI string, logger *log.Logge
 			localdir.CreateLocalDirClient(ctx, logger), /*repoClient*/
 			nil, /*ossFuzzClient*/
 			nil, /*ciiClient*/
-			nil, /*vulnClient*/
+			clients.DefaultVulnerabilitiesClient(), /*vulnClient*/
 			retErr
 	}
 

checks/evaluation/vulnerabilities.go

@@ -18,6 +18,8 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/google/osv-scanner/pkg/grouper"
+
 	"github.com/ossf/scorecard/v4/checker"
 	sce "github.com/ossf/scorecard/v4/errors"
 )
@@ -31,21 +33,25 @@ func Vulnerabilities(name string, dl checker.DetailLogger,
 		return checker.CreateRuntimeErrorResult(name, e)
 	}
 
-	score := checker.MaxResultScore
-	IDs := []string{}
+	aliasVulnerabilities := []grouper.IDAliases{}
 	for _, vuln := range r.Vulnerabilities {
-		IDs = append(IDs, vuln.ID)
-		score--
+		aliasVulnerabilities = append(aliasVulnerabilities, grouper.IDAliases(vuln))
 	}
 
+	IDs := grouper.Group(aliasVulnerabilities)
+	score := checker.MaxResultScore - len(IDs)
+
 	if score < checker.MinResultScore {
 		score = checker.MinResultScore
 	}
 
 	if len(IDs) > 0 {
-		dl.Warn(&checker.LogMessage{
-			Text: fmt.Sprintf("HEAD is vulnerable to %s", strings.Join(IDs, ", ")),
-		})
+		for _, v := range IDs {
+			dl.Warn(&checker.LogMessage{
+				Text: fmt.Sprintf("Project is vulnerable to: %s", strings.Join(v.IDs, " / ")),
+			})
+		}
+
 		return checker.CreateResultWithScore(name,
 			fmt.Sprintf("%v existing vulnerabilities detected", len(IDs)), score)
 	}

checks/raw/vulnerabilities.go

@@ -23,18 +23,19 @@ import (
 
 // Vulnerabilities retrieves the raw data for the Vulnerabilities check.
 func Vulnerabilities(c *checker.CheckRequest) (checker.VulnerabilitiesData, error) {
+	commitHash := ""
 	commits, err := c.RepoClient.ListCommits()
-	if err != nil {
-		return checker.VulnerabilitiesData{}, fmt.Errorf("repoClient.ListCommits: %w", err)
+	if err == nil && len(commits) > 0 && !allOf(commits, hasEmptySHA) {
+		commitHash = commits[0].SHA
 	}
 
-	if len(commits) < 1 || allOf(commits, hasEmptySHA) {
-		return checker.VulnerabilitiesData{}, nil
+	localPath, err := c.RepoClient.LocalPath()
+	if err != nil {
+		return checker.VulnerabilitiesData{}, fmt.Errorf("RepoClient.LocalPath: %w", err)
 	}
-
-	resp, err := c.VulnerabilitiesClient.HasUnfixedVulnerabilities(c.Ctx, commits[0].SHA)
+	resp, err := c.VulnerabilitiesClient.ListUnfixedVulnerabilities(c.Ctx, commitHash, localPath)
 	if err != nil {
-		return checker.VulnerabilitiesData{}, fmt.Errorf("vulnerabilitiesClient.HasUnfixedVulnerabilities: %w", err)
+		return checker.VulnerabilitiesData{}, fmt.Errorf("vulnerabilitiesClient.ListUnfixedVulnerabilities: %w", err)
 	}
 	return checker.VulnerabilitiesData{
 		Vulnerabilities: resp.Vulnerabilities,

checks/raw/vulnerabilities_test.go

@@ -85,9 +85,13 @@ func TestVulnerabilities(t *testing.T) {
 				return []clients.Commit{{SHA: "test"}}, nil
 			}).AnyTimes()
 
+			mockRepo.EXPECT().LocalPath().DoAndReturn(func() (string, error) {
+				return "test_path", nil
+			}).AnyTimes()
+
 			mockVulnClient := mockrepo.NewMockVulnerabilitiesClient(ctrl)
-			mockVulnClient.EXPECT().HasUnfixedVulnerabilities(context.TODO(), gomock.Any()).DoAndReturn(
-				func(ctx context.Context, repo string) (clients.VulnerabilitiesResponse, error) {
+			mockVulnClient.EXPECT().ListUnfixedVulnerabilities(context.TODO(), gomock.Any(), gomock.Any()).DoAndReturn(
+				func(ctx context.Context, commit string, localPath string) (clients.VulnerabilitiesResponse, error) {
 					if tt.vulnsError {
 						//nolint
 						return clients.VulnerabilitiesResponse{}, errors.New("error")

checks/vulnerabilities.go

@@ -28,6 +28,7 @@ const CheckVulnerabilities = "Vulnerabilities"
 func init() {
 	supportedRequestTypes := []checker.RequestType{
 		checker.CommitBased,
+		checker.FileBased,
 	}
 	if err := registerCheck(CheckVulnerabilities, Vulnerabilities, supportedRequestTypes); err != nil {
 		// this should never happen

checks/vulnerabilities_test.go

@@ -55,9 +55,13 @@ func TestVulnerabilities(t *testing.T) {
 				return []clients.Commit{{SHA: "test"}}, nil
 			}).MinTimes(1)
 
+			mockRepo.EXPECT().LocalPath().DoAndReturn(func() (string, error) {
+				return "test_path", nil
+			}).AnyTimes()
+
 			mockVulnClient := mockrepo.NewMockVulnerabilitiesClient(ctrl)
-			mockVulnClient.EXPECT().HasUnfixedVulnerabilities(context.TODO(), gomock.Any()).DoAndReturn(
-				func(ctx context.Context, repo string) (clients.VulnerabilitiesResponse, error) {
+			mockVulnClient.EXPECT().ListUnfixedVulnerabilities(context.TODO(), gomock.Any(), gomock.Any()).DoAndReturn(
+				func(ctx context.Context, commit string, localPath string) (clients.VulnerabilitiesResponse, error) {
 					return tt.expected, tt.err
 				}).MinTimes(1)
 

clients/githubrepo/client.go

@@ -130,6 +130,11 @@ func (client *Client) URI() string {
 	return fmt.Sprintf("github.com/%s/%s", client.repourl.owner, client.repourl.repo)
 }
 
+// LocalPath implements RepoClient.LocalPath.
+func (client *Client) LocalPath() (string, error) {
+	return client.tarball.getLocalPath()
+}
+
 // ListFiles implements RepoClient.ListFiles.
 func (client *Client) ListFiles(predicate func(string) (bool, error)) ([]string, error) {
 	return client.tarball.listFiles(predicate)

clients/githubrepo/tarball.go

@@ -243,6 +243,17 @@ func (handler *tarballHandler) listFiles(predicate func(string) (bool, error)) (
 	return ret, nil
 }
 
+func (handler *tarballHandler) getLocalPath() (string, error) {
+	if err := handler.setup(); err != nil {
+		return "", fmt.Errorf("error during tarballHandler.setup: %w", err)
+	}
+	absTempDir, err := filepath.Abs(handler.tempDir)
+	if err != nil {
+		return "", fmt.Errorf("error during filepath.Abs: %w", err)
+	}
+	return absTempDir, nil
+}
+
 func (handler *tarballHandler) getFileContent(filename string) ([]byte, error) {
 	if err := handler.setup(); err != nil {
 		return nil, fmt.Errorf("error during tarballHandler.setup: %w", err)

clients/gitlabrepo/client.go

@@ -136,6 +136,10 @@ func (client *Client) URI() string {
 	return fmt.Sprintf("%s/%s/%s", client.repourl.hostname, client.repourl.owner, client.repourl.projectID)
 }
 
+func (client *Client) LocalPath() (string, error) {
+	return "", nil
+}
+
 func (client *Client) ListFiles(predicate func(string) (bool, error)) ([]string, error) {
 	return nil, nil
 }

clients/localdir/client.go

@@ -142,6 +142,15 @@ func applyPredicate(
 	return files, nil
 }
 
+// LocalPath implements RepoClient.LocalPath.
+func (client *localDirClient) LocalPath() (string, error) {
+	clientPath, err := filepath.Abs(client.path)
+	if err != nil {
+		return "", fmt.Errorf("error during filepath.Abs: %w", err)
+	}
+	return clientPath, nil
+}
+
 // ListFiles implements RepoClient.ListFiles.
 func (client *localDirClient) ListFiles(predicate func(string) (bool, error)) ([]string, error) {
 	client.once.Do(func() {

clients/osv.go

@@ -15,62 +15,68 @@
 package clients
 
 import (
-	"bytes"
 	"context"
-	"encoding/json"
-	"net/http"
+	"fmt"
 
-	"github.com/ossf/scorecard/v4/errors"
+	"github.com/google/osv-scanner/pkg/osvscanner"
 )
 
 var _ VulnerabilitiesClient = osvClient{}
 
 type osvClient struct{}
 
-const osvQueryEndpoint = "https://api.osv.dev/v1/query"
-
-type osvQuery struct {
-	Commit string `json:"commit"`
-}
-
-type osvResp struct {
-	Vulns []struct {
-		ID string `json:"id"`
-	} `json:"vulns"`
-}
-
-// HasUnfixedVulnerabilities implements VulnerabilityClient.HasUnfixedVulnerabilities.
-func (v osvClient) 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")
+// ListUnfixedVulnerabilities implements VulnerabilityClient.ListUnfixedVulnerabilities.
+func (v osvClient) ListUnfixedVulnerabilities(
+	ctx context.Context,
+	commit,
+	localPath string,
+) (VulnerabilitiesResponse, error) {
+	directoryPaths := []string{}
+	if localPath != "" {
+		directoryPaths = append(directoryPaths, localPath)
 	}
-
-	req, err := http.NewRequestWithContext(ctx, http.MethodPost, osvQueryEndpoint, bytes.NewReader(query))
-	if err != nil {
-		return VulnerabilitiesResponse{}, errors.WithMessage(err, "failed to create request")
+	gitCommits := []string{}
+	if commit != "" {
+		gitCommits = append(gitCommits, commit)
 	}
-
-	httpClient := &http.Client{}
-	resp, err := httpClient.Do(req)
+	res, err := osvscanner.DoScan(osvscanner.ScannerActions{
+		DirectoryPaths: directoryPaths,
+		SkipGit:        true,
+		Recursive:      true,
+		GitCommits:     gitCommits,
+	}, nil) // TODO: Do logging?
 	if err != nil {
-		return VulnerabilitiesResponse{}, errors.WithMessage(err, "failed to send request")
+		return VulnerabilitiesResponse{}, fmt.Errorf("osvscanner.DoScan: %w", err)
 	}
-	defer resp.Body.Close()
 
-	var osvresp osvResp
-	decoder := json.NewDecoder(resp.Body)
-	if err := decoder.Decode(&osvresp); err != nil {
-		return VulnerabilitiesResponse{}, errors.WithMessage(err, "failed to decode response")
+	response := VulnerabilitiesResponse{}
+	vulns := res.Flatten()
+	for i := range vulns {
+		response.Vulnerabilities = append(response.Vulnerabilities, Vulnerability{
+			ID:      vulns[i].Vulnerability.ID,
+			Aliases: vulns[i].Vulnerability.Aliases,
+		})
+		// Remove duplicate vulnerability IDs for now as we don't report information
+		// on the source of each vulnerability yet, therefore having multiple identical
+		// vuln IDs might be confusing.
+		response.Vulnerabilities = removeDuplicate(
+			response.Vulnerabilities,
+			func(key Vulnerability) string { return key.ID },
+		)
 	}
+	return response, nil
+}
 
-	var ret VulnerabilitiesResponse
-	for _, vuln := range osvresp.Vulns {
-		ret.Vulnerabilities = append(ret.Vulnerabilities, Vulnerability{
-			ID: vuln.ID,
-		})
+// RemoveDuplicate removes duplicate entries from a slice.
+func removeDuplicate[T any, K comparable](sliceList []T, keyExtract func(T) K) []T {
+	allKeys := make(map[K]bool)
+	list := []T{}
+	for _, item := range sliceList {
+		key := keyExtract(item)
+		if _, value := allKeys[key]; !value {
+			allKeys[key] = true
+			list = append(list, item)
+		}
 	}
-	return ret, nil
+	return list
 }

clients/osv_test.go

@@ -0,0 +1,48 @@
+// Copyright 2022 OpenSSF 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 (
+	"reflect"
+	"testing"
+)
+
+func TestRemoveDuplicate(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name       string
+		keyExtract func(string) string
+		list       []string
+		want       []string
+	}{
+		{
+			name: "Basic list with dup items",
+			list: []string{"A", "B", "C", "B"},
+			want: []string{"A", "B", "C"},
+			keyExtract: func(in string) string {
+				return in
+			},
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := removeDuplicate(tt.list, tt.keyExtract)
+			if !reflect.DeepEqual(tt.want, got) {
+				t.Errorf("got %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

clients/repo_client.go

@@ -32,6 +32,9 @@ type RepoClient interface {
 	URI() string
 	IsArchived() (bool, error)
 	ListFiles(predicate func(string) (bool, error)) ([]string, error)
+	// Returns an absolute path to the local repository
+	// in the format that matches the local OS
+	LocalPath() (string, error)
 	GetFileContent(filename string) ([]byte, error)
 	GetBranch(branch string) (*BranchRef, error)
 	GetCreatedAt() (time.Time, error)