Skip to content

Commit

Permalink
Make component identity matching strict
Browse files Browse the repository at this point in the history 
To address DependencyTrack/dependency-track#2519 (comment).

Also add regression test for this specific issue.

Signed-off-by: nscuro <nscuro@protonmail.com>
  • Loading branch information
@nscuro
nscuro committed Jul 5, 2023
1 parent ea1dac7 commit 6418879
Show file tree
Hide file tree
Showing 6 changed files with 63,432 additions and 38 deletions.
11 changes: 8 additions & 3 deletions src/main/java/org/dependencytrack/persistence/ComponentQueryManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,19 +424,24 @@ public Component matchSingleIdentity(final Project project, final ComponentIdent
final var params = new HashMap<String, Object>();

if (cid.getPurl() != null) {
filterParts.add("((purl != null && purl == :purl) || (purlCoordinates != null && purlCoordinates == :purlCoordinates))");
filterParts.add("(purl != null && purl == :purl)");
params.put("purl", cid.getPurl().canonicalize());
params.put("purlCoordinates", cid.getPurlCoordinates().canonicalize());
} else {
filterParts.add("purl == null");
}

if (cid.getCpe() != null) {
filterParts.add("(cpe != null && cpe == :cpe)");
params.put("cpe", cid.getCpe());
} else {
filterParts.add("cpe == null");
}

if (cid.getSwidTagId() != null) {
filterParts.add("(swidTagId != null && swidTagId == :swidTagId)");
params.put("swidTagId", cid.getSwidTagId());
} else {
filterParts.add("swidTagId == null");
}

var coordinatesFilter = "(";
Expand All @@ -457,7 +462,7 @@ public Component matchSingleIdentity(final Project project, final ComponentIdent
coordinatesFilter += ")";
filterParts.add(coordinatesFilter);

final var filter = "project == :project && (" + String.join(" || ", filterParts) + ")";
final var filter = "project == :project && (" + String.join(" && ", filterParts) + ")";
params.put("project", project);

final Query<Component> query = pm.newQuery(Component.class, filter);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/dependencytrack/tasks/BomUploadProcessingTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ private static Project processMetadataComponent(final Context ctx, final Persist
changed |= applyIfChanged(project, metadataComponent, Project::getPublisher, project::setPublisher);
changed |= applyIfChanged(project, metadataComponent, Project::getClassifier, project::setClassifier);
// TODO: Currently these properties are "decoupled" from the BOM and managed directly by DT users.
// Perhaps there could be a flag for BOM uploads saying "use BOM properties" or something?
// Perhaps there could be a flag for BOM uploads saying "use BOM properties" or something?
// changed |= applyIfChanged(project, metadataComponent, Project::getGroup, project::setGroup);
// changed |= applyIfChanged(project, metadataComponent, Project::getName, project::setName);
// changed |= applyIfChanged(project, metadataComponent, Project::getVersion, project::setVersion);
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ public void exportComponentAsCycloneDxInvalid() {
public void uploadBomTest() throws Exception {
initializeWithPermissions(Permissions.BOM_UPLOAD);
Project project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
BomSubmitRequest request = new BomSubmitRequest(project.getUuid().toString(), null, null, false, bomString);
Response response = target(V1_BOM).request()
Expand All @@ -684,7 +684,7 @@ public void uploadBomTest() throws Exception {
@Test
public void uploadBomInvalidProjectTest() throws Exception {
initializeWithPermissions(Permissions.BOM_UPLOAD);
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
BomSubmitRequest request = new BomSubmitRequest(UUID.randomUUID().toString(), null, null, false, bomString);
Response response = target(V1_BOM).request()
Expand All @@ -699,7 +699,7 @@ public void uploadBomInvalidProjectTest() throws Exception {
@Test
public void uploadBomAutoCreateTest() throws Exception {
initializeWithPermissions(Permissions.BOM_UPLOAD, Permissions.PROJECT_CREATION_UPLOAD);
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
BomSubmitRequest request = new BomSubmitRequest(null, "Acme Example", "1.0", true, bomString);
Response response = target(V1_BOM).request()
Expand All @@ -716,7 +716,7 @@ public void uploadBomAutoCreateTest() throws Exception {

@Test
public void uploadBomUnauthorizedTest() throws Exception {
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
BomSubmitRequest request = new BomSubmitRequest(null, "Acme Example", "1.0", true, bomString);
Response response = target(V1_BOM).request()
Expand All @@ -730,7 +730,7 @@ public void uploadBomUnauthorizedTest() throws Exception {
@Test
public void uploadBomAutoCreateTestWithParentTest() throws Exception {
initializeWithPermissions(Permissions.BOM_UPLOAD, Permissions.PROJECT_CREATION_UPLOAD);
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
// Upload parent project
BomSubmitRequest request = new BomSubmitRequest(null, "Acme Parent", "1.0", true, bomString);
Expand Down Expand Up @@ -794,7 +794,7 @@ public void uploadBomAutoCreateTestWithParentTest() throws Exception {
@Test
public void uploadBomInvalidParentTest() throws Exception {
initializeWithPermissions(Permissions.BOM_UPLOAD, Permissions.PROJECT_CREATION_UPLOAD);
File file = new File(IOUtils.resourceToURL("/bom-1.xml").toURI());
File file = new File(IOUtils.resourceToURL("/unit/bom-1.xml").toURI());
String bomString = Base64.getEncoder().encodeToString(FileUtils.readFileToByteArray(file));
BomSubmitRequest request = new BomSubmitRequest(null, "Acme Example", "1.0", true, UUID.randomUUID().toString(), null, null, bomString);
Response response = target(V1_BOM).request()
Expand Down
76 changes: 48 additions & 28 deletions src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.junit.Before;
import org.junit.Test;

import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand Down Expand Up @@ -66,13 +67,7 @@ public void setUp() {
public void informTest() throws Exception {
Project project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);

// The task will delete the input file after processing it,
// so create a temporary copy to not impact other tests.
final Path bomFilePath = Files.createTempFile(null, null);
Files.copy(Paths.get(resourceToURL("/bom-1.xml").toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
final var bomFile = bomFilePath.toFile();

final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomFile);
final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-1.xml"));
new BomUploadProcessingTask().inform(bomUploadEvent);
assertConditionWithTimeout(() -> kafkaMockProducer.history().size() >= 5, Duration.ofSeconds(5));
assertThat(kafkaMockProducer.history()).satisfiesExactly(
Expand Down Expand Up @@ -111,13 +106,7 @@ public void informTest() throws Exception {
public void informWithEmptyBomTest() throws Exception {
Project project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);

// The task will delete the input file after processing it,
// so create a temporary copy to not impact other tests.
final Path bomFilePath = Files.createTempFile(null, null);
Files.copy(Paths.get(resourceToURL("/unit/bom-empty.json").toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
final var bomFile = bomFilePath.toFile();

final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomFile);
final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-empty.json"));
new BomUploadProcessingTask().inform(bomUploadEvent);
assertConditionWithTimeout(() -> kafkaMockProducer.history().size() >= 3, Duration.ofSeconds(5));
assertThat(kafkaMockProducer.history()).satisfiesExactly(
Expand All @@ -141,13 +130,7 @@ public void informWithEmptyBomTest() throws Exception {
public void informWithInvalidBomTest() throws Exception {
Project project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);

// The task will delete the input file after processing it,
// so create a temporary copy to not impact other tests.
final Path bomFilePath = Files.createTempFile(null, null);
Files.copy(Paths.get(resourceToURL("/unit/bom-invalid.json").toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
final var bomFile = bomFilePath.toFile();

new BomUploadProcessingTask().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), bomFile));
new BomUploadProcessingTask().inform(new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-invalid.json")));
assertConditionWithTimeout(() -> kafkaMockProducer.history().size() >= 2, Duration.ofSeconds(5));
assertThat(kafkaMockProducer.history()).satisfiesExactly(
event -> assertThat(event.topic()).isEqualTo(KafkaTopics.NOTIFICATION_PROJECT_CREATED.name()),
Expand Down Expand Up @@ -184,13 +167,7 @@ public void informWithInvalidBomTest() throws Exception {
public void informWithBloatedBomTest() throws Exception {
final var project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);

// The task will delete the input file after processing it,
// so create a temporary copy to not impact other tests.
final Path bomFilePath = Files.createTempFile(null, null);
Files.copy(Paths.get(resourceToURL("/unit/bloated.bom.json").toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
final var bomFile = bomFilePath.toFile();

final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), bomFile);
final var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-bloated.json"));
new BomUploadProcessingTask().inform(bomUploadEvent);

assertThat(kafkaMockProducer.history())
Expand Down Expand Up @@ -276,4 +253,47 @@ public void informWithBloatedBomTest() throws Exception {
assertThat(repoMetaAnalysisCommandsSent).isEqualTo(9056);
}

@Test // https://github.com/DependencyTrack/dependency-track/issues/2519
public void informIssue2519Test() throws Exception {
final var project = qm.createProject("Acme Example", null, "1.0", null, null, null, true, false);

var bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-issue2519.xml"));
new BomUploadProcessingTask().inform(bomUploadEvent);

// Make sure processing did not fail.
assertThat(kafkaMockProducer.history())
.noneSatisfy(record -> {
assertThat(record.topic()).isEqualTo(KafkaTopics.NOTIFICATION_BOM.name());
final Notification notification = deserializeValue(KafkaTopics.NOTIFICATION_BOM, record);
assertThat(notification.getGroup()).isEqualTo(GROUP_BOM_PROCESSING_FAILED);
});

// Ensure the expected amount of components is present.
assertThat(qm.getAllComponents(project)).hasSize(1756);

// Upload the same BOM again a few times.
// Ensure processing does not fail, and the number of components ingested doesn't change.
for (int i = 0; i < 3; i++) {
bomUploadEvent = new BomUploadEvent(qm.detach(Project.class, project.getId()), createTempBomFile("bom-issue2519.xml"));
new BomUploadProcessingTask().inform(bomUploadEvent);

assertThat(kafkaMockProducer.history())
.noneSatisfy(record -> {
assertThat(record.topic()).isEqualTo(KafkaTopics.NOTIFICATION_BOM.name());
final Notification notification = deserializeValue(KafkaTopics.NOTIFICATION_BOM, record);
assertThat(notification.getGroup()).isEqualTo(GROUP_BOM_PROCESSING_FAILED);
});

assertThat(qm.getAllComponents(project)).hasSize(1756);
}
}

private static File createTempBomFile(final String testFileName) throws Exception {
// The task will delete the input file after processing it,
// so create a temporary copy to not impact other tests.
final Path bomFilePath = Files.createTempFile(null, null);
Files.copy(Paths.get(resourceToURL("/unit/" + testFileName).toURI()), bomFilePath, StandardCopyOption.REPLACE_EXISTING);
return bomFilePath.toFile();
}

}
0 src/test/resources/bom-1.xml → src/test/resources/unit/bom-1.xml
View file
File renamed without changes.
Loading

0 comments on commit 6418879

Please sign in to comment.