internal/lsp/mod: adjust vulncheck diagnostics suppression logic

Vulnerabilities associated with a module should be suppressed
after the module is updated. Previously, we checked whether the
module version in go.mod 'require' matches the FoundVersion reported
by the vulncheck. However, we realized that we cannot always assume
the module version in require is the actually used module version
(due to how minimal version selection works, and how exclude/replace
affects). Instead check whether the module version is newer or equals
to the suggested fixed version and if this go.mod require is newer,
assume that the user updated the module already and suppress diagnostics
about the module. This is not perfect but a heuristic to reduce
confusion from the stale vulncheck report right after applying the
quick fixes and upgrading modules.

Change-Id: I40f4c3e70b19af3f6edd98f30de3ccb7a6bd7498
Reviewed-on: https://go-review.googlesource.com/c/tools/+/450277
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
This commit is contained in:
Hana (Hyang-Ah) Kim 2022-11-14 07:28:05 -05:00 committed by Hyang-Ah Hana Kim
parent c099dff179
commit fc039936a9
1 changed files with 20 additions and 11 deletions

View File

@ -215,13 +215,23 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
var warning, info []string
for _, mv := range vulns {
mod, vuln := mv.mod, mv.vuln
// Only show the diagnostic if the vulnerability was calculated
// for the module at the current version.
// TODO(hyangah): this prevents from surfacing vulnerable modules
// since module version selection is affected by dependency module
// requirements and replace/exclude/go.work use. Drop this check
// and annotate only the module path.
if semver.IsValid(mod.FoundVersion) && semver.Compare(req.Mod.Version, mod.FoundVersion) != 0 {
// It is possible that the source code was changed since the last
// govulncheck run and information in the `vulns` info is stale.
// For example, imagine that a user is in the middle of updating
// problematic modules detected by the govulncheck run by applying
// quick fixes. Stale diagnostics can be confusing and prevent the
// user from quickly locating the next module to fix.
// Ideally we should rerun the analysis with the updated module
// dependencies or any other code changes, but we are not yet
// in the position of automatically triggerring the analysis
// (govulncheck can take a while). We also don't know exactly what
// part of source code was changed since `vulns` was computed.
// As a heuristic, we assume that a user upgrades the affecting
// module to the version with the fix or the latest one, and if the
// version in the require statement is equal to or higher than the
// fixed version, skip generating a diagnostic about the vulnerability.
// Eventually, the user has to rerun govulncheck.
if mod.FixedVersion != "" && semver.IsValid(req.Mod.Version) && semver.Compare(mod.FixedVersion, req.Mod.Version) <= 0 {
continue
}
if !vuln.IsCalled() {
@ -233,15 +243,14 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
if fixedVersion := mod.FixedVersion; semver.IsValid(fixedVersion) && semver.Compare(req.Mod.Version, fixedVersion) < 0 {
cmd, err := getUpgradeCodeAction(fh, req, fixedVersion)
if err != nil {
return nil, err
return nil, err // TODO: bug report
}
// Add an upgrade for module@latest.
// TODO(suzmue): verify if latest is the same as fixedVersion.
latest, err := getUpgradeCodeAction(fh, req, "latest")
if err != nil {
return nil, err
return nil, err // TODO: bug report
}
fixes = []source.SuggestedFix{
source.SuggestedFixFromCommand(cmd, protocol.QuickFix),
source.SuggestedFixFromCommand(latest, protocol.QuickFix),
@ -250,7 +259,7 @@ func ModVulnerabilityDiagnostics(ctx context.Context, snapshot source.Snapshot,
}
if len(warning) == 0 && len(info) == 0 {
return nil, nil
continue
}
severity := protocol.SeverityInformation
if len(warning) > 0 {