From 5d80fbd8039076095a5019b5515acfb08f5f5649 Mon Sep 17 00:00:00 2001 From: Tatiana Bradley Date: Mon, 21 Nov 2022 19:27:37 -0500 Subject: [PATCH] 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 TryBot-Result: Gopher Robot Run-TryBot: Tatiana Bradley Reviewed-by: Jonathan Amsterdam --- cmd/checkdb/main.go | 11 +- cmd/checkdeploy/main.go | 32 ++++ internal/database/database.go | 16 +- internal/database/database_test.go | 19 +- internal/database/generate_test.go | 6 - internal/database/load.go | 173 ++++++++++++++++- internal/database/load_test.go | 3 + .../testdata/db/existing/ID/GO-1999-0001.json | 55 ++++++ .../testdata/db/existing/ID/GO-2000-0002.json | 49 +++++ .../testdata/db/existing/ID/index.json | 4 + .../testdata/db/existing/aliases.json | 8 + .../db/existing/example.com/module.json | 57 ++++++ .../db/existing/example.com/module2.json | 51 +++++ .../database/testdata/db/existing/index.json | 4 + internal/database/validate.go | 174 +++--------------- internal/database/validate_test.go | 128 ++++++++++++- 16 files changed, 598 insertions(+), 192 deletions(-) create mode 100644 cmd/checkdeploy/main.go create mode 100644 internal/database/testdata/db/existing/ID/GO-1999-0001.json create mode 100644 internal/database/testdata/db/existing/ID/GO-2000-0002.json create mode 100644 internal/database/testdata/db/existing/ID/index.json create mode 100644 internal/database/testdata/db/existing/aliases.json create mode 100644 internal/database/testdata/db/existing/example.com/module.json create mode 100644 internal/database/testdata/db/existing/example.com/module2.json create mode 100644 internal/database/testdata/db/existing/index.json diff --git a/cmd/checkdb/main.go b/cmd/checkdb/main.go index e747087c..ce5446bc 100644 --- a/cmd/checkdb/main.go +++ b/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) } } diff --git a/cmd/checkdeploy/main.go b/cmd/checkdeploy/main.go new file mode 100644 index 00000000..c1c7b70a --- /dev/null +++ b/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) + } +} diff --git a/internal/database/database.go b/internal/database/database.go index 0f29c820..db51f60e 100644 --- a/internal/database/database.go +++ b/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) diff --git a/internal/database/database_test.go b/internal/database/database_test.go index 56e87626..b4bde5db 100644 --- a/internal/database/database_test.go +++ b/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) } } diff --git a/internal/database/generate_test.go b/internal/database/generate_test.go index 2a61ec5a..ed62cab3 100644 --- a/internal/database/generate_test.go +++ b/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 { diff --git a/internal/database/load.go b/internal/database/load.go index c5278677..aa2dcd20 100644 --- a/internal/database/load.go +++ b/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 { diff --git a/internal/database/load_test.go b/internal/database/load_test.go index ed77e90e..eed7b970 100644 --- a/internal/database/load_test.go +++ b/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) diff --git a/internal/database/testdata/db/existing/ID/GO-1999-0001.json b/internal/database/testdata/db/existing/ID/GO-1999-0001.json new file mode 100644 index 00000000..56b71959 --- /dev/null +++ b/internal/database/testdata/db/existing/ID/GO-1999-0001.json @@ -0,0 +1,55 @@ +{ + "id": "GO-1999-0001", + "published": "1999-01-01T00:00:00Z", + "modified": "2001-01-01T00:00:00Z", + "aliases": [ + "CVE-1999-1111" + ], + "details": "Some details", + "affected": [ + { + "package": { + "name": "example.com/module", + "ecosystem": "Go" + }, + "ranges": [ + { + "type": "SEMVER", + "events": [ + { + "introduced": "0" + }, + { + "fixed": "1.1.0" + }, + { + "introduced": "1.2.0" + }, + { + "fixed": "1.2.2" + } + ] + } + ], + "database_specific": { + "url": "https://pkg.go.dev/vuln/GO-1999-0001" + }, + "ecosystem_specific": { + "imports": [ + { + "path": "package", + "symbols": [ + "Symbol" + ] + } + ] + } + } + ], + "references": [ + { + "type": "FIX", + "url": "https://example.com/cl/123" + } + ] +} \ No newline at end of file diff --git a/internal/database/testdata/db/existing/ID/GO-2000-0002.json b/internal/database/testdata/db/existing/ID/GO-2000-0002.json new file mode 100644 index 00000000..b9c20bf2 --- /dev/null +++ b/internal/database/testdata/db/existing/ID/GO-2000-0002.json @@ -0,0 +1,49 @@ +{ + "id": "GO-2000-0002", + "published": "2000-01-01T00:00:00Z", + "modified": "2001-01-01T00:00:00Z", + "aliases": [ + "CVE-1999-2222" + ], + "details": "Some details", + "affected": [ + { + "package": { + "name": "example.com/module2", + "ecosystem": "Go" + }, + "ranges": [ + { + "type": "SEMVER", + "events": [ + { + "introduced": "0" + }, + { + "fixed": "1.2.0" + } + ] + } + ], + "database_specific": { + "url": "https://pkg.go.dev/vuln/GO-2000-0002" + }, + "ecosystem_specific": { + "imports": [ + { + "path": "package", + "symbols": [ + "Symbol" + ] + } + ] + } + } + ], + "references": [ + { + "type": "FIX", + "url": "https://example.com/cl/543" + } + ] +} \ No newline at end of file diff --git a/internal/database/testdata/db/existing/ID/index.json b/internal/database/testdata/db/existing/ID/index.json new file mode 100644 index 00000000..949e2f21 --- /dev/null +++ b/internal/database/testdata/db/existing/ID/index.json @@ -0,0 +1,4 @@ +[ + "GO-1999-0001", + "GO-2000-0002" +] \ No newline at end of file diff --git a/internal/database/testdata/db/existing/aliases.json b/internal/database/testdata/db/existing/aliases.json new file mode 100644 index 00000000..05eaf766 --- /dev/null +++ b/internal/database/testdata/db/existing/aliases.json @@ -0,0 +1,8 @@ +{ + "CVE-1999-1111": [ + "GO-1999-0001" + ], + "CVE-1999-2222": [ + "GO-2000-0002" + ] +} \ No newline at end of file diff --git a/internal/database/testdata/db/existing/example.com/module.json b/internal/database/testdata/db/existing/example.com/module.json new file mode 100644 index 00000000..6187acd2 --- /dev/null +++ b/internal/database/testdata/db/existing/example.com/module.json @@ -0,0 +1,57 @@ +[ + { + "id": "GO-1999-0001", + "published": "1999-01-01T00:00:00Z", + "modified": "2001-01-01T00:00:00Z", + "aliases": [ + "CVE-1999-1111" + ], + "details": "Some details", + "affected": [ + { + "package": { + "name": "example.com/module", + "ecosystem": "Go" + }, + "ranges": [ + { + "type": "SEMVER", + "events": [ + { + "introduced": "0" + }, + { + "fixed": "1.1.0" + }, + { + "introduced": "1.2.0" + }, + { + "fixed": "1.2.2" + } + ] + } + ], + "database_specific": { + "url": "https://pkg.go.dev/vuln/GO-1999-0001" + }, + "ecosystem_specific": { + "imports": [ + { + "path": "package", + "symbols": [ + "Symbol" + ] + } + ] + } + } + ], + "references": [ + { + "type": "FIX", + "url": "https://example.com/cl/123" + } + ] + } +] \ No newline at end of file diff --git a/internal/database/testdata/db/existing/example.com/module2.json b/internal/database/testdata/db/existing/example.com/module2.json new file mode 100644 index 00000000..b8d2738a --- /dev/null +++ b/internal/database/testdata/db/existing/example.com/module2.json @@ -0,0 +1,51 @@ +[ + { + "id": "GO-2000-0002", + "published": "2000-01-01T00:00:00Z", + "modified": "2001-01-01T00:00:00Z", + "aliases": [ + "CVE-1999-2222" + ], + "details": "Some details", + "affected": [ + { + "package": { + "name": "example.com/module2", + "ecosystem": "Go" + }, + "ranges": [ + { + "type": "SEMVER", + "events": [ + { + "introduced": "0" + }, + { + "fixed": "1.2.0" + } + ] + } + ], + "database_specific": { + "url": "https://pkg.go.dev/vuln/GO-2000-0002" + }, + "ecosystem_specific": { + "imports": [ + { + "path": "package", + "symbols": [ + "Symbol" + ] + } + ] + } + } + ], + "references": [ + { + "type": "FIX", + "url": "https://example.com/cl/543" + } + ] + } +] \ No newline at end of file diff --git a/internal/database/testdata/db/existing/index.json b/internal/database/testdata/db/existing/index.json new file mode 100644 index 00000000..9fe1aef2 --- /dev/null +++ b/internal/database/testdata/db/existing/index.json @@ -0,0 +1,4 @@ +{ + "example.com/module": "2001-01-01T00:00:00Z", + "example.com/module2": "2001-01-01T00:00:00Z" +} \ No newline at end of file diff --git a/internal/database/validate.go b/internal/database/validate.go index 53e65978..7283ac0e 100644 --- a/internal/database/validate.go +++ b/internal/database/validate.go @@ -6,175 +6,43 @@ package database import ( "fmt" - "io/fs" - "path/filepath" - "reflect" - "strings" - "time" - "golang.org/x/exp/slices" - "golang.org/x/vuln/client" "golang.org/x/vulndb/internal/derrors" - "golang.org/x/vulndb/internal/report" ) -func Validate(dbPath string) (err error) { - derrors.Wrap(&err, "Validate(%s)", dbPath) +// Validate checks that the databases in newPath and oldPath are +// both valid databases, and that the database in newPath can +// be safely deployed on top of the database in oldPath. +func Validate(newPath, oldPath string) (err error) { + derrors.Wrap(&err, "Validate(new=%s, old=%s)", newPath, oldPath) - // Load will fail if any files are missing. - d, err := Load(dbPath) + // Load will fail if either of the databases is internally + // inconsistent. + new, err := Load(newPath) if err != nil { return err } - if err = d.validate(dbPath); err != nil { + old, err := Load(oldPath) + if err != nil { return err } - return nil -} -func (d *Database) validate(dbPath string) error { - if err := d.checkNoUnexpectedFiles(dbPath); err != nil { - return err - } - if err := d.checkInternalConsistency(); err != nil { - return err - } - return nil + return validate(new, old) } -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 +// validate checks for deleted files and inconsistent timestamps. +func validate(new, old *Database) error { + for id, oldEntry := range old.EntriesByID { + newEntry, ok := new.EntriesByID[id] + if !ok { + return fmt.Errorf("%s is not present in new database. Use the %q field to delete an entry", id, "withdrawn") } - - 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) - } + if newEntry.Published != oldEntry.Published { + return fmt.Errorf("%s: published time cannot change (new %s, old %s)", id, newEntry.Published, oldEntry.Published) } - 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 newEntry.Modified.Before(oldEntry.Modified) { + return fmt.Errorf("%s: modified time cannot decrease (new %s, old %s)", id, newEntry.Modified, oldEntry.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 } diff --git a/internal/database/validate_test.go b/internal/database/validate_test.go index c29eec3d..7fa1b425 100644 --- a/internal/database/validate_test.go +++ b/internal/database/validate_test.go @@ -4,13 +4,131 @@ package database -import "testing" +import ( + "strings" + "testing" -// TODO(https://github.com/golang/go#56417): Write unit tests for various -// invalid databases. + "golang.org/x/vuln/osv" +) + +var existingDir = "testdata/db/existing" func TestValidate(t *testing.T) { - if err := Validate(validDir); err != nil { - t.Error(err) + t.Run("valid", func(t *testing.T) { + // validDir contains a new entry and correct modified and + // published timestamps with respect to existing. + if err := Validate(validDir, existingDir); err != nil { + t.Error(err) + } + }) + + t.Run("old on new fails", func(t *testing.T) { + // Attempting to deploy the existing database on top of the new + // database should fail, as an entry is missing and timestamps are + // incorrect. + if err := Validate(existingDir, validDir); err == nil { + t.Error("expected error, got nil") + } + }) +} + +func newTestDB(entries ...*osv.Entry) *Database { + d := newEmpty() + for _, entry := range entries { + d.addEntry(entry) + } + return d +} + +func TestValidateInternal(t *testing.T) { + successTests := []struct { + name string + newDB *Database + oldDB *Database + }{ + { + name: "valid updates ok", + oldDB: newTestDB( + &osv.Entry{ + ID: "GO-1999-0001", + Published: jan1999, + Modified: jan1999, + }), + newDB: newTestDB(&osv.Entry{ + ID: "GO-1999-0001", + Published: jan1999, + Modified: jan2000, + }, &osv.Entry{ + ID: "GO-1999-0002", + Published: jan2000, + Modified: jan2000, + }), + }, + { + name: "same db ok", + oldDB: valid, + newDB: valid, + }, + } + for _, test := range successTests { + t.Run(test.name, func(t *testing.T) { + if err := validate(test.newDB, test.oldDB); err != nil { + t.Errorf("validate(): unexpected error %v", err) + } + }) + } + + failTests := []struct { + name string + newDB *Database + oldDB *Database + wantErr string + }{ + { + name: "published time changed", + oldDB: newTestDB( + &osv.Entry{ + ID: "GO-1999-0001", + Published: jan1999, + Modified: jan1999, + }), + newDB: newTestDB(&osv.Entry{ + ID: "GO-1999-0001", + Published: jan2000, + Modified: jan2000, + }), + wantErr: "published time cannot change", + }, + { + name: "deleted entry", + oldDB: newTestDB( + &osv.Entry{ + ID: "GO-1999-0001", + Published: jan1999, + Modified: jan1999, + }), + newDB: newTestDB(), + wantErr: "GO-1999-0001 is not present in new database", + }, + { + name: "modified time decreased", + oldDB: newTestDB( + &osv.Entry{ + ID: "GO-1999-0001", + Modified: jan2000, + }), + newDB: newTestDB(&osv.Entry{ + ID: "GO-1999-0001", + Modified: jan1999, + }), + wantErr: "modified time cannot decrease", + }, + } + for _, test := range failTests { + t.Run(test.name, func(t *testing.T) { + if err := validate(test.newDB, test.oldDB); err == nil || !strings.Contains(err.Error(), test.wantErr) { + t.Errorf("validate(): want error containing %q, got %v", test.wantErr, err) + } + }) } }