internal/worker: update false positives only if newer
Instead of inserting false positives only once when the DB
is new, update them on each run.
That way, we can add to our list of false positives and have
those changes reflected in the DB.
To do that safely, we have to make sure we don't update a false
positive record that has changed since the program's internal data was
created. We use the commit times for that.
Change-Id: I39d21940af2a302f7860a7e8563a91c88621b12a
Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/376298
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/internal/worker/false_positives.go b/internal/worker/false_positives.go
index b9d5681..ce35ad1 100644
--- a/internal/worker/false_positives.go
+++ b/internal/worker/false_positives.go
@@ -11,17 +11,33 @@
"golang.org/x/vulndb/internal/worker/store"
)
-func InsertFalsePositives(ctx context.Context, st store.Store) (err error) {
- defer derrors.Wrap(&err, "InsertFalsePositives")
+// updateFalsePositives makes sure the store reflects the list of false positives.
+func updateFalsePositives(ctx context.Context, st store.Store) (err error) {
+ defer derrors.Wrap(&err, "updateFalsePositives")
for i := 0; i < len(falsePositives); i += maxTransactionWrites {
j := i + maxTransactionWrites
if j >= len(falsePositives) {
j = len(falsePositives)
}
+ batch := falsePositives[i:j]
err := st.RunTransaction(ctx, func(ctx context.Context, tx store.Transaction) error {
- for _, cr := range falsePositives[i:j] {
- if err := tx.CreateCVERecord(cr); err != nil {
+ oldRecords, err := readCVERecords(tx, batch)
+ if err != nil {
+ return err
+ }
+ for i, cr := range batch {
+ old := oldRecords[i]
+ var err error
+ if old == nil {
+ err = tx.CreateCVERecord(cr)
+ } else if old.CommitHash != cr.CommitHash && !old.CommitTime.IsZero() && old.CommitTime.Before(cr.CommitTime) {
+ // If the false positive data is more recent than what is in
+ // the store, then update the DB. But ignore records whose
+ // commit time hasn't been populated.
+ err = tx.SetCVERecord(cr)
+ }
+ if err != nil {
return err
}
}
@@ -34,19 +50,18 @@
return nil
}
-// falsePositivesInserted reports whether the list of false positives has been
-// added to the store.
-func falsePositivesInserted(ctx context.Context, st store.Store) (bool, error) {
- // Check the first and last IDs. See gen_false_positives.go for the list.
- ids := []string{"CVE-2013-2124", "CVE-2021-3391"}
- for _, id := range ids {
- cr, err := st.GetCVERecord(ctx, id)
+func readCVERecords(tx store.Transaction, crs []*store.CVERecord) ([]*store.CVERecord, error) {
+ var olds []*store.CVERecord
+ for _, cr := range crs {
+ dbcrs, err := tx.GetCVERecords(cr.ID, cr.ID)
if err != nil {
- return false, err
+ return nil, err
}
- if cr == nil {
- return false, nil
+ var old *store.CVERecord
+ if len(dbcrs) > 0 {
+ old = dbcrs[0]
}
+ olds = append(olds, old)
}
- return true, nil
+ return olds, nil
}
diff --git a/internal/worker/false_positives_test.go b/internal/worker/false_positives_test.go
index eb0c2b6..890d0a0 100644
--- a/internal/worker/false_positives_test.go
+++ b/internal/worker/false_positives_test.go
@@ -2,6 +2,9 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
+//go:build go1.17
+// +build go1.17
+
package worker
import (
@@ -14,17 +17,46 @@
"golang.org/x/vulndb/internal/worker/store"
)
-func TestInsertFalsePositives(t *testing.T) {
- mstore := store.NewMemStore()
- if err := InsertFalsePositives(context.Background(), mstore); err != nil {
- t.Fatal(err)
- }
- // Spot-check a couple of records.
+func TestUpdateFalsePositives(t *testing.T) {
const commitHash = "17294f1a2af61a2a2df52ac89cbd7c516f0c4e6a"
commitTime := time.Date(2021, time.April, 12, 23, 0, 56, 0, time.UTC)
+
+ mstore := store.NewMemStore()
+ createCVERecords(t, mstore, []*store.CVERecord{
+ // This DB record is older than the matching false positive record
+ // embedded in the program, so it will be updated.
+ {
+ ID: "CVE-2020-15112",
+ Path: "2020/15xxx/CVE-2020-15112.json",
+ CommitHash: "xyz",
+ CommitTime: time.Date(2021, time.March, 1, 0, 0, 0, 0, time.UTC),
+ BlobHash: "3d87891317ff107037bc0145194ab72df1890411",
+ CVEState: cveschema.StatePublic,
+ TriageState: store.TriageStateNeedsIssue,
+ TriageStateReason: "will be replaced",
+ },
+ // This DB record is newer, so it won't be changed.
+ {
+ ID: "CVE-2020-15113",
+ Path: "2020/15xxx/CVE-2020-15113.json",
+ BlobHash: "9133c3be68ef84771bad74ec8770e1efff7bf0de",
+ CommitHash: commitHash,
+ CommitTime: commitTime,
+ CVEState: cveschema.StatePublic,
+ TriageState: store.TriageStateNoActionNeeded,
+ ReferenceURLs: []string{
+ "https://github.com/etcd-io/etcd/security/advisories/GHSA-chh6-ppwq-jh92",
+ },
+ },
+ })
+
+ if err := updateFalsePositives(context.Background(), mstore); err != nil {
+ t.Fatal(err)
+ }
got := mstore.CVERecords()
for _, want := range []*store.CVERecord{
{
+ // Doesn't exist in DB.
ID: "CVE-2016-0216",
Path: "2016/0xxx/CVE-2016-0216.json",
CommitHash: commitHash,
@@ -38,6 +70,7 @@
},
},
{
+ // Newer than DB.
ID: "CVE-2020-15112",
Path: "2020/15xxx/CVE-2020-15112.json",
CommitHash: commitHash,
@@ -47,6 +80,18 @@
TriageState: store.TriageStateHasVuln,
TriageStateReason: "GO-2020-0005",
},
+ {
+ ID: "CVE-2020-15113",
+ Path: "2020/15xxx/CVE-2020-15113.json",
+ BlobHash: "9133c3be68ef84771bad74ec8770e1efff7bf0de",
+ CommitHash: commitHash,
+ CommitTime: commitTime,
+ CVEState: cveschema.StatePublic,
+ TriageState: store.TriageStateNoActionNeeded,
+ ReferenceURLs: []string{
+ "https://github.com/etcd-io/etcd/security/advisories/GHSA-chh6-ppwq-jh92",
+ },
+ },
} {
if diff := cmp.Diff(want, got[want.ID]); diff != "" {
t.Errorf("mismatch (-want, +got):\n%s", diff)
diff --git a/internal/worker/update_test.go b/internal/worker/update_test.go
index a3c8b49..ba9d9a6 100644
--- a/internal/worker/update_test.go
+++ b/internal/worker/update_test.go
@@ -348,7 +348,6 @@
}
func createCVERecords(t *testing.T, s store.Store, crs []*store.CVERecord) {
- t.Helper()
err := s.RunTransaction(context.Background(), func(_ context.Context, tx store.Transaction) error {
for _, cr := range crs {
if err := tx.CreateCVERecord(cr); err != nil {
diff --git a/internal/worker/worker.go b/internal/worker/worker.go
index b839547..8e82feb 100644
--- a/internal/worker/worker.go
+++ b/internal/worker/worker.go
@@ -33,35 +33,29 @@
// UpdateCommit performs an update on the store using the given commit.
// Unless force is true, it checks that the update makes sense before doing it.
-func UpdateCommit(ctx context.Context, repoPath, commitHash string, st store.Store, pkgsiteURL string, force bool) (err error) {
- defer derrors.Wrap(&err, "RunCommitUpdate(%q, %q, force=%t)", repoPath, commitHash, force)
+func UpdateCommit(ctx context.Context, repoPath, commitHashString string, st store.Store, pkgsiteURL string, force bool) (err error) {
+ defer derrors.Wrap(&err, "RunCommitUpdate(%q, %q, force=%t)", repoPath, commitHashString, force)
- b, err := falsePositivesInserted(ctx, st)
- if err != nil {
+ log.Infof(ctx, "updating false positives")
+ if err := updateFalsePositives(ctx, st); err != nil {
return err
}
- if !b {
- log.Infof(ctx, "inserting false positives")
- if err := InsertFalsePositives(ctx, st); err != nil {
- return err
- }
- }
repo, err := gitrepo.CloneOrOpen(ctx, repoPath)
if err != nil {
return err
}
- var ch plumbing.Hash
- if commitHash == "HEAD" {
+ var commitHash plumbing.Hash
+ if commitHashString == "HEAD" {
ref, err := repo.Reference(plumbing.HEAD, true)
if err != nil {
return err
}
- ch = ref.Hash()
+ commitHash = ref.Hash()
} else {
- ch = plumbing.NewHash(commitHash)
+ commitHash = plumbing.NewHash(commitHashString)
}
- commit, err := repo.CommitObject(ch)
+ commit, err := repo.CommitObject(commitHash)
if err != nil {
return err
}
diff --git a/internal/worker/worker_test.go b/internal/worker/worker_test.go
index f10fbfd..db3dc6d 100644
--- a/internal/worker/worker_test.go
+++ b/internal/worker/worker_test.go
@@ -77,7 +77,7 @@
},
} {
mstore := store.NewMemStore()
- if err := InsertFalsePositives(ctx, mstore); err != nil {
+ if err := updateFalsePositives(ctx, mstore); err != nil {
t.Fatal(err)
}
if test.latestUpdate != nil {
