From f983c1a9a8743fcd092fc60a89aa3f07bbdfdfe1 Mon Sep 17 00:00:00 2001 From: Tatiana Bradley Date: Wed, 23 Nov 2022 12:36:07 -0500 Subject: [PATCH] internal/database: add more tests for Load Adds tests for Load failure cases, adds more context to error message, and adds an additional failure case. For golang/go#56417 Change-Id: If4927c11f433c931827b262ee65a04f3594a125a Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/453175 Reviewed-by: Damien Neil Reviewed-by: Jonathan Amsterdam Run-TryBot: Tatiana Bradley TryBot-Result: Gopher Robot --- internal/database/load.go | 28 ++- internal/database/load_test.go | 222 +++++++++++++++++- .../db/missing-file/ID/GO-1999-0001.json | 55 +++++ .../db/missing-file/ID/GO-2000-0002.json | 49 ++++ .../testdata/db/missing-file/ID/index.json | 4 + .../testdata/db/missing-file/aliases.json | 8 + .../db/missing-file/example.com/module.json | 57 +++++ .../testdata/db/missing-file/index.json | 4 + .../db/unexpected-file/ID/GO-1999-0001.json | 55 +++++ .../db/unexpected-file/ID/GO-2000-0002.json | 49 ++++ .../testdata/db/unexpected-file/ID/index.json | 3 + .../testdata/db/unexpected-file/aliases.json | 5 + .../unexpected-file/example.com/module.json | 57 +++++ .../testdata/db/unexpected-file/index.json | 3 + 14 files changed, 581 insertions(+), 18 deletions(-) create mode 100644 internal/database/testdata/db/missing-file/ID/GO-1999-0001.json create mode 100644 internal/database/testdata/db/missing-file/ID/GO-2000-0002.json create mode 100644 internal/database/testdata/db/missing-file/ID/index.json create mode 100644 internal/database/testdata/db/missing-file/aliases.json create mode 100644 internal/database/testdata/db/missing-file/example.com/module.json create mode 100644 internal/database/testdata/db/missing-file/index.json create mode 100644 internal/database/testdata/db/unexpected-file/ID/GO-1999-0001.json create mode 100644 internal/database/testdata/db/unexpected-file/ID/GO-2000-0002.json create mode 100644 internal/database/testdata/db/unexpected-file/ID/index.json create mode 100644 internal/database/testdata/db/unexpected-file/aliases.json create mode 100644 internal/database/testdata/db/unexpected-file/example.com/module.json create mode 100644 internal/database/testdata/db/unexpected-file/index.json diff --git a/internal/database/load.go b/internal/database/load.go index aa2dcd20..c5c61603 100644 --- a/internal/database/load.go +++ b/internal/database/load.go @@ -54,7 +54,7 @@ func rawLoad(dbPath string) (_ *Database, err error) { } if err := report.UnmarshalFromFile(filepath.Join(dbPath, indexFile), &d.Index); err != nil { - return nil, err + return nil, fmt.Errorf("invalid or missing index.json: %v", err) } d.EntriesByModule, err = loadEntriesByModule(dbPath, d.Index) @@ -68,7 +68,7 @@ func rawLoad(dbPath string) (_ *Database, err error) { } if err := report.UnmarshalFromFile(filepath.Join(dbPath, aliasesFile), &d.IDsByAlias); err != nil { - return nil, err + return nil, fmt.Errorf("invalid or missing aliases.json: %v", err) } return d, nil @@ -102,7 +102,7 @@ func (d *Database) checkNoUnexpectedFiles(dbPath string) error { } 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)) + return fmt.Errorf("found unexpected file %q which is not present in %s", fname, filepath.Join(idDirectory, indexFile)) } // All other files should have corresponding entries in // EntriesByModule. @@ -153,6 +153,18 @@ func (d *Database) checkInternalConsistency() error { if !reflect.DeepEqual(entry, entryByID) { return fmt.Errorf("inconsistent OSV contents in module and ID advisory for %s", entry.ID) } + + var found bool + for _, affected := range entry.Affected { + m := affected.Package.Name + if m == module { + found = true + break + } + } + if !found { + return fmt.Errorf("%s does not reference %s", entry.ID, module) + } } if modified != wantModified { return fmt.Errorf("incorrect modified timestamp for module %s: want %s, got %s", module, wantModified, modified) @@ -190,7 +202,7 @@ func (d *Database) checkInternalConsistency() error { } } if !found { - return fmt.Errorf("%s is not listed as an alias of %s", entry.ID, alias) + return fmt.Errorf("%s is not listed as an alias of %s in aliases.json", entry.ID, alias) } } if entry.Published.After(entry.Modified) { @@ -202,7 +214,7 @@ func (d *Database) checkInternalConsistency() error { 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) + return fmt.Errorf("no advisory found for %s listed under %s", goID, alias) } if !slices.Contains(entry.Aliases, alias) { @@ -217,7 +229,7 @@ func (d *Database) checkInternalConsistency() error { func loadEntriesByID(dbPath string) (EntriesByID, error) { var ids []string if err := report.UnmarshalFromFile(filepath.Join(dbPath, idDirectory, indexFile), &ids); err != nil { - return nil, err + return nil, fmt.Errorf("invalid or missing ID/index.json: %v", err) } entriesByID := make(EntriesByID, len(ids)) @@ -225,7 +237,7 @@ func loadEntriesByID(dbPath string) (EntriesByID, error) { var entry osv.Entry err := report.UnmarshalFromFile(filepath.Join(dbPath, idDirectory, id+".json"), &entry) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid or missing OSV file: %v", err) } entriesByID[id] = &entry } @@ -243,7 +255,7 @@ func loadEntriesByModule(dbPath string, index client.DBIndex) (EntriesByModule, var entries []*osv.Entry err = report.UnmarshalFromFile(fpath, &entries) if err != nil { - return nil, err + return nil, fmt.Errorf("invalid or missing module directory: %v", err) } entriesByModule[module] = entries } diff --git a/internal/database/load_test.go b/internal/database/load_test.go index eed7b970..1097abec 100644 --- a/internal/database/load_test.go +++ b/internal/database/load_test.go @@ -5,6 +5,7 @@ package database import ( + "strings" "testing" "time" @@ -13,9 +14,6 @@ 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) @@ -123,13 +121,217 @@ var valid = &Database{ } func TestLoad(t *testing.T) { - path := validDir - got, err := Load(path) - if err != nil { - t.Fatalf("Load(%s): want succeess, got %s", path, err) + t.Run("ok", func(t *testing.T) { + path := validDir + got, err := Load(path) + if err != nil { + t.Fatalf("Load(%s): want succeess, got %s", path, err) + } + want := valid + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Load(%s): unexpected diff (want- got+):\n %s", path, diff) + } + }) + + failTests := []struct { + name string + dbPath string + wantErr string + }{ + { + name: "missing file", + dbPath: "testdata/db/missing-file", + wantErr: "invalid or missing", + }, + { + name: "unexpected file", + dbPath: "testdata/db/unexpected-file", + wantErr: "found unexpected file", + }, } - want := valid - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("Load(%s): unexpected diff (want- got+):\n %s", path, diff) + for _, test := range failTests { + t.Run(test.name, func(t *testing.T) { + _, err := Load(test.dbPath) + if err == nil || !strings.Contains(err.Error(), test.wantErr) { + t.Fatalf("Load(%s): want err containing %s, got %v", test.dbPath, test.wantErr, err) + } + }) + } +} + +func TestCheckInternalConsistency(t *testing.T) { + t.Run("ok", func(t *testing.T) { + if err := valid.checkInternalConsistency(); err != nil { + t.Error(err) + } + }) + + failTests := []struct { + name string + db *Database + wantErr string + }{ + { + name: "too many modules", + db: &Database{ + EntriesByModule: EntriesByModule{"module": []*osv.Entry{}}, + }, + wantErr: "length mismatch", + }, + { + name: "missing module from index", + db: &Database{ + Index: client.DBIndex{"module": time.Time{}}, + EntriesByModule: EntriesByModule{"module2": []*osv.Entry{}}, + }, + wantErr: "no module directory found", + }, + { + name: "missing OSV from module reference", + db: &Database{ + Index: client.DBIndex{"module": time.Time{}}, + EntriesByModule: EntriesByModule{"module": []*osv.Entry{ + {ID: "GO-1999-0001"}, + }}, + EntriesByID: EntriesByID{}, + }, + wantErr: "no advisory found for ID GO-1999-0001", + }, + { + name: "inconsistent OSV", + db: &Database{ + Index: client.DBIndex{"module": time.Time{}}, + EntriesByModule: EntriesByModule{"module": []*osv.Entry{ + {ID: "GO-1999-0001"}, + }}, + EntriesByID: EntriesByID{"GO-1999-0001": {ID: "GO-1999-0001", + Published: jan1999}}, + }, + wantErr: "inconsistent OSV contents", + }, + { + name: "incorrect modified timestamp in index", + db: &Database{ + Index: client.DBIndex{"module": jan2000}, + EntriesByModule: EntriesByModule{"module": []*osv.Entry{ + {ID: "GO-1999-0001", Modified: jan1999, + Affected: []osv.Affected{ + { + Package: osv.Package{ + Name: "module", + }, + }, + }}, + }}, + EntriesByID: EntriesByID{"GO-1999-0001": {ID: "GO-1999-0001", + Modified: jan1999, Affected: []osv.Affected{ + { + Package: osv.Package{ + Name: "module", + }, + }, + }}}, + }, + wantErr: "incorrect modified timestamp", + }, + { + name: "missing module referenced by OSV", + db: &Database{ + Index: client.DBIndex{}, + EntriesByModule: EntriesByModule{}, + EntriesByID: EntriesByID{"GO-1999-0001": {ID: "GO-1999-0001", + Affected: []osv.Affected{ + { + Package: osv.Package{ + Name: "a/module", + }, + }, + }, + }}}, + wantErr: "module a/module not found", + }, + { + name: "OSV does not reference module", + db: &Database{ + Index: client.DBIndex{"module": time.Time{}}, + EntriesByModule: EntriesByModule{"module": []*osv.Entry{ + {ID: "GO-1999-0001"}, + }}, + EntriesByID: EntriesByID{"GO-1999-0001": {ID: "GO-1999-0001"}}, + }, + wantErr: "GO-1999-0001 does not reference module", + }, + { + name: "missing OSV entry in module", + db: &Database{ + Index: client.DBIndex{"module": time.Time{}}, + EntriesByModule: EntriesByModule{"module": []*osv.Entry{ + {ID: "GO-1999-0002", + Affected: []osv.Affected{ + { + Package: osv.Package{ + Name: "module", + }, + }, + }, + }}}, + EntriesByID: EntriesByID{"GO-1999-0001": {ID: "GO-1999-0001", + Affected: []osv.Affected{ + { + Package: osv.Package{ + Name: "module", + }, + }, + }, + }, "GO-1999-0002": {ID: "GO-1999-0002", + Affected: []osv.Affected{ + { + Package: osv.Package{ + Name: "module", + }, + }, + }, + }, + }}, + wantErr: "GO-1999-0001 does not have an entry in module", + }, + { + name: "missing alias in aliases.json", + db: &Database{ + EntriesByID: EntriesByID{"GO-1999-0001": {ID: "GO-1999-0001", Aliases: []string{"CVE-1999-0001"}}}, + IDsByAlias: IDsByAlias{}, + }, + wantErr: "alias CVE-1999-0001 not found", + }, + { + name: "missing OSV reference in aliases.json", + db: &Database{ + EntriesByID: EntriesByID{"GO-1999-0001": {ID: "GO-1999-0001", Aliases: []string{"CVE-1999-0001"}}}, + IDsByAlias: IDsByAlias{"CVE-1999-0001": []string{"GO-2000-2222"}}, + }, + wantErr: "GO-1999-0001 is not listed as an alias of CVE-1999-0001", + }, + { + name: "missing OSV referenced by aliases.json", + db: &Database{ + IDsByAlias: IDsByAlias{"CVE-1999-0001": []string{"GO-1999-0001"}}, + }, + wantErr: "no advisory found for GO-1999-0001 listed under CVE-1999-0001", + }, + { + name: "missing alias in OSV", + db: &Database{ + EntriesByID: EntriesByID{"GO-1999-0001": {ID: "GO-1999-0001"}}, + IDsByAlias: IDsByAlias{"CVE-1999-0001": []string{"GO-1999-0001"}}, + }, + wantErr: "advisory GO-1999-0001 does not reference alias CVE-1999-0001", + }, + } + for _, test := range failTests { + t.Run(test.name, func(t *testing.T) { + if err := test.db.checkInternalConsistency(); err == nil || !strings.Contains(err.Error(), test.wantErr) { + t.Errorf("want error containing %q, got %v", test.wantErr, err) + } + }) } } diff --git a/internal/database/testdata/db/missing-file/ID/GO-1999-0001.json b/internal/database/testdata/db/missing-file/ID/GO-1999-0001.json new file mode 100644 index 00000000..56b71959 --- /dev/null +++ b/internal/database/testdata/db/missing-file/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/missing-file/ID/GO-2000-0002.json b/internal/database/testdata/db/missing-file/ID/GO-2000-0002.json new file mode 100644 index 00000000..b9c20bf2 --- /dev/null +++ b/internal/database/testdata/db/missing-file/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/missing-file/ID/index.json b/internal/database/testdata/db/missing-file/ID/index.json new file mode 100644 index 00000000..949e2f21 --- /dev/null +++ b/internal/database/testdata/db/missing-file/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/missing-file/aliases.json b/internal/database/testdata/db/missing-file/aliases.json new file mode 100644 index 00000000..05eaf766 --- /dev/null +++ b/internal/database/testdata/db/missing-file/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/missing-file/example.com/module.json b/internal/database/testdata/db/missing-file/example.com/module.json new file mode 100644 index 00000000..6187acd2 --- /dev/null +++ b/internal/database/testdata/db/missing-file/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/missing-file/index.json b/internal/database/testdata/db/missing-file/index.json new file mode 100644 index 00000000..9fe1aef2 --- /dev/null +++ b/internal/database/testdata/db/missing-file/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/testdata/db/unexpected-file/ID/GO-1999-0001.json b/internal/database/testdata/db/unexpected-file/ID/GO-1999-0001.json new file mode 100644 index 00000000..d5282060 --- /dev/null +++ b/internal/database/testdata/db/unexpected-file/ID/GO-1999-0001.json @@ -0,0 +1,55 @@ +{ + "id": "GO-1999-0001", + "published": "1999-01-01T00:00:00Z", + "modified": "2002-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/unexpected-file/ID/GO-2000-0002.json b/internal/database/testdata/db/unexpected-file/ID/GO-2000-0002.json new file mode 100644 index 00000000..75f020fb --- /dev/null +++ b/internal/database/testdata/db/unexpected-file/ID/GO-2000-0002.json @@ -0,0 +1,49 @@ +{ + "id": "GO-2000-0002", + "published": "2000-01-01T00:00:00Z", + "modified": "2002-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/unexpected-file/ID/index.json b/internal/database/testdata/db/unexpected-file/ID/index.json new file mode 100644 index 00000000..286d5c83 --- /dev/null +++ b/internal/database/testdata/db/unexpected-file/ID/index.json @@ -0,0 +1,3 @@ +[ + "GO-1999-0001" +] \ No newline at end of file diff --git a/internal/database/testdata/db/unexpected-file/aliases.json b/internal/database/testdata/db/unexpected-file/aliases.json new file mode 100644 index 00000000..ab04a2a6 --- /dev/null +++ b/internal/database/testdata/db/unexpected-file/aliases.json @@ -0,0 +1,5 @@ +{ + "CVE-1999-1111": [ + "GO-1999-0001" + ] +} \ No newline at end of file diff --git a/internal/database/testdata/db/unexpected-file/example.com/module.json b/internal/database/testdata/db/unexpected-file/example.com/module.json new file mode 100644 index 00000000..e59562da --- /dev/null +++ b/internal/database/testdata/db/unexpected-file/example.com/module.json @@ -0,0 +1,57 @@ +[ + { + "id": "GO-1999-0001", + "published": "1999-01-01T00:00:00Z", + "modified": "2002-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/unexpected-file/index.json b/internal/database/testdata/db/unexpected-file/index.json new file mode 100644 index 00000000..cbbec59b --- /dev/null +++ b/internal/database/testdata/db/unexpected-file/index.json @@ -0,0 +1,3 @@ +{ + "example.com/module": "2002-01-01T00:00:00Z" +} \ No newline at end of file