From bc08991678f9f60b920f053f72129878f0791805 Mon Sep 17 00:00:00 2001 From: "Hana (Hyang-Ah) Kim" Date: Wed, 9 Nov 2022 06:57:43 -0500 Subject: [PATCH] gopls: sync golang.org/x/vuln@3af8368ee4fe golang.org/x/vuln/exp/govulncheck API was significantly changed. The previous Main is gone, and we need to use Source. The returned data types changed significantly too. Change-Id: Ic702ffe9a94a8ddd1867a0f2766bb49e2133d3a3 Reviewed-on: https://go-review.googlesource.com/c/tools/+/448975 TryBot-Result: Gopher Robot gopls-CI: kokoro Reviewed-by: Robert Findley Run-TryBot: Hyang-Ah Hana Kim --- gopls/go.mod | 4 +- gopls/go.sum | 5 +- gopls/internal/govulncheck/cache.go | 145 ------------------ gopls/internal/govulncheck/cache_test.go | 165 --------------------- gopls/internal/govulncheck/copy.sh | 32 ---- gopls/internal/govulncheck/summary.go | 46 ------ gopls/internal/govulncheck/types_118.go | 44 ++++++ gopls/internal/govulncheck/types_not118.go | 133 +++++++++++++++++ gopls/internal/lsp/cache/session.go | 2 +- gopls/internal/lsp/cache/view.go | 8 +- gopls/internal/lsp/cmd/vulncheck.go | 8 +- gopls/internal/lsp/command.go | 27 ++-- gopls/internal/lsp/mod/diagnostics.go | 41 +++-- gopls/internal/lsp/mod/hover.go | 64 ++++---- gopls/internal/lsp/source/view.go | 4 +- gopls/internal/vulncheck/command.go | 36 +++-- gopls/internal/vulncheck/vulncheck.go | 2 +- 17 files changed, 289 insertions(+), 477 deletions(-) delete mode 100644 gopls/internal/govulncheck/cache.go delete mode 100644 gopls/internal/govulncheck/cache_test.go delete mode 100755 gopls/internal/govulncheck/copy.sh delete mode 100644 gopls/internal/govulncheck/summary.go create mode 100644 gopls/internal/govulncheck/types_118.go create mode 100644 gopls/internal/govulncheck/types_not118.go diff --git a/gopls/go.mod b/gopls/go.mod index 979174c67c..a36bccdea1 100644 --- a/gopls/go.mod +++ b/gopls/go.mod @@ -11,8 +11,8 @@ require ( golang.org/x/sync v0.1.0 golang.org/x/sys v0.2.0 golang.org/x/text v0.4.0 - golang.org/x/tools v0.2.0 - golang.org/x/vuln v0.0.0-20221010193109-563322be2ea9 + golang.org/x/tools v0.2.1-0.20221108172846-9474ca31d0df + golang.org/x/vuln v0.0.0-20221109205719-3af8368ee4fe gopkg.in/yaml.v3 v3.0.1 honnef.co/go/tools v0.3.3 mvdan.cc/gofumpt v0.4.0 diff --git a/gopls/go.sum b/gopls/go.sum index 9810051995..ccc7756fb9 100644 --- a/gopls/go.sum +++ b/gopls/go.sum @@ -61,7 +61,6 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco= golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220819030929-7fc1605a5dde/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -80,8 +79,8 @@ golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= -golang.org/x/vuln v0.0.0-20221010193109-563322be2ea9 h1:KaYZQUtEEaV8aVADIHAuYBTjo77aUcCvC7KTGKM3J1I= -golang.org/x/vuln v0.0.0-20221010193109-563322be2ea9/go.mod h1:F12iebNzxRMpJsm4W7ape+r/KdnXiSy3VC94WsyCG68= +golang.org/x/vuln v0.0.0-20221109205719-3af8368ee4fe h1:qptQiQwEpETwDiz85LKtChqif9xhVkAm8Nhxs0xnTww= +golang.org/x/vuln v0.0.0-20221109205719-3af8368ee4fe/go.mod h1:8nFLBv8KFyZ2VuczUYssYKh+fcBR3BuXDG/HIWcxlwM= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/gopls/internal/govulncheck/cache.go b/gopls/internal/govulncheck/cache.go deleted file mode 100644 index 2fa6a05d72..0000000000 --- a/gopls/internal/govulncheck/cache.go +++ /dev/null @@ -1,145 +0,0 @@ -// Copyright 2021 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. - -//go:build go1.18 -// +build go1.18 - -// Package govulncheck supports the govulncheck command. -package govulncheck - -import ( - "encoding/json" - "go/build" - "os" - "path/filepath" - "sync" - "time" - - "golang.org/x/vuln/client" - "golang.org/x/vuln/osv" -) - -// The cache uses a single JSON index file for each vulnerability database -// which contains the map from packages to the time the last -// vulnerability for that package was added/modified and the time that -// the index was retrieved from the vulnerability database. The JSON -// format is as follows: -// -// $GOPATH/pkg/mod/cache/download/vulndb/{db hostname}/indexes/index.json -// { -// Retrieved time.Time -// Index client.DBIndex -// } -// -// Each package also has a JSON file which contains the array of vulnerability -// entries for the package. The JSON format is as follows: -// -// $GOPATH/pkg/mod/cache/download/vulndb/{db hostname}/{import path}/vulns.json -// []*osv.Entry - -// FSCache is a thread-safe file-system cache implementing osv.Cache -// -// TODO: use something like cmd/go/internal/lockedfile for thread safety? -type FSCache struct { - mu sync.Mutex - rootDir string -} - -// Assert that *FSCache implements client.Cache. -var _ client.Cache = (*FSCache)(nil) - -// use cfg.GOMODCACHE available in cmd/go/internal? -var defaultCacheRoot = filepath.Join(build.Default.GOPATH, "/pkg/mod/cache/download/vulndb") - -func DefaultCache() *FSCache { - return &FSCache{rootDir: defaultCacheRoot} -} - -type cachedIndex struct { - Retrieved time.Time - Index client.DBIndex -} - -func (c *FSCache) ReadIndex(dbName string) (client.DBIndex, time.Time, error) { - c.mu.Lock() - defer c.mu.Unlock() - - b, err := os.ReadFile(filepath.Join(c.rootDir, dbName, "index.json")) - if err != nil { - if os.IsNotExist(err) { - return nil, time.Time{}, nil - } - return nil, time.Time{}, err - } - var index cachedIndex - if err := json.Unmarshal(b, &index); err != nil { - return nil, time.Time{}, err - } - return index.Index, index.Retrieved, nil -} - -func (c *FSCache) WriteIndex(dbName string, index client.DBIndex, retrieved time.Time) error { - c.mu.Lock() - defer c.mu.Unlock() - - path := filepath.Join(c.rootDir, dbName) - if err := os.MkdirAll(path, 0755); err != nil { - return err - } - j, err := json.Marshal(cachedIndex{ - Index: index, - Retrieved: retrieved, - }) - if err != nil { - return err - } - if err := os.WriteFile(filepath.Join(path, "index.json"), j, 0666); err != nil { - return err - } - return nil -} - -func (c *FSCache) ReadEntries(dbName string, p string) ([]*osv.Entry, error) { - c.mu.Lock() - defer c.mu.Unlock() - - ep, err := client.EscapeModulePath(p) - if err != nil { - return nil, err - } - b, err := os.ReadFile(filepath.Join(c.rootDir, dbName, ep, "vulns.json")) - if err != nil { - if os.IsNotExist(err) { - return nil, nil - } - return nil, err - } - var entries []*osv.Entry - if err := json.Unmarshal(b, &entries); err != nil { - return nil, err - } - return entries, nil -} - -func (c *FSCache) WriteEntries(dbName string, p string, entries []*osv.Entry) error { - c.mu.Lock() - defer c.mu.Unlock() - - ep, err := client.EscapeModulePath(p) - if err != nil { - return err - } - path := filepath.Join(c.rootDir, dbName, ep) - if err := os.MkdirAll(path, 0777); err != nil { - return err - } - j, err := json.Marshal(entries) - if err != nil { - return err - } - if err := os.WriteFile(filepath.Join(path, "vulns.json"), j, 0666); err != nil { - return err - } - return nil -} diff --git a/gopls/internal/govulncheck/cache_test.go b/gopls/internal/govulncheck/cache_test.go deleted file mode 100644 index 57e8765904..0000000000 --- a/gopls/internal/govulncheck/cache_test.go +++ /dev/null @@ -1,165 +0,0 @@ -// Copyright 2021 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. - -//go:build go1.18 -// +build go1.18 - -package govulncheck - -import ( - "fmt" - "os" - "path/filepath" - "reflect" - "testing" - "time" - - "golang.org/x/sync/errgroup" - "golang.org/x/vuln/client" - "golang.org/x/vuln/osv" -) - -func TestCache(t *testing.T) { - tmpDir := t.TempDir() - - cache := &FSCache{rootDir: tmpDir} - dbName := "vulndb.golang.org" - - _, _, err := cache.ReadIndex(dbName) - if err != nil { - t.Fatalf("ReadIndex failed for non-existent database: %v", err) - } - - if err = os.Mkdir(filepath.Join(tmpDir, dbName), 0777); err != nil { - t.Fatalf("os.Mkdir failed: %v", err) - } - _, _, err = cache.ReadIndex(dbName) - if err != nil { - t.Fatalf("ReadIndex failed for database without cached index: %v", err) - } - - now := time.Now() - expectedIdx := client.DBIndex{ - "a.vuln.example.com": time.Time{}.Add(time.Hour), - "b.vuln.example.com": time.Time{}.Add(time.Hour * 2), - "c.vuln.example.com": time.Time{}.Add(time.Hour * 3), - } - if err = cache.WriteIndex(dbName, expectedIdx, now); err != nil { - t.Fatalf("WriteIndex failed to write index: %v", err) - } - - idx, retrieved, err := cache.ReadIndex(dbName) - if err != nil { - t.Fatalf("ReadIndex failed for database with cached index: %v", err) - } - if !reflect.DeepEqual(idx, expectedIdx) { - t.Errorf("ReadIndex returned unexpected index, got:\n%s\nwant:\n%s", idx, expectedIdx) - } - if !retrieved.Equal(now) { - t.Errorf("ReadIndex returned unexpected retrieved: got %s, want %s", retrieved, now) - } - - if _, err = cache.ReadEntries(dbName, "vuln.example.com"); err != nil { - t.Fatalf("ReadEntires failed for non-existent package: %v", err) - } - - expectedEntries := []*osv.Entry{ - {ID: "001"}, - {ID: "002"}, - {ID: "003"}, - } - if err := cache.WriteEntries(dbName, "vuln.example.com", expectedEntries); err != nil { - t.Fatalf("WriteEntries failed: %v", err) - } - - entries, err := cache.ReadEntries(dbName, "vuln.example.com") - if err != nil { - t.Fatalf("ReadEntries failed for cached package: %v", err) - } - if !reflect.DeepEqual(entries, expectedEntries) { - t.Errorf("ReadEntries returned unexpected entries, got:\n%v\nwant:\n%v", entries, expectedEntries) - } -} - -func TestConcurrency(t *testing.T) { - tmpDir := t.TempDir() - - cache := &FSCache{rootDir: tmpDir} - dbName := "vulndb.golang.org" - - g := new(errgroup.Group) - for i := 0; i < 1000; i++ { - i := i - g.Go(func() error { - id := i % 5 - p := fmt.Sprintf("example.com/package%d", id) - - entries, err := cache.ReadEntries(dbName, p) - if err != nil { - return err - } - - err = cache.WriteEntries(dbName, p, append(entries, &osv.Entry{ID: fmt.Sprint(id)})) - if err != nil { - return err - } - return nil - }) - } - - if err := g.Wait(); err != nil { - t.Errorf("error in parallel cache entries read/write: %v", err) - } - - // sanity checking - for i := 0; i < 5; i++ { - id := fmt.Sprint(i) - p := fmt.Sprintf("example.com/package%s", id) - - es, err := cache.ReadEntries(dbName, p) - if err != nil { - t.Fatalf("failed to read entries: %v", err) - } - for _, e := range es { - if e.ID != id { - t.Errorf("want %s ID for vuln entry; got %s", id, e.ID) - } - } - } - - // do similar for cache index - start := time.Now() - for i := 0; i < 1000; i++ { - i := i - g.Go(func() error { - id := i % 5 - p := fmt.Sprintf("package%v", id) - - idx, _, err := cache.ReadIndex(dbName) - if err != nil { - return err - } - - if idx == nil { - idx = client.DBIndex{} - } - - // sanity checking - if rt, ok := idx[p]; ok && rt.Before(start) { - return fmt.Errorf("unexpected past time in index: %v before start %v", rt, start) - } - - now := time.Now() - idx[p] = now - if err := cache.WriteIndex(dbName, idx, now); err != nil { - return err - } - return nil - }) - } - - if err := g.Wait(); err != nil { - t.Errorf("error in parallel cache index read/write: %v", err) - } -} diff --git a/gopls/internal/govulncheck/copy.sh b/gopls/internal/govulncheck/copy.sh deleted file mode 100755 index 398cde2f46..0000000000 --- a/gopls/internal/govulncheck/copy.sh +++ /dev/null @@ -1,32 +0,0 @@ -#!/bin/bash -eu - -# Copyright 2020 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. - -set -o pipefail - -# Copy golang.org/x/vuln/cmd/govulncheck/internal/govulncheck into this directory. -# Assume the x/vuln repo is a sibling of the tools repo. - -rm -f *.go -cp ../../../../vuln/cmd/govulncheck/internal/govulncheck/*.go . - -sed -i '' 's/\"golang.org\/x\/vuln\/internal\/semver\"/\"golang.org\/x\/tools\/gopls\/internal\/govulncheck\/semver\"/g' *.go -sed -i '' -e '4 i\ -' -e '4 i\ -//go:build go1.18' -e '4 i\ -// +build go1.18' *.go - -# Copy golang.org/x/vuln/internal/semver that -# golang.org/x/vuln/cmd/govulncheck/internal/govulncheck -# depends on. - -mkdir -p semver -cd semver -rm -f *.go -cp ../../../../../vuln/internal/semver/*.go . -sed -i '' -e '4 i\ -' -e '4 i\ -//go:build go1.18' -e '4 i\ -// +build go1.18' *.go diff --git a/gopls/internal/govulncheck/summary.go b/gopls/internal/govulncheck/summary.go deleted file mode 100644 index e389b89a4a..0000000000 --- a/gopls/internal/govulncheck/summary.go +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2020 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. - -package govulncheck - -import "golang.org/x/vuln/osv" - -// TODO(hyangah): Find a better package for these types -// unless golang.org/x/vuln/exp/govulncheck starts to export these. - -// Summary is the govulncheck result. -type Summary struct { - // Vulnerabilities affecting the analysis target binary or source code. - Affecting []Vuln - // Vulnerabilities that may be imported but the vulnerable symbols are - // not called. For binary analysis, this will be always empty. - NonAffecting []Vuln -} - -// Vuln represents a vulnerability relevant to a (module, package). -type Vuln struct { - OSV *osv.Entry - PkgPath string // Package path. - ModPath string // Module path. - FoundIn string // @ if we know when it was introduced. Empty otherwise. - FixedIn string // @ if fix is available. Empty otherwise. - // Trace contains a call stack for each affecting symbol. - // For vulnerabilities found from binary analysis, and vulnerabilities - // that are reported as Unaffecting ones, this will be always empty. - Trace []Trace -} - -// Trace represents a sample trace for a vulnerable symbol. -type Trace struct { - Symbol string // Name of the detected vulnerable function or method. - Desc string // One-line description of the callstack. - Stack []StackEntry // Call stack. - Seen int // Number of similar call stacks. -} - -// StackEntry represents a call stack entry. -type StackEntry struct { - FuncName string // Function name is the function name, adjusted to remove pointer annotation. - CallSite string // Position of the call/reference site. It is one of the formats token.Pos.String() returns or empty if unknown. -} diff --git a/gopls/internal/govulncheck/types_118.go b/gopls/internal/govulncheck/types_118.go new file mode 100644 index 0000000000..7410963195 --- /dev/null +++ b/gopls/internal/govulncheck/types_118.go @@ -0,0 +1,44 @@ +// 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. + +//go:build go1.18 +// +build go1.18 + +// Package govulncheck provides an experimental govulncheck API. +package govulncheck + +import "golang.org/x/vuln/exp/govulncheck" + +var ( + // Source reports vulnerabilities that affect the analyzed packages. + Source = govulncheck.Source + + // DefaultCache constructs cache for a vulnerability database client. + DefaultCache = govulncheck.DefaultCache +) + +type ( + // Config is the configuration for Main. + Config = govulncheck.Config + + // Result is the result of executing Source. + Result = govulncheck.Result + + // Vuln represents a single OSV entry. + Vuln = govulncheck.Vuln + + // Module represents a specific vulnerability relevant to a + // single module or package. + Module = govulncheck.Module + + // Package is a Go package with known vulnerable symbols. + Package = govulncheck.Package + + // CallStacks contains a representative call stack for each + // vulnerable symbol that is called. + CallStack = govulncheck.CallStack + + // StackFrame represents a call stack entry. + StackFrame = govulncheck.StackFrame +) diff --git a/gopls/internal/govulncheck/types_not118.go b/gopls/internal/govulncheck/types_not118.go new file mode 100644 index 0000000000..ae92caa5ea --- /dev/null +++ b/gopls/internal/govulncheck/types_not118.go @@ -0,0 +1,133 @@ +// 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. + +//go:build !go1.18 +// +build !go1.18 + +package govulncheck + +import ( + "go/token" + + "golang.org/x/vuln/osv" +) + +// Result is the result of executing Source or Binary. +type Result struct { + // Vulns contains all vulnerabilities that are called or imported by + // the analyzed module. + Vulns []*Vuln +} + +// Vuln represents a single OSV entry. +type Vuln struct { + // OSV contains all data from the OSV entry for this vulnerability. + OSV *osv.Entry + + // Modules contains all of the modules in the OSV entry where a + // vulnerable package is imported by the target source code or binary. + // + // For example, a module M with two packages M/p1 and M/p2, where only p1 + // is vulnerable, will appear in this list if and only if p1 is imported by + // the target source code or binary. + Modules []*Module +} + +func (v *Vuln) IsCalled() bool { + return false +} + +// Module represents a specific vulnerability relevant to a single module. +type Module struct { + // Path is the module path of the module containing the vulnerability. + // + // Importable packages in the standard library will have the path "stdlib". + Path string + + // FoundVersion is the module version where the vulnerability was found. + FoundVersion string + + // FixedVersion is the module version where the vulnerability was + // fixed. If there are multiple fixed versions in the OSV report, this will + // be the latest fixed version. + // + // This is empty if a fix is not available. + FixedVersion string + + // Packages contains all the vulnerable packages in OSV entry that are + // imported by the target source code or binary. + // + // For example, given a module M with two packages M/p1 and M/p2, where + // both p1 and p2 are vulnerable, p1 and p2 will each only appear in this + // list they are individually imported by the target source code or binary. + Packages []*Package +} + +// Package is a Go package with known vulnerable symbols. +type Package struct { + // Path is the import path of the package containing the vulnerability. + Path string + + // CallStacks contains a representative call stack for each + // vulnerable symbol that is called. + // + // For vulnerabilities found from binary analysis, only CallStack.Symbol + // will be provided. + // + // For non-affecting vulnerabilities reported from the source mode + // analysis, this will be empty. + CallStacks []CallStack +} + +// CallStacks contains a representative call stack for a vulnerable +// symbol. +type CallStack struct { + // Symbol is the name of the detected vulnerable function + // or method. + // + // This follows the naming convention in the OSV report. + Symbol string + + // Summary is a one-line description of the callstack, used by the + // default govulncheck mode. + // + // Example: module3.main calls github.com/shiyanhui/dht.DHT.Run + Summary string + + // Frames contains an entry for each stack in the call stack. + // + // Frames are sorted starting from the entry point to the + // imported vulnerable symbol. The last frame in Frames should match + // Symbol. + Frames []*StackFrame +} + +// StackFrame represents a call stack entry. +type StackFrame struct { + // PackagePath is the import path. + PkgPath string + + // FuncName is the function name. + FuncName string + + // RecvType is the fully qualified receiver type, + // if the called symbol is a method. + // + // The client can create the final symbol name by + // prepending RecvType to FuncName. + RecvType string + + // Position describes an arbitrary source position + // including the file, line, and column location. + // A Position is valid if the line number is > 0. + Position token.Position +} + +func (sf *StackFrame) Name() string { + return "" +} + +func (sf *StackFrame) Pos() string { + return "" +} diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go index 57bba47df8..f2651a75a8 100644 --- a/gopls/internal/lsp/cache/session.go +++ b/gopls/internal/lsp/cache/session.go @@ -239,7 +239,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, name: name, folder: folder, moduleUpgrades: map[span.URI]map[string]string{}, - vulns: map[span.URI][]govulncheck.Vuln{}, + vulns: map[span.URI][]*govulncheck.Vuln{}, filesByURI: map[span.URI]*fileBase{}, filesByBase: map[string][]*fileBase{}, rootURI: root, diff --git a/gopls/internal/lsp/cache/view.go b/gopls/internal/lsp/cache/view.go index a408ee7a03..5974fab726 100644 --- a/gopls/internal/lsp/cache/view.go +++ b/gopls/internal/lsp/cache/view.go @@ -61,7 +61,7 @@ type View struct { // Each modfile has a map of module name to upgrade version. moduleUpgrades map[span.URI]map[string]string - vulns map[span.URI][]govulncheck.Vuln + vulns map[span.URI][]*govulncheck.Vuln // keep track of files by uri and by basename, a single file may be mapped // to multiple uris, and the same basename may map to multiple files @@ -1035,16 +1035,16 @@ func (v *View) ClearModuleUpgrades(modfile span.URI) { delete(v.moduleUpgrades, modfile) } -func (v *View) Vulnerabilities(modfile span.URI) []govulncheck.Vuln { +func (v *View) Vulnerabilities(modfile span.URI) []*govulncheck.Vuln { v.mu.Lock() defer v.mu.Unlock() - vulns := make([]govulncheck.Vuln, len(v.vulns[modfile])) + vulns := make([]*govulncheck.Vuln, len(v.vulns[modfile])) copy(vulns, v.vulns[modfile]) return vulns } -func (v *View) SetVulnerabilities(modfile span.URI, vulns []govulncheck.Vuln) { +func (v *View) SetVulnerabilities(modfile span.URI, vulns []*govulncheck.Vuln) { v.mu.Lock() defer v.mu.Unlock() diff --git a/gopls/internal/lsp/cmd/vulncheck.go b/gopls/internal/lsp/cmd/vulncheck.go index 93b9c106ae..c8f0358d39 100644 --- a/gopls/internal/lsp/cmd/vulncheck.go +++ b/gopls/internal/lsp/cmd/vulncheck.go @@ -82,9 +82,11 @@ func (v *vulncheck) Run(ctx context.Context, args ...string) error { } if v.AsSummary { - // vulnchecklib.Main calls os.Exit and never returns. - vulnchecklib.Main(loadCfg, args...) - return nil + if err := vulnchecklib.Main(loadCfg, args...); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) } // TODO(hyangah): delete. res, err := vulnchecklib.Govulncheck(ctx, &loadCfg, pattern) diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go index e231b4bba4..9722b7864e 100644 --- a/gopls/internal/lsp/command.go +++ b/gopls/internal/lsp/command.go @@ -880,34 +880,31 @@ func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.Vulnc return fmt.Errorf("failed to run govulncheck: %v", err) } - var summary govulncheck.Summary - if err := json.Unmarshal(stdout, &summary); err != nil { + var result govulncheck.Result + if err := json.Unmarshal(stdout, &result); err != nil { // TODO: for easy debugging, log the failed stdout somewhere? return fmt.Errorf("failed to parse govulncheck output: %v", err) } - - vulns := append(summary.Affecting, summary.NonAffecting...) + vulns := result.Vulns deps.snapshot.View().SetVulnerabilities(args.URI.SpanURI(), vulns) c.s.diagnoseSnapshot(deps.snapshot, nil, false) - if len(summary.Affecting) == 0 { + affecting := make([]string, 0, len(vulns)) + for _, v := range vulns { + if v.IsCalled() { + affecting = append(affecting, v.OSV.ID) + } + } + if len(affecting) == 0 { return c.s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ Type: protocol.Info, Message: "No vulnerabilities found", }) } - set := make(map[string]bool) - for _, v := range summary.Affecting { - set[v.OSV.ID] = true - } - list := make([]string, 0, len(set)) - for k := range set { - list = append(list, k) - } - sort.Strings(list) + sort.Strings(affecting) return c.s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ Type: protocol.Warning, - Message: fmt.Sprintf("Found %v", strings.Join(list, ", ")), + Message: fmt.Sprintf("Found %v", strings.Join(affecting, ", ")), }) }) return err diff --git a/gopls/internal/lsp/mod/diagnostics.go b/gopls/internal/lsp/mod/diagnostics.go index 770fb3729f..fbbd34e278 100644 --- a/gopls/internal/lsp/mod/diagnostics.go +++ b/gopls/internal/lsp/mod/diagnostics.go @@ -197,13 +197,19 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, vs := snapshot.View().Vulnerabilities(fh.URI()) // TODO(suzmue): should we just store the vulnerabilities like this? - affecting := make(map[string][]govulncheck.Vuln) - nonaffecting := make(map[string][]govulncheck.Vuln) + type modVuln struct { + mod *govulncheck.Module + vuln *govulncheck.Vuln + } + affecting := make(map[string][]modVuln) + nonaffecting := make(map[string][]modVuln) for _, v := range vs { - if len(v.Trace) > 0 { - affecting[v.ModPath] = append(affecting[v.ModPath], v) - } else { - nonaffecting[v.ModPath] = append(nonaffecting[v.ModPath], v) + for _, m := range v.Modules { + if v.IsCalled() { + affecting[m.Path] = append(affecting[m.Path], modVuln{mod: m, vuln: v}) + } else { + nonaffecting[m.Path] = append(nonaffecting[m.Path], modVuln{mod: m, vuln: v}) + } } } @@ -222,25 +228,30 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, // Fixes will include only the upgrades for warning level diagnostics. var fixes []source.SuggestedFix var warning, info []string - for _, v := range nonaffectingVulns { + for _, mv := range nonaffectingVulns { + mod, vuln := mv.mod, mv.vuln // Only show the diagnostic if the vulnerability was calculated // for the module at the current version. - if semver.IsValid(v.FoundIn) && semver.Compare(req.Mod.Version, v.FoundIn) != 0 { + // TODO(hyangah): this prevents from surfacing vulnerable modules + // since module version selection is affected by dependency module + // requirements and replace/exclude/go.work use. Drop this check + // and annotate only the module path. + if semver.IsValid(mod.FoundVersion) && semver.Compare(req.Mod.Version, mod.FoundVersion) != 0 { continue } - info = append(info, v.OSV.ID) + info = append(info, vuln.OSV.ID) } - for _, v := range affectingVulns { + for _, mv := range affectingVulns { + mod, vuln := mv.mod, mv.vuln // Only show the diagnostic if the vulnerability was calculated // for the module at the current version. - if semver.IsValid(v.FoundIn) && semver.Compare(req.Mod.Version, v.FoundIn) != 0 { + if semver.IsValid(mod.FoundVersion) && semver.Compare(req.Mod.Version, mod.FoundVersion) != 0 { continue } - warning = append(warning, v.OSV.ID) + warning = append(warning, vuln.OSV.ID) // Upgrade to the exact version we offer the user, not the most recent. // TODO(hakim): Produce fixes only for affecting vulnerabilities (if len(v.Trace) > 0) - - if fixedVersion := v.FixedIn; semver.IsValid(fixedVersion) && semver.Compare(req.Mod.Version, fixedVersion) < 0 { + if fixedVersion := mod.FixedVersion; semver.IsValid(fixedVersion) && semver.Compare(req.Mod.Version, fixedVersion) < 0 { cmd, err := getUpgradeCodeAction(fh, req, fixedVersion) if err != nil { return nil, err @@ -297,7 +308,7 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, return vulnDiagnostics, nil } -func formatMessage(v govulncheck.Vuln) string { +func formatMessage(v *govulncheck.Vuln) string { details := []byte(v.OSV.Details) // Remove any new lines that are not preceded or followed by a new line. for i, r := range details { diff --git a/gopls/internal/lsp/mod/hover.go b/gopls/internal/lsp/mod/hover.go index 5d5b615821..547f25270b 100644 --- a/gopls/internal/lsp/mod/hover.go +++ b/gopls/internal/lsp/mod/hover.go @@ -71,7 +71,7 @@ func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, } // Get the vulnerability info. - affecting, nonaffecting := lookupVulns(snapshot.View().Vulnerabilities(fh.URI()), req) + affecting, nonaffecting := lookupVulns(snapshot.View().Vulnerabilities(fh.URI()), req.Mod.Path) // Get the `go mod why` results for the given file. why, err := snapshot.ModWhy(ctx, fh) @@ -94,7 +94,7 @@ func Hover(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, isPrivate := snapshot.View().IsGoPrivatePath(req.Mod.Path) header := formatHeader(req.Mod.Path, options) explanation = formatExplanation(explanation, req, options, isPrivate) - vulns := formatVulnerabilities(affecting, nonaffecting, options) + vulns := formatVulnerabilities(req.Mod.Path, affecting, nonaffecting, options) return &protocol.Hover{ Contents: protocol.MarkupContent{ @@ -117,33 +117,25 @@ func formatHeader(modpath string, options *source.Options) string { return b.String() } -func compareVuln(i, j govulncheck.Vuln) bool { - if i.OSV.ID == j.OSV.ID { - return i.PkgPath < j.PkgPath - } - return i.OSV.ID < j.OSV.ID -} - -func lookupVulns(vulns []govulncheck.Vuln, req *modfile.Require) (affecting, nonaffecting []govulncheck.Vuln) { - modpath, modversion := req.Mod.Path, req.Mod.Version - - var info, warning []govulncheck.Vuln +func lookupVulns(vulns []*govulncheck.Vuln, modpath string) (affecting, nonaffecting []*govulncheck.Vuln) { for _, vuln := range vulns { - if vuln.ModPath != modpath || vuln.FoundIn != modversion { - continue - } - if len(vuln.Trace) == 0 { - info = append(info, vuln) - } else { - warning = append(warning, vuln) + for _, mod := range vuln.Modules { + if mod.Path != modpath { + continue + } + if vuln.IsCalled() { + affecting = append(affecting, vuln) + } else { + nonaffecting = append(nonaffecting, vuln) + } } } - sort.Slice(info, func(i, j int) bool { return compareVuln(info[i], info[j]) }) - sort.Slice(warning, func(i, j int) bool { return compareVuln(warning[i], warning[j]) }) - return warning, info + sort.Slice(nonaffecting, func(i, j int) bool { return nonaffecting[i].OSV.ID < nonaffecting[j].OSV.ID }) + sort.Slice(affecting, func(i, j int) bool { return affecting[i].OSV.ID < affecting[j].OSV.ID }) + return affecting, nonaffecting } -func formatVulnerabilities(affecting, nonaffecting []govulncheck.Vuln, options *source.Options) string { +func formatVulnerabilities(modPath string, affecting, nonaffecting []*govulncheck.Vuln, options *source.Options) string { if len(affecting) == 0 && len(nonaffecting) == 0 { return "" } @@ -163,10 +155,7 @@ func formatVulnerabilities(affecting, nonaffecting []govulncheck.Vuln, options * } } for _, v := range affecting { - fix := "No fix is available." - if v.FixedIn != "" { - fix = "Fixed in " + v.FixedIn + "." - } + fix := fixedVersionInfo(v, modPath) if useMarkdown { fmt.Fprintf(&b, "- [**%v**](%v) %v %v\n", v.OSV.ID, href(v.OSV), formatMessage(v), fix) @@ -178,10 +167,7 @@ func formatVulnerabilities(affecting, nonaffecting []govulncheck.Vuln, options * fmt.Fprintf(&b, "\n**FYI:** The project imports packages with known vulnerabilities, but does not call the vulnerable code.\n") } for _, v := range nonaffecting { - fix := "No fix is available." - if v.FixedIn != "" { - fix = "Fixed in " + v.FixedIn + "." - } + fix := fixedVersionInfo(v, modPath) if useMarkdown { fmt.Fprintf(&b, "- [%v](%v) %v %v\n", v.OSV.ID, href(v.OSV), formatMessage(v), fix) } else { @@ -192,6 +178,20 @@ func formatVulnerabilities(affecting, nonaffecting []govulncheck.Vuln, options * return b.String() } +func fixedVersionInfo(v *govulncheck.Vuln, modPath string) string { + fix := "No fix is available." + for _, m := range v.Modules { + if m.Path != modPath { + continue + } + if m.FixedVersion != "" { + fix = "Fixed in " + m.FixedVersion + "." + } + break + } + return fix +} + func formatExplanation(text string, req *modfile.Require, options *source.Options, isPrivate bool) string { text = strings.TrimSuffix(text, "\n") splt := strings.Split(text, "\n") diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go index 25fa7d704d..54c942f1d3 100644 --- a/gopls/internal/lsp/source/view.go +++ b/gopls/internal/lsp/source/view.go @@ -278,11 +278,11 @@ type View interface { // Vulnerabilites returns known vulnerabilities for the given modfile. // TODO(suzmue): replace command.Vuln with a different type, maybe // https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck/govulnchecklib#Summary? - Vulnerabilities(modfile span.URI) []govulncheck.Vuln + Vulnerabilities(modfile span.URI) []*govulncheck.Vuln // SetVulnerabilities resets the list of vulnerabilites that exists for the given modules // required by modfile. - SetVulnerabilities(modfile span.URI, vulnerabilities []govulncheck.Vuln) + SetVulnerabilities(modfile span.URI, vulnerabilities []*govulncheck.Vuln) // FileKind returns the type of a file FileKind(FileHandle) FileKind diff --git a/gopls/internal/vulncheck/command.go b/gopls/internal/vulncheck/command.go index e607eb415e..510dd84496 100644 --- a/gopls/internal/vulncheck/command.go +++ b/gopls/internal/vulncheck/command.go @@ -9,6 +9,7 @@ package vulncheck import ( "context" + "encoding/json" "fmt" "log" "os" @@ -216,19 +217,32 @@ func trimPosPrefix(summary string) string { const GoVersionForVulnTest = "_GOPLS_TEST_VULNCHECK_GOVERSION" func init() { - Main = func(cfg packages.Config, patterns ...string) { - // never return - err := gvcapi.Main(gvcapi.Config{ - AnalysisType: "source", - OutputType: "summary", - Patterns: patterns, - SourceLoadConfig: &cfg, - GoVersion: os.Getenv(GoVersionForVulnTest), + Main = func(cfg packages.Config, patterns ...string) error { + // Set the mode that Source needs. + cfg.Mode = packages.NeedName | packages.NeedImports | packages.NeedTypes | + packages.NeedSyntax | packages.NeedTypesInfo | packages.NeedDeps | + packages.NeedModule + + pkgs, err := packages.Load(&cfg, patterns...) + if err != nil { + return err + } + cli, err := client.NewClient(findGOVULNDB(&cfg), client.Options{ + HTTPCache: gvc.DefaultCache(), }) if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) + return err } - os.Exit(0) + res, err := gvcapi.Source(context.Background(), &gvcapi.Config{ + Client: cli, + GoVersion: os.Getenv(GoVersionForVulnTest), + }, vulncheck.Convert(pkgs)) + if err != nil { + return err + } + if err := json.NewEncoder(os.Stdout).Encode(res); err != nil { + return err + } + return nil } } diff --git a/gopls/internal/vulncheck/vulncheck.go b/gopls/internal/vulncheck/vulncheck.go index 7167ec1c26..1cbf18e86d 100644 --- a/gopls/internal/vulncheck/vulncheck.go +++ b/gopls/internal/vulncheck/vulncheck.go @@ -19,4 +19,4 @@ import ( // With go1.18+, this is swapped with the real implementation. var Govulncheck func(ctx context.Context, cfg *packages.Config, patterns string) (res command.VulncheckResult, _ error) = nil -var Main func(cfg packages.Config, patterns ...string) = nil +var Main func(cfg packages.Config, patterns ...string) error = nil