Merge remote-tracking branch 'origin/release/17.3' into release/17.4

config/locales/en.yml

@@ -4556,6 +4556,7 @@ en:
   label_total_days_off: "Total days off"
   macro_execution_error: "Error executing the macro %{macro_name}"
   macro_unavailable: "Macro %{macro_name} cannot be displayed."
+  macro_unknown: "Unknown or unsupported macro."
   macros:
     placeholder: "[Placeholder] Macro %{macro_name}"
     errors:

lib/open_project/text_formatting/filters/macro_filter.rb

@@ -41,9 +41,13 @@ module OpenProject::TextFormatting
 
       def call # rubocop:disable Metrics/AbcSize
         doc.search("macro").each do |macro|
+          matched = false
+
           registered.each do |macro_class|
             next unless macro_applies?(macro_class, macro)
 
+            matched = true
+
             # If requested to skip macro expansion, do that
             if context[:disable_macro_expansion]
               macro.replace macro_placeholder(macro_class)
@@ -60,6 +64,8 @@ module OpenProject::TextFormatting
               break
             end
           end
+
+          macro.replace unknown_macro_placeholder unless matched
         end
 
         doc
@@ -67,6 +73,13 @@ module OpenProject::TextFormatting
 
       private
 
+      def unknown_macro_placeholder
+        ApplicationController.helpers.content_tag :macro,
+                                                  I18n.t(:macro_unknown),
+                                                  class: "macro-unavailable",
+                                                  data: { macro_name: "unknown" }
+      end
+
       def macro_error_placeholder(macro_class, message)
         ApplicationController.helpers.content_tag :macro,
                                                   "#{I18n.t(:macro_execution_error,

lib/open_project/text_formatting/filters/sanitization_filter.rb

@@ -52,8 +52,8 @@ module OpenProject::TextFormatting
           remove_contents: Array(base[:remove_contents]) | %w[svg style],
 
           attributes: base_attrs.deep_merge(
-            # Whitelist class and data-* attributes on all macros
-            "macro" => ["class", :data],
+            # Explicit allowlist of data-* attributes used by registered macros.
+            "macro" => %w[class data-type data-classes data-page data-include-parent data-macro-name data-query-props data-pull-request-id data-pull-request-state],
             # mentions
             "mention" => %w[data-type data-text data-id class],
             # add styles to tables

spec/lib/open_project/text_formatting/filters/macro_attribute_handling_spec.rb

@@ -0,0 +1,109 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+
+RSpec.describe "macro element attribute handling" do # rubocop:disable RSpec/DescribeClass
+  def sanitize(html)
+    filter = OpenProject::TextFormatting::Filters::SanitizationFilter.new(html, {})
+    result = filter.call
+    result.respond_to?(:to_html) ? result.to_html : result.to_s
+  end
+
+  def apply_macro_filter(html)
+    filter = OpenProject::TextFormatting::Filters::MacroFilter.new(html, {})
+    result = filter.call
+    result.respond_to?(:to_html) ? result.to_html : result.to_s
+  end
+
+  describe OpenProject::TextFormatting::Filters::SanitizationFilter do
+    describe "macro element data attribute restrictions" do
+      it "strips data-controller from macro elements" do
+        html = '<macro class="x" data-controller="poll-for-changes">.</macro>'
+        expect(sanitize(html)).not_to include("data-controller")
+      end
+
+      it "strips data-action from macro elements" do
+        html = '<macro class="x" data-action="click->foo#bar">.</macro>'
+        expect(sanitize(html)).not_to include("data-action")
+      end
+
+      it "strips arbitrary data-* stimulus value attributes from macro elements" do
+        html = '<macro class="x" data-poll-for-changes-url-value="/api/v3/attachments/1/content" ' \
+               'data-poll-for-changes-interval-value="2000">.</macro>'
+        output = sanitize(html)
+        expect(output).not_to include("data-poll-for-changes-url-value")
+        expect(output).not_to include("data-poll-for-changes-interval-value")
+      end
+
+      it "strips data-controller from arbitrary non-macro elements" do
+        html = '<div data-controller="poll-for-changes"><p data-controller="evil">text</p></div>'
+        output = sanitize(html)
+        expect(output).not_to include("data-controller")
+      end
+
+      it "preserves data-type on macro elements (used by create-work-package-link macro)" do
+        html = '<macro class="create-work-package-link" data-type="Task">.</macro>'
+        expect(sanitize(html)).to include('data-type="Task"')
+      end
+
+      it "preserves data-page on macro elements (used by child-pages macro)" do
+        html = '<macro class="child-pages" data-page="some-page" data-include-parent="true">.</macro>'
+        output = sanitize(html)
+        expect(output).to include('data-page="some-page"')
+        expect(output).to include('data-include-parent="true"')
+      end
+
+      it "preserves data-macro-name on macro elements (used by placeholder rendering)" do
+        html = '<macro class="macro-placeholder" data-macro-name="toc">placeholder</macro>'
+        expect(sanitize(html)).to include('data-macro-name="toc"')
+      end
+    end
+  end
+
+  describe OpenProject::TextFormatting::Filters::MacroFilter do
+    describe "unrecognized macro elements" do
+      it "replaces macro elements whose class does not match any registered macro with an unavailable placeholder" do
+        html = '<p><macro class="x">.</macro></p>'
+        output = apply_macro_filter(html)
+        expect(output).not_to include('class="x"')
+        expect(output).to include("macro-unavailable")
+        expect(output).to include("Unknown or unsupported macro.")
+      end
+
+      it "replaces macro elements with no class with an unavailable placeholder" do
+        html = "<p><macro>.</macro></p>"
+        output = apply_macro_filter(html)
+        expect(output).to include("macro-unavailable")
+        expect(output).to include("Unknown or unsupported macro.")
+      end
+    end
+  end
+end