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

Integrate vulnerability policy evaluation into scan result processing #474

Merged
merged 12 commits into from
Dec 15, 2023

Conversation

nscuro
Copy link
Member

@nscuro nscuro commented Dec 6, 2023

Description

This PR integrates the evaluation and application of vulnerability policies into the vulnerability scan result processing logic. The application includes both analyses and custom ratings, as well as according population of the audit trail.

In order to reduce the performance hit for doing this, the code has been slightly refactored to utilize batched SQL statements wherever possible. The majority of the logic has been migrated to use JDBI instead of DataNucleus, which again results in performance gains due to less ORM overhead.

Addressed Issue

Closes DependencyTrack/hyades#940

Additional Details

  • There's quite a bit of code in VulnerabilityScanResultProcessor that could theoretically be moved to separate classes. I left it there for now, as it is kind of specific to what the processor needs. It may not be applicable to other areas of the codebase.
  • PROJECT_AUDIT_CHANGE notifications are currently not sent for analyses applied via policy. I added TODOs in the code and raised Send PROJECT_AUDIT_CHANGE notifications for analyses applied via policy hyades#968 to implement that.
  • NEW_VULNERABILITY and NEW_VULNERABLE_DEPENDENCY notifications do not currently reflect the rating overwrites applied via policy. I added FIXMEs in the code and raised Rating overrides should reflect in notifications hyades#967 to address that.

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

@nscuro nscuro added the enhancement New feature or request label Dec 6, 2023
@nscuro nscuro force-pushed the issue-940 branch 3 times, most recently from 257fda3 to 7c9361f Compare December 10, 2023 22:19
@nscuro nscuro force-pushed the issue-940 branch 2 times, most recently from fd778af to e4748ae Compare December 13, 2023 10:23
@nscuro nscuro marked this pull request as ready for review December 13, 2023 16:01
@mehab mehab self-requested a review December 13, 2023 16:37
mehab
mehab previously approved these changes Dec 14, 2023
Copy link
Collaborator

@mehab mehab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Signed-off-by: nscuro <nscuro@protonmail.com>
Signed-off-by: nscuro <nscuro@protonmail.com>
Not sure why this changed as I didn't change the vulnerability sync logic. But the new values are in fact correct, according to https://owasp-risk-rating.com/?vector=(SL:1/M:4/O:4/S:9/ED:7/EE:3/A:4/ID:3/LC:9/LI:1/LAV:5/LAC:1/FD:3/RD:4/NC:7/PV:9).

Signed-off-by: nscuro <nscuro@protonmail.com>
…otos

Signed-off-by: nscuro <nscuro@protonmail.com>
Signed-off-by: nscuro <nscuro@protonmail.com>
Signed-off-by: nscuro <nscuro@protonmail.com>
... and record respective changes in the audit log.

Signed-off-by: nscuro <nscuro@protonmail.com>
Signed-off-by: nscuro <nscuro@protonmail.com>
Signed-off-by: nscuro <nscuro@protonmail.com>
Signed-off-by: nscuro <nscuro@protonmail.com>
Signed-off-by: nscuro <nscuro@protonmail.com>
Signed-off-by: nscuro <nscuro@protonmail.com>
Copy link
Collaborator

@mehab mehab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nscuro nscuro merged commit 850a988 into main Dec 15, 2023
6 checks passed
@nscuro nscuro deleted the issue-940 branch December 15, 2023 11:54
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2024
@nscuro nscuro added this to the 5.3.0 milestone Feb 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate VulnerabilityPolicyEvaluator in VulnerabilityScanResultProcessor
2 participants