[gopls-release-branch.0.11] 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 <[email protected]>
Auto-Submit: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Suzy Mueller <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/tools/+/456639

Author: hyangah

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 @@ type Result struct {
 	// 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

gopls/internal/lsp/cache/view.go

@@ -19,6 +19,7 @@ import (
 	"sort"
 	"strings"
 	"sync"
+	"time"
 
 	"golang.org/x/mod/modfile"
 	"golang.org/x/mod/semver"
@@ -1034,19 +1035,27 @@ func (v *View) ClearModuleUpgrades(modfile span.URI) {
 	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
 }

gopls/internal/lsp/cache/view_test.go

@@ -5,11 +5,15 @@ package cache
 
 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 TestSuffixes(t *testing.T) {
 		}
 	}
 }
+
+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)
+}

gopls/internal/lsp/command.go

@@ -17,6 +17,7 @@ import (
 	"path/filepath"
 	"sort"
 	"strings"
+	"time"
 
 	"golang.org/x/mod/modfile"
 	"golang.org/x/tools/go/ast/astutil"
@@ -927,6 +928,7 @@ func (c *commandHandler) RunGovulncheck(ctx context.Context, args command.Vulnch
 			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)