Trusty: Support blocking PRs through reviews #3392
Conversation
| Action: pr_actions.ActionSummary, | ||
| Action: pr_actions.ActionReviewPr, | ||
| Block: map[string]bool{ | ||
| "malicious": true, |
There was a problem hiding this comment.
Isn't it safer if we invert this and don't rely on initialising the bool with true?
Also I think we can even delete this option in the Review action. I mean turning it on kind of overlaps with the other actions and if someone doesn't want Minder to block the PR, they can use some of them, right?
I think this is how the vulnerability/OSV rule works like, right?
There was a problem hiding this comment.
Ah so the idea is that this option is for the review action. In addition to malicious, we will have another entry to block (or not) when deprecated packages are found. This will let users control when they want to block or just get feedback from minder separately for those two specific dependency types.
There was a problem hiding this comment.
Also note that this is initializing missing map entries from the default settings here.
There was a problem hiding this comment.
I'm actually fine with this config granularity @rdimitrov . The way I read it the action tells you the "what" to do with the PR and the "block" tells you which of the attributes to take into account when deciding whether to block. And I also think all of them should be on by default.
There was a problem hiding this comment.
Ah, thanks for flipping the setting! 😃 Yeah, my motivation was to keep vulncheck and trusty behaviour similar, but I agree on the functionality of this + we'll be refactoring that part so we might as well make it available for all PR related Minder actions not just Trusty.
This commit adds to minder the capability to request changes in PRs when minder finds something odd using trusty data. We now also introduce a new setting to disable blocking on malicious deps (all power to the users, but why would you want that?!). Signed-off-by: Adolfo García Veytia (puerco) <puerco@stacklok.com>
|
In the rush to write this yesterday I noticed that the configuration flags I added didn't match those in the PRD. I rewrote the logic to rename the options and to make them configurable per ecosystem. @rdimitrov you'll be glad to see that the logic of the blocking flags is now flipped :) This is the config block now, note that with the last push it is now configurable per ecosystem: action: review
ecosystem_config:
- name: npm
score: 5
allow_malicious: false
- name: pypi
score: 5
allow_malicious: false
- name: go
score: 5
allow_malicious: falseSee mindersec/minder-rules-and-profiles#116 for the updated rule config |
Summary
Minder now has the capability to request changes in PRs when it finds something odd based on Trusty dependency data.
This PR also introduces a new setting in the trusty ruletype configuration to disable blocking when malicious deps are detected (all power to the users, but why would you want that?!).
Fixes #3380
Part of #3379
Change Type
Mark the type of change your PR introduces:
Testing
Review Checklist: