Fix #2955: Import components that are located under 'metadata.component.components' #2956
Conversation
…etadata.component.components'. Signed-off-by: Roland Asmann <[email protected]>
| @@ -92,6 +92,14 @@ public static List<Component> convertComponents(final QueryManager qm, final Bom | |||
| } | |||
| } | |||
| } | |||
| if (bom.getMetadata() != null && bom.getMetadata().getComponent() != null && bom.getMetadata().getComponent().getComponents() != null) { | |||
There was a problem hiding this comment.
As per the spec, such nesting is allowed even with component.components.
| for (int i = 0; i < bom.getMetadata().getComponent().getComponents().size(); i++) { | ||
| final org.cyclonedx.model.Component cycloneDxComponent = bom.getMetadata().getComponent().getComponents().get(i); | ||
| if (cycloneDxComponent != null) { | ||
| components.add(convert(qm, cycloneDxComponent, project)); |
There was a problem hiding this comment.
Can we track the bom-ref to avoid duplicate components?
|
I've been checking the specs again:
So, yes, we could, but only if they are used and are not unique. |
What do you think about the value returned by the convert method? Does it include a bom-ref? |
From what I can tell, the convert method simply copies the bom-ref it received from the SBOM. This makes sense, because it would need this ref to resolve the dependencies later on. Also, when there are duplicate components, the refs should (imho) be the same (and not unique), otherwise the dependency-tree would not be correct! During the conversion, DT does check if a certain component already exists (in persistent storage) for the project, however components are only persisted after the whole SBOM has been read and converted. This check is done on any of several fields (purl, swid, cpe, group/name/version-combo), which we could maybe do when converting as well? Anyway, I just tested importing the same SBOM again and now there are no more duplicates! This is similar to something I had already discussed with the devs of cyclonedx-cocoapods. It's not 100% the same problem, but there it was decided to document this behavior, as it was not actually incorrect. |
Signed-off-by: Roland Asmann <[email protected]>
|
I implemented a simple duplicate-check on |
| for (org.cyclonedx.model.Component component : components) { | ||
| if (component != null) { | ||
| boolean hasPurl = component.getPurl() != null && !component.getPurl().isBlank(); | ||
| if (hasPurl && purls.contains(component.getPurl())) { |
There was a problem hiding this comment.
Since purl is also optional, we might also need bom-ref and CPE-based checks. I suggest waiting for a response from DT developers before making this change.
|
@malice00 We recently re-worked the BOM ingestion area in hyades-apiserver and will backport that soon to this code base. Components are flattened prior to de-duplication: https://github.com/DependencyTrack/hyades-apiserver/blob/23725258b8681ada8a5fc605a06dd45350fd83ef/src/main/java/org/dependencytrack/tasks/BomUploadProcessingTask.java#L213-L214 I believe this is where your change could be implemented as well. We de-duplicate based on There was also a relevant change to how existing components are looked up in the DB (see #2519 (comment)); We made this more strict. It may be easier to incorporate your proposed change to that logic, rather than retrofitting it onto the existing one. If you can, maybe look at the changes we made and let us know if you think your requirement can be integrated there. |
|
@nscuro Might take me until the weekend to get to it, but I'll give it a look! |
|
@nscuro Nice clean codebase, looks like I can integrate very easily! Now for a test to make sure that integration actually worked... :-) |
Description
Certain SBOM-generators save sub-projects or -modules as part of the component, instead of in the list of dependencies. This change makes sure to also import these as components, so a correct dependency-tree can be generated.
Addressed Issue
Fixes #2955
Checklist
and I have provided tests to verify that the fix is effectiveThis PR implements an enhancement, and I have provided tests to verify that it works as intendedThis PR introduces changes to the database model, and I have added corresponding update logicThis PR introduces new or alters existing behavior, and I have updated the documentation accordinglyTesting was done manually, since no tests for this part of the code exists and I wasn't sure on how to add them there.