-
Notifications
You must be signed in to change notification settings - Fork 416
update govulncheck integration #431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Implementation looks good!, can you move the deleted test-project fixtures into sourceanalysis so we can still have the integration test?
internal/sourceanalysis/go.go
Outdated
if !ok { // If vulnerability not found, check if it contain any source information | ||
fillNotImportedAnalysisInfo(vulnsByID, vulnID, pv, analysis) | ||
continue | ||
} | ||
// Module list is unlikely to be very big, linear search is fine | ||
containsModule := slices.ContainsFunc(gvcVuln.Modules, func(module *govulncheck.Module) bool { | ||
return module.Path == pv.Package.Name | ||
var modulePaths []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part can be replaced a direct map key check, something like:
called, hasModule := moduleToCalled[pv.Package.Name];
(*analysis)[vulnID] = models.AnalysisInfo{
Called: hasModule && called,
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! I left out the hasModule
bit, since we want to set Called
to false
regardless of if pv.Package.Name
is not in the map, or is present but has a value of false
Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
No description provided.