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

Fix cvss and response override #248

Merged
merged 4 commits into from
Jun 21, 2024
Merged

Conversation

Dakes
Copy link
Contributor

@Dakes Dakes commented Jun 18, 2024

This fixes the problem, that cvss and response by overrides aren't included in Findings.

I think this also, at least partially, fixes #222

@Dakes Dakes requested a review from izar as a code owner June 18, 2024 13:29
Copy link
Collaborator

@izar izar left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@izar
Copy link
Collaborator

izar commented Jun 18, 2024

Can you add this case to the tests, before we merge? Thanks!

@Dakes
Copy link
Contributor Author

Dakes commented Jun 21, 2024

I now added a test for the private function encode_threat_data, because the alternative would be to test the generated reports. But I couldn't get the threats to show up in the report during the test. And it would also change the results if the threats.json would be updated.

Also during writing the test I noticed another small bug, which I fixed in the last commit. If the inScope property of an Element was set to False, it would not reset the Findings, causing other tests to fail, besides the one, where the Finding was set.
With these changes there is no code any more, which requires this fix, but I thought I'd add it anyway.

@izar
Copy link
Collaborator

izar commented Jun 21, 2024

Thanks!

@izar izar merged commit e96ee5f into OWASP:master Jun 21, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot override findings, threats remain, DFD impacted, exception thrown for overrides len > 1
2 participants