Merge pull request #4684 from nscuro/pebble-disable-include

DependencyTrack · Feb 23, 2025 · d9a4b10 · d9a4b10
2 parents 8cab3ea + 8601a77
commit d9a4b10
Show file tree

Hide file tree

Showing 12 changed files with 82 additions and 53 deletions.
diff --git a/pom.xml b/pom.xml
@@ -111,7 +111,7 @@
         <lib.mockserver-netty.version>5.15.0</lib.mockserver-netty.version>
         <lib.open-vulnerability-clients.version>7.2.2</lib.open-vulnerability-clients.version>
         <lib.packageurl.version>1.5.0</lib.packageurl.version>
-        <lib.pebble.version>3.2.2</lib.pebble.version>
+        <lib.pebble.version>3.2.3</lib.pebble.version>
         <lib.protobuf-java.version>4.29.3</lib.protobuf-java.version>
         <lib.resilience4j.version>2.2.0</lib.resilience4j.version>
         <lib.swagger-parser.version>2.1.22</lib.swagger-parser.version>

diff --git a/src/main/java/org/dependencytrack/notification/publisher/AbstractWebhookPublisher.java b/src/main/java/org/dependencytrack/notification/publisher/AbstractWebhookPublisher.java
@@ -19,6 +19,8 @@
 package org.dependencytrack.notification.publisher;
 
 import alpine.notification.Notification;
+import io.pebbletemplates.pebble.PebbleEngine;
+import io.pebbletemplates.pebble.extension.core.DisallowExtensionCustomizerBuilder;
 import io.pebbletemplates.pebble.template.PebbleTemplate;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpPost;
@@ -31,9 +33,17 @@
 
 import jakarta.json.JsonObject;
 import java.io.IOException;
+import java.util.List;
 
 public abstract class AbstractWebhookPublisher implements Publisher {
 
+    private static final PebbleEngine DEFAULT_PEBBLE_ENGINE = new PebbleEngine.Builder()
+            .registerExtensionCustomizer(new DisallowExtensionCustomizerBuilder()
+                    .disallowedTokenParserTags(List.of("include"))
+                    .build())
+            .defaultEscapingStrategy("json")
+            .build();
+
     public void publish(final PublishContext ctx, final PebbleTemplate template, final Notification notification, final JsonObject config) {
         final Logger logger = LoggerFactory.getLogger(getClass());
 
@@ -101,6 +111,11 @@ public void publish(final PublishContext ctx, final PebbleTemplate template, fin
         }
     }
 
+    @Override
+    public PebbleEngine getTemplateEngine() {
+        return DEFAULT_PEBBLE_ENGINE;
+    }
+
     protected String getDestinationUrl(final JsonObject config) {
         return config.getString(CONFIG_DESTINATION, null);
     }

diff --git a/src/main/java/org/dependencytrack/notification/publisher/ConsolePublisher.java b/src/main/java/org/dependencytrack/notification/publisher/ConsolePublisher.java
@@ -22,15 +22,22 @@
 import alpine.notification.Notification;
 import alpine.notification.NotificationLevel;
 import io.pebbletemplates.pebble.PebbleEngine;
+import io.pebbletemplates.pebble.extension.core.DisallowExtensionCustomizerBuilder;
 
 import jakarta.json.JsonObject;
 import java.io.IOException;
 import java.io.PrintStream;
+import java.util.List;
 
 public class ConsolePublisher implements Publisher {
 
     private static final Logger LOGGER = Logger.getLogger(ConsolePublisher.class);
-    private static final PebbleEngine ENGINE = new PebbleEngine.Builder().newLineTrimming(false).build();
+    private static final PebbleEngine ENGINE = new PebbleEngine.Builder()
+            .registerExtensionCustomizer(new DisallowExtensionCustomizerBuilder()
+                    .disallowedTokenParserTags(List.of("include"))
+                    .build())
+            .newLineTrimming(false)
+            .build();
 
     public void inform(final PublishContext ctx, final Notification notification, final JsonObject config) {
         final String content;

diff --git a/src/main/java/org/dependencytrack/notification/publisher/CsWebexPublisher.java b/src/main/java/org/dependencytrack/notification/publisher/CsWebexPublisher.java
@@ -19,21 +19,13 @@
 package org.dependencytrack.notification.publisher;
 
 import alpine.notification.Notification;
-import io.pebbletemplates.pebble.PebbleEngine;
 
 import jakarta.json.JsonObject;
 
 public class CsWebexPublisher extends AbstractWebhookPublisher implements Publisher {
 
-    private static final PebbleEngine ENGINE = new PebbleEngine.Builder().defaultEscapingStrategy("json").build();
-
     public void inform(final PublishContext ctx, final Notification notification, final JsonObject config) {
         publish(ctx, getTemplate(config), notification, config);
     }
 
-    @Override
-    public PebbleEngine getTemplateEngine() {
-        return ENGINE;
-    }
-
 }
diff --git a/src/main/java/org/dependencytrack/notification/publisher/JiraPublisher.java b/src/main/java/org/dependencytrack/notification/publisher/JiraPublisher.java
@@ -21,7 +21,6 @@
 import alpine.common.logging.Logger;
 import alpine.model.ConfigProperty;
 import alpine.notification.Notification;
-import io.pebbletemplates.pebble.PebbleEngine;
 import org.dependencytrack.exception.PublisherException;
 import org.dependencytrack.persistence.QueryManager;
 import org.dependencytrack.util.DebugDataEncryption;
@@ -42,7 +41,6 @@
 public class JiraPublisher extends AbstractWebhookPublisher implements Publisher {
 
     private static final Logger LOGGER = Logger.getLogger(JiraPublisher.class);
-    private static final PebbleEngine ENGINE = new PebbleEngine.Builder().defaultEscapingStrategy("json").build();
 
     private String jiraProjectKey;
     private String jiraTicketType;
@@ -69,11 +67,6 @@ public void inform(final PublishContext ctx, final Notification notification, fi
         publish(ctx, getTemplate(config), notification, config);
     }
 
-    @Override
-    public PebbleEngine getTemplateEngine() {
-        return ENGINE;
-    }
-
     @Override
     public String getDestinationUrl(final JsonObject config) {
         try (final QueryManager qm = new QueryManager()) {

diff --git a/src/main/java/org/dependencytrack/notification/publisher/MattermostPublisher.java b/src/main/java/org/dependencytrack/notification/publisher/MattermostPublisher.java
@@ -19,20 +19,13 @@
 package org.dependencytrack.notification.publisher;
 
 import alpine.notification.Notification;
-import io.pebbletemplates.pebble.PebbleEngine;
 
 import jakarta.json.JsonObject;
 
 public class MattermostPublisher extends AbstractWebhookPublisher implements Publisher {
 
-    private static final PebbleEngine ENGINE = new PebbleEngine.Builder().defaultEscapingStrategy("json").build();
-
     public void inform(final PublishContext ctx, final Notification notification, final JsonObject config) {
         publish(ctx, getTemplate(config), notification, config);
     }
 
-    @Override
-    public PebbleEngine getTemplateEngine() {
-        return ENGINE;
-    }
 }
diff --git a/src/main/java/org/dependencytrack/notification/publisher/MsTeamsPublisher.java b/src/main/java/org/dependencytrack/notification/publisher/MsTeamsPublisher.java
@@ -19,21 +19,13 @@
 package org.dependencytrack.notification.publisher;
 
 import alpine.notification.Notification;
-import io.pebbletemplates.pebble.PebbleEngine;
 
 import jakarta.json.JsonObject;
 
 public class MsTeamsPublisher extends AbstractWebhookPublisher implements Publisher {
 
-    private static final PebbleEngine ENGINE = new PebbleEngine.Builder().defaultEscapingStrategy("json").build();
-
     public void inform(final PublishContext ctx, final Notification notification, final JsonObject config) {
         publish(ctx, getTemplate(config), notification, config);
     }
 
-    @Override
-    public PebbleEngine getTemplateEngine() {
-        return ENGINE;
-    }
-
 }
diff --git a/src/main/java/org/dependencytrack/notification/publisher/SendMailPublisher.java b/src/main/java/org/dependencytrack/notification/publisher/SendMailPublisher.java
@@ -27,15 +27,14 @@
 import alpine.server.mail.SendMail;
 import alpine.server.mail.SendMailException;
 import io.pebbletemplates.pebble.PebbleEngine;
+import io.pebbletemplates.pebble.extension.core.DisallowExtensionCustomizerBuilder;
 import io.pebbletemplates.pebble.template.PebbleTemplate;
-
 import org.apache.commons.text.StringEscapeUtils;
 import org.dependencytrack.persistence.QueryManager;
 import org.dependencytrack.util.DebugDataEncryption;
 
 import jakarta.json.JsonObject;
 import jakarta.json.JsonString;
-
 import jakarta.ws.rs.core.MediaType;
 import java.io.IOException;
 import java.util.Arrays;
@@ -59,7 +58,12 @@
 public class SendMailPublisher implements Publisher {
 
     private static final Logger LOGGER = Logger.getLogger(SendMailPublisher.class);
-    private static final PebbleEngine ENGINE = new PebbleEngine.Builder().newLineTrimming(false).build();
+    private static final PebbleEngine ENGINE = new PebbleEngine.Builder()
+            .registerExtensionCustomizer(new DisallowExtensionCustomizerBuilder()
+                    .disallowedTokenParserTags(List.of("include"))
+                    .build())
+            .newLineTrimming(false)
+            .build();
 
     public void inform(final PublishContext ctx, final Notification notification, final JsonObject config) {
         if (config == null) {

diff --git a/src/main/java/org/dependencytrack/notification/publisher/SlackPublisher.java b/src/main/java/org/dependencytrack/notification/publisher/SlackPublisher.java
@@ -19,21 +19,13 @@
 package org.dependencytrack.notification.publisher;
 
 import alpine.notification.Notification;
-import io.pebbletemplates.pebble.PebbleEngine;
 
 import jakarta.json.JsonObject;
 
 public class SlackPublisher extends AbstractWebhookPublisher implements Publisher {
 
-    private static final PebbleEngine ENGINE = new PebbleEngine.Builder().defaultEscapingStrategy("json").build();
-
     public void inform(final PublishContext ctx, final Notification notification, final JsonObject config) {
         publish(ctx, getTemplate(config), notification, config);
     }
 
-    @Override
-    public PebbleEngine getTemplateEngine() {
-        return ENGINE;
-    }
-
 }
diff --git a/src/main/java/org/dependencytrack/notification/publisher/WebhookPublisher.java b/src/main/java/org/dependencytrack/notification/publisher/WebhookPublisher.java
@@ -19,21 +19,13 @@
 package org.dependencytrack.notification.publisher;
 
 import alpine.notification.Notification;
-import io.pebbletemplates.pebble.PebbleEngine;
 
 import jakarta.json.JsonObject;
 
 public class WebhookPublisher extends AbstractWebhookPublisher implements Publisher {
 
-    private static final PebbleEngine ENGINE = new PebbleEngine.Builder().defaultEscapingStrategy("json").build();
-
     public void inform(final PublishContext ctx, final Notification notification, final JsonObject config) {
         publish(ctx, getTemplate(config), notification, config);
     }
 
-    @Override
-    public PebbleEngine getTemplateEngine() {
-        return ENGINE;
-    }
-
 }
diff --git a/src/test/java/org/dependencytrack/notification/publisher/AbstractPublisherTest.java b/src/test/java/org/dependencytrack/notification/publisher/AbstractPublisherTest.java
@@ -20,6 +20,7 @@
 
 import alpine.notification.Notification;
 import alpine.notification.NotificationLevel;
+import io.pebbletemplates.pebble.error.ParserException;
 import org.apache.commons.io.IOUtils;
 import org.dependencytrack.PersistenceCapableTest;
 import org.dependencytrack.model.Analysis;
@@ -39,7 +40,6 @@
 import org.dependencytrack.notification.vo.BomProcessingFailed;
 import org.dependencytrack.notification.vo.BomValidationFailed;
 import org.dependencytrack.notification.vo.NewVulnerabilityIdentified;
-import org.dependencytrack.resources.v1.problems.InvalidBomProblemDetails;
 import org.dependencytrack.notification.vo.NewVulnerableDependency;
 import org.junit.Test;
 
@@ -54,6 +54,7 @@
 import java.util.UUID;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.assertThatNoException;
 
 public abstract class AbstractPublisherTest<T extends Publisher> extends PersistenceCapableTest {
@@ -229,6 +230,24 @@ public void testInformWithEscapedData() {
                 .isThrownBy(() -> publisherInstance.inform(PublishContext.from(notification), notification, createConfig()));
     }
 
+    @Test
+    public void testInformWithTemplateInclude() throws Exception {
+        final var notification = new Notification()
+                .scope(NotificationScope.SYSTEM)
+                .group(NotificationGroup.ANALYZER)
+                .title(NotificationConstants.Title.NOTIFICATION_TEST)
+                .level(NotificationLevel.ERROR)
+                .timestamp(LocalDateTime.ofEpochSecond(66666, 666, ZoneOffset.UTC));
+
+        final JsonObject config = Json.createObjectBuilder(createConfig())
+                .add(Publisher.CONFIG_TEMPLATE_KEY, "{% include '/some/path' %}")
+                .build();
+
+        assertThatExceptionOfType(ParserException.class)
+                .isThrownBy(() -> publisherInstance.inform(PublishContext.from(notification), notification, config))
+                .withMessage("Unexpected tag name \"include\" ({% include '/some/path' %}:1)");
+    }
+
     private static Component createComponent(final Project project) {
         final var component = new Component();
         component.setProject(project);
@@ -287,7 +306,7 @@ private static Analysis createAnalysis(final Component component, final Vulnerab
         return analysis;
     }
 
-    private JsonObject createConfig() throws Exception {
+    JsonObject createConfig() throws Exception {
         return Json.createObjectBuilder()
                 .add(Publisher.CONFIG_TEMPLATE_MIME_TYPE_KEY, publisher.getTemplateMimeType())
                 .add(Publisher.CONFIG_TEMPLATE_KEY, IOUtils.resourceToString(publisher.getPublisherTemplateFile(), UTF_8))

diff --git a/src/test/java/org/dependencytrack/notification/publisher/SendMailPublisherTest.java b/src/test/java/org/dependencytrack/notification/publisher/SendMailPublisherTest.java
@@ -4,9 +4,14 @@
 import alpine.model.ManagedUser;
 import alpine.model.OidcUser;
 import alpine.model.Team;
+import alpine.notification.Notification;
+import alpine.notification.NotificationLevel;
 import alpine.security.crypto.DataEncryption;
 import com.icegreen.greenmail.junit4.GreenMailRule;
 import com.icegreen.greenmail.util.ServerSetup;
+import org.dependencytrack.notification.NotificationConstants;
+import org.dependencytrack.notification.NotificationGroup;
+import org.dependencytrack.notification.NotificationScope;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
@@ -17,11 +22,14 @@
 import jakarta.json.JsonObjectBuilder;
 import jakarta.mail.internet.MimeBodyPart;
 import jakarta.mail.internet.MimeMultipart;
+import java.time.LocalDateTime;
+import java.time.ZoneOffset;
 import java.util.ArrayList;
 import java.util.List;
 
 import static com.icegreen.greenmail.configuration.GreenMailConfiguration.aConfig;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 import static org.dependencytrack.model.ConfigPropertyConstants.EMAIL_PREFIX;
 import static org.dependencytrack.model.ConfigPropertyConstants.EMAIL_SMTP_ENABLED;
 import static org.dependencytrack.model.ConfigPropertyConstants.EMAIL_SMTP_FROM_ADDR;
@@ -426,6 +434,28 @@ public void testInformWithEscapedData() {
 
     }
 
+    @Override
+    public void testInformWithTemplateInclude() throws Exception {
+        final var notification = new Notification()
+                .scope(NotificationScope.SYSTEM)
+                .group(NotificationGroup.ANALYZER)
+                .title(NotificationConstants.Title.NOTIFICATION_TEST)
+                .level(NotificationLevel.ERROR)
+                .timestamp(LocalDateTime.ofEpochSecond(66666, 666, ZoneOffset.UTC));
+
+        final JsonObject config = Json.createObjectBuilder(createConfig())
+                .add(Publisher.CONFIG_TEMPLATE_KEY, "{% include '/etc/passwd' %}")
+                .build();
+
+        // NB: In contrast to other publishers, SendMailPublisher catches and logs
+        // failures during template evaluation. Instead of expecting an exception
+        // being thrown, we verify that no email was sent.
+        assertThatNoException()
+                .isThrownBy(() -> publisherInstance.inform(PublishContext.from(notification), notification, config));
+
+        assertThat(greenMail.getReceivedMessages()).isEmpty();
+    }
+
     @Override
     JsonObjectBuilder extraConfig() {
         return super.extraConfig()