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

Feat: Add notification when severity of a vulnerability changes #2730

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ehoky
Copy link

@Ehoky Ehoky commented May 4, 2023

Description

Feat: Add notification when severity of a vulnerability changes

Addressed Issue

#2361

Checklist

  • I have read and understand the contributing guidelines
  • This PR fixes a defect, and I have provided tests to verify that the fix is effective
  • This PR implements an enhancement, and I have provided tests to verify that it works as intended
  • This PR introduces changes to the database model, and I have added corresponding update logic
  • This PR introduces new or alters existing behavior, and I have updated the documentation accordingly

@Ehoky Ehoky force-pushed the feat-2361-indicate-update-of-vulnerability-severity-via-notification branch from 651a984 to 2c89a1a Compare June 26, 2023 08:36
Comment on lines 408 to 424
if (vo.getVulnerabilityUpdateDiff() != null) {
builder.add("old severity", vo.getVulnerabilityUpdateDiff().getOldSeverity().toString());
}
Copy link
Member

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()));
}
Copy link
Member

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.

Copy link
Member

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 <enora.germond@deveryware.com>
@Ehoky Ehoky force-pushed the feat-2361-indicate-update-of-vulnerability-severity-via-notification branch from 2c89a1a to 14b68e0 Compare July 18, 2023 15:05
@Ehoky
Copy link
Author

Ehoky commented Jul 18, 2023

Thank you for your feedback. I modified the generation of the JSON structure accordingly, PTAL

@melba-lopez melba-lopez added the enhancement New feature or request label Jul 28, 2023
@melba-lopez
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants