internal/database: add logic to validate a new deploy

Adds a function, Validate, which checks a candidate Go vulnerability
database against an existing one, to ensure that both databases are
valid, timestamps are consistent and no OSV entries would be deleted.

Moves single-database validation logic (previously called Validate) to
the Load function, so that Load now loads and checks a database.

Also adds a command line tool, "checkdeploy" which calls the new
Validate function. This tool will be used in the deploy script for vulndb.

For golang/go#56417

Change-Id: Ifa12234376f2a3fd577d96978919b167fcb25f64
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/452443
Reviewed-by: Damien Neil <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Tatiana Bradley <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>

cmd/checkdb/main.go

@@ -12,16 +12,13 @@ import (
 	"golang.org/x/vulndb/internal/database"
 )
 
-var (
-	path = flag.String("path", "", "path to database")
-)
-
 func main() {
 	flag.Parse()
-	if *path == "" {
-		log.Fatalf("flag -path must be set")
+	path := flag.Arg(0)
+	if path == "" {
+		log.Fatal("path must be set\nusage: checkdb [path]")
 	}
-	if err := database.Validate(*path); err != nil {
+	if _, err := database.Load(path); err != nil {
 		log.Fatal(err)
 	}
 }

cmd/checkdeploy/main.go

@@ -0,0 +1,32 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Command checkdeploy validates that it is safe to deploy a new
+// vulnerability database.
+package main
+
+import (
+	"flag"
+	"log"
+
+	"golang.org/x/vulndb/internal/database"
+)
+
+var (
+	newPath      = flag.String("new", "", "path to new database")
+	existingPath = flag.String("existing", "", "path to existing database")
+)
+
+func main() {
+	flag.Parse()
+	if *newPath == "" {
+		log.Fatalf("flag -new must be set")
+	}
+	if *existingPath == "" {
+		log.Fatalf("flag -existing must be set")
+	}
+	if err := database.Validate(*newPath, *existingPath); err != nil {
+		log.Fatal(err)
+	}
+}

internal/database/database.go

@@ -67,12 +67,7 @@ const (
 func New(ctx context.Context, repo *git.Repository) (_ *Database, err error) {
 	defer derrors.Wrap(&err, "New()")
 
-	d := &Database{
-		Index:           make(client.DBIndex),
-		EntriesByID:     make(EntriesByID),
-		EntriesByModule: make(EntriesByModule),
-		IDsByAlias:      make(IDsByAlias),
-	}
+	d := newEmpty()
 
 	root, err := gitrepo.Root(repo)
 	if err != nil {
@@ -118,6 +113,15 @@ func New(ctx context.Context, repo *git.Repository) (_ *Database, err error) {
 	return d, nil
 }
 
+func newEmpty() *Database {
+	return &Database{
+		Index:           make(client.DBIndex),
+		EntriesByID:     make(EntriesByID),
+		EntriesByModule: make(EntriesByModule),
+		IDsByAlias:      make(IDsByAlias),
+	}
+}
+
 func (d *Database) addEntry(entry *osv.Entry) {
 	for _, module := range report.ModulesForEntry(*entry) {
 		d.EntriesByModule[module] = append(d.EntriesByModule[module], entry)

internal/database/database_test.go

@@ -54,16 +54,13 @@ func TestAll(t *testing.T) {
 		t.Error(err)
 	}
 
-	loaded, err := Load(writeDir)
+	validated, err := Load(writeDir)
 	if err != nil {
 		t.Error(err)
 	}
-	if diff := cmp.Diff(loaded, new); diff != "" {
-		t.Errorf("unexpected diff (loaded-, new+):\n%s", diff)
-	}
 
-	if err = Validate(writeDir); err != nil {
-		t.Error(err)
+	if diff := cmp.Diff(validated, new); diff != "" {
+		t.Errorf("unexpected diff (validated-, new+):\n%s", diff)
 	}
 }
 
@@ -88,16 +85,12 @@ func TestAllIntegration(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	loaded, err := Load(writeDir)
+	validated, err := Load(writeDir)
 	if err != nil {
 		t.Error(err)
 	}
 
-	if diff := cmp.Diff(loaded, new); diff != "" {
-		t.Errorf("unexpected diff (loaded-, new+):\n%s", diff)
-	}
-
-	if err = Validate(writeDir); err != nil {
-		t.Error(err)
+	if diff := cmp.Diff(validated, new); diff != "" {
+		t.Errorf("unexpected diff (validated-, new+):\n%s", diff)
 	}
 }

internal/database/generate_test.go

@@ -43,12 +43,6 @@ func TestGenerateIntegration(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	t.Run("Generate outputs valid DB", func(t *testing.T) {
-		if err := Validate(genDir); err != nil {
-			t.Error(err)
-		}
-	})
-
 	t.Run("Generate equivalent to New then Write", func(t *testing.T) {
 		writeDir := t.TempDir()
 		if err = new.Write(writeDir, false); err != nil {

internal/database/load.go

@@ -5,18 +5,47 @@
 package database
 
 import (
+	"fmt"
+	"io/fs"
 	"path/filepath"
+	"reflect"
+	"strings"
+	"time"
 
 	"golang.org/x/exp/maps"
+	"golang.org/x/exp/slices"
 	"golang.org/x/vuln/client"
 	"golang.org/x/vuln/osv"
 	"golang.org/x/vulndb/internal/derrors"
 	"golang.org/x/vulndb/internal/report"
 )
 
-// Load reads the contents of dbPath into a Database, and errors
-// if the database has missing files (based on the module and ID indexes).
+// Load reads the contents of dbPath into a Database, and errors if:
+//   - Any files are malformed (cannot be unmarshaled)
+//   - The database has missing files (based on the module and ID indexes)
+//   - The database has unexpected files not listed in the indexes
+//   - The database is internally inconsistent
 func Load(dbPath string) (_ *Database, err error) {
+	derrors.Wrap(&err, "Load(%s)", dbPath)
+
+	d, err := rawLoad(dbPath)
+	if err != nil {
+		return nil, err
+	}
+	if err := d.checkNoUnexpectedFiles(dbPath); err != nil {
+		return nil, err
+	}
+	if err := d.checkInternalConsistency(); err != nil {
+		return nil, err
+	}
+
+	return d, nil
+}
+
+// rawLoad reads the contents of dbPath into a Database, and errors
+// if any files are malformed, or the database has missing files
+// (based on the module and ID indexes).
+func rawLoad(dbPath string) (_ *Database, err error) {
 	defer derrors.Wrap(&err, "Load(%q)", dbPath)
 
 	d := &Database{
@@ -45,6 +74,146 @@ func Load(dbPath string) (_ *Database, err error) {
 	return d, nil
 }
 
+func (d *Database) checkNoUnexpectedFiles(dbPath string) error {
+	return filepath.WalkDir(dbPath, func(path string, f fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		fname := f.Name()
+		ext := filepath.Ext(fname)
+		dir := filepath.Dir(path)
+
+		switch {
+		// Skip directories.
+		case f.IsDir():
+			return nil
+		// In the top-level directory, web files and index files are OK.
+		case dir == dbPath && isIndexOrWebFile(fname, ext):
+			return nil
+		// All non-directory and non-web files should end in ".json".
+		case ext != ".json":
+			return fmt.Errorf("found unexpected non-JSON file %s", path)
+		// All files in the ID directory (except the index) should have
+		// corresponding entries in EntriesByID.
+		case dir == filepath.Join(dbPath, idDirectory):
+			if fname == indexFile {
+				return nil
+			}
+			id := report.GetGoIDFromFilename(fname)
+			if _, ok := d.EntriesByID[id]; !ok {
+				return fmt.Errorf("found unexpected ID %q which is not present in %s", id, filepath.Join(idDirectory, indexFile))
+			}
+		// All other files should have corresponding entries in
+		// EntriesByModule.
+		default:
+			module := strings.TrimSuffix(strings.TrimPrefix(strings.TrimPrefix(path, dbPath), "/"), ".json")
+			unescaped, err := client.UnescapeModulePath(module)
+			if err != nil {
+				return fmt.Errorf("could not unescape module file %s: %v", path, err)
+			}
+			if _, ok := d.EntriesByModule[unescaped]; !ok {
+				return fmt.Errorf("found unexpected module %q which is not present in %s", unescaped, indexFile)
+			}
+		}
+		return nil
+	})
+}
+
+func isIndexOrWebFile(filename, ext string) bool {
+	return ext == ".ico" ||
+		ext == ".html" ||
+		// HTML files may have no extension.
+		ext == "" ||
+		filename == indexFile ||
+		filename == aliasesFile
+}
+
+func (d *Database) checkInternalConsistency() error {
+	if il, ml := len(d.Index), len(d.EntriesByModule); il != ml {
+		return fmt.Errorf("length mismatch: there are %d module entries in the index, and %d module directory entries", il, ml)
+	}
+
+	for module, modified := range d.Index {
+		entries, ok := d.EntriesByModule[module]
+		if !ok || len(entries) == 0 {
+			return fmt.Errorf("no module directory found for indexed module %s", module)
+		}
+
+		var wantModified time.Time
+		for _, entry := range entries {
+			if mod := entry.Modified; mod.After(wantModified) {
+				wantModified = mod
+			}
+
+			entryByID, ok := d.EntriesByID[entry.ID]
+			if !ok {
+				return fmt.Errorf("no advisory found for ID %s listed in %s", entry.ID, module)
+			}
+			if !reflect.DeepEqual(entry, entryByID) {
+				return fmt.Errorf("inconsistent OSV contents in module and ID advisory for %s", entry.ID)
+			}
+		}
+		if modified != wantModified {
+			return fmt.Errorf("incorrect modified timestamp for module %s: want %s, got %s", module, wantModified, modified)
+		}
+	}
+
+	for id, entry := range d.EntriesByID {
+		for _, affected := range entry.Affected {
+			module := affected.Package.Name
+			entries, ok := d.EntriesByModule[module]
+			if !ok || len(entries) == 0 {
+				return fmt.Errorf("module %s not found (referenced by %s)", module, id)
+			}
+			found := false
+			for _, gotEntry := range entries {
+				if gotEntry.ID == id {
+					found = true
+					break
+				}
+			}
+			if !found {
+				return fmt.Errorf("%s does not have an entry in %s", id, module)
+			}
+		}
+		for _, alias := range entry.Aliases {
+			gotEntries, ok := d.IDsByAlias[alias]
+			if !ok || len(gotEntries) == 0 {
+				return fmt.Errorf("alias %s not found in aliases.json (alias of %s)", alias, id)
+			}
+			found := false
+			for _, gotID := range gotEntries {
+				if gotID == id {
+					found = true
+					break
+				}
+			}
+			if !found {
+				return fmt.Errorf("%s is not listed as an alias of %s", entry.ID, alias)
+			}
+		}
+		if entry.Published.After(entry.Modified) {
+			return fmt.Errorf("%s: published time (%s) cannot be after modified time (%s)", entry.ID, entry.Published, entry.Modified)
+		}
+	}
+
+	for alias, goIDs := range d.IDsByAlias {
+		for _, goID := range goIDs {
+			entry, ok := d.EntriesByID[goID]
+			if !ok {
+				return fmt.Errorf("no advisory found for ID %s listed under %s", goID, alias)
+			}
+
+			if !slices.Contains(entry.Aliases, alias) {
+				return fmt.Errorf("advisory %s does not reference alias %s", goID, alias)
+			}
+		}
+	}
+
+	return nil
+}
+
 func loadEntriesByID(dbPath string) (EntriesByID, error) {
 	var ids []string
 	if err := report.UnmarshalFromFile(filepath.Join(dbPath, idDirectory, indexFile), &ids); err != nil {

internal/database/load_test.go

@@ -13,6 +13,9 @@ import (
 	"golang.org/x/vuln/osv"
 )
 
+// TODO(https://github.com/golang/go#56417): Write unit tests for various
+// invalid databases.
+
 var (
 	validDir = "testdata/db/valid"
 	jan1999  = time.Date(1999, 1, 1, 0, 0, 0, 0, time.UTC)