Skip to content

Commit c4ec0f5

Browse files
ethanalee-workgopherbot
authored andcommitted
internal/server: list vulnerabilities within vulncheck prompt
- List vulnerabilities and their CVE links in preparation for adding remediation functionality. - Refactor command.go to generalize vulncheck scanning functionality. Change-Id: I567f514eba2b7ed59ad3c4be23622f8e44fb9fa1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/735740 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Hongxiang Jiang <hxjiang@golang.org> Auto-Submit: Ethan Lee <ethanalee@google.com>
1 parent 80d1715 commit c4ec0f5

File tree

2 files changed

+103
-52
lines changed

2 files changed

+103
-52
lines changed

gopls/internal/server/command.go

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,28 +1246,13 @@ func (c *commandHandler) Vulncheck(ctx context.Context, args command.VulncheckAr
12461246
jsonrpc2.Async(ctx) // run this in parallel with other requests: vulncheck can be slow.
12471247

12481248
workDoneWriter := progress.NewWorkDoneWriter(ctx, deps.work)
1249-
dir := args.URI.DirPath()
1250-
pattern := args.Pattern
1251-
1252-
result, err := scan.RunGovulncheck(ctx, pattern, deps.snapshot, dir, workDoneWriter)
1249+
result, err := c.s.runVulncheck(ctx, deps.snapshot, args.URI, args.Pattern, workDoneWriter)
12531250
if err != nil {
12541251
return err
12551252
}
12561253
commandResult.Result = result
12571254
commandResult.Token = deps.work.Token()
12581255

1259-
snapshot, release, err := c.s.session.InvalidateView(ctx, deps.snapshot.View(), cache.StateChange{
1260-
Vulns: map[protocol.DocumentURI]*vulncheck.Result{args.URI: result},
1261-
})
1262-
if err != nil {
1263-
return err
1264-
}
1265-
defer release()
1266-
1267-
// Diagnosing with the background context ensures new snapshots are fully
1268-
// diagnosed.
1269-
c.s.diagnoseSnapshot(snapshot.BackgroundContext(), snapshot, nil, 0)
1270-
12711256
affecting := make(map[string]bool, len(result.Entries))
12721257
for _, finding := range result.Findings {
12731258
if len(finding.Trace) > 1 { // at least 2 frames if callstack exists (vulnerability, entry)
@@ -1285,7 +1270,6 @@ func (c *commandHandler) Vulncheck(ctx context.Context, args command.VulncheckAr
12851270
sort.Strings(affectingOSVs)
12861271

12871272
showMessage(ctx, c.s.client, protocol.Warning, fmt.Sprintf("Found %v", strings.Join(affectingOSVs, ", ")))
1288-
12891273
return nil
12901274
})
12911275
if err != nil {
@@ -1332,26 +1316,11 @@ func (c *commandHandler) RunGovulncheck(ctx context.Context, args command.Vulnch
13321316
tokenChan <- deps.work.Token()
13331317

13341318
workDoneWriter := progress.NewWorkDoneWriter(ctx, deps.work)
1335-
dir := filepath.Dir(args.URI.Path())
1336-
pattern := args.Pattern
1337-
1338-
result, err := scan.RunGovulncheck(ctx, pattern, deps.snapshot, dir, workDoneWriter)
1319+
result, err := c.s.runVulncheck(ctx, deps.snapshot, args.URI, args.Pattern, workDoneWriter)
13391320
if err != nil {
13401321
return err
13411322
}
13421323

1343-
snapshot, release, err := c.s.session.InvalidateView(ctx, deps.snapshot.View(), cache.StateChange{
1344-
Vulns: map[protocol.DocumentURI]*vulncheck.Result{args.URI: result},
1345-
})
1346-
if err != nil {
1347-
return err
1348-
}
1349-
defer release()
1350-
1351-
// Diagnosing with the background context ensures new snapshots are fully
1352-
// diagnosed.
1353-
c.s.diagnoseSnapshot(snapshot.BackgroundContext(), snapshot, nil, 0)
1354-
13551324
affecting := make(map[string]bool, len(result.Entries))
13561325
for _, finding := range result.Findings {
13571326
if len(finding.Trace) > 1 { // at least 2 frames if callstack exists (vulnerability, entry)
@@ -1369,7 +1338,6 @@ func (c *commandHandler) RunGovulncheck(ctx context.Context, args command.Vulnch
13691338
sort.Strings(affectingOSVs)
13701339

13711340
showMessage(ctx, c.s.client, protocol.Warning, fmt.Sprintf("Found %v", strings.Join(affectingOSVs, ", ")))
1372-
13731341
return nil
13741342
})
13751343
if err != nil {
@@ -1383,6 +1351,30 @@ func (c *commandHandler) RunGovulncheck(ctx context.Context, args command.Vulnch
13831351
}
13841352
}
13851353

1354+
// runVulncheck executes a vulnerability scan and updates the server's state.
1355+
func (s *server) runVulncheck(ctx context.Context, snapshot *cache.Snapshot, uri protocol.DocumentURI, pattern string, out io.Writer) (*vulncheck.Result, error) {
1356+
dir := uri.DirPath()
1357+
result, err := scan.RunGovulncheck(ctx, pattern, snapshot, dir, out)
1358+
if err != nil {
1359+
return nil, err
1360+
}
1361+
1362+
// Invalidate the view to store findings in the vulnerability cache.
1363+
newSnapshot, release, err := s.session.InvalidateView(ctx, snapshot.View(), cache.StateChange{
1364+
Vulns: map[protocol.DocumentURI]*vulncheck.Result{uri: result},
1365+
})
1366+
if err != nil {
1367+
return nil, err
1368+
}
1369+
defer release()
1370+
1371+
// Diagnosing with the background context ensures new snapshots are fully
1372+
// diagnosed.
1373+
s.diagnoseSnapshot(newSnapshot.BackgroundContext(), newSnapshot, nil, 0)
1374+
1375+
return result, nil
1376+
}
1377+
13861378
// MemStats implements the MemStats command. It returns an error as a
13871379
// future-proof API, but the resulting error is currently always nil.
13881380
func (c *commandHandler) MemStats(ctx context.Context) (command.MemStatsResult, error) {

gopls/internal/server/vulncheck_prompt.go

Lines changed: 77 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,26 @@ import (
1010
"encoding/hex"
1111
"encoding/json"
1212
"fmt"
13+
"maps"
1314
"os"
1415
"path/filepath"
16+
"slices"
17+
"sort"
18+
"strings"
1519

1620
"golang.org/x/mod/modfile"
1721
"golang.org/x/tools/gopls/internal/filecache"
22+
"golang.org/x/tools/gopls/internal/progress"
1823
"golang.org/x/tools/gopls/internal/protocol"
19-
"golang.org/x/tools/gopls/internal/protocol/command"
2024
"golang.org/x/tools/gopls/internal/settings"
2125
"golang.org/x/tools/internal/event"
26+
"golang.org/x/tools/internal/xcontext"
2227
)
2328

2429
const (
2530
// goModHashKind is the kind for the go.mod hash in the filecache.
26-
goModHashKind = "gomodhash"
31+
goModHashKind = "gomodhash"
32+
maxVulnsToShow = 10
2733
)
2834

2935
// computeGoModHash computes the SHA256 hash of the go.mod file's dependencies.
@@ -95,17 +101,10 @@ func (s *server) checkGoModDeps(ctx context.Context, uri protocol.DocumentURI) {
95101
if err != nil {
96102
event.Error(ctx, "reading vulncheck preference failed", err)
97103
}
98-
args := command.VulncheckArgs{
99-
URI: uri,
100-
Pattern: "./...",
101-
}
102-
cmd := command.NewRunGovulncheckCommand("Run govulncheck", args)
103104

104105
switch action {
105106
case "Always":
106-
if _, err := s.executeCommand(ctx, cmd); err != nil {
107-
event.Error(ctx, "executing govulncheck command failed", err)
108-
}
107+
go s.handleVulncheck(ctx, uri)
109108
case "Never":
110109
return
111110
case "":
@@ -125,9 +124,7 @@ func (s *server) checkGoModDeps(ctx context.Context, uri protocol.DocumentURI) {
125124
}
126125
}
127126
if action == "Yes" || action == "Always" {
128-
if _, err := s.executeCommand(ctx, cmd); err != nil {
129-
event.Error(ctx, "executing govulncheck command failed", err)
130-
}
127+
go s.handleVulncheck(ctx, uri)
131128
}
132129
if action == "" {
133130
// No user input gathered from prompt.
@@ -142,12 +139,74 @@ func (s *server) checkGoModDeps(ctx context.Context, uri protocol.DocumentURI) {
142139
}
143140
}
144141

145-
func (s *server) executeCommand(ctx context.Context, cmd *protocol.Command) (any, error) {
146-
params := &protocol.ExecuteCommandParams{
147-
Command: cmd.Command,
148-
Arguments: cmd.Arguments,
142+
func (s *server) handleVulncheck(ctx context.Context, uri protocol.DocumentURI) {
143+
_, snapshot, release, err := s.session.FileOf(ctx, uri)
144+
if err != nil {
145+
event.Error(ctx, "getting file snapshot failed", err)
146+
return
147+
}
148+
defer release()
149+
ctx = xcontext.Detach(ctx)
150+
151+
work := s.progress.Start(ctx, GoVulncheckCommandTitle, "Running govulncheck...", nil, nil)
152+
defer work.End(ctx, "Done.")
153+
workDoneWriter := progress.NewWorkDoneWriter(ctx, work)
154+
result, err := s.runVulncheck(ctx, snapshot, uri, "./...", workDoneWriter)
155+
if err != nil {
156+
event.Error(ctx, "vulncheck failed", err)
157+
return
158+
}
159+
160+
affecting := make(map[string]struct{})
161+
upgrades := make(map[string]string)
162+
for _, f := range result.Findings {
163+
if len(f.Trace) > 1 {
164+
affecting[f.OSV] = struct{}{}
165+
if f.FixedVersion != "" && f.Trace[0].Module != "stdlib" {
166+
upgrades[f.Trace[0].Module] = f.FixedVersion
167+
}
168+
}
169+
}
170+
171+
if len(affecting) == 0 {
172+
showMessage(ctx, s.client, protocol.Info, "No vulnerabilities found.")
173+
return
174+
}
175+
176+
affectingOSVs := slices.Sorted(maps.Keys(affecting))
177+
sort.Strings(affectingOSVs)
178+
179+
var b strings.Builder
180+
fmt.Fprintf(&b, "Found %d vulnerabilities affecting your dependencies:\n\n", len(affectingOSVs))
181+
182+
for i, id := range affectingOSVs {
183+
if i >= maxVulnsToShow {
184+
break
185+
}
186+
cveURL := fmt.Sprintf("https://pkg.go.dev/vuln/%s", id)
187+
if s.Options().LinkifyShowMessage {
188+
fmt.Fprintf(&b, "Vulnerability #%d: [%s](%s)", i+1, id, cveURL)
189+
} else {
190+
fmt.Fprintf(&b, "Vulnerability #%d: %s (%s)", i+1, id, cveURL)
191+
}
192+
if i < len(affectingOSVs)-1 && i < maxVulnsToShow-1 {
193+
b.WriteString(", ")
194+
}
195+
}
196+
if len(affectingOSVs) > maxVulnsToShow {
197+
fmt.Fprintf(&b, "\n\n...and %d more.", len(affectingOSVs)-maxVulnsToShow)
198+
}
199+
200+
action, err := showMessageRequest(ctx, s.client, protocol.Warning, b.String(), "Upgrade All", "Ignore")
201+
if err != nil {
202+
event.Error(ctx, "vulncheck remediation failed", err)
203+
return
204+
}
205+
206+
if action == "Upgrade All" {
207+
// TODO: Add dependency upgrade functionality.
208+
showMessage(ctx, s.client, protocol.Info, "Upgrading all modules... (not yet implemented)")
149209
}
150-
return s.ExecuteCommand(ctx, params)
151210
}
152211

153212
type vulncheckConfig struct {

0 commit comments

Comments
 (0)