diff --git a/gopls/go.mod b/gopls/go.mod index e42749e71b..9a16f38c1e 100644 --- a/gopls/go.mod +++ b/gopls/go.mod @@ -12,7 +12,7 @@ require ( golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664 golang.org/x/text v0.3.7 golang.org/x/tools v0.1.13-0.20220928184430-f80e98464e27 - golang.org/x/vuln v0.0.0-20221004232641-2aa0553d353b + golang.org/x/vuln v0.0.0-20221010193109-563322be2ea9 gopkg.in/yaml.v3 v3.0.1 honnef.co/go/tools v0.3.3 mvdan.cc/gofumpt v0.3.1 diff --git a/gopls/go.sum b/gopls/go.sum index 9ced54e968..e4293b40ac 100644 --- a/gopls/go.sum +++ b/gopls/go.sum @@ -71,6 +71,10 @@ golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/vuln v0.0.0-20221004232641-2aa0553d353b h1:8Tu9pgIV7kt8ulNtzidzpLl9E9l1i+U4QLdKG0ZzHyE= golang.org/x/vuln v0.0.0-20221004232641-2aa0553d353b/go.mod h1:F12iebNzxRMpJsm4W7ape+r/KdnXiSy3VC94WsyCG68= +golang.org/x/vuln v0.0.0-20221006005703-27389ae96df4 h1:rj0uNKXz70TlwVjkDL/rF4qGHp0RzIXzDg7d7b0pnQo= +golang.org/x/vuln v0.0.0-20221006005703-27389ae96df4/go.mod h1:F12iebNzxRMpJsm4W7ape+r/KdnXiSy3VC94WsyCG68= +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/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/summary.go b/gopls/internal/govulncheck/summary.go new file mode 100644 index 0000000000..e389b89a4a --- /dev/null +++ b/gopls/internal/govulncheck/summary.go @@ -0,0 +1,46 @@ +// 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/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go index cb18834d1c..57bba47df8 100644 --- a/gopls/internal/lsp/cache/session.go +++ b/gopls/internal/lsp/cache/session.go @@ -13,7 +13,7 @@ import ( "sync" "sync/atomic" - "golang.org/x/tools/gopls/internal/lsp/command" + "golang.org/x/tools/gopls/internal/govulncheck" "golang.org/x/tools/gopls/internal/lsp/progress" "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/gopls/internal/span" @@ -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][]command.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 6244d909c4..bdf2281757 100644 --- a/gopls/internal/lsp/cache/view.go +++ b/gopls/internal/lsp/cache/view.go @@ -24,7 +24,7 @@ import ( "golang.org/x/mod/semver" exec "golang.org/x/sys/execabs" "golang.org/x/tools/go/packages" - "golang.org/x/tools/gopls/internal/lsp/command" + "golang.org/x/tools/gopls/internal/govulncheck" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/gopls/internal/span" @@ -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][]command.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 @@ -1032,16 +1032,16 @@ func (v *View) ClearModuleUpgrades(modfile span.URI) { delete(v.moduleUpgrades, modfile) } -func (v *View) Vulnerabilities(modfile span.URI) []command.Vuln { +func (v *View) Vulnerabilities(modfile span.URI) []govulncheck.Vuln { v.mu.Lock() defer v.mu.Unlock() - vulns := make([]command.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 []command.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/usage/vulncheck.hlp b/gopls/internal/lsp/cmd/usage/vulncheck.hlp index 4cdb8f3b64..4fbe573e22 100644 --- a/gopls/internal/lsp/cmd/usage/vulncheck.hlp +++ b/gopls/internal/lsp/cmd/usage/vulncheck.hlp @@ -13,3 +13,5 @@ Usage: -config If true, the command reads a JSON-encoded package load configuration from stdin + -summary + If true, outputs a JSON-encoded govulnchecklib.Summary JSON diff --git a/gopls/internal/lsp/cmd/vulncheck.go b/gopls/internal/lsp/cmd/vulncheck.go index 3ae2175a96..93b9c106ae 100644 --- a/gopls/internal/lsp/cmd/vulncheck.go +++ b/gopls/internal/lsp/cmd/vulncheck.go @@ -18,8 +18,9 @@ import ( // vulncheck implements the vulncheck command. type vulncheck struct { - Config bool `flag:"config" help:"If true, the command reads a JSON-encoded package load configuration from stdin"` - app *Application + Config bool `flag:"config" help:"If true, the command reads a JSON-encoded package load configuration from stdin"` + AsSummary bool `flag:"summary" help:"If true, outputs a JSON-encoded govulnchecklib.Summary JSON"` + app *Application } type pkgLoadConfig struct { @@ -58,6 +59,7 @@ func (v *vulncheck) Run(ctx context.Context, args ...string) error { return fmt.Errorf("vulncheck command is available only in gopls compiled with go1.18 or newer") } + // TODO(hyangah): what's wrong with allowing multiple targets? if len(args) > 1 { return tool.CommandLineErrorf("vulncheck accepts at most one package pattern") } @@ -65,25 +67,27 @@ func (v *vulncheck) Run(ctx context.Context, args ...string) error { if len(args) == 1 { pattern = args[0] } + var cfg pkgLoadConfig if v.Config { if err := json.NewDecoder(os.Stdin).Decode(&cfg); err != nil { return tool.CommandLineErrorf("failed to parse cfg: %v", err) } } - - if vulnchecklib.Govulncheck == nil { - return fmt.Errorf("vulncheck feature is not available") - } - - loadCfg := &packages.Config{ + loadCfg := packages.Config{ Context: ctx, Tests: cfg.Tests, BuildFlags: cfg.BuildFlags, // inherit the current process's cwd and env. } - res, err := vulnchecklib.Govulncheck(ctx, loadCfg, pattern) + if v.AsSummary { + // vulnchecklib.Main calls os.Exit and never returns. + vulnchecklib.Main(loadCfg, args...) + return nil + } + // TODO(hyangah): delete. + res, err := vulnchecklib.Govulncheck(ctx, &loadCfg, pattern) if err != nil { return fmt.Errorf("vulncheck failed: %v", err) } diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go index 7f7380e299..e231b4bba4 100644 --- a/gopls/internal/lsp/command.go +++ b/gopls/internal/lsp/command.go @@ -20,6 +20,7 @@ import ( "golang.org/x/mod/modfile" "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/gopls/internal/govulncheck" "golang.org/x/tools/gopls/internal/lsp/command" "golang.org/x/tools/gopls/internal/lsp/debug" "golang.org/x/tools/gopls/internal/lsp/progress" @@ -849,7 +850,7 @@ func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.Vulnc return errors.New("vulncheck feature is not available") } - cmd := exec.CommandContext(ctx, os.Args[0], "vulncheck", "-config", args.Pattern) + cmd := exec.CommandContext(ctx, os.Args[0], "vulncheck", "-summary", "-config", args.Pattern) cmd.Dir = filepath.Dir(args.URI.SpanURI().Filename()) var viewEnv []string @@ -879,28 +880,26 @@ func (c *commandHandler) RunVulncheckExp(ctx context.Context, args command.Vulnc return fmt.Errorf("failed to run govulncheck: %v", err) } - var vulns command.VulncheckResult - if err := json.Unmarshal(stdout, &vulns); err != nil { + var summary govulncheck.Summary + if err := json.Unmarshal(stdout, &summary); err != nil { // TODO: for easy debugging, log the failed stdout somewhere? return fmt.Errorf("failed to parse govulncheck output: %v", err) } - deps.snapshot.View().SetVulnerabilities(args.URI.SpanURI(), vulns.Vuln) + vulns := append(summary.Affecting, summary.NonAffecting...) + deps.snapshot.View().SetVulnerabilities(args.URI.SpanURI(), vulns) c.s.diagnoseSnapshot(deps.snapshot, nil, false) - set := make(map[string]bool) - for _, v := range vulns.Vuln { - if len(v.CallStackSummaries) > 0 { - set[v.ID] = true - } - } - if len(set) == 0 { + if len(summary.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) diff --git a/gopls/internal/lsp/command/interface.go b/gopls/internal/lsp/command/interface.go index fc5057580f..23b9f65504 100644 --- a/gopls/internal/lsp/command/interface.go +++ b/gopls/internal/lsp/command/interface.go @@ -336,6 +336,7 @@ type StackEntry struct { } // Vuln models an osv.Entry and representative call stacks. +// TODO: deprecate type Vuln struct { // ID is the vulnerability ID (osv.Entry.ID). // https://ossf.github.io/osv-schema/#id-modified-fields diff --git a/gopls/internal/lsp/mod/diagnostics.go b/gopls/internal/lsp/mod/diagnostics.go index 256abd13cf..546c84cb07 100644 --- a/gopls/internal/lsp/mod/diagnostics.go +++ b/gopls/internal/lsp/mod/diagnostics.go @@ -10,13 +10,16 @@ import ( "bytes" "context" "fmt" + "strings" "golang.org/x/mod/semver" + "golang.org/x/tools/gopls/internal/govulncheck" "golang.org/x/tools/gopls/internal/lsp/command" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/lsp/source" "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/event/tag" + "golang.org/x/vuln/osv" ) // Diagnostics returns diagnostics for the modules in the workspace. @@ -158,6 +161,21 @@ func ModUpgradeDiagnostics(ctx context.Context, snapshot source.Snapshot, fh sou return upgradeDiagnostics, nil } +func pkgVersion(pkgVersion string) (pkg, ver string) { + if pkgVersion == "" { + return "", "" + } + at := strings.Index(pkgVersion, "@") + switch { + case at < 0: + return pkgVersion, "" + case at == 0: + return "", pkgVersion[1:] + default: + return pkgVersion[:at], pkgVersion[at+1:] + } +} + // ModVulnerabilityDiagnostics adds diagnostics for vulnerabilities in individual modules // if the vulnerability is recorded in the view. func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) (vulnDiagnostics []*source.Diagnostic, err error) { @@ -173,7 +191,7 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, vs := snapshot.View().Vulnerabilities(fh.URI()) // TODO(suzmue): should we just store the vulnerabilities like this? - vulns := make(map[string][]command.Vuln) + vulns := make(map[string][]govulncheck.Vuln) for _, v := range vs { vulns[v.ModPath] = append(vulns[v.ModPath], v) } @@ -190,14 +208,14 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, for _, v := range vulnList { // Only show the diagnostic if the vulnerability was calculated // for the module at the current version. - if semver.IsValid(v.CurrentVersion) && semver.Compare(req.Mod.Version, v.CurrentVersion) != 0 { + if semver.IsValid(v.FoundIn) && semver.Compare(req.Mod.Version, v.FoundIn) != 0 { continue } - // Upgrade to the exact version we offer the user, not the most recent. // TODO(suzmue): Add an upgrade for module@latest. + // TODO(hakim): Produce fixes only for affecting vulnerabilities (if len(v.Trace) > 0) var fixes []source.SuggestedFix - if fixedVersion := v.FixedVersion; semver.IsValid(fixedVersion) && semver.Compare(req.Mod.Version, fixedVersion) < 0 { + if fixedVersion := v.FixedIn; semver.IsValid(fixedVersion) && semver.Compare(req.Mod.Version, fixedVersion) < 0 { title := fmt.Sprintf("Upgrade to %v", fixedVersion) cmd, err := command.NewUpgradeDependencyCommand(title, command.DependencyArgs{ URI: protocol.URIFromSpanURI(fh.URI()), @@ -211,7 +229,7 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, } severity := protocol.SeverityInformation - if len(v.CallStacks) > 0 { + if len(v.Trace) > 0 { severity = protocol.SeverityWarning } @@ -220,9 +238,9 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, Range: rng, Severity: severity, Source: source.Vulncheck, - Code: v.ID, - CodeHref: v.URL, - Message: formatMessage(&v), + Code: v.OSV.ID, + CodeHref: href(v.OSV), + Message: formatMessage(v), SuggestedFixes: fixes, }) } @@ -232,8 +250,8 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, return vulnDiagnostics, nil } -func formatMessage(v *command.Vuln) string { - details := []byte(v.Details) +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 { if r == '\n' && i > 0 && details[i-1] != '\n' && i+1 < len(details) && details[i+1] != '\n' { @@ -242,3 +260,20 @@ func formatMessage(v *command.Vuln) string { } return fmt.Sprintf("%s has a known vulnerability: %s", v.ModPath, string(bytes.TrimSpace(details))) } + +// href returns a URL embedded in the entry if any. +// If no suitable URL is found, it returns a default entry in +// pkg.go.dev/vuln. +func href(vuln *osv.Entry) string { + for _, affected := range vuln.Affected { + if url := affected.DatabaseSpecific.URL; url != "" { + return url + } + } + for _, r := range vuln.References { + if r.Type == "WEB" { + return r.URL + } + } + return fmt.Sprintf("https://pkg.go.dev/vuln/%s", vuln.ID) +} diff --git a/gopls/internal/lsp/source/view.go b/gopls/internal/lsp/source/view.go index cd0c8417f4..e986d1261f 100644 --- a/gopls/internal/lsp/source/view.go +++ b/gopls/internal/lsp/source/view.go @@ -21,7 +21,7 @@ import ( "golang.org/x/mod/module" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/packages" - "golang.org/x/tools/gopls/internal/lsp/command" + "golang.org/x/tools/gopls/internal/govulncheck" "golang.org/x/tools/gopls/internal/lsp/protocol" "golang.org/x/tools/gopls/internal/span" "golang.org/x/tools/internal/gocommand" @@ -271,11 +271,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) []command.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 []command.Vuln) + SetVulnerabilities(modfile span.URI, vulnerabilities []govulncheck.Vuln) // FileKind returns the type of a file FileKind(FileHandle) FileKind diff --git a/gopls/internal/regtest/misc/vuln_test.go b/gopls/internal/regtest/misc/vuln_test.go index 2fb9e31a3c..1b55a6cabe 100644 --- a/gopls/internal/regtest/misc/vuln_test.go +++ b/gopls/internal/regtest/misc/vuln_test.go @@ -16,6 +16,7 @@ import ( "golang.org/x/tools/gopls/internal/lsp/protocol" . "golang.org/x/tools/gopls/internal/lsp/regtest" "golang.org/x/tools/gopls/internal/lsp/tests/compare" + "golang.org/x/tools/gopls/internal/vulncheck" "golang.org/x/tools/gopls/internal/vulncheck/vulntest" "golang.org/x/tools/internal/testenv" ) @@ -139,9 +140,7 @@ func main() { // 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", + vulncheck.GoVersionForVulnTest: "go1.18", "_GOPLS_TEST_BINARY_RUN_AS_GOPLS": "true", // needed to run `gopls vulncheck`. }, Settings{ @@ -301,9 +300,7 @@ func TestRunVulncheckExp(t *testing.T) { // When fetching 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", + vulncheck.GoVersionForVulnTest: "go1.18", "_GOPLS_TEST_BINARY_RUN_AS_GOPLS": "true", // needed to run `gopls vulncheck`. }, Settings{ diff --git a/gopls/internal/vulncheck/command.go b/gopls/internal/vulncheck/command.go index 641c9ddeb6..e607eb415e 100644 --- a/gopls/internal/vulncheck/command.go +++ b/gopls/internal/vulncheck/command.go @@ -19,6 +19,7 @@ import ( gvc "golang.org/x/tools/gopls/internal/govulncheck" "golang.org/x/tools/gopls/internal/lsp/command" "golang.org/x/vuln/client" + gvcapi "golang.org/x/vuln/exp/govulncheck" "golang.org/x/vuln/osv" "golang.org/x/vuln/vulncheck" ) @@ -208,3 +209,26 @@ func trimPosPrefix(summary string) string { } return after } + +// GoVersionForVulnTest is an internal environment variable used in gopls +// testing to examine govulncheck behavior with a go version different +// than what `go version` returns in the system. +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), + }) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) + } +} diff --git a/gopls/internal/vulncheck/vulncheck.go b/gopls/internal/vulncheck/vulncheck.go index d452045a5e..7167ec1c26 100644 --- a/gopls/internal/vulncheck/vulncheck.go +++ b/gopls/internal/vulncheck/vulncheck.go @@ -18,3 +18,5 @@ import ( // Govulncheck runs the in-process govulncheck implementation. // 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