refactor: Remove osv package from many places (#1497)

I am attempting to split a big refactor PR into several small pieces, this is part 4, which just removes the need for the osv package from many places since we are replacing it with the osvdev package. This mostly means user agent need to be passed in via the client instead of being pulled from a global variable (this was already started in part 3: #1491 )
google · Jan 15, 2025 · 404b7f3 · 404b7f3
1 parent 0809439
commit 404b7f3
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 43 deletions.
diff --git a/cmd/osv-scanner/main.go b/cmd/osv-scanner/main.go
@@ -10,7 +10,6 @@ import (
 	"github.com/google/osv-scanner/cmd/osv-scanner/scan"
 	"github.com/google/osv-scanner/cmd/osv-scanner/update"
 	"github.com/google/osv-scanner/internal/version"
-	"github.com/google/osv-scanner/pkg/osv"
 	"github.com/google/osv-scanner/pkg/osvscanner"
 	"github.com/google/osv-scanner/pkg/reporter"
 
@@ -30,8 +29,6 @@ func run(args []string, stdout, stderr io.Writer) int {
 		r.Infof("osv-scanner version: %s\ncommit: %s\nbuilt at: %s\n", ctx.App.Version, commit, date)
 	}
 
-	osv.RequestUserAgent = "osv-scanner/" + version.OSVVersion
-
 	app := &cli.App{
 		Name:           "osv-scanner",
 		Version:        version.OSVVersion,

diff --git a/internal/clients/clientimpl/localmatcher/localmatcher.go b/internal/clients/clientimpl/localmatcher/localmatcher.go
@@ -26,11 +26,13 @@ type LocalMatcher struct {
 	downloadDB bool
 	// failedDBs keeps track of the errors when getting databases for each ecosystem
 	failedDBs map[osvschema.Ecosystem]error
+	// userAgent sets the user agent requests for db zips are made with
+	userAgent string
 	// TODO(v2 logging): Remove this reporter
 	r reporter.Reporter
 }
 
-func NewLocalMatcher(r reporter.Reporter, localDBPath string, downloadDB bool) (*LocalMatcher, error) {
+func NewLocalMatcher(r reporter.Reporter, localDBPath string, userAgent string, downloadDB bool) (*LocalMatcher, error) {
 	dbBasePath, err := setupLocalDBDirectory(localDBPath)
 	if err != nil {
 		return nil, fmt.Errorf("could not create %s: %w", dbBasePath, err)
@@ -41,6 +43,7 @@ func NewLocalMatcher(r reporter.Reporter, localDBPath string, downloadDB bool) (
 		dbs:        make(map[osvschema.Ecosystem]*ZipDB),
 		downloadDB: downloadDB,
 		r:          r,
+		userAgent:  userAgent,
 		failedDBs:  make(map[osvschema.Ecosystem]error),
 	}, nil
 }
@@ -97,7 +100,7 @@ func (matcher *LocalMatcher) loadDBFromCache(ctx context.Context, ecosystem ecos
 		return nil, matcher.failedDBs[ecosystem.Ecosystem]
 	}
 
-	db, err := NewZippedDB(ctx, matcher.dbBasePath, string(ecosystem.Ecosystem), fmt.Sprintf("%s/%s/all.zip", zippedDBRemoteHost, ecosystem.Ecosystem), !matcher.downloadDB)
+	db, err := NewZippedDB(ctx, matcher.dbBasePath, string(ecosystem.Ecosystem), fmt.Sprintf("%s/%s/all.zip", zippedDBRemoteHost, ecosystem.Ecosystem), matcher.userAgent, !matcher.downloadDB)
 
 	if err != nil {
 		matcher.failedDBs[ecosystem.Ecosystem] = err

diff --git a/internal/clients/clientimpl/localmatcher/zip.go b/internal/clients/clientimpl/localmatcher/zip.go
@@ -20,7 +20,6 @@ import (
 	"github.com/google/osv-scanner/internal/utility/vulns"
 	"github.com/google/osv-scanner/pkg/lockfile"
 	"github.com/google/osv-scanner/pkg/models"
-	"github.com/google/osv-scanner/pkg/osv"
 )
 
 type ZipDB struct {
@@ -34,6 +33,8 @@ type ZipDB struct {
 	StoredAt string
 	// the vulnerabilities that are loaded into this database
 	vulnerabilities []models.Vulnerability
+	// User agent to query with
+	UserAgent string
 }
 
 var ErrOfflineDatabaseNotFound = errors.New("no offline version of the OSV database is available")
@@ -105,8 +106,8 @@ func (db *ZipDB) fetchZip(ctx context.Context) ([]byte, error) {
 		return nil, fmt.Errorf("could not retrieve OSV database archive: %w", err)
 	}
 
-	if osv.RequestUserAgent != "" {
-		req.Header.Set("User-Agent", osv.RequestUserAgent)
+	if db.UserAgent != "" {
+		req.Header.Set("User-Agent", db.UserAgent)
 	}
 
 	resp, err := http.DefaultClient.Do(req)
@@ -203,12 +204,13 @@ func (db *ZipDB) load(ctx context.Context) error {
 	return nil
 }
 
-func NewZippedDB(ctx context.Context, dbBasePath, name, url string, offline bool) (*ZipDB, error) {
+func NewZippedDB(ctx context.Context, dbBasePath, name, url, userAgent string, offline bool) (*ZipDB, error) {
 	db := &ZipDB{
 		Name:       name,
 		ArchiveURL: url,
 		Offline:    offline,
 		StoredAt:   path.Join(dbBasePath, name, "all.zip"),
+		UserAgent:  userAgent,
 	}
 	if err := db.load(ctx); err != nil {
 		return nil, fmt.Errorf("unable to fetch OSV database: %w", err)

diff --git a/internal/clients/clientimpl/localmatcher/zip_test.go b/internal/clients/clientimpl/localmatcher/zip_test.go
@@ -19,9 +19,12 @@ import (
 
 	"github.com/google/osv-scanner/internal/clients/clientimpl/localmatcher"
 	"github.com/google/osv-scanner/internal/testutility"
+	"github.com/google/osv-scanner/internal/version"
 	"github.com/google/osv-scanner/pkg/models"
 )
 
+const userAgent = "osv-scanner_test/" + version.OSVVersion
+
 func expectDBToHaveOSVs(
 	t *testing.T,
 	db interface {
@@ -146,7 +149,7 @@ func TestNewZippedDB_Offline_WithoutCache(t *testing.T) {
 		t.Errorf("a server request was made when running offline")
 	})
 
-	_, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, true)
+	_, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, true)
 
 	if !errors.Is(err, localmatcher.ErrOfflineDatabaseNotFound) {
 		t.Errorf("expected \"%v\" error but got \"%v\"", localmatcher.ErrOfflineDatabaseNotFound, err)
@@ -178,7 +181,7 @@ func TestNewZippedDB_Offline_WithCache(t *testing.T) {
 		"GHSA-5.json": {ID: "GHSA-5"},
 	}))
 
-	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, true)
+	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, true)
 
 	if err != nil {
 		t.Fatalf("unexpected error \"%v\"", err)
@@ -196,7 +199,7 @@ func TestNewZippedDB_BadZip(t *testing.T) {
 		_, _ = w.Write([]byte("this is not a zip"))
 	})
 
-	_, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, false)
+	_, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, false)
 
 	if err == nil {
 		t.Errorf("expected an error but did not get one")
@@ -208,7 +211,7 @@ func TestNewZippedDB_UnsupportedProtocol(t *testing.T) {
 
 	testDir := testutility.CreateTestDir(t)
 
-	_, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", "file://hello-world", false)
+	_, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", "file://hello-world", userAgent, false)
 
 	if err == nil {
 		t.Errorf("expected an error but did not get one")
@@ -238,7 +241,7 @@ func TestNewZippedDB_Online_WithoutCache(t *testing.T) {
 		})
 	})
 
-	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, false)
+	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, false)
 
 	if err != nil {
 		t.Fatalf("unexpected error \"%v\"", err)
@@ -270,7 +273,7 @@ func TestNewZippedDB_Online_WithoutCacheAndNoHashHeader(t *testing.T) {
 		}))
 	})
 
-	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, false)
+	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, false)
 
 	if err != nil {
 		t.Fatalf("unexpected error \"%v\"", err)
@@ -308,7 +311,7 @@ func TestNewZippedDB_Online_WithSameCache(t *testing.T) {
 
 	cacheWrite(t, determineStoredAtPath(testDir, "my-db"), cache)
 
-	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, false)
+	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, false)
 
 	if err != nil {
 		t.Fatalf("unexpected error \"%v\"", err)
@@ -346,7 +349,7 @@ func TestNewZippedDB_Online_WithDifferentCache(t *testing.T) {
 		"GHSA-3.json": {ID: "GHSA-3"},
 	}))
 
-	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, false)
+	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, false)
 
 	if err != nil {
 		t.Fatalf("unexpected error \"%v\"", err)
@@ -376,7 +379,7 @@ func TestNewZippedDB_Online_WithCacheButNoHashHeader(t *testing.T) {
 		"GHSA-3.json": {ID: "GHSA-3"},
 	}))
 
-	_, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, false)
+	_, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, false)
 
 	if err == nil {
 		t.Errorf("expected an error but did not get one")
@@ -404,7 +407,7 @@ func TestNewZippedDB_Online_WithBadCache(t *testing.T) {
 
 	cacheWriteBad(t, determineStoredAtPath(testDir, "my-db"), "this is not json!")
 
-	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, false)
+	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, false)
 
 	if err != nil {
 		t.Fatalf("unexpected error \"%v\"", err)
@@ -430,7 +433,7 @@ func TestNewZippedDB_FileChecks(t *testing.T) {
 		})
 	})
 
-	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, false)
+	db, err := localmatcher.NewZippedDB(context.Background(), testDir, "my-db", ts.URL, userAgent, false)
 
 	if err != nil {
 		t.Fatalf("unexpected error \"%v\"", err)

diff --git a/pkg/osvscanner/osvscanner.go b/pkg/osvscanner/osvscanner.go
@@ -129,7 +129,7 @@ func DoScan(actions ScannerActions, r reporter.Reporter) (models.VulnerabilityRe
 		var matcher clientinterfaces.VulnerabilityMatcher
 		var err error
 		if actions.CompareOffline {
-			matcher, err = localmatcher.NewLocalMatcher(r, actions.LocalDBPath, actions.DownloadDatabases)
+			matcher, err = localmatcher.NewLocalMatcher(r, actions.LocalDBPath, "osv-scanner_scan/"+version.OSVVersion, actions.DownloadDatabases)
 			if err != nil {
 				return models.VulnerabilityResults{}, err
 			}

diff --git a/pkg/osvscanner/vulnerability_result_internal_test.go b/pkg/osvscanner/vulnerability_result_internal_test.go
@@ -10,7 +10,6 @@ import (
 	"github.com/google/osv-scanner/internal/scalibrextract/ecosystemmock"
 	"github.com/google/osv-scanner/internal/testutility"
 	"github.com/google/osv-scanner/pkg/models"
-	"github.com/google/osv-scanner/pkg/osv"
 	"github.com/google/osv-scanner/pkg/reporter"
 )
 
@@ -47,22 +46,21 @@ func Test_assembleResult(t *testing.T) {
 			},
 		},
 	}
-	vulnsResp := &osv.HydratedBatchedResponse{
-		Results: []osv.Response{
-			{Vulns: models.Vulnerabilities([]models.Vulnerability{
-				{
-					ID:      "GHSA-123",
-					Aliases: []string{"CVE-123"},
-				}, {
-					ID: "CVE-123",
-				},
-			})},
-			{Vulns: models.Vulnerabilities{}},
-			{Vulns: models.Vulnerabilities([]models.Vulnerability{
-				{
-					ID: "GHSA-456",
-				},
-			})},
+	vulnsResp := [][]*models.Vulnerability{
+		{
+			{
+				ID:      "GHSA-123",
+				Aliases: []string{"CVE-123"},
+			},
+			{
+				ID: "CVE-123",
+			},
+		},
+		{},
+		{
+			{
+				ID: "GHSA-456",
+			},
 		},
 	}
 
@@ -86,13 +84,9 @@ func Test_assembleResult(t *testing.T) {
 		}
 		licensesResp = makeLicensesResp()
 		for i := range packages {
-			vulnPointers := []*models.Vulnerability{}
-			for _, vuln := range vulnsResp.Results[i].Vulns {
-				vulnPointers = append(vulnPointers, &vuln)
-			}
 			scanResults.PackageScanResults = append(scanResults.PackageScanResults, imodels.PackageScanResult{
 				PackageInfo:     packages[i],
-				Vulnerabilities: vulnPointers,
+				Vulnerabilities: vulnsResp[i],
 				Licenses:        licensesResp[i],
 			})
 		}