Skip to content

Conversation

mwager
Copy link
Contributor

@mwager mwager commented Jul 7, 2025

This PR enhances the Kiuwan SCA Parser by modifying the logic to create one finding per component, instead of taking only the first component from the components array.

The buggy code:

grafik

Motivation

In the current implementation, for a given CVE from a Kiuwan SCA scan, only the first component listed is used to create a finding. However, many CVEs in Kiuwan are related to multiple components. This leads to loss of detail and incomplete representation of risks in DefectDojo.

Test results

  • Manual test with real-world Kiuwan JSON input shows that all findings are now preserved and properly associated with the correct component.

This checklist is for your information.

  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.11 compliant.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

@mwager mwager requested review from Maffooch and mtesauro as code owners July 7, 2025 13:09
@github-actions github-actions bot added the parser label Jul 7, 2025
@mwager
Copy link
Contributor Author

mwager commented Jul 7, 2025

TODO: Ideally you extend the test suite in tests/ and dojo/unittests to cover the changed in this PR.

I will soon check how to update & run unit tests, any info/docs on it appreciated :)

@valentijnscholten
Copy link
Member

Copy link
Contributor

@Maffooch Maffooch left a comment

Choose a reason for hiding this comment

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

@valentijnscholten what do you think of this approach?

@Maffooch
Copy link
Contributor

@mwager how are things going here?

@mwager
Copy link
Contributor Author

mwager commented Jul 29, 2025

@Maffooch Sorry, I forgot to update the tests. Done from my side now!

@mwager
Copy link
Contributor Author

mwager commented Aug 8, 2025

@Maffooch @valentijnscholten Does it look good to you? :)

@valentijnscholten valentijnscholten added this to the 2.50.0 milestone Aug 24, 2025
@mwager
Copy link
Contributor Author

mwager commented Aug 26, 2025

@valentijnscholten should be good to go now?

cc @shodanwashere

@valentijnscholten
Copy link
Member

We've realized that the PR will result in violations of the (logical) "unique" constraint for the unique_id_from_tool field. We're discussing how we could handle this scenario now and in the future. Possible compromise for this parser could be to store the row id in the vuln_id_from_tool field? The unique_id_from_tool is not used in dedupe currently for this parser.

@valentijnscholten valentijnscholten modified the milestones: 2.50.0, 2.50.1 Sep 2, 2025
@mwager
Copy link
Contributor Author

mwager commented Sep 3, 2025

@shodanwashere Could you have a look and discuss this with the team? Thx!

@valentijnscholten valentijnscholten modified the milestones: 2.50.1, 2.50.2 Sep 9, 2025
@shodanwashere
Copy link
Contributor

@valentijnscholten true, since two components could have the exact same vuln, the IDs would no longer be unique. does the unique_id_from_tool have some constraint preventing it from being null? otherwise, we could go with having the id on the vuln_id_from_tool field.

@valentijnscholten
Copy link
Member

@shodanwashere Using the vuln_id_from_tool field is acceptable in this case.

@mwager
Copy link
Contributor Author

mwager commented Sep 16, 2025

@valentijnscholten does it look good now?

cc @shodanwashere

@shodanwashere
Copy link
Contributor

i'll assume that's what we're intending to do. @valentijnscholten will it work like how @mwager has implemented?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants