-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat: remediate to greater than or equal versions for github alerts #31393
Conversation
Another reason for this refactor was to be able to enable #31395 |
@Churro I didn't make the changes to OSV alerts yet. This functionality is intended to work side-by-side and allow a later refactoring |
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.
Since matchCurrentVersion
also matches version or range, for consistency I'd advocate calling the field vulnerabilityFixVersion
. With vulnerabilityFix
I'm somewhat lacking the "what it is"-suffix.
I tested this PR on https://github.com/renovate-demo/31393-github-alerts/pulls and the result of PRs created is basically identical to #29600 and the reproducer in https://github.com/renovate-demo/29600-github-vuln-alerts/pulls.
I also tested it with OSV, changing only all occurrences of
The issue is that go uses semver and renovate/lib/workers/repository/process/lookup/filter.ts Lines 78 to 95 in ffe2b4c
However, this shouldn't be an issue with GitHub alerts, as they always return a single version. Just mentioning it with regards to porting this change also to OSV. If OSV advisories use "last affected", it will inevitably result in a constraint like "> 1.2.3". |
@Churro regarding using OSV, can we return vulnerabilityFixVersion as a single version? The warning you describe above is due to Renovate go versioning not having the concept of range syntax plus the lack of semver fallback you already identified. The thing is that in go versioning, ">= 1.3.1" is unnecessary anyway - "1.3.1" is essentially the same thing (although technically it's like ^1.3.1 and does not go past 2.0.0). |
With OSV, it depends which fix info is provided in the advisory:
A workaround to omit the fallback would be to use |
@Churro let's discuss that later on then. This PR already has more logic than it needs to work here, i.e. the support for ranges is unnecessary for the github alerts. If OSV requires special treatment then I'd prefer to do it there |
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.
otherwise LGTM
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.
we should split to smaller functions in a follow up PR
🎉 This PR is included in version 38.88.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Changes
Adds new field
vulnerabilityFixVersion
to use in place ofallowedVersions
for vulnerability rules.Context
When we (currently) use
allowedVersions
for vulnerability fixes then it means we override/break anyallowedVersions
rules which users have configured.Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: