From 6f7f22ab0a5ca783cd00bfc4d29eec8dbd18f423 Mon Sep 17 00:00:00 2001 From: nscuro Date: Tue, 2 Jul 2024 15:54:52 +0200 Subject: [PATCH 1/2] Fix `invalid stream header` when modifying analyses with associated vuln policy When an existing analysis, which is assigned to a vulnerability policy, was updated, DataNucleus failed with `invalid stream header`. This happened because, when updating an analysis, DataNucleus fetches all associated objects, which includes `VulnerabilityPolicy`. The `VulnerabilityPolicy` class does not map cleanly to the database schema: Some fields use PostgreSQL-native arrays, others use JSONB. It also turned out that the model class isn't really used anywhere, since we modify and query `VulnerabilityPolicy` via JDBI. Hence the class was removed entirely, replacing it with a simple ID column of type `Long`. Signed-off-by: nscuro --- .../org/dependencytrack/model/Analysis.java | 10 +- .../model/VulnerabilityPolicy.java | 200 ------------------ .../v1/VulnerabilityPolicyResource.java | 2 +- src/main/resources/META-INF/persistence.xml | 1 - .../VulnerabilityScanResultProcessorTest.java | 4 +- .../jdbi/VulnerabilityPolicyDaoTest.java | 2 +- .../resources/v1/AnalysisResourceTest.java | 66 ++++++ 7 files changed, 75 insertions(+), 210 deletions(-) delete mode 100644 src/main/java/org/dependencytrack/model/VulnerabilityPolicy.java diff --git a/src/main/java/org/dependencytrack/model/Analysis.java b/src/main/java/org/dependencytrack/model/Analysis.java index d9af0c1f3..b3dc2dffd 100644 --- a/src/main/java/org/dependencytrack/model/Analysis.java +++ b/src/main/java/org/dependencytrack/model/Analysis.java @@ -135,7 +135,7 @@ public class Analysis implements Serializable { @Persistent @Column(name = "VULNERABILITY_POLICY_ID", allowsNull = "true") @JsonIgnore - private VulnerabilityPolicy vulnerabilityPolicy; + private Long vulnerabilityPolicyId; public long getId() { return id; @@ -270,11 +270,11 @@ public void setOwaspScore(BigDecimal owaspScore) { this.owaspScore = owaspScore; } - public VulnerabilityPolicy getVulnerabilityPolicy() { - return vulnerabilityPolicy; + public Long getVulnerabilityPolicyId() { + return vulnerabilityPolicyId; } - public void setVulnerabilityPolicy(VulnerabilityPolicy vulnerabilityPolicy) { - this.vulnerabilityPolicy = vulnerabilityPolicy; + public void setVulnerabilityPolicyId(Long vulnerabilityPolicyId) { + this.vulnerabilityPolicyId = vulnerabilityPolicyId; } } diff --git a/src/main/java/org/dependencytrack/model/VulnerabilityPolicy.java b/src/main/java/org/dependencytrack/model/VulnerabilityPolicy.java deleted file mode 100644 index ce3747161..000000000 --- a/src/main/java/org/dependencytrack/model/VulnerabilityPolicy.java +++ /dev/null @@ -1,200 +0,0 @@ -/* - * This file is part of Dependency-Track. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright (c) OWASP Foundation. All Rights Reserved. - */ -package org.dependencytrack.model; - -import alpine.common.validation.RegexSequence; -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonInclude; -import org.dependencytrack.policy.vulnerability.VulnerabilityPolicyAnalysis; -import org.dependencytrack.policy.vulnerability.VulnerabilityPolicyOperation; -import org.dependencytrack.policy.vulnerability.VulnerabilityPolicyRating; - -import javax.jdo.annotations.Column; -import javax.jdo.annotations.IdGeneratorStrategy; -import javax.jdo.annotations.Index; -import javax.jdo.annotations.PersistenceCapable; -import javax.jdo.annotations.Persistent; -import javax.jdo.annotations.PrimaryKey; -import javax.validation.constraints.NotBlank; -import javax.validation.constraints.Pattern; -import javax.validation.constraints.Size; -import java.io.Serializable; -import java.util.Date; -import java.util.List; - -@PersistenceCapable(table = "VULNERABILITY_POLICY") -@JsonInclude(JsonInclude.Include.NON_NULL) -@JsonIgnoreProperties(ignoreUnknown = true) -public class VulnerabilityPolicy implements Serializable { - - @PrimaryKey - @Persistent(valueStrategy = IdGeneratorStrategy.NATIVE) - @JsonIgnore - private long id; - - @Persistent - @Column(name = "NAME", allowsNull = "false") - @Index(name = "VULNERABILITY_POLICY_NAME_IDX", unique = "true") - @NotBlank - @Size(min = 1, max = 255) - @Pattern(regexp = RegexSequence.Definition.PRINTABLE_CHARS, message = "The name may only contain printable characters") - private String name; - - @Persistent - @Column(name = "DESCRIPTION", allowsNull = "true") - @Size(min = 1, max = 512) - @Pattern(regexp = RegexSequence.Definition.PRINTABLE_CHARS, message = "The description may only contain printable characters") - private String description; - - @Persistent - @Column(name = "AUTHOR", allowsNull = "true", jdbcType = "VARCHAR") - @Size(min = 1, max = 255) - @Pattern(regexp = RegexSequence.Definition.PRINTABLE_CHARS, message = "The author may only contain printable characters") - private String author; - - @Persistent - @Column(name = "CREATED", allowsNull = "true") - private Date created; - - @Persistent - @Column(name = "UPDATED", allowsNull = "true") - private Date updated; - - @Persistent - @Column(name = "VALID_FROM", allowsNull = "true") - private Date validFrom; - - @Persistent - @Column(name = "VALID_UNTIL", allowsNull = "true") - private Date validUntil; - - @Persistent(defaultFetchGroup = "true") - @Column(name = "CONDITIONS", allowsNull = "false") - private String[] conditions; - - @Column(name = "ANALYSIS", allowsNull = "false") - @Persistent(defaultFetchGroup = "true") - private VulnerabilityPolicyAnalysis analysis; - - @Column(name = "RATINGS", allowsNull = "true") - @Persistent(defaultFetchGroup = "true") - private List ratings; - - @Column(name = "OPERATION_MODE", allowsNull = "false") - @Persistent(defaultFetchGroup = "true") - private VulnerabilityPolicyOperation operationMode; - - public long getId() { - return id; - } - - public void setId(long id) { - this.id = id; - } - - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public String getDescription() { - return description; - } - - public void setDescription(String description) { - this.description = description; - } - - public String getAuthor() { - return author; - } - - public void setAuthor(String author) { - this.author = author; - } - - public Date getCreated() { - return created; - } - - public void setCreated(Date created) { - this.created = created; - } - - public Date getUpdated() { - return updated; - } - - public void setUpdated(Date updated) { - this.updated = updated; - } - - public Date getValidFrom() { - return validFrom; - } - - public void setValidFrom(Date validFrom) { - this.validFrom = validFrom; - } - - public Date getValidUntil() { - return validUntil; - } - - public void setValidUntil(Date validUntil) { - this.validUntil = validUntil; - } - - public String[] getConditions() { - return conditions; - } - - public void setConditions(String[] conditions) { - this.conditions = conditions; - } - - public VulnerabilityPolicyAnalysis getAnalysis() { - return analysis; - } - - public void setAnalysis(VulnerabilityPolicyAnalysis vulnerabilityPolicyAnalysis) { - this.analysis = vulnerabilityPolicyAnalysis; - } - - public List getRatings() { - return ratings; - } - - public void setRatings(List ratings) { - this.ratings = ratings; - } - - public VulnerabilityPolicyOperation getOperationMode() { - return operationMode; - } - - public void setOperationMode(VulnerabilityPolicyOperation operationMode) { - this.operationMode = operationMode; - } -} - diff --git a/src/main/java/org/dependencytrack/resources/v1/VulnerabilityPolicyResource.java b/src/main/java/org/dependencytrack/resources/v1/VulnerabilityPolicyResource.java index 528a3c798..1d2037040 100644 --- a/src/main/java/org/dependencytrack/resources/v1/VulnerabilityPolicyResource.java +++ b/src/main/java/org/dependencytrack/resources/v1/VulnerabilityPolicyResource.java @@ -34,11 +34,11 @@ import org.dependencytrack.auth.Permissions; import org.dependencytrack.common.ConfigKey; import org.dependencytrack.event.VulnerabilityPolicyFetchEvent; -import org.dependencytrack.model.VulnerabilityPolicy; import org.dependencytrack.model.WorkflowState; import org.dependencytrack.model.WorkflowStatus; import org.dependencytrack.model.WorkflowStep; import org.dependencytrack.persistence.QueryManager; +import org.dependencytrack.policy.vulnerability.VulnerabilityPolicy; import org.dependencytrack.policy.vulnerability.VulnerabilityPolicyProvider; import org.dependencytrack.policy.vulnerability.VulnerabilityPolicyProviderFactory; import org.dependencytrack.resources.v1.openapi.PaginatedApi; diff --git a/src/main/resources/META-INF/persistence.xml b/src/main/resources/META-INF/persistence.xml index f8fd9f31a..9701f0522 100644 --- a/src/main/resources/META-INF/persistence.xml +++ b/src/main/resources/META-INF/persistence.xml @@ -58,7 +58,6 @@ org.dependencytrack.model.VulnerableSoftware org.dependencytrack.model.WorkflowState org.dependencytrack.model.IntegrityAnalysis - org.dependencytrack.model.VulnerabilityPolicy org.dependencytrack.model.VulnerabilityPolicyBundle true diff --git a/src/test/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessorTest.java b/src/test/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessorTest.java index 3002106f4..152366d79 100644 --- a/src/test/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessorTest.java +++ b/src/test/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessorTest.java @@ -1235,7 +1235,7 @@ public void analysisThroughPolicyResetOnNoMatchTest() { assertThat(v.getVulnId()).isEqualTo("CVE-100"); assertThat(qm.getAnalysis(component, v)).satisfies(a -> { assertThat(a.getAnalysisState()).isEqualTo(AnalysisState.NOT_SET); - assertThat(a.getVulnerabilityPolicy()).isNull(); + assertThat(a.getVulnerabilityPolicyId()).isNull(); assertThat(a.getAnalysisComments()).extracting(AnalysisComment::getCommenter).containsOnly("[Policy{None}]"); assertThat(a.getAnalysisComments()).extracting(AnalysisComment::getComment).containsExactlyInAnyOrder( "No longer covered by any policy", @@ -1267,7 +1267,7 @@ public void analysisThroughPolicyResetOnNoMatchTest() { assertThat(a.getCvssV2Score()).isNull(); assertThat(a.getCvssV3Vector()).isNull(); assertThat(a.getCvssV3Score()).isNull(); - assertThat(a.getVulnerabilityPolicy()).isNull(); + assertThat(a.getVulnerabilityPolicyId()).isNull(); assertThat(a.getAnalysisComments()).isEmpty(); }); }); diff --git a/src/test/java/org/dependencytrack/persistence/jdbi/VulnerabilityPolicyDaoTest.java b/src/test/java/org/dependencytrack/persistence/jdbi/VulnerabilityPolicyDaoTest.java index 236b858c2..054a8a1da 100644 --- a/src/test/java/org/dependencytrack/persistence/jdbi/VulnerabilityPolicyDaoTest.java +++ b/src/test/java/org/dependencytrack/persistence/jdbi/VulnerabilityPolicyDaoTest.java @@ -369,7 +369,7 @@ public void testVulnerabilityPolicyIsUnassignedAndDeleted() throws Exception { assertThat(analysis.getAnalysisJustification()).isNull(); assertThat(analysis.getAnalysisResponse()).isNull(); assertThat(analysis.getAnalysisDetails()).isNull(); - assertThat(analysis.getVulnerabilityPolicy()).isNull(); + assertThat(analysis.getVulnerabilityPolicyId()).isNull(); assertThat(analysis.getAnalysisComments()).extracting(AnalysisComment::getCommenter).containsOnly("[Policy{Name=name}]"); assertThat(analysis.getAnalysisComments()).extracting(AnalysisComment::getComment).containsExactlyInAnyOrder( "Policy removed", diff --git a/src/test/java/org/dependencytrack/resources/v1/AnalysisResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/AnalysisResourceTest.java index 5c67d16c5..8a05be5fa 100644 --- a/src/test/java/org/dependencytrack/resources/v1/AnalysisResourceTest.java +++ b/src/test/java/org/dependencytrack/resources/v1/AnalysisResourceTest.java @@ -31,11 +31,15 @@ import org.dependencytrack.model.AnalysisJustification; import org.dependencytrack.model.AnalysisResponse; import org.dependencytrack.model.AnalysisState; +import org.dependencytrack.model.AnalyzerIdentity; import org.dependencytrack.model.Component; import org.dependencytrack.model.Project; import org.dependencytrack.model.Severity; import org.dependencytrack.model.Vulnerability; import org.dependencytrack.notification.NotificationConstants; +import org.dependencytrack.persistence.jdbi.VulnerabilityPolicyDao; +import org.dependencytrack.policy.vulnerability.VulnerabilityPolicy; +import org.dependencytrack.policy.vulnerability.VulnerabilityPolicyAnalysis; import org.dependencytrack.proto.notification.v1.Notification; import org.dependencytrack.resources.v1.vo.AnalysisRequest; import org.dependencytrack.util.NotificationUtil; @@ -55,6 +59,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.dependencytrack.assertion.Assertions.assertConditionWithTimeout; +import static org.dependencytrack.persistence.jdbi.JdbiFactory.useJdbiHandle; +import static org.dependencytrack.persistence.jdbi.JdbiFactory.withJdbiHandle; import static org.dependencytrack.proto.notification.v1.Group.GROUP_PROJECT_AUDIT_CHANGE; import static org.dependencytrack.proto.notification.v1.Level.LEVEL_INFORMATIONAL; import static org.dependencytrack.proto.notification.v1.Scope.SCOPE_PORTFOLIO; @@ -767,4 +773,64 @@ public void updateAnalysisUnauthorizedTest() { assertThat(response.getHeaderString(TOTAL_COUNT_HEADER)).isNull(); } + @Test + public void updateAnalysisWithAssociatedVulnerabilityPolicyTest() { + initializeWithPermissions(Permissions.VULNERABILITY_ANALYSIS); + + final var project = new Project(); + project.setName("acme-app"); + qm.persist(project); + + final var component = new Component(); + component.setProject(project); + component.setName("acme-lib"); + qm.persist(component); + + final var vuln = new Vulnerability(); + vuln.setVulnId("CVE-123"); + vuln.setSource(Vulnerability.Source.NVD); + qm.persist(vuln); + + qm.addVulnerability(vuln, component, AnalyzerIdentity.INTERNAL_ANALYZER); + final Analysis analysis = qm.makeAnalysis(component, vuln, AnalysisState.NOT_AFFECTED, null, null, null, true); + + final VulnerabilityPolicy vulnPolicy = withJdbiHandle(handle -> { + final var policyAnalysis = new VulnerabilityPolicyAnalysis(); + policyAnalysis.setState(VulnerabilityPolicyAnalysis.State.EXPLOITABLE); + + final var policy = new VulnerabilityPolicy(); + policy.setName("foo"); + policy.setAnalysis(policyAnalysis); + policy.setConditions(List.of("true")); + return handle.attach(VulnerabilityPolicyDao.class).create(policy); + }); + + useJdbiHandle(handle -> handle.createUpdate(""" + WITH "VULN_POLICY" AS ( + SELECT "ID" + FROM "VULNERABILITY_POLICY" + WHERE "NAME" = :policyName + ) + UPDATE "ANALYSIS" + SET "VULNERABILITY_POLICY_ID" = (SELECT "ID" FROM "VULN_POLICY") + WHERE "ID" = :analysisId + """) + .bind("policyName", vulnPolicy.getName()) + .bind("analysisId", analysis.getId()) + .execute()); + + final Response response = jersey.target(V1_ANALYSIS) + .request() + .header(X_API_KEY, apiKey) + .put(Entity.entity(""" + { + "project": "%s", + "component": "%s", + "vulnerability": "%s", + "comment": "foo" + } + """.formatted(project.getUuid(), component.getUuid(), vuln.getUuid()), MediaType.APPLICATION_JSON)); + assertThat(response.getStatus()).isEqualTo(200); + } + } From 8ec0a06a7d15df8c39458f30171181832b14ac97 Mon Sep 17 00:00:00 2001 From: nscuro Date: Tue, 2 Jul 2024 15:58:54 +0200 Subject: [PATCH 2/2] Fix vuln policy assignment getting lost upon project cloning Signed-off-by: nscuro --- .../persistence/ProjectQueryManager.java | 1 + .../resources/v1/ProjectResourceTest.java | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java index 22c13db4c..0df7bc66f 100644 --- a/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java +++ b/src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java @@ -614,6 +614,7 @@ public Project clone(UUID from, String newVersion, boolean includeTags, boolean analysis.setAnalysisJustification(sourceAnalysis.getAnalysisJustification()); analysis.setAnalysisState(sourceAnalysis.getAnalysisState()); analysis.setAnalysisDetails(sourceAnalysis.getAnalysisDetails()); + analysis.setVulnerabilityPolicyId(sourceAnalysis.getVulnerabilityPolicyId()); analysis = persist(analysis); if (sourceAnalysis.getAnalysisComments() != null) { for (final AnalysisComment sourceComment : sourceAnalysis.getAnalysisComments()) { diff --git a/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java b/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java index 37667b703..609447926 100644 --- a/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java +++ b/src/test/java/org/dependencytrack/resources/v1/ProjectResourceTest.java @@ -46,6 +46,9 @@ import org.dependencytrack.model.WorkflowStatus; import org.dependencytrack.model.WorkflowStep; import org.dependencytrack.notification.NotificationConstants; +import org.dependencytrack.persistence.jdbi.VulnerabilityPolicyDao; +import org.dependencytrack.policy.vulnerability.VulnerabilityPolicy; +import org.dependencytrack.policy.vulnerability.VulnerabilityPolicyAnalysis; import org.dependencytrack.tasks.CloneProjectTask; import org.glassfish.jersey.client.HttpUrlConnectorProvider; import org.glassfish.jersey.server.ResourceConfig; @@ -78,6 +81,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; import static org.dependencytrack.assertion.Assertions.assertConditionWithTimeout; +import static org.dependencytrack.persistence.jdbi.JdbiFactory.useJdbiHandle; +import static org.dependencytrack.persistence.jdbi.JdbiFactory.withJdbiHandle; import static org.dependencytrack.proto.notification.v1.Group.GROUP_PROJECT_CREATED; import static org.dependencytrack.proto.notification.v1.Level.LEVEL_INFORMATIONAL; import static org.dependencytrack.proto.notification.v1.Scope.SCOPE_PORTFOLIO; @@ -971,6 +976,31 @@ public void cloneProjectTest() { AnalysisJustification.REQUIRES_ENVIRONMENT, AnalysisResponse.WILL_NOT_FIX, "details", false); qm.makeAnalysisComment(analysis, "comment", "commenter"); + final VulnerabilityPolicy vulnPolicy = withJdbiHandle(handle -> { + final var policyAnalysis = new VulnerabilityPolicyAnalysis(); + policyAnalysis.setState(VulnerabilityPolicyAnalysis.State.EXPLOITABLE); + + final var policy = new VulnerabilityPolicy(); + policy.setName("foo"); + policy.setAnalysis(policyAnalysis); + policy.setConditions(List.of("true")); + return handle.attach(VulnerabilityPolicyDao.class).create(policy); + }); + useJdbiHandle(handle -> handle.createUpdate(""" + WITH "VULN_POLICY" AS ( + SELECT "ID" + FROM "VULNERABILITY_POLICY" + WHERE "NAME" = :policyName + ) + UPDATE "ANALYSIS" + SET "VULNERABILITY_POLICY_ID" = (SELECT "ID" FROM "VULN_POLICY") + WHERE "ID" = :analysisId + """) + .bind("policyName", vulnPolicy.getName()) + .bind("analysisId", analysis.getId()) + .execute()); + + final Response response = jersey.target("%s/clone".formatted(V1_PROJECT)).request() .header(X_API_KEY, apiKey) .put(Entity.json(""" @@ -1052,6 +1082,7 @@ public void cloneProjectTest() { assertThat(clonedAnalysis.getAnalysisResponse()).isEqualTo(AnalysisResponse.WILL_NOT_FIX); assertThat(clonedAnalysis.getAnalysisDetails()).isEqualTo("details"); assertThat(clonedAnalysis.isSuppressed()).isFalse(); + assertThat(clonedAnalysis.getVulnerabilityPolicyId()).isNotNull(); }); });