-
-
Notifications
You must be signed in to change notification settings - Fork 595
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: Add notification when severity of a vulnerability changes #2730
base: master
Are you sure you want to change the base?
Feat: Add notification when severity of a vulnerability changes #2730
Conversation
651a984
to
2c89a1a
Compare
if (vo.getVulnerabilityUpdateDiff() != null) { | ||
builder.add("old severity", vo.getVulnerabilityUpdateDiff().getOldSeverity().toString()); | ||
} |
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.
Thinking of future use-cases here, where perhaps we would want to capture more than just a change in severity, WDYT about having an old
and new
object instead? This would give us the flexibility to add more properties of interest over time, e.g. CVSS or EPSS scores.
Looking at popular solutions for Change Data Capture, like Debezium, they typically do this, too: https://debezium.io/documentation/reference/stable/connectors/mysql.html#mysql-update-events
I'd imagine something like:
{
"component": {
...
},
"project": {
...
},
"vulnerability": {
"old": {
"severity": "MEDIUM"
},
"new": {
"severity": "HIGH"
}
}
Where old
and new
may just as well be before
and after
.
From the DT perspective, this form of communicating changes could be re-used in other notification types as well. WDYT?
cc @Mvld3r
if (!affectedProjects.containsKey(c.getProject().getId())) { | ||
affectedProjects.put(c.getProject().getId(), qm.detach(Project.class, c.getProject().getId())); | ||
} |
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.
This computation needs to happen outside of the for
loop that dispatches notifications.
Otherwise affectedProjects
will grow with each iteration, resulting in all components other than the very last one to not get the complete set of projects.
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.
Please also include an example JSON of the new notification on this page.
Signed-off-by: Enora Germond <[email protected]>
2c89a1a
to
14b68e0
Compare
Thank you for your feedback. I modified the generation of the JSON structure accordingly, PTAL |
Thanks for the contributions @Ehoky! I'm having a hard time visualizing how this would actually work if 100 CVEs get updated. Is it at a global level? a component level? would you pop up 100 alerts? I definitely have been wanting a notification that tells me if something in a project or about a component has changed. If we think of some BOMs having THOUSANDS of components, or a DT with hundreds of projects, would you inundate the user with too many notifications? Would there be a separate page to show the changes? |
Description
Feat: Add notification when severity of a vulnerability changes
Addressed Issue
#2361
Checklist