From ea5b338a364762551d0d90458d53890b06ddd3cd Mon Sep 17 00:00:00 2001 From: MAEDA Go Date: Fri, 9 Oct 2026 13:25:12 +0900 Subject: [PATCH 1/4] Extract the sanitization rules common to all text formatters from CommonMark::SanitizationFilter into a base class --- .../common_mark/sanitization_filter.rb | 162 ++--------------- .../wiki_formatting/sanitization_filter.rb | 169 ++++++++++++++++++ 2 files changed, 187 insertions(+), 144 deletions(-) create mode 100644 lib/redmine/wiki_formatting/sanitization_filter.rb diff --git a/lib/redmine/wiki_formatting/common_mark/sanitization_filter.rb b/lib/redmine/wiki_formatting/common_mark/sanitization_filter.rb index 32b839122..815c393b7 100644 --- a/lib/redmine/wiki_formatting/common_mark/sanitization_filter.rb +++ b/lib/redmine/wiki_formatting/common_mark/sanitization_filter.rb @@ -20,130 +20,28 @@ module Redmine module WikiFormatting module CommonMark - # sanitizes rendered HTML using the Sanitize gem - class SanitizationFilter - include Redmine::Helpers::URL - - attr_accessor :allowlist - - LISTS = Set.new(%w[ul ol].freeze) - LIST_ITEM = 'li' - - # List of table child elements. These must be contained by a element - # or they are not allowed through. Otherwise they can be used to break out - # of places we're using tables to contain formatted user content (like pull - # request review comments). - TABLE_ITEMS = Set.new(%w[tr td th].freeze) - TABLE = 'table' - TABLE_SECTIONS = Set.new(%w[thead tbody tfoot].freeze) - - # The main sanitization allowlist. Only these elements and attributes are - # allowed through by default. - ALLOWLIST = { - :elements => %w[ - h1 h2 h3 h4 h5 h6 br b i strong em a pre code img input tt u - div ins del sup sub p ol ul table thead tbody tfoot blockquote - dl dt dd kbd q samp var hr ruby rt rp li tr td th s strike summary - details caption figure figcaption - abbr bdo cite dfn mark small span time wbr - ].freeze, - :remove_contents => ['script'].freeze, - :attributes => { - 'a' => %w[href id name].freeze, - 'img' => %w[src longdesc].freeze, - 'code' => ['class'].freeze, - 'div' => %w[class itemscope itemtype].freeze, - 'li' => %w[id class].freeze, - 'input' => %w[class type].freeze, - 'p' => ['class'].freeze, - 'ul' => ['class'].freeze, - 'blockquote' => ['cite'].freeze, - 'del' => ['cite'].freeze, - 'ins' => ['cite'].freeze, - 'q' => ['cite'].freeze, - :all => %w[ - abbr accept accept-charset - accesskey action align alt - aria-describedby aria-hidden aria-label aria-labelledby - axis border cellpadding cellspacing char - charoff charset checked - clear cols colspan color - compact coords datetime dir - disabled enctype for frame - headers height hreflang - hspace ismap label lang - maxlength media method - multiple nohref noshade - nowrap open progress prompt readonly rel rev - role rows rowspan rules scope - selected shape size span - start style summary tabindex target - title type usemap valign value - vspace width itemprop - ].freeze - }.freeze, - :protocols => { - 'blockquote' => { 'cite' => ['http', 'https', :relative].freeze }, - 'del' => { 'cite' => ['http', 'https', :relative].freeze }, - 'ins' => { 'cite' => ['http', 'https', :relative].freeze }, - 'q' => { 'cite' => ['http', 'https', :relative].freeze }, - 'img' => { - 'src' => ['http', 'https', :relative].freeze, - 'longdesc' => ['http', 'https', :relative].freeze - }.freeze - }, - :transformers => [ - # Top-level
  • elements are removed because they can break out of - # containing markup. - lambda { |env| - name = env[:node_name] - node = env[:node] - if name == LIST_ITEM && node.ancestors.none? { |n| LISTS.include?(n.name) } - node.replace(node.children) - end - }, - - # Table child elements that are not contained by a
    • are removed. - lambda { |env| - name = env[:node_name] - node = env[:node] - if (TABLE_SECTIONS.include?(name) || TABLE_ITEMS.include?(name)) && node.ancestors.none? { |n| n.name == TABLE } - node.replace(node.children) - end - } - ].freeze, - :css => { - :properties => %w[ - color background-color - width min-width max-width - height min-height max-height - padding padding-left padding-right padding-top padding-bottom - margin margin-left margin-right margin-top margin-bottom - border border-left border-right border-top border-bottom border-radius border-style border-collapse border-spacing - font font-style font-variant font-weight font-stretch font-size line-height font-family - text-align text-decoration - float - ].freeze - } - }.freeze - - RELAXED_PROTOCOL_ATTRS = { - "a" => %w(href).freeze, - }.freeze - - def initialize - @allowlist = default_allowlist - add_transformers - end + # Sanitizes the HTML generated by the CommonMark formatter + class SanitizationFilter < Redmine::WikiFormatting::SanitizationFilter + private - def call(doc) - # Sanitize is applied to the whole document, so the API is different from loofeh's scrubber. - Sanitize.clean_node!(doc, allowlist) + def default_allowlist + allowlist = super + allowlist[:elements] += %w[input] + allowlist[:attributes].merge!( + 'a' => %w[href id name], + 'code' => %w[class], + 'div' => %w[class itemscope itemtype], + 'li' => %w[id class], + 'input' => %w[class type], + 'p' => %w[class], + 'ul' => %w[class] + ) + allowlist end - private - def add_transformers + super + # allow class on code tags (this holds the language info from fenced # code bocks and has the format language-foo) allowlist[:transformers].push lambda {|env| @@ -227,30 +125,6 @@ module Redmine node.remove_attribute("class") } - - # https://github.com/rgrove/sanitize/issues/209 - allowlist[:transformers].push lambda {|env| - node = env[:node] - return if node.type != Nokogiri::XML::Node::ELEMENT_NODE - - name = env[:node_name] - return unless RELAXED_PROTOCOL_ATTRS.include?(name) - - RELAXED_PROTOCOL_ATTRS[name].each do |attr| - next unless node.has_attribute?(attr) - - node[attr] = node[attr].strip - unless !node[attr].empty? && uri_with_link_safe_scheme?(node[attr]) - node.remove_attribute(attr) - end - end - } - end - - # The allowlist to use when sanitizing. This can be passed in the context - # hash to the filter but defaults to ALLOWLIST constant value above. - def default_allowlist - ALLOWLIST.deep_dup end end end diff --git a/lib/redmine/wiki_formatting/sanitization_filter.rb b/lib/redmine/wiki_formatting/sanitization_filter.rb new file mode 100644 index 000000000..f280ffd24 --- /dev/null +++ b/lib/redmine/wiki_formatting/sanitization_filter.rb @@ -0,0 +1,169 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006- Jean-Philippe Lang +# +# 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. + +module Redmine + module WikiFormatting + # Sanitizes rendered HTML using the Sanitize gem. The allowlist only covers + # what is common to all formatters; classes, ids and elements specific to a + # formatter are allowed by its own subclass. + class SanitizationFilter + include Redmine::Helpers::URL + + attr_accessor :allowlist + + LISTS = Set.new(%w[ul ol].freeze) + LIST_ITEM = 'li' + + # List of table child elements. These must be contained by a
    element + # or they are not allowed through. Otherwise they can be used to break out + # of places we're using tables to contain formatted user content (like pull + # request review comments). + TABLE_ITEMS = Set.new(%w[tr td th].freeze) + TABLE = 'table' + TABLE_SECTIONS = Set.new(%w[thead tbody tfoot].freeze) + + # The main sanitization allowlist. Only these elements and attributes are + # allowed through by default. + ALLOWLIST = { + :elements => %w[ + h1 h2 h3 h4 h5 h6 br b i strong em a pre code img tt u + div ins del sup sub p ol ul table thead tbody tfoot blockquote + dl dt dd kbd q samp var hr ruby rt rp li tr td th s strike summary + details caption figure figcaption + abbr bdo cite dfn mark small span time wbr + ].freeze, + :remove_contents => ['script'].freeze, + :attributes => { + 'a' => %w[href name].freeze, + 'img' => %w[src longdesc].freeze, + 'div' => %w[itemscope itemtype].freeze, + 'blockquote' => ['cite'].freeze, + 'del' => ['cite'].freeze, + 'ins' => ['cite'].freeze, + 'q' => ['cite'].freeze, + :all => %w[ + abbr accept accept-charset + accesskey action align alt + aria-describedby aria-hidden aria-label aria-labelledby + axis border cellpadding cellspacing char + charoff charset checked + clear cols colspan color + compact coords datetime dir + disabled enctype for frame + headers height hreflang + hspace ismap label lang + maxlength media method + multiple nohref noshade + nowrap open progress prompt readonly rel rev + role rows rowspan rules scope + selected shape size span + start style summary tabindex target + title type usemap valign value + vspace width itemprop + ].freeze + }.freeze, + :protocols => { + 'blockquote' => { 'cite' => ['http', 'https', :relative].freeze }, + 'del' => { 'cite' => ['http', 'https', :relative].freeze }, + 'ins' => { 'cite' => ['http', 'https', :relative].freeze }, + 'q' => { 'cite' => ['http', 'https', :relative].freeze }, + 'img' => { + 'src' => ['http', 'https', :relative].freeze, + 'longdesc' => ['http', 'https', :relative].freeze + }.freeze + }, + :transformers => [ + # Top-level
  • elements are removed because they can break out of + # containing markup. + lambda { |env| + name = env[:node_name] + node = env[:node] + if name == LIST_ITEM && node.ancestors.none? { |n| LISTS.include?(n.name) } + node.replace(node.children) + end + }, + + # Table child elements that are not contained by a
    • are removed. + lambda { |env| + name = env[:node_name] + node = env[:node] + if (TABLE_SECTIONS.include?(name) || TABLE_ITEMS.include?(name)) && node.ancestors.none? { |n| n.name == TABLE } + node.replace(node.children) + end + } + ].freeze, + :css => { + :properties => %w[ + color background-color + width min-width max-width + height min-height max-height + padding padding-left padding-right padding-top padding-bottom + margin margin-left margin-right margin-top margin-bottom + border border-left border-right border-top border-bottom border-radius border-style border-collapse border-spacing + font font-style font-variant font-weight font-stretch font-size line-height font-family + text-align text-decoration + float + ].freeze + } + }.freeze + + RELAXED_PROTOCOL_ATTRS = { + "a" => %w(href).freeze, + }.freeze + + def initialize + @allowlist = default_allowlist + add_transformers + end + + def call(doc) + # Sanitize is applied to the whole document, so the API is different from loofeh's scrubber. + Sanitize.clean_node!(doc, allowlist) + end + + private + + def add_transformers + # https://github.com/rgrove/sanitize/issues/209 + allowlist[:transformers].push lambda {|env| + node = env[:node] + return if node.type != Nokogiri::XML::Node::ELEMENT_NODE + + name = env[:node_name] + return unless RELAXED_PROTOCOL_ATTRS.include?(name) + + RELAXED_PROTOCOL_ATTRS[name].each do |attr| + next unless node.has_attribute?(attr) + + node[attr] = node[attr].strip + unless !node[attr].empty? && uri_with_link_safe_scheme?(node[attr]) + node.remove_attribute(attr) + end + end + } + end + + # The allowlist to use when sanitizing. This can be passed in the context + # hash to the filter but defaults to ALLOWLIST constant value above. + def default_allowlist + ALLOWLIST.deep_dup + end + end + end +end -- 2.55.0