Skip to content
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

When no fix for a CVE exists, don't mark a PR as changes requested #1860

Closed
jhrozek opened this issue Dec 7, 2023 · 7 comments
Closed

When no fix for a CVE exists, don't mark a PR as changes requested #1860

jhrozek opened this issue Dec 7, 2023 · 7 comments
Assignees
Labels
bug Something isn't working P2 Nice to fix: non-critical items that should be evaluated and planned during issue triage priority: medium Medium Priority

Comments

@jhrozek
Copy link
Contributor

jhrozek commented Dec 7, 2023

As discussed in #1806 (comment), when a CVE exists, but no fixed version exists, we should not mark the PR as changes requested (at least not by default)

@jhrozek jhrozek added bug Something isn't working priority: medium Medium Priority labels Dec 7, 2023
@jhrozek
Copy link
Contributor Author

jhrozek commented May 23, 2024

@eleftherias I think you fixed this didn't you?

@eleftherias
Copy link
Contributor

@jhrozek No, I don't think so. The fix I had done was about not commenting a new version on PR when no fixed version exists, but I didn't change the review status.

@evankanderson evankanderson added the P2 Nice to fix: non-critical items that should be evaluated and planned during issue triage label Aug 27, 2024
@evankanderson
Copy link
Member

@ethomson -- we're not sure if this hits the "beta quality bar" list or not

@ethomson
Copy link
Member

I think that it's okay to keep it as "changes requested" - that makes sense within the context of what we're doing (we found a vulnerability, we don't want you to check in a vulnerability, changes requested makes a certain amount of sense). I do think that we should update the verbiage.

Minder found vulnerable dependencies in this PR. Either push an updated
version or accept the proposed changes. Note that accepting the changes will
include Minder as a co-author of this PR.

Given this, I think yes for beta. If we do propose a change, then let's keep this guidance 👆

If we cannot propose a change, let's make the verbiage make sense:

Minder found vulnerable dependencies in this PR, but could not find a new version of the dependency that is not vulnerable. Please push an updated version.

@evankanderson
Copy link
Member

This looks like we need to change the verbiage on the warning.

@eleftherias
Copy link
Contributor

We should consider making this configurable, some users may want a warning.

@eleftherias
Copy link
Contributor

Closing in favour of mindersec/minder-rules-and-profiles#274 in minder-rules-and-profiles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P2 Nice to fix: non-critical items that should be evaluated and planned during issue triage priority: medium Medium Priority
Projects
None yet
Development

No branches or pull requests

4 participants