feat: add experimental support for updating config files#2534
feat: add experimental support for updating config files#2534G-Rath wants to merge 43 commits intogoogle:mainfrom
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
1b5d45c to
bdd81c3
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2534 +/- ##
==========================================
+ Coverage 67.99% 68.14% +0.15%
==========================================
Files 173 174 +1
Lines 13387 13511 +124
==========================================
+ Hits 9102 9207 +105
- Misses 3576 3589 +13
- Partials 709 715 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Being able to automatically remove not found vulnerabilities from the config files seem quite useful.
However I'm not sure how much we want to encourage people to automatically add all their vulns to the ignore file. At least we should separate the two functionalities, maybe by making UpdateConfigIgnores not a boolean flag
| id = "GHSA-123" | ||
|
|
||
| [[IgnoredVulns]] | ||
| id = "GHSA-456" |
There was a problem hiding this comment.
Should we add a comment/reason saying this is automatically added by the new flag?
There was a problem hiding this comment.
at this point I'm leaning to not doing that just for simplicity, and I'd rather add it later to see how we feel - I'll explore this once I'm done with the core implementation
I originally felt against this kind of thing too, similar to also having tools like this exit as an error if there were unused ignores, but I've found in practice at least in an agency company kind of context it can really help reduce the friction - I'm able to tell other developers who are less familiar with this space to just add "xyz flag" rather than needing to try explain how to write ignores (which they can still learn about because they see the diff + it helps keep the actual order and format consistent), and when onboarding apps from other vendors we can get the security pipelines setup faster by initially ignoring all existing vulns and then go back to triage. With which has no hope of preserving any other config content including other properties and comments (the latter of which are not being preserved "yet")
I really like this idea, because then we could also later introduce an "interactive" mode which could let you set reasons and "until" dates. One side thing to consider with this is if we do the updating before or after our filtering - currently I have it being done before which means the tool always presents "no vulns found" and with a non-zero exit code; currently Since this is being added an experimental flag we don't need to decide right now, but something to think about - I can explore this more once we've gotten the rest of the implementation settled, since it should just be a case of moving a function call to see the difference 🤷 |
0b21ce3 to
4da338d
Compare
495d044 to
880a80c
Compare
This comment was marked as resolved.
This comment was marked as resolved.
d26adc2 to
a99c857
Compare
a99c857 to
72b86d8
Compare
71c87bb to
25dc0aa
Compare
25dc0aa to
0ba5d86
Compare
This is an attempt to port the
--update-config-ignoresfeature fromosv-detector, which has been useful especially for onboarding new codebases and updating existing ones after patching has been done to prune out old entries.Currently this does not preserve comments or specific formatting, but I'm hopeful those are things we can improve on future especially if this proves to be a popular flag