Guided Remediation: non-interactive mode #798
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #798 +/- ##
===========================================
+ Coverage 58.89% 71.79% +12.90%
===========================================
Files 116 118 +2
Lines 8452 8677 +225
===========================================
+ Hits 4978 6230 +1252
+ Misses 3271 2078 -1193
- Partials 203 369 +166 ☔ View full report in Codecov by Sentry. |
|
|
||
| cmd := opts.RelockCmd | ||
| if cmd == "" { | ||
| cmd = "npm install --package-lock-only" |
There was a problem hiding this comment.
does this mean that npm needs to be installed to re-lock?
There was a problem hiding this comment.
Yeah, we need npm to regenerate the package-lock.json from scratch.
It doesn't need to be installed if osv-scanner fix isn't regenerating the lockfile, but npm install would need to be run eventually.
| } | ||
|
|
||
| res.FilterVulns(opts.MatchVuln) | ||
| // TODO: count vulnerabilities per unique version as scan action does |
There was a problem hiding this comment.
how much work is this to do?
There was a problem hiding this comment.
It's probably not too much work for just the counts, as long as we still take the all-or-nothing approach to resolving the vulns.
I think I'll look at this after I port the interactive mode, since it might change how I list the vulnerabilities there.
| // returns the top {maxUpgrades} compatible patches, the number of vulns fixed, and the number unfixable vulns | ||
| // if maxUpgrades is < 0, do as many patches as possible | ||
| func autoChooseRelockPatches(diffs []resolution.ResolutionDiff, maxUpgrades int) ([]manifest.DependencyPatch, int, int) { | ||
| unfixableVulnIDs := make(map[string]struct{}) |
There was a problem hiding this comment.
add some high level comments explaining the logic in this function.
| Version: p.OrigVersion, | ||
| } | ||
|
|
||
| if _, seen := seenVKs[vk]; maxUpgrades != 0 && !seen { |
There was a problem hiding this comment.
add a comment explaining this loop logic.
Added two basic end-to-end tests, bringing the coverage back up to ~70%. Might make the tests take slightly longer to run. Hopefully these don't break easily if new versions of packages are released, but I've tried to pick packages that are unlikely to change. |
|
Is this ready for merge @michaelkedar ? |
|
Oh yeah - this was ready to merge last week 🙂 |
It actually can be used now 🎉
This is basically re-written from the internal equivalent, but it should be functionally similar.
Probably will end up extracting some code to use with the interactive code when I end up needing it.
imo, non-interactive mode is currently too opaque to be generally useful - there's no indication of what vulnerabilities exist and which get fixed.