Maintenance: Improve template rendering.

lib/notification_factory/mailer.rb

@@ -324,7 +324,8 @@ returns
       locale:   data[:locale],
       timezone: data[:timezone],
       template: template[:subject],
-      escape:   false
+      escape:   false,
+      trusted:  true,
     ).render
 
     # strip off the extra newline at the end of the subject to avoid =0A suffixes (see #2726)
@@ -334,7 +335,8 @@ returns
       objects:  data[:objects],
       locale:   data[:locale],
       timezone: data[:timezone],
-      template: template[:body]
+      template: template[:body],
+      trusted:  true,
     ).render
 
     if !data[:raw]
@@ -348,7 +350,8 @@ returns
         objects:  data[:objects],
         locale:   data[:locale],
         timezone: data[:timezone],
-        template: application_template
+        template: application_template,
+        trusted:  true,
       ).render
     end
     {

lib/notification_factory/renderer.rb

@@ -13,7 +13,8 @@ examples how to use
       locale: 'de-de',
       timezone: 'America/Port-au-Prince',
       template: 'some template <b>#{ticket.title}</b> {config.fqdn}',
-      escape: false
+      escape: false,
+      trusted: false, # Allow ERB tags in the template?
     ).render
 
     message_body = NotificationFactory::Renderer.new(
@@ -27,11 +28,11 @@ examples how to use
 
 =end
 
-  def initialize(objects:, template:, locale: nil, timezone: nil, escape: true)
+  def initialize(objects:, template:, locale: nil, timezone: nil, escape: true, trusted: false) # rubocop:disable Metrics/ParameterLists
     @objects  = objects
     @locale   = locale || Locale.default
     @timezone = timezone || Setting.get('timezone_default')
-    @template = NotificationFactory::Template.new(template, escape)
+    @template = NotificationFactory::Template.new(template, escape, trusted)
     @escape = escape
   end
 

lib/notification_factory/slack.rb

@@ -46,14 +46,16 @@ returns
       locale:   data[:locale],
       timezone: data[:timezone],
       template: template[:subject],
-      escape:   false
+      escape:   false,
+      trusted:  true
     ).render
     message_body = NotificationFactory::Renderer.new(
       objects:  data[:objects],
       locale:   data[:locale],
       timezone: data[:timezone],
       template: template[:body],
-      escape:   false
+      escape:   false,
+      trusted:  true
     ).render
 
     if !data[:raw]
@@ -68,7 +70,8 @@ returns
         locale:   data[:locale],
         timezone: data[:timezone],
         template: application_template,
-        escape:   false
+        escape:   false,
+        trusted:  true
       ).render
     end
     {

lib/notification_factory/template.rb

@@ -9,17 +9,21 @@ examples how to use
     cleaned_template = NotificationFactory::Template.new(
       'some template <b>#{ticket.title}</b> #{config.fqdn}',
       true,
+      false,  # Allow ERB tags in the template?
     ).to_s
 
 =end
 
-  def initialize(template, escape)
+  def initialize(template, escape, trusted)
     @template = template
     @escape   = escape
+    @trusted  = trusted
   end
 
   def to_s
-    @template.gsub(%r{\#{\s*(.*?)\s*}}m) do
+    result = @template
+    result.gsub!(%r{<%(?!%)}, '<%%') if !@trusted
+    result.gsub(%r{\#{\s*(.*?)\s*}}m) do
       # some browsers start adding HTML tags
       # fixes https://github.com/zammad/zammad/issues/385
       input_template = $1.gsub(%r{\A<.+?>\s*|\s*<.+?>\z}, '')

spec/factories/notification_factory/renderer.rb

@@ -6,7 +6,8 @@ FactoryBot.define do
     locale   { 'en-en' }
     template { '' }
     escape   { true }
+    trusted  { false }
 
-    initialize_with { new(objects: objects, locale: locale, template: template, escape: escape) }
+    initialize_with { new(objects: objects, locale: locale, template: template, escape: escape, trusted: trusted) }
   end
 end

spec/lib/notification_factory/renderer_spec.rb

@@ -12,6 +12,21 @@ RSpec.describe NotificationFactory::Renderer do
       expect(renderer.render).to eq ''
     end
 
+    context 'when rendering templates with ERB tags' do
+
+      let(:template) { '<%% <%= "<%" %> %%>' }
+
+      it 'ignores pre-existing ERB tags in an untrusted template' do
+        renderer = build :notification_factory_renderer, template: template
+        expect(renderer.render).to eq '<% <%= "<%" %> %%>'
+      end
+
+      it 'executes pre-existing ERB tags in a trusted template' do
+        renderer = build :notification_factory_renderer, template: template, trusted: true
+        expect(renderer.render).to eq '<% <% %%>'
+      end
+    end
+
     it 'correctly renders chained object references' do
       user = User.where(firstname: 'Nicole').first
       ticket = create :ticket, customer: user
@@ -30,17 +45,17 @@ RSpec.describe NotificationFactory::Renderer do
     end
 
     it 'raises a StandardError when rendering a template with a broken syntax' do
-      renderer = build :notification_factory_renderer, template: 'test <% if %>', objects: {}
+      renderer = build :notification_factory_renderer, template: 'test <% if %>', objects: {}, trusted: true
       expect { renderer.render }.to raise_error(StandardError)
     end
 
     it 'raises a StandardError when rendering a template calling a non existant method' do
-      renderer = build :notification_factory_renderer, template: 'test <% Ticket.non_existant_method %>', objects: {}
+      renderer = build :notification_factory_renderer, template: 'test <% Ticket.non_existant_method %>', objects: {}, trusted: true
       expect { renderer.render }.to raise_error(StandardError)
     end
 
     it 'raises a StandardError when rendering a template referencing a non existant object' do
-      renderer = build :notification_factory_renderer, template: 'test <% NonExistantObject.first %>', objects: {}
+      renderer = build :notification_factory_renderer, template: 'test <% NonExistantObject.first %>', objects: {}, trusted: true
       expect { renderer.render }.to raise_error(StandardError)
     end
 

spec/lib/notification_factory/template_spec.rb

@@ -4,9 +4,11 @@ require 'rails_helper'
 
 RSpec.describe NotificationFactory::Template do
   subject(:template) do
-    described_class.new(template_string, escape)
+    described_class.new(template_string, escape, trusted)
   end
 
+  let(:trusted) { false }
+
   describe '#to_s' do
     context 'for empty input template (incl. whitespace-only)' do
       let(:template_string) { "\#{ }" }
@@ -28,6 +30,28 @@ RSpec.describe NotificationFactory::Template do
       end
     end
 
+    context 'for sanitizing the template string' do
+      let(:escape) { false }
+
+      context 'for strings containing ERB' do
+        let(:template_string) { '<%% <% "<%" %> <%# comment %> <%= "<%" %> <%- "" %> %%>' }
+
+        context 'for untrusted templates' do
+          it 'mutes all pre-existing ERB tags' do
+            expect(template.to_s).to eq('<%% <%% "<%%" %> <%%# comment %> <%%= "<%%" %> <%%- "" %> %%>')
+          end
+        end
+
+        context 'for trusted templates' do
+          let(:trusted) { true }
+
+          it 'keeps all pre-existing ERB tags' do
+            expect(template.to_s).to eq(template_string)
+          end
+        end
+      end
+    end
+
     context 'for input template using #t helper' do
       let(:template_string) { "\#{t('some text')}" }
       let(:escape) { false }