Skip to content

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
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