internal/lsp/command: let RunVulncheckExp call gopls vulncheck

By making gopls.run_vulncheck_exp (RunVulncheckExp implements)
call `gopls vulncheck`, we achieve

  - gopls.run_vulncheck_exp can run asynchronously and be cancellable
  - log information can be forwarded as progress messages
  - isolate any failures during vulncheck execution

In this CL, we also changed not to include test files in the analysis
(match the default of govulncheck). We will add an option in the future.

TODO:
 - prevent concurrent gopls.run_vulncheck_exp
 - convert the gopls vulncheck output to diagnostics and publish it
 - remove timestamps from the `gopls vulncheck` log messages
   for simplify progress messages
 - add test to check vulnerability in third-party dependencies

Change-Id: I21592e03794cd9e9d96ed3989973a2ab7d75c538
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420717
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>

9 files changed, 123 insertions, 44 deletions

diff --git a/gopls/doc/commands.md b/gopls/doc/commands.md
index f868a4893..37d8dcbdc 100644
--- a/gopls/doc/commands.md
+++ b/gopls/doc/commands.md
@@ -281,26 +281,6 @@ Args:
 }
 ```
 
-Result:
-
-```
-{
-	"Vuln": []{
-		"ID": string,
-		"Details": string,
-		"Aliases": []string,
-		"Symbol": string,
-		"PkgPath": string,
-		"ModPath": string,
-		"URL": string,
-		"CurrentVersion": string,
-		"FixedVersion": string,
-		"CallStacks": [][]golang.org/x/tools/internal/lsp/command.StackEntry,
-		"CallStackSummaries": []string,
-	},
-}
-```
-
 ### **Start the gopls debug server**
 Identifier: `gopls.start_debugging`
 
diff --git a/gopls/internal/regtest/misc/testdata/vulndb/golang.org/x/crypto.json b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/x/crypto.json
new file mode 100644
index 000000000..e0a279129
--- /dev/null
+++ b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/x/crypto.json
@@ -0,0 +1 @@
+[{"id":"GO-2020-0012","published":"2021-04-14T20:04:52Z","modified":"2021-04-14T20:04:52Z","aliases":["CVE-2020-9283"],"details":"An attacker can craft an ssh-ed25519 or sk-ssh-ed25519@openssh.com public\nkey, such that the library will panic when trying to verify a signature\nwith it. If verifying signatures using user supplied public keys, this\nmay be used as a denial of service vector.\n","affected":[{"package":{"name":"golang.org/x/crypto/ssh","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.0.0-20200220183623-bac4c82f6975"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2020-0012"},"ecosystem_specific":{"symbols":["parseED25519","ed25519PublicKey.Verify","parseSKEd25519","skEd25519PublicKey.Verify","NewPublicKey"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/c/crypto/+/220357"},{"type":"FIX","url":"https://go.googlesource.com/crypto/+/bac4c82f69751a6dd76e702d54b3ceb88adab236"},{"type":"WEB","url":"https://groups.google.com/g/golang-announce/c/3L45YRc91SY"}]},{"id":"GO-2020-0013","published":"2021-04-14T20:04:52Z","modified":"2021-04-14T20:04:52Z","aliases":["CVE-2017-3204"],"details":"By default host key verification is disabled which allows for\nman-in-the-middle attacks against SSH clients if\nClientConfig.HostKeyCallback is not set.\n","affected":[{"package":{"name":"golang.org/x/crypto/ssh","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.0.0-20170330155735-e4e2799dd7aa"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2020-0013"},"ecosystem_specific":{"symbols":["NewClientConn"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/38701"},{"type":"FIX","url":"https://go.googlesource.com/crypto/+/e4e2799dd7aab89f583e1d898300d96367750991"},{"type":"WEB","url":"https://go.dev/issue/19767"},{"type":"WEB","url":"https://bridge.grumpy-troll.org/2017/04/golang-ssh-security/"}]},{"id":"GO-2021-0227","published":"2022-02-17T17:35:32Z","modified":"2022-02-17T17:35:32Z","aliases":["CVE-2020-29652"],"details":"Clients can cause a panic in SSH servers. An attacker can craft\nan authentication request message for the “gssapi-with-mic” method\nwhich will cause NewServerConn to panic via a nil pointer dereference\nif ServerConfig.GSSAPIWithMICConfig is nil.\n","affected":[{"package":{"name":"golang.org/x/crypto/ssh","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.0.0-20201216223049-8b5274cf687f"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2021-0227"},"ecosystem_specific":{"symbols":["connection.serverAuthenticate"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/c/crypto/+/278852"},{"type":"FIX","url":"https://go.googlesource.com/crypto/+/8b5274cf687fd9316b4108863654cc57385531e8"},{"type":"WEB","url":"https://groups.google.com/g/golang-announce/c/ouZIlBimOsE?pli=1"}]}]
\ No newline at end of file
diff --git a/gopls/internal/regtest/misc/testdata/vulndb/golang.org/x/text.json b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/x/text.json
new file mode 100644
index 000000000..eee052ffc
--- /dev/null
+++ b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/x/text.json
@@ -0,0 +1 @@
+[{"id":"GO-2020-0015","published":"2021-04-14T20:04:52Z","modified":"2021-06-07T12:00:00Z","aliases":["CVE-2020-14040"],"details":"An attacker could provide a single byte to a UTF16 decoder instantiated with\nUseBOM or ExpectBOM to trigger an infinite loop if the String function on\nthe Decoder is called, or the Decoder is passed to transform.String.\nIf used to parse user supplied input, this may be used as a denial of service\nvector.\n","affected":[{"package":{"name":"golang.org/x/text/encoding/unicode","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.3.3"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2020-0015"},"ecosystem_specific":{"symbols":["utf16Decoder.Transform","bomOverride.Transform"]}},{"package":{"name":"golang.org/x/text/transform","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.3.3"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2020-0015"},"ecosystem_specific":{"symbols":["Transform"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/c/text/+/238238"},{"type":"FIX","url":"https://go.googlesource.com/text/+/23ae387dee1f90d29a23c0e87ee0b46038fbed0e"},{"type":"WEB","url":"https://go.dev/issue/39491"},{"type":"WEB","url":"https://groups.google.com/g/golang-announce/c/bXVeAmGOqz0"}]},{"id":"GO-2021-0113","published":"2021-10-06T17:51:21Z","modified":"2021-10-06T17:51:21Z","aliases":["CVE-2021-38561"],"details":"Due to improper index calculation, an incorrectly formatted language tag can cause Parse\nto panic via an out of bounds read. If Parse is used to process untrusted user inputs,\nthis may be used as a vector for a denial of service attack.\n","affected":[{"package":{"name":"golang.org/x/text/language","ecosystem":"Go"},"ranges":[{"type":"SEMVER","events":[{"introduced":"0"},{"fixed":"0.3.7"}]}],"database_specific":{"url":"https://pkg.go.dev/vuln/GO-2021-0113"},"ecosystem_specific":{"symbols":["Parse","MatchStrings","MustParse","ParseAcceptLanguage"]}}],"references":[{"type":"FIX","url":"https://go-review.googlesource.com/c/text/+/340830"},{"type":"FIX","url":"https://go.googlesource.com/text/+/383b2e75a7a4198c42f8f87833eefb772868a56f"}]}]
\ No newline at end of file
diff --git a/gopls/internal/regtest/misc/testdata/vulndb/stdlib.json b/gopls/internal/regtest/misc/testdata/vulndb/stdlib.json
new file mode 100644
index 000000000..240ee4943
--- /dev/null
+++ b/gopls/internal/regtest/misc/testdata/vulndb/stdlib.json
@@ -0,0 +1 @@
+[{"id":"STD","affected":[{"package":{"name":"archive/zip"},"ranges":[{"type":"SEMVER","events":[{"introduced":"1.18.0"}]}],"ecosystem_specific":{"symbols":["OpenReader"]}}]}]
diff --git a/gopls/internal/regtest/misc/vuln_test.go b/gopls/internal/regtest/misc/vuln_test.go
index 94fde715c..d39fd74ef 100644
--- a/gopls/internal/regtest/misc/vuln_test.go
+++ b/gopls/internal/regtest/misc/vuln_test.go
@@ -5,11 +5,14 @@
 package misc
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
 	"golang.org/x/tools/internal/lsp/command"
 	"golang.org/x/tools/internal/lsp/protocol"
 	. "golang.org/x/tools/internal/lsp/regtest"
+	"golang.org/x/tools/internal/testenv"
 )
 
 func TestRunVulncheckExpError(t *testing.T) {
@@ -41,3 +44,56 @@ package foo
 		}
 	})
 }
+
+func TestRunVulncheckExp(t *testing.T) {
+	testenv.NeedsGo1Point(t, 18)
+	const files = `
+-- go.mod --
+module mod.com
+
+go 1.18
+-- main.go --
+package main
+
+import (
+        "archive/zip"
+        "fmt"
+)
+
+func main() {
+        _, err := zip.OpenReader("file.zip")  // vulnerable.
+        fmt.Println(err)
+}
+`
+
+	cwd, _ := os.Getwd()
+	WithOptions(
+		EnvVars{
+			// Let the analyzer read vulnerabilities data from the testdata/vulndb.
+			"GOVULNDB": "file://" + filepath.Join(cwd, "testdata", "vulndb"),
+			// When fetchinging stdlib package vulnerability info,
+			// behave as if our go version is go1.18 for this testing.
+			// The default behavior is to run `go env GOVERSION` (which isn't mutable env var).
+			// See gopls/internal/vulncheck.goVersion
+			// which follows the convention used in golang.org/x/vuln/cmd/govulncheck.
+			"GOVERSION":                       "go1.18",
+			"_GOPLS_TEST_BINARY_RUN_AS_GOPLS": "true",
+		},
+	).Run(t, files, func(t *testing.T, env *Env) {
+		cmd, err := command.NewRunVulncheckExpCommand("Run Vulncheck Exp", command.VulncheckArgs{
+			Dir: env.Sandbox.Workdir.RootURI(),
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		env.ExecuteCommand(&protocol.ExecuteCommandParams{
+			Command:   command.RunVulncheckExp.ID(),
+			Arguments: cmd.Arguments,
+		}, nil)
+		env.Await(
+			CompletedWork("Checking vulnerability", 1, true),
+			// TODO(hyangah): once the diagnostics are published, wait for diagnostics.
+		)
+	})
+}
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index c173ef235..23ade896a 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -13,13 +13,13 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"sort"
 	"strings"
 
 	"golang.org/x/mod/modfile"
 	"golang.org/x/tools/go/ast/astutil"
-	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/gocommand"
 	"golang.org/x/tools/internal/lsp/command"
@@ -790,9 +790,29 @@ func (c *commandHandler) StartDebugging(ctx context.Context, args command.Debugg
 	return result, nil
 }
 
-func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.VulncheckArgs) (result command.VulncheckResult, _ error) {
+// Copy of pkgLoadConfig defined in internal/lsp/cmd/vulncheck.go
+// TODO(hyangah): decide where to define this.
+type pkgLoadConfig struct {
+	// BuildFlags is a list of command-line flags to be passed through to
+	// the build system's query tool.
+	BuildFlags []string
+
+	// Env is the environment to use when invoking the build system's query tool.
+	// If Env is nil, the current environment is used.
+	// TODO: This seems unnecessary. Delete.
+	Env []string
+
+	// If Tests is set, the loader includes related test packages.
+	Tests bool
+}
+
+func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.VulncheckArgs) error {
+	if args.Dir == "" {
+		return errors.New("VulncheckArgs is missing Dir field")
+	}
 	err := c.run(ctx, commandConfig{
-		progress:    "Running vulncheck",
+		async:       true, // need to be async to be cancellable
+		progress:    "Checking vulnerability",
 		requireSave: true,
 		forURI:      args.Dir, // Will dir work?
 	}, func(ctx context.Context, deps commandDeps) error {
@@ -802,22 +822,44 @@ func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.Vulnc
 			return errors.New("vulncheck feature is not available")
 		}
 
-		buildFlags := opts.BuildFlags // XXX: is session.Options equivalent to view.Options?
+		cmd := exec.Command(os.Args[0], "vulncheck", "-config", args.Pattern)
+		cmd.Dir = args.Dir.SpanURI().Filename()
+
 		var viewEnv []string
 		if e := opts.EnvSlice(); e != nil {
 			viewEnv = append(os.Environ(), e...)
 		}
-		cfg := &packages.Config{
-			Context:    ctx,
-			Tests:      true, // TODO(hyangah): add a field in args.
-			BuildFlags: buildFlags,
-			Env:        viewEnv,
-			Dir:        args.Dir.SpanURI().Filename(),
-			// TODO(hyangah): configure overlay
+		cmd.Env = viewEnv
+
+		// stdin: gopls vulncheck expects JSON-encoded configuration from STDIN when -config flag is set.
+		var stdin bytes.Buffer
+		cmd.Stdin = &stdin
+
+		if err := json.NewEncoder(&stdin).Encode(pkgLoadConfig{
+			BuildFlags: opts.BuildFlags,
+			// TODO(hyangah): add `tests` flag in command.VulncheckArgs
+		}); err != nil {
+			return fmt.Errorf("failed to pass package load config: %v", err)
 		}
-		var err error
-		result, err = opts.Hooks.Govulncheck(ctx, cfg, args)
-		return err
+
+		// stderr: stream gopls vulncheck's STDERR as progress reports
+		er := progress.NewEventWriter(ctx, "vulncheck")
+		stderr := io.MultiWriter(er, progress.NewWorkDoneWriter(ctx, deps.work))
+		cmd.Stderr = stderr
+		// TODO: can we stream stdout?
+		stdout, err := cmd.Output()
+		if err != nil {
+			return fmt.Errorf("failed to run govulncheck: %v", err)
+		}
+
+		var vulns command.VulncheckResult
+		if err := json.Unmarshal(stdout, &vulns); err != nil {
+			// TODO: for easy debugging, log the failed stdout somewhere?
+			return fmt.Errorf("failed to parse govulncheck output: %v", err)
+		}
+
+		// TODO(hyangah): convert the results to diagnostics & code actions.
+		return nil
 	})
-	return result, err
+	return err
 }
diff --git a/internal/lsp/command/command_gen.go b/internal/lsp/command/command_gen.go
index 22cfeff5b..207def42e 100644
--- a/internal/lsp/command/command_gen.go
+++ b/internal/lsp/command/command_gen.go
@@ -159,7 +159,7 @@ func Dispatch(ctx context.Context, params *protocol.ExecuteCommandParams, s Inte
 		if err := UnmarshalArgs(params.Arguments, &a0); err != nil {
 			return nil, err
 		}
-		return s.RunVulncheckExp(ctx, a0)
+		return nil, s.RunVulncheckExp(ctx, a0)
 	case "gopls.start_debugging":
 		var a0 DebuggingArgs
 		if err := UnmarshalArgs(params.Arguments, &a0); err != nil {
diff --git a/internal/lsp/command/interface.go b/internal/lsp/command/interface.go
index 1f3b092fa..280fd616e 100644
--- a/internal/lsp/command/interface.go
+++ b/internal/lsp/command/interface.go
@@ -147,7 +147,7 @@ type Interface interface {
 	// RunVulncheckExp: Run vulncheck (experimental)
 	//
 	// Run vulnerability check (`govulncheck`).
-	RunVulncheckExp(context.Context, VulncheckArgs) (VulncheckResult, error)
+	RunVulncheckExp(context.Context, VulncheckArgs) error
 }
 
 type RunTestsArgs struct {
@@ -320,8 +320,7 @@ type VulncheckArgs struct {
 	// Package pattern. E.g. "", ".", "./...".
 	Pattern string
 
-	// TODO: Flag []string (flags accepted by govulncheck, e.g., -tests)
-	// TODO: Format string (json, text)
+	// TODO: -tests
 }
 
 type VulncheckResult struct {
diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go
index e20b8a671..a6130063f 100755
--- a/internal/lsp/source/api_json.go
+++ b/internal/lsp/source/api_json.go
@@ -726,11 +726,10 @@ var GeneratedAPIJSON = &APIJSON{
 			ArgDoc:  "{\n\t// The test file containing the tests to run.\n\t\"URI\": string,\n\t// Specific test names to run, e.g. TestFoo.\n\t\"Tests\": []string,\n\t// Specific benchmarks to run, e.g. BenchmarkFoo.\n\t\"Benchmarks\": []string,\n}",
 		},
 		{
-			Command:   "gopls.run_vulncheck_exp",
-			Title:     "Run vulncheck (experimental)",
-			Doc:       "Run vulnerability check (`govulncheck`).",
-			ArgDoc:    "{\n\t// Dir is the directory from which vulncheck will run from.\n\t\"Dir\": string,\n\t// Package pattern. E.g. \"\", \".\", \"./...\".\n\t\"Pattern\": string,\n}",
-			ResultDoc: "{\n\t\"Vuln\": []{\n\t\t\"ID\": string,\n\t\t\"Details\": string,\n\t\t\"Aliases\": []string,\n\t\t\"Symbol\": string,\n\t\t\"PkgPath\": string,\n\t\t\"ModPath\": string,\n\t\t\"URL\": string,\n\t\t\"CurrentVersion\": string,\n\t\t\"FixedVersion\": string,\n\t\t\"CallStacks\": [][]golang.org/x/tools/internal/lsp/command.StackEntry,\n\t\t\"CallStackSummaries\": []string,\n\t},\n}",
+			Command: "gopls.run_vulncheck_exp",
+			Title:   "Run vulncheck (experimental)",
+			Doc:     "Run vulnerability check (`govulncheck`).",
+			ArgDoc:  "{\n\t// Dir is the directory from which vulncheck will run from.\n\t\"Dir\": string,\n\t// Package pattern. E.g. \"\", \".\", \"./...\".\n\t\"Pattern\": string,\n}",
 		},
 		{
 			Command:   "gopls.start_debugging",