gopls/internal/lsp/cache: invalidate govulncheck results older than 1hr
1hr is long enough to make the computed results stale either because
the vuln db is updated or the code is changed. Drop old results.
Note: OSV entry caching TTL used by the govulncheck command line tool is 2hr.
https://github.com/golang/vuln/blob/05fb7250142cc6010c39968839f2f3710afdd918/client/client.go#L230
Change-Id: I17ac5307a0e966bb91be74dcaf25cee14781eb2d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/456255
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Auto-Submit: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/govulncheck/types.go b/gopls/internal/govulncheck/types.go
index 75240aa..7198451 100644
--- a/gopls/internal/govulncheck/types.go
+++ b/gopls/internal/govulncheck/types.go
@@ -4,11 +4,13 @@
package govulncheck
+import "time"
+
// Result is the result of vulnerability scanning.
type Result struct {
// Vulns contains all vulnerabilities that are called or imported by
// the analyzed module.
- Vulns []*Vuln
+ Vulns []*Vuln `json:",omitempty"`
// Mode contains the source of the vulnerability info.
// Clients of the gopls.fetch_vulncheck_result command may need
@@ -19,7 +21,11 @@
// without callstack traces just implies the package with the
// vulnerability is known to the workspace and we do not know
// whether the vulnerable symbols are actually used or not.
- Mode AnalysisMode
+ Mode AnalysisMode `json:",omitempty"`
+
+ // AsOf describes when this Result was computed using govulncheck.
+ // It is valid only with the govulncheck analysis mode.
+ AsOf time.Time `json:",omitempty"`
}
type AnalysisMode string
diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go
index 0f4272e..7b5c05b7 100644
--- a/gopls/internal/lsp/cache/view.go
+++ b/gopls/internal/lsp/cache/view.go
@@ -19,6 +19,7 @@
"sort"
"strings"
"sync"
+ "time"
"golang.org/x/mod/modfile"
"golang.org/x/mod/semver"
@@ -1038,19 +1039,27 @@
delete(v.moduleUpgrades, modfile)
}
-func (v *View) Vulnerabilities(modfile ...span.URI) map[span.URI]*govulncheck.Result {
+const maxGovulncheckResultAge = 1 * time.Hour // Invalidate results older than this limit.
+var timeNow = time.Now // for testing
+
+func (v *View) Vulnerabilities(modfiles ...span.URI) map[span.URI]*govulncheck.Result {
m := make(map[span.URI]*govulncheck.Result)
+ now := timeNow()
v.mu.Lock()
defer v.mu.Unlock()
- if len(modfile) == 0 {
- for k, v := range v.vulns {
- m[k] = v
+ if len(modfiles) == 0 { // empty means all modfiles
+ for modfile := range v.vulns {
+ modfiles = append(modfiles, modfile)
}
- return m
}
- for _, f := range modfile {
- m[f] = v.vulns[f]
+ for _, modfile := range modfiles {
+ vuln := v.vulns[modfile]
+ if vuln != nil && now.Sub(vuln.AsOf) > maxGovulncheckResultAge {
+ v.vulns[modfile] = nil // same as SetVulnerabilities(modfile, nil)
+ vuln = nil
+ }
+ m[modfile] = vuln
}
return m
}
diff --git a/gopls/internal/lsp/cache/view_test.go b/gopls/internal/lsp/cache/view_test.go
index f57fc80..b2fca90 100644
--- a/gopls/internal/lsp/cache/view_test.go
+++ b/gopls/internal/lsp/cache/view_test.go
@@ -5,11 +5,15 @@
import (
"context"
+ "encoding/json"
"io/ioutil"
"os"
"path/filepath"
"testing"
+ "time"
+ "github.com/google/go-cmp/cmp"
+ "golang.org/x/tools/gopls/internal/govulncheck"
"golang.org/x/tools/gopls/internal/lsp/fake"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/span"
@@ -208,3 +212,67 @@
}
}
}
+
+func TestView_Vulnerabilities(t *testing.T) {
+ // TODO(hyangah): use t.Cleanup when we get rid of go1.13 legacy CI.
+ defer func() { timeNow = time.Now }()
+
+ now := time.Now()
+
+ view := &View{
+ vulns: make(map[span.URI]*govulncheck.Result),
+ }
+ file1, file2 := span.URIFromPath("f1/go.mod"), span.URIFromPath("f2/go.mod")
+
+ vuln1 := &govulncheck.Result{AsOf: now.Add(-(maxGovulncheckResultAge * 3) / 4)} // already ~3/4*maxGovulncheckResultAge old
+ view.SetVulnerabilities(file1, vuln1)
+
+ vuln2 := &govulncheck.Result{AsOf: now} // fresh.
+ view.SetVulnerabilities(file2, vuln2)
+
+ t.Run("fresh", func(t *testing.T) {
+ got := view.Vulnerabilities()
+ want := map[span.URI]*govulncheck.Result{
+ file1: vuln1,
+ file2: vuln2,
+ }
+
+ if diff := cmp.Diff(toJSON(want), toJSON(got)); diff != "" {
+ t.Errorf("view.Vulnerabilities() mismatch (-want +got):\n%s", diff)
+ }
+ })
+
+ // maxGovulncheckResultAge/2 later
+ timeNow = func() time.Time { return now.Add(maxGovulncheckResultAge / 2) }
+ t.Run("after30min", func(t *testing.T) {
+ got := view.Vulnerabilities()
+ want := map[span.URI]*govulncheck.Result{
+ file1: nil, // expired.
+ file2: vuln2,
+ }
+
+ if diff := cmp.Diff(toJSON(want), toJSON(got)); diff != "" {
+ t.Errorf("view.Vulnerabilities() mismatch (-want +got):\n%s", diff)
+ }
+ })
+
+ // maxGovulncheckResultAge later
+ timeNow = func() time.Time { return now.Add(maxGovulncheckResultAge + time.Minute) }
+
+ t.Run("after1hr", func(t *testing.T) {
+ got := view.Vulnerabilities()
+ want := map[span.URI]*govulncheck.Result{
+ file1: nil,
+ file2: nil,
+ }
+
+ if diff := cmp.Diff(toJSON(want), toJSON(got)); diff != "" {
+ t.Errorf("view.Vulnerabilities() mismatch (-want +got):\n%s", diff)
+ }
+ })
+}
+
+func toJSON(x interface{}) string {
+ b, _ := json.MarshalIndent(x, "", " ")
+ return string(b)
+}
diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go
index 4bbf0a9..2d89f44 100644
--- a/gopls/internal/lsp/command.go
+++ b/gopls/internal/lsp/command.go
@@ -17,6 +17,7 @@
"path/filepath"
"sort"
"strings"
+ "time"
"golang.org/x/mod/modfile"
"golang.org/x/tools/go/ast/astutil"
@@ -933,6 +934,7 @@
return fmt.Errorf("failed to parse govulncheck output: %v", err)
}
result.Mode = govulncheck.ModeGovulncheck
+ result.AsOf = time.Now()
deps.snapshot.View().SetVulnerabilities(args.URI.SpanURI(), &result)
c.s.diagnoseSnapshot(deps.snapshot, nil, false)
