Give your feedback about the osvVulnerabilityAlerts experimental feature
#20542
Replies: 46 comments 110 replies
-
|
It's a really cool feature! Especially when adding a label like 'security' it gives you a nice cross repository overview. One improvement that would be welcome is the ability to have more control over major dependency updates and the 'dependencyDashboardApproval' flag. Right now, this configuration has no effect when fixedVersion reflects a major upgrade "major": {
"dependencyDashboardApproval": true
}Example issue: GHSA-r8f7-9pfq-mjmv Another improvement / feature request. Would it be possible to reflect the security risk in the PR title? Something like a Low/Medium/High/Critical suffix? |
Beta Was this translation helpful? Give feedback.
-
|
We enabled this feature yesterday and it's really really cool!
+1 - this would be an great enhancement! |
Beta Was this translation helpful? Give feedback.
-
|
i don't like the idea to enable approval for security updates. to easy to miss those updates. |
Beta Was this translation helpful? Give feedback.
-
That does make sense thinking about it. My first reaction was to initially disable it as some developers where surprised. Seeing the Maybe related. Could it be that custom packages rules where |
Beta Was this translation helpful? Give feedback.
-
Not sure about this. Depending on who reports vulns, initially they are often overrated (esp. when found via Bug Bounty programs). Over time, GHSA and other sources react on community feedback, adapt the score and renovate will silently also update the PR description. Changing the PR title would cause additional noise by renovate closing and reopeing PRs. |
Beta Was this translation helpful? Give feedback.
-
|
Relevant to some of the discussions: renovate/lib/config/options/index.ts Lines 1653 to 1672 in e87af92 I'm not aware of anyone modifying that in the past, but it was intended to be possible. It says github-only there, but I think that's only used for documentation. |
Beta Was this translation helpful? Give feedback.
-
Only if the first PR was closed. In this case you'd get a replacement with the new severity. Otherwise, you would see the title updated in-placed without closing/opening. |
Beta Was this translation helpful? Give feedback.
-
|
saw a strange security PR autoclose
|
Beta Was this translation helpful? Give feedback.
-
@viceice, you've hit an edge case that occurs with only 0.58% (83 / 14311) of OSV vulnerabilities (congrats on that 😉):
PR #20512 intends to corrects this. Note that renovate still depends on semantically valid versions for comparisons. Hence, it will still not flag GHSA-gxr4-xjj5-5px2 for jquery 2.2.4, although affected. IMHO such cases should rather be corrected in the OSV advisory itself (1.2 -> 1.2.0), rather than in renovate by trying workarounds like semver-coerced versioning. |
Beta Was this translation helpful? Give feedback.
-
|
Hi @Churro I'm with the OSV team, is there an issue with GHSA-gxr4-xjj5-5px2 that needs to be corrected? I totally agree that we should be fixing the underlying data rather than downstream consumers doing contortions to correct for it. |
Beta Was this translation helpful? Give feedback.
-
|
@andrewpollock, thanks for reaching out. It seems to me there is indeed an issue with GHSA-gxr4-xjj5-5px2:
I ran a check across all OSV datasources that renovate uses and extracted all entries with versions that are listed as In some cases, the GitHub advisories can be corrected. For all others, OSV could tackle this by adding a trailing |
Beta Was this translation helpful? Give feedback.
-
|
Hey @Churro you're a champion! Thanks so much for doing some of the additional legwork to identify problematic records, this directly helps us maintain a high standard of quality in OSV data. FYI we're working on doing stricter validation at record import time as part of google/osv.dev#771 (and subsequently surfacing import issues to our data sources) We've flagged these records with GitHub to determine the best course of action. We would much rather see OSV source data be correct (at the source) than try to paper over it by having OSV.dev do creative interpolation, as you've suggested as one of the options. |
Beta Was this translation helpful? Give feedback.
-
|
@Churro thanks for finding the cause ❤️ can you upstream the version fix too? |
Beta Was this translation helpful? Give feedback.
-
|
I really love the idea of this vulnerability alert. We also had this issue with flapping merge requests in hosted gitlab. All merge requests created by this feature were autoclosed several times. Since we have just started using renovate on that repository it didn't bother that much. I noticed it though. Another thing I noticed was the github ratelimit in this case. We authenticate to github for the release notes already with a personal token, but it seems that toen is not used for these alert. Could this be the reason for the flip flopping? If yes, how can I make renovate use the github token for these calls? |
Beta Was this translation helpful? Give feedback.
-
|
We've been running into this as well – currently maybe 20% of Renovate runs succeed. On balance, it's nice to get vulnerability notifications, but it would be better if Renovate went back to running regularly (when the API rate limit is exceeded, Renovate doesn't do anything - no creating or rebasing of PRs either I believe). |
Beta Was this translation helpful? Give feedback.
-
|
Did you define a Github token (see here)? Vulnerability alerts now also use that token, so that you don't hit the rate limit as fast as before but it may still happen, of course ... |
Beta Was this translation helpful? Give feedback.
-
|
Sorry for the slow response @Churro – I am travelling at the moment. I didn't read the original thread carefully. I am actually on GitHub.com and I'm using the cloud-hosted renovate through Mend.io – I see frequent (~90% of attempts) errors both for personal repositories under @kkom and for work ones under @isometric. |
Beta Was this translation helpful? Give feedback.
-
|
This may be related to #22502 then. |
Beta Was this translation helpful? Give feedback.
-
|
@vquie, flapping PRs are most likely related to hitting the API limit. This has been addressed with renovatebot/osv-offline#230, which found its way into renovate 34.142.0. Can you please check if the issue is gone after updating renovate? |
Beta Was this translation helpful? Give feedback.
-
|
Thanks @Churro . I just updated to the latest helm release and will monitor it. |
Beta Was this translation helpful? Give feedback.
-
|
Currently, renovate raises no vulnerability fix PRs for dependencies that are disabled explicitly or by using a preset like Proposition: Add an opt-in config flag (= disabled by default) to Any thoughts on that? Let me know if this is something you'd also find useful 👍 or not 👎 |
Beta Was this translation helpful? Give feedback.
-
|
Update: Found out that this already possible, even without adding a new config option. Just specify: "osvVulnerabilityAlerts": true,
"vulnerabilityAlerts": {
"enabled": true
}The |
Beta Was this translation helpful? Give feedback.
-
|
Not sure if it's really an issue but I've observed the following message in logs for a project with NPM manager:
Is the unsupported version because of the |
Beta Was this translation helpful? Give feedback.
-
|
Correct, the |
Beta Was this translation helpful? Give feedback.
-
|
Pretty sure it happened on a project with a |
Beta Was this translation helpful? Give feedback.
-
|
What would be the correct way of making renovate still open an MR even if the version is pinned with I have a project which uses @gaeljw did you manage to solve this? |
Beta Was this translation helpful? Give feedback.
-
|
I noticed another peculiarity... It seems that when a package in the Pipfile does not match the case of the package in the OSV, a security update is not created (with the So the Pipfile has: In the log of renovate runs I see (note lowercase pillow): When it outputs the json for the update it outputs this for Pillow (note depName mixedcase and no branch with Let me know if I gave you enough information or if you need me to give you any more details... Geoff |
Beta Was this translation helpful? Give feedback.
-
|
Here is the simple repro case: config.js You will see that the in the log it creates a regular branch not a security branch (look at branchName in the logs) : If you change the Pipfile to be pillow in lowercase and re-run renovate, you see that it then creates a security vulnerability branch... |
Beta Was this translation helpful? Give feedback.
-
|
@gzmarstone, when I find some time the next few days, I'll make a PR to address this... it's an easy fix. |
Beta Was this translation helpful? Give feedback.
-
|
Would something like https://github.com/AppThreat/vulnerability-db positively broaden the available offline data for vulnerability analysis? |
Beta Was this translation helpful? Give feedback.
-
|
I have created an issue for this: renovatebot/osv-offline#630 You can download latest |
Beta Was this translation helpful? Give feedback.
-
|
Hey, it's not quite clear to me from the docs whether the options under |
Beta Was this translation helpful? Give feedback.
-
|
As far as I noticed, |
Beta Was this translation helpful? Give feedback.
-
|
Is there a way to get notified when a vulnerability can't be fixed by a PR? Maybe by creating an issue. Example: the I saw that I can use https://docs.renovatebot.com/configuration-options/#dependencydashboardosvvulnerabilitysummary but it would be nice to get a notification. I'm using gitlab. |
Beta Was this translation helpful? Give feedback.
-
|
I'm trying to get only security PRs to work using the regex manager, yet something appears to go wrong. The debug logs indicate it finds the packages and versions, yet it skips opening any python PR because all dependencies are disabled. I have a reproducer, which does open a PR for the requirements.txt dependency (aiohttp=3.8.3), but not for the regex manager based config (with tornado=6.2.0). The mend renovate app log clearly shows it has found vulnerabilities, yet it only opens a branch for the requirements.txt vulnerability. |
Beta Was this translation helpful? Give feedback.
-
|
This may need to be made into an issue as I'd expect renovate's behavior to be independent of the manager, as long as that manager is correctly configured. |
Beta Was this translation helpful? Give feedback.
-
|
I added Now, despite I like this very much as security should be treated seriously there might be scenarios where something has been accepted as a dev dependency for example and will not be resolved because the new version brings breaking changes for example. How can I make Renovate when a security vulnerability is found and there is a constraint, to respect the constraint and make the information on the dependency dashboard? |
Beta Was this translation helpful? Give feedback.
-
|
I can make it into an discussion itself if would help |
Beta Was this translation helpful? Give feedback.
-
|
Raised a separate discussion #29433 but I think I know my answer by this comment #20542 (comment) |
Beta Was this translation helpful? Give feedback.
-
|
In |
Beta Was this translation helpful? Give feedback.
-
|
It should work to enable it using matchBaseBranches inside packageRules |
Beta Was this translation helpful? Give feedback.
-
|
I haven't been able to make it work. When I put it in a packageRule it does nothing. renovate/lib/config/presets/internal/security.ts Lines 24 to 38 in 833df8e |
Beta Was this translation helpful? Give feedback.
-
|
When using For example we have this renovate config: {
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
"osvVulnerabilityAlerts": true,
"vulnerabilityAlerts": {
"enabled": true
}
}a GitHub Token and a go.mod file like module example.com/m
go 1.21
require (
github.com/argoproj/argo-cd/v2 v2.3.17
k8s.io/kubernetes v1.24.2 // indirect
)If we now run renovate we get security updates for |
Beta Was this translation helpful? Give feedback.
-
|
This sounds like a feature, not a bug |
Beta Was this translation helpful? Give feedback.
-
The doumentation states though:
It still creates updates for indirect dependencies, even if no direct dependency is being updated, which would lead to an update of the indirect dependency. This leads to unwanted behaviour: The default of not updating indirect dependencies is not being taken into account here, which seems to be the root cause for this issue. |
Beta Was this translation helpful? Give feedback.
-
|
The documentation can be updated/fixed, and you are welcome to submit a PR if it's urgent to you. The Please separate this topic off into a new config help discussion with a reproduction repo demonstrating the remediation of an indirect dependency. It's possible that remediation of indirect dependencies can be disabled via existing config options. |
Beta Was this translation helpful? Give feedback.
-
|
Sorry for the late response but in the logs we see this: so |
Beta Was this translation helpful? Give feedback.
-
|
If you disable a dependency by config then it's not intended that package alerts would reenable it. If you can reproduce it in a public repo then please create a new discussion |
Beta Was this translation helpful? Give feedback.
-
|
Is there a way to set "packageRules": [
{
"matchManagers": ["npm"],
"rangeStrategy": "bump",
"osvVulnerabilityAlerts": true
}
]...and a package with |
Beta Was this translation helpful? Give feedback.
-
|
@aarongoldenthal please raise that as a separate "suggest an idea" discussion |
Beta Was this translation helpful? Give feedback.
-
Raised the proposal at #30854 |
Beta Was this translation helpful? Give feedback.
-
|
I like the feature but we are running into issues in our setup. We use bundle-audit in our CI to check if the dependencies include known vulnerabilities. This is for ruby btw. Bundle-audit checks also indirect dependencies, which totally makes sense. Unfortunately renovate does not create prs for these. According to the documentation this is on purpose. However it would be nice if it would also update indirect dependencies, as those are equally dangerous imo. Using bundler those can be updated also if they are not part of the Gemfile. However you cannot specify a specific target version. but e.g. renovate could try to run Edit: Thinking about it more I think this could create kind of a loop or prs that update something but not fix the vulnerability. renovate would need to extract the version of the indirect dependenc from the Gemfile.lock to actually verify that a safe version is used. Not sure how straight forward that is |
Beta Was this translation helpful? Give feedback.
-
|
Hi, I would like to propose a feature request.
|
Beta Was this translation helpful? Give feedback.
-
|
Actually the 'severity' field is part of the schema, but it's often not set. The question is how should renovate handle the ones without 'severity' rating. There's even an option to set different severities for different versions and then the toplevel 'severity' should not be set. Not too easy to automate it. |
Beta Was this translation helpful? Give feedback.
-
|
Works fine so far for us. For critical cve we need to back port on older versions. Don't know if we can already do that with some workaround in the config. |
Beta Was this translation helpful? Give feedback.
-
|
Hi @ggjulio, 👋 I created a config that I think will do what you want. Please read (and subscribe) to this discsusion: |
Beta Was this translation helpful? Give feedback.
-
|
We've been using this for over a year now and get a lot of value from it. One additional thing we'd like to be able to do is add the vuln id's as labels to the MRs. Does anyone know if this is possible? |
Beta Was this translation helpful? Give feedback.
-
|
"Great feature! The osvVulnerabilityAlerts provide valuable insights, but it would be helpful to have more detailed filtering options for different severity levels. Also, integrating notifications for high-priority alerts could make it even more useful. Keep up the good work!" |
Beta Was this translation helpful? Give feedback.
-
|
It looks like security updates disregard groups. Is this by design? I don't see how this would be a good thing. |
Beta Was this translation helpful? Give feedback.
and others
-
Tell us how you like the new
osvVulnerabilityAlertsexperimental feature. What works for you, what doesn't work for you? Do you see anything we could improve?Let us know what you think! 😉
Beta Was this translation helpful? Give feedback.
All reactions