Merge pull request #3876 from OSC/backport-motd-erb

markdown_erb needs to sanitize html too (backport #3675)

apps/dashboard/app/models/motd_formatter/base_formatter.rb

@@ -0,0 +1,14 @@
+module MotdFormatter
+
+  class BaseFormatter
+    include ActionView::Helpers::SanitizeHelper
+
+    def safe_content(content)
+      if Configuration.motd_render_html?
+        content.html_safe
+      else
+        sanitize(content)
+      end
+    end
+  end
+end

apps/dashboard/app/models/motd_formatter/markdown.rb

@@ -1,9 +1,6 @@
 module MotdFormatter
   # Utility class for rendering Markdown MOTD files.
-  class Markdown
-
-    include ActionView::Helpers::SanitizeHelper
-
+  class Markdown < BaseFormatter
     attr_reader :content, :title
 
     # @param [MotdFile] motd_file an MotdFile object that contains a URI path to a message of the day in OSC format
@@ -17,13 +14,5 @@ def initialize(motd_file)
     def to_partial_path
       "dashboard/motd_markdown"
     end
-
-    def safe_content(content)
-      if Configuration.motd_render_html?
-        content.html_safe
-      else
-        sanitize(content)
-      end
-    end
   end
 end

apps/dashboard/app/models/motd_formatter/markdown_erb.rb

@@ -1,16 +1,19 @@
+# frozen_string_literal: true
+
 module MotdFormatter
   # Utility class for rendering Markdown MOTD files after ERB rendering them.
- class MarkdownErb
+  class MarkdownErb < BaseFormatter
     attr_reader :content, :title
 
     def initialize(motd_file)
       motd_file ||= MotdFile.new unless motd_file
       @title = motd_file.title
-      @content = OodAppkit.markdown.render(ERB.new(motd_file.content).result)
+      content = OodAppkit.markdown.render(ERB.new(motd_file.content).result)
+      @content = safe_content(content)
     end
-    
+
     def to_partial_path
-      "dashboard/motd_markdown"
+      'dashboard/motd_markdown'
     end
- end  
+  end
 end

apps/dashboard/test/fixtures/files/motd_md_erb_w_html

@@ -0,0 +1,3 @@
+# Some Markdown file
+
+<script>var msg = 'this was a <%= 'script' %>';</script>

apps/dashboard/test/models/motd_formatter/motd_formatter_markdown_erb_test.rb

@@ -51,5 +51,38 @@ class MotdFormatterMarkdownErbTest < ActiveSupport::TestCase
 
     assert_not_nil formatted_motd.content
   end
+
+  test 'content is html safe by default' do
+    path = "#{Rails.root}/test/fixtures/files/motd_md_erb_w_html"
+    motd = MotdFile.new(path)
+    formatted_motd = MotdFormatterMarkdownErb.new(motd)
+    content = formatted_motd.content
+
+    expected_content = "<h1>Some Markdown file</h1>\n" \
+    "\n" \
+    "var msg = 'this was a script';\n"
+
+    assert_not_nil(content)
+    assert_equal(expected_content, content)
+  end
+
+  # this test is very similar to above, but the content
+  # has a <script> tag still in it.
+  test 'content can contain html if configured' do
+    Configuration.stubs(:motd_render_html).returns(true)
+
+    path = "#{Rails.root}/test/fixtures/files/motd_md_erb_w_html"
+    motd = MotdFile.new(path)
+    formatted_motd = MotdFormatterMarkdownErb.new(motd)
+    content = formatted_motd.content
+
+    expected_content = "<h1>Some Markdown file</h1>\n" \
+    "\n" \
+    "<script>var msg = 'this was a script';</script>\n"
+
+    assert_not_nil(content)
+    assert_equal(expected_content, content)
+  end
+
 end