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

Add support for storing exploitability and weighted severity #1646

Merged
merged 10 commits into from
Nov 19, 2024

Conversation

ziadhany
Copy link
Collaborator

@ziadhany ziadhany commented Nov 9, 2024

Vulnerability model.
Create a pipeline for vulnerability risk assessment.

Signed-off-by: ziad hany <ziadhany2016@gmail.com>
Copy link
Contributor

@tdruez tdruez left a comment

Choose a reason for hiding this comment

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

See the various comments.
Also, do we have unit tests for all the new code?

vulnerabilities/models.py Outdated Show resolved Hide resolved
vulnerabilities/models.py Outdated Show resolved Hide resolved
vulnerabilities/models.py Outdated Show resolved Hide resolved
vulnerabilities/risk.py Outdated Show resolved Hide resolved
vulnerabilities/risk.py Outdated Show resolved Hide resolved
vulnerabilities/risk.py Outdated Show resolved Hide resolved
Comment on lines 127 to 129
data-tooltip="Exploitability refers to the potential or probability of a software package vulnerability being
exploited by malicious actors to compromise systems, applications, or networks.
It is determined automatically by the discovery of exploits.">
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be taken from the model help instead of duplicated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, you are right, but I think we should handle this separately because we do this for a lot of fields. Based on my understanding, there is no direct way to display the help_text of the model without using a view, form, or a template tag.

vulnerabilities/templates/vulnerability_details.html Outdated Show resolved Hide resolved
…_score function. Rename the help text for the model.

Signed-off-by: ziad hany <ziadhany2016@gmail.com>
Signed-off-by: ziad hany <ziadhany2016@gmail.com>
@TG1999 TG1999 added the 2-next label Nov 13, 2024
@TG1999
Copy link
Contributor

TG1999 commented Nov 13, 2024

@ziadhany please attach the issues that will be fixed by this PR. Additionally please do not merge this until these 2 #1649 #1636 are merged.

Copy link
Contributor

@tdruez tdruez left a comment

Choose a reason for hiding this comment

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

Hi @ziadhany we are nearly there. See the few minor comments.

Also, could you provide some input on aboutcode-org/dejacode#194 (comment)

In the design documents, the decimal values are always presented with 1 decimal place: 9.0 - 10.0 but the implementation was made with 2 on the VCIO side. I don't know if that was decided on purpose, but I'm not sure that the second decimal place is adding any values. It makes the UI more dense and does not fit the filter choices. Let's clarify this.

What's your take on this? Was it a particular reason to go with 2 decimal places? I just want to make sure that we are consistent across the apps.

vulnerabilities/models.py Outdated Show resolved Hide resolved
vulnerabilities/tests/test_risk.py Outdated Show resolved Hide resolved
@TG1999
Copy link
Contributor

TG1999 commented Nov 14, 2024

@ziadhany #1649 #1636 are merged now, please mention the issue in ticket description.

vulnerabilities/models.py Outdated Show resolved Hide resolved
# Conflicts:
#	vulnerabilities/models.py
#	vulnerabilities/pipelines/compute_package_risk.py
#	vulnerabilities/risk.py
… api_v2

Signed-off-by: ziad hany <ziadhany2016@gmail.com>
vulnerabilities/models.py Outdated Show resolved Hide resolved
Signed-off-by: ziad hany <ziadhany2016@gmail.com>
Copy link
Collaborator Author

@ziadhany ziadhany left a comment

Choose a reason for hiding this comment

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

@tdruez I've updated the help text and resolved the merge conflict. Please let me know if I missed anything so I can address it as soon as possible.

vulnerabilities/risk.py Outdated Show resolved Hide resolved
Comment on lines 127 to 129
data-tooltip="Exploitability refers to the potential or probability of a software package vulnerability being
exploited by malicious actors to compromise systems, applications, or networks.
It is determined automatically by the discovery of exploits.">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, you are right, but I think we should handle this separately because we do this for a lot of fields. Based on my understanding, there is no direct way to display the help_text of the model without using a view, form, or a template tag.


for vulnerability in progress.iter(affected_vulnerabilities.paginated()):

vulnerability = compute_vulnerability_risk(vulnerability)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

vulnerabilities/models.py Outdated Show resolved Hide resolved
Copy link
Contributor

@TG1999 TG1999 left a comment

Choose a reason for hiding this comment

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

See my comment!

Copy link
Member

@keshav-space keshav-space left a comment

Choose a reason for hiding this comment

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

Thanks @ziadhany, see some suggestions to improve performance.

vulnerabilities/pipelines/compute_package_risk.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/compute_package_risk.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/compute_package_risk.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/compute_package_risk.py Outdated Show resolved Hide resolved
vulnerabilities/pipelines/compute_package_risk.py Outdated Show resolved Hide resolved
vulnerabilities/risk.py Outdated Show resolved Hide resolved
…or compute_and_store_package_risk_score

Signed-off-by: ziad hany <ziadhany2016@gmail.com>
Update the tests for exploits and the simple_risk_pipeline.

Signed-off-by: ziad hany <ziadhany2016@gmail.com>
@ziadhany
Copy link
Collaborator Author

@keshav-space Thank you for the review, Please let me know if I overlooked anything, and I hope we can move forward with the merge.

Signed-off-by: ziad hany <ziadhany2016@gmail.com>
Copy link
Member

@keshav-space keshav-space left a comment

Choose a reason for hiding this comment

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

Thanks @ziadhany, Looking Good!

Copy link
Contributor

@TG1999 TG1999 left a comment

Choose a reason for hiding this comment

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

LGTM!

@TG1999 TG1999 merged commit 9c23eb8 into aboutcode-org:main Nov 19, 2024
5 checks passed
@pombredanne pombredanne added this to the v35.0.0 - 2-next milestone Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

5 participants