From 27641fbc7c81239bb0d55ca26d6364b6ca02676b Mon Sep 17 00:00:00 2001 From: Suzy Mueller Date: Wed, 28 Sep 2022 17:54:14 -0400 Subject: [PATCH] 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 TryBot-Result: Gopher Robot gopls-CI: kokoro Reviewed-by: Robert Findley --- gopls/internal/lsp/code_action.go | 11 ++-- gopls/internal/lsp/mod/diagnostics.go | 19 ++++++- .../misc/testdata/vulndb/golang.org/amod.json | 2 +- gopls/internal/regtest/misc/vuln_test.go | 50 ++++++++++++++++--- 4 files changed, 69 insertions(+), 13 deletions(-) diff --git a/gopls/internal/lsp/code_action.go b/gopls/internal/lsp/code_action.go index 11013b5bb1..e1f0dc23c9 100644 --- a/gopls/internal/lsp/code_action.go +++ b/gopls/internal/lsp/code_action.go @@ -81,8 +81,12 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara if err != nil { return nil, err } - // TODO(suzmue): get upgrades code actions from vulnerabilities. - quickFixes, err := codeActionsMatchingDiagnostics(ctx, snapshot, diagnostics, append(diags, udiags...)) + vdiags, err := mod.ModVulnerabilityDiagnostics(ctx, snapshot, fh) + if err != nil { + return nil, err + } + // TODO(suzmue): Consider deduping upgrades from ModUpgradeDiagnostics and ModVulnerabilityDiagnostics. + quickFixes, err := codeActionsMatchingDiagnostics(ctx, snapshot, diagnostics, append(append(diags, udiags...), vdiags...)) if err != nil { return nil, err } @@ -426,7 +430,8 @@ func codeActionsForDiagnostic(ctx context.Context, snapshot source.Snapshot, sd } func sameDiagnostic(pd protocol.Diagnostic, sd *source.Diagnostic) bool { - return pd.Message == sd.Message && protocol.CompareRange(pd.Range, sd.Range) == 0 && pd.Source == string(sd.Source) + return pd.Message == strings.TrimSpace(sd.Message) && // extra space may have been trimmed when converting to protocol.Diagnostic + protocol.CompareRange(pd.Range, sd.Range) == 0 && pd.Source == string(sd.Source) } func goTest(ctx context.Context, snapshot source.Snapshot, uri span.URI, rng protocol.Range) ([]protocol.CodeAction, error) { diff --git a/gopls/internal/lsp/mod/diagnostics.go b/gopls/internal/lsp/mod/diagnostics.go index 0405fc7687..af5009e7fb 100644 --- a/gopls/internal/lsp/mod/diagnostics.go +++ b/gopls/internal/lsp/mod/diagnostics.go @@ -193,6 +193,22 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, continue } + // Upgrade to the exact version we offer the user, not the most recent. + // TODO(suzmue): Add an upgrade for module@latest. + var fixes []source.SuggestedFix + if fixedVersion := v.FixedVersion; 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()), + AddRequire: false, + GoCmdArgs: []string{req.Mod.Path + "@" + fixedVersion}, + }) + if err != nil { + return nil, err + } + fixes = append(fixes, source.SuggestedFixFromCommand(cmd, protocol.QuickFix)) + } + severity := protocol.SeverityInformation if len(v.CallStacks) > 0 { severity = protocol.SeverityWarning @@ -206,7 +222,8 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot, Code: v.ID, CodeHref: v.URL, // TODO(suzmue): replace the newlines in v.Details to allow the editor to handle formatting. - Message: v.Details, + Message: v.Details, + SuggestedFixes: fixes, }) } diff --git a/gopls/internal/regtest/misc/testdata/vulndb/golang.org/amod.json b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/amod.json index b6790bf29e..59808ed5ff 100644 --- a/gopls/internal/regtest/misc/testdata/vulndb/golang.org/amod.json +++ b/gopls/internal/regtest/misc/testdata/vulndb/golang.org/amod.json @@ -21,7 +21,7 @@ "affected":[ { "package":{"name":"golang.org/amod","ecosystem":"Go"}, - "ranges":[{"type":"SEMVER","events":[{"introduced":"1.0.0"},{"fixed":"1.0.4"}]}], + "ranges":[{"type":"SEMVER","events":[{"introduced":"1.0.0"},{"fixed":"1.0.6"}]}], "ecosystem_specific":{ "imports":[{"path":"golang.org/amod/avuln","symbols":["nonExisting"]}] } diff --git a/gopls/internal/regtest/misc/vuln_test.go b/gopls/internal/regtest/misc/vuln_test.go index f05a959470..c13acf82a2 100644 --- a/gopls/internal/regtest/misc/vuln_test.go +++ b/gopls/internal/regtest/misc/vuln_test.go @@ -13,6 +13,7 @@ import ( "golang.org/x/tools/gopls/internal/lsp/command" "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/internal/testenv" ) @@ -126,12 +127,12 @@ go 1.18 require golang.org/cmod v1.1.3 require ( - golang.org/amod v1.1.3 // indirect + golang.org/amod v1.0.0 // indirect golang.org/bmod v0.5.0 // indirect ) -- go.sum -- -golang.org/amod v1.1.3 h1:E9ohW9ayc6iCFrT/VNq8tCI4hgYM+tEbo8txbtbyS3o= -golang.org/amod v1.1.3/go.mod h1:yvny5/2OtYFomKt8ax+WJGvN6pfN1pqjGnn7DQLUi6E= +golang.org/amod v1.0.0 h1:EUQOI2m5NhQZijXZf8WimSnnWubaFNrrKUH/PopTN8k= +golang.org/amod v1.0.0/go.mod h1:yvny5/2OtYFomKt8ax+WJGvN6pfN1pqjGnn7DQLUi6E= golang.org/bmod v0.5.0 h1:0kt1EI53298Ta9w4RPEAzNUQjtDoHUA6cc0c7Rwxhlk= golang.org/bmod v0.5.0/go.mod h1:f6o+OhF66nz/0BBc/sbCsshyPRKMSxZIlG50B/bsM4c= golang.org/cmod v1.1.3 h1:PJ7rZFTk7xGAunBRDa0wDe7rZjZ9R/vr1S2QkVVCngQ= @@ -188,16 +189,27 @@ func C1() I { func C2() func() { return bvuln.Vuln } --- golang.org/amod@v1.1.3/go.mod -- +-- golang.org/amod@v1.0.0/go.mod -- module golang.org/amod go 1.14 --- golang.org/amod@v1.1.3/avuln/avuln.go -- +-- golang.org/amod@v1.0.0/avuln/avuln.go -- package avuln type VulnData struct {} func (v VulnData) Vuln1() {} func (v VulnData) Vuln2() {} +-- golang.org/amod@v1.0.4/go.mod -- +module golang.org/amod + +go 1.14 +-- golang.org/amod@v1.0.4/avuln/avuln.go -- +package avuln + +type VulnData struct {} +func (v VulnData) Vuln1() {} +func (v VulnData) Vuln2() {} + -- golang.org/bmod@v0.5.0/go.mod -- module golang.org/bmod @@ -234,13 +246,35 @@ func TestRunVulncheckExp(t *testing.T) { }, ).Run(t, workspace1, func(t *testing.T, env *Env) { env.OpenFile("go.mod") - env.ExecuteCodeLensCommand("go.mod", command.RunVulncheckExp) + env.ExecuteCodeLensCommand("go.mod", command.Tidy) + env.ExecuteCodeLensCommand("go.mod", command.RunVulncheckExp) + d := &protocol.PublishDiagnosticsParams{} env.Await( CompletedWork("govulncheck", 1, true), ShownMessage("Found"), - env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/amod`, "vuln in amod"), - env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/bmod`, "vuln in bmod"), + OnceMet( + env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/amod`, "vuln in amod"), + env.DiagnosticAtRegexpWithMessage("go.mod", `golang.org/bmod`, "vuln in bmod"), + ReadDiagnostics("go.mod", d), + ), ) + + env.ApplyQuickFixes("go.mod", d.Diagnostics) + env.Await(env.DoneWithChangeWatchedFiles()) + wantGoMod := `module golang.org/entry + +go 1.18 + +require golang.org/cmod v1.1.3 + +require ( + golang.org/amod v1.0.4 // indirect + golang.org/bmod v0.5.0 // indirect +) +` + if got := env.Editor.BufferText("go.mod"); got != wantGoMod { + t.Fatalf("go.mod vulncheck fix failed:\n%s", compare.Text(wantGoMod, got)) + } }) }