JFrog xray api corrections #7190
Conversation
madeoninfo
commented
Nov 28, 2022
madeoninfo
commented
- Removed populating the to-be-deprecated cve and replaced it with populating unsaved_vulnerability_ids
- Added population of the vuln_id_from_tool (XRAY-xxxxxx)
- Improved the unique_id_from_tool to make sure that all cases are covered (With unique id, only cve, and no ids provided by Xray)
- Title changed to the summary of the Xray finding
- Minor code refactoring changes
- Removed populating the to be deprecated cve and replaced it with populating unsaved_vulnerability_ids - Added population of the the vuln_id_from_tool (XRAY-xxxxxx) - Improved the unique_id_from_tool to make sure that all cases are covered (With unique id, only cve and no ids provided by Xray) - Title changed to the summary of the Xray finding - Minor code structure changes -
|
There is no publicly available reference for XRAY findings, hence the lack of url |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
- Removed unused import to pass flake8 check - Corrected unit test to include the check for the unsaved vulnerability ids - Replaced deprecated assertEquals with assertEqual
…-DefectDojo into jfrog_xray_api
|
@madeoninfo this looks good to me so far. Left one comment above. |
- Added the Artifact unique sha256 in the parser to uniquely identify issues per artifact (microservice) - Altered the unit tests to include more meaningful data - Added a unit test to assure that the unique id is created as expected
- Getting the sha256 of the Artifact from the right location
|
After adding the MD5 hash id I am having some strange issues where the duplicates are not identified, even though the unique ids are the same. It might be my setup so please don't merge yet! Will have a look over the weekend |
|
Same issue with sha256 hash. I double-checked the database and with 2 identical unique ids, the duplicate is not detected. It is only noted as a similar finding. Any idea why this is the case?
|
|
One thing that I noticed is that all necessary services are up but Celery is reported in the settings that it is not running
|
|
It was the celery worker on overdrive. It managed to catch up after a few hours and the duplicates have been discovered!
Good to go as is ;) |
|
@madeoninfo awesome work. I'm reviewing your PR. |
|
Hi @damiencarol, I just realised something. The top level Artifact (microservice) is also a component. It would make more sense to have this mentioned as a component in DefectDojo instead of the second level component. Reporting would be more intuitive for developers and they could still see the 2nd level dependent component in the path. Is there a way to pass the 2 options as a parameter or do I need to create a separate parser? |
- added the 2nd level component in the description - corrected the unit tests to accommodate the switch from 2nd to 1st level component that a finding is discovered in
…tespace after ',' (E231)
|
Sorry for changing my mind about the parser level of component tracking and grouping. It makes much more sense to have the 1st level component (Artifact/Microservice) the one that is tracked in DefectDojo. When looking into the findings it is much more intuitive for the developers to focus on one Artifact at a time to resolve the 3rd party, 2nd level, component findings that are affecting the 1st level component. Filtering the findings is easier and the reporting is more intuitive and easy to understand by the developers and the client. |
For some reason the integration test for deleting a finding did not work. This is probably not related to the parser of the PR.
|
Hi @damiencarol and @Maffooch, some integration tests are failing and I'm not sure if they are related to the parser changes or not. |
|
@madeoninfo this is good work. Sadly I didn't time to provide more feedback. Some comments you've made are right. |
