Skip to content

Commit 27641fb

Browse files
suzmuesuzmue
committed
gopls: suggest upgrading to fixed version for vulncheck diagnostics
If there is a fixed version of the module with a vulnerability, provide a code action to upgrade to that version. Change-Id: I2e0d72e7a86dc097f139d60893c204d1ec55dad1 Reviewed-on: https://go-review.googlesource.com/c/tools/+/436216 Run-TryBot: Suzy Mueller <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 199742a commit 27641fb

File tree

4 files changed

+69
-13
lines changed

4 files changed

+69
-13
lines changed

‎gopls/internal/lsp/code_action.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,12 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
8181
if err != nil {
8282
return nil, err
8383
}
84-
// TODO(suzmue): get upgrades code actions from vulnerabilities.
85-
quickFixes, err := codeActionsMatchingDiagnostics(ctx, snapshot, diagnostics, append(diags, udiags...))
84+
vdiags, err := mod.ModVulnerabilityDiagnostics(ctx, snapshot, fh)
85+
if err != nil {
86+
return nil, err
87+
}
88+
// TODO(suzmue): Consider deduping upgrades from ModUpgradeDiagnostics and ModVulnerabilityDiagnostics.
89+
quickFixes, err := codeActionsMatchingDiagnostics(ctx, snapshot, diagnostics, append(append(diags, udiags...), vdiags...))
8690
if err != nil {
8791
return nil, err
8892
}
@@ -426,7 +430,8 @@ func codeActionsForDiagnostic(ctx context.Context, snapshot source.Snapshot, sd
426430
}
427431

428432
func sameDiagnostic(pd protocol.Diagnostic, sd *source.Diagnostic) bool {
429-
return pd.Message == sd.Message && protocol.CompareRange(pd.Range, sd.Range) == 0 && pd.Source == string(sd.Source)
433+
return pd.Message == strings.TrimSpace(sd.Message) && // extra space may have been trimmed when converting to protocol.Diagnostic
434+
protocol.CompareRange(pd.Range, sd.Range) == 0 && pd.Source == string(sd.Source)
430435
}
431436

432437
func goTest(ctx context.Context, snapshot source.Snapshot, uri span.URI, rng protocol.Range) ([]protocol.CodeAction, error) {

‎gopls/internal/lsp/mod/diagnostics.go

+18-1
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,22 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
193193
continue
194194
}
195195

196+
// Upgrade to the exact version we offer the user, not the most recent.
197+
// TODO(suzmue): Add an upgrade for module@latest.
198+
var fixes []source.SuggestedFix
199+
if fixedVersion := v.FixedVersion; semver.IsValid(fixedVersion) && semver.Compare(req.Mod.Version, fixedVersion) < 0 {
200+
title := fmt.Sprintf("Upgrade to %v", fixedVersion)
201+
cmd, err := command.NewUpgradeDependencyCommand(title, command.DependencyArgs{
202+
URI: protocol.URIFromSpanURI(fh.URI()),
203+
AddRequire: false,
204+
GoCmdArgs: []string{req.Mod.Path + "@" + fixedVersion},
205+
})
206+
if err != nil {
207+
return nil, err
208+
}
209+
fixes = append(fixes, source.SuggestedFixFromCommand(cmd, protocol.QuickFix))
210+
}
211+
196212
severity := protocol.SeverityInformation
197213
if len(v.CallStacks) > 0 {
198214
severity = protocol.SeverityWarning
@@ -206,7 +222,8 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
206222
Code: v.ID,
207223
CodeHref: v.URL,
208224
// TODO(suzmue): replace the newlines in v.Details to allow the editor to handle formatting.
209-
Message: v.Details,
225+
Message: v.Details,
226+
SuggestedFixes: fixes,
210227
})
211228
}
212229

‎gopls/internal/regtest/misc/testdata/vulndb/golang.org/amod.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"affected":[
2222
{
2323
"package":{"name":"golang.org/amod","ecosystem":"Go"},
24-
"ranges":[{"type":"SEMVER","events":[{"introduced":"1.0.0"},{"fixed":"1.0.4"}]}],
24+
"ranges":[{"type":"SEMVER","events":[{"introduced":"1.0.0"},{"fixed":"1.0.6"}]}],
2525
"ecosystem_specific":{
2626
"imports":[{"path":"golang.org/amod/avuln","symbols":["nonExisting"]}]
2727
}

‎gopls/internal/regtest/misc/vuln_test.go

+42-8
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"golang.org/x/tools/gopls/internal/lsp/command"
1414
"golang.org/x/tools/gopls/internal/lsp/protocol"
1515
. "golang.org/x/tools/gopls/internal/lsp/regtest"
16+
"golang.org/x/tools/gopls/internal/lsp/tests/compare"
1617
"golang.org/x/tools/internal/testenv"
1718
)
1819

@@ -126,12 +127,12 @@ go 1.18
126127
require golang.org/cmod v1.1.3
127128
128129
require (
129-
golang.org/amod v1.1.3 // indirect
130+
golang.org/amod v1.0.0 // indirect
130131
golang.org/bmod v0.5.0 // indirect
131132
)
132133
-- go.sum --
133-
golang.org/amod v1.1.3 h1:E9ohW9ayc6iCFrT/VNq8tCI4hgYM+tEbo8txbtbyS3o=
134-
golang.org/amod v1.1.3/go.mod h1:yvny5/2OtYFomKt8ax+WJGvN6pfN1pqjGnn7DQLUi6E=
134+
golang.org/amod v1.0.0 h1:EUQOI2m5NhQZijXZf8WimSnnWubaFNrrKUH/PopTN8k=
135+
golang.org/amod v1.0.0/go.mod h1:yvny5/2OtYFomKt8ax+WJGvN6pfN1pqjGnn7DQLUi6E=
135136
golang.org/bmod v0.5.0 h1:0kt1EI53298Ta9w4RPEAzNUQjtDoHUA6cc0c7Rwxhlk=
136137
golang.org/bmod v0.5.0/go.mod h1:f6o+OhF66nz/0BBc/sbCsshyPRKMSxZIlG50B/bsM4c=
137138
golang.org/cmod v1.1.3 h1:PJ7rZFTk7xGAunBRDa0wDe7rZjZ9R/vr1S2QkVVCngQ=
@@ -188,16 +189,27 @@ func C1() I {
188189
func C2() func() {
189190
return bvuln.Vuln
190191
}
191-
-- golang.org/amod@v1.1.3/go.mod --
192+
-- golang.org/amod@v1.0.0/go.mod --
192193
module golang.org/amod
193194
194195
go 1.14
195-
-- golang.org/amod@v1.1.3/avuln/avuln.go --
196+
-- golang.org/amod@v1.0.0/avuln/avuln.go --
196197
package avuln
197198
198199
type VulnData struct {}
199200
func (v VulnData) Vuln1() {}
200201
func (v VulnData) Vuln2() {}
202+
-- golang.org/[email protected]/go.mod --
203+
module golang.org/amod
204+
205+
go 1.14
206+
-- golang.org/[email protected]/avuln/avuln.go --
207+
package avuln
208+
209+
type VulnData struct {}
210+
func (v VulnData) Vuln1() {}
211+
func (v VulnData) Vuln2() {}
212+
201213
-- golang.org/[email protected]/go.mod --
202214
module golang.org/bmod
203215
@@ -234,13 +246,35 @@ func TestRunVulncheckExp(t *testing.T) {
234246
},
235247
).Run(t, workspace1, func(t *testing.T, env *Env) {
236248
env.OpenFile("go.mod")
237-
env.ExecuteCodeLensCommand("go.mod", command.RunVulncheckExp)
249+
env.ExecuteCodeLensCommand("go.mod", command.Tidy)
238250

251+
env.ExecuteCodeLensCommand("go.mod", command.RunVulncheckExp)
252+
d := &protocol.PublishDiagnosticsParams{}
239253
env.Await(
240254
CompletedWork("govulncheck", 1, true),
241255
ShownMessage("Found"),
242-
env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/amod`, "vuln in amod"),
243-
env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/bmod`, "vuln in bmod"),
256+
OnceMet(
257+
env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/amod`, "vuln in amod"),
258+
env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/bmod`, "vuln in bmod"),
259+
ReadDiagnostics("go.mod", d),
260+
),
244261
)
262+
263+
env.ApplyQuickFixes("go.mod", d.Diagnostics)
264+
env.Await(env.DoneWithChangeWatchedFiles())
265+
wantGoMod := `module golang.org/entry
266+
267+
go 1.18
268+
269+
require golang.org/cmod v1.1.3
270+
271+
require (
272+
golang.org/amod v1.0.4 // indirect
273+
golang.org/bmod v0.5.0 // indirect
274+
)
275+
`
276+
if got := env.Editor.BufferText("go.mod"); got != wantGoMod {
277+
t.Fatalf("go.mod vulncheck fix failed:\n%s", compare.Text(wantGoMod, got))
278+
}
245279
})
246280
}

0 commit comments

Comments
 (0)