diff --git a/CHANGES.rst b/CHANGES.rst index e4eec3ddc..061eff043 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -12,12 +12,14 @@ New Features - Add no-cache headers to fix stale comment display (`#1110`_, pkvach) - Add webhook support for comment events (`#1123`_, p1slave) - Update comment counts in the admin interface dynamically after moderation. (`#1113`_, pkvach) +- Provide full control of allowed HTML elements via the configuration file (`#1007`_, pkvach) .. _#1111: https://github.com/isso-comments/isso/pull/1111 .. _#1117: https://github.com/isso-comments/isso/pull/1117 .. _#1110: https://github.com/isso-comments/isso/pull/1110 .. _#1123: https://github.com/isso-comments/isso/pull/1123 .. _#1113: https://github.com/isso-comments/isso/pull/1113 +.. _#1007: https://github.com/isso-comments/isso/pull/1007 0.14.0 (2026-03-26) -------------------- diff --git a/contrib/isso-dev.cfg b/contrib/isso-dev.cfg index 760494b4a..c509c3d79 100644 --- a/contrib/isso-dev.cfg +++ b/contrib/isso-dev.cfg @@ -38,7 +38,7 @@ reply-to-self = true [markup] options = autolink, fenced-code, no-intra-emphasis, strikethrough, superscript flags = -allowed-elements = +allowed-html-elements = allowed-attributes = [hash] diff --git a/docs/docs/reference/server-config.rst b/docs/docs/reference/server-config.rst index 1517ef735..086e9cebc 100644 --- a/docs/docs/reference/server-config.rst +++ b/docs/docs/reference/server-config.rst @@ -438,7 +438,7 @@ For a more detailed explanation, see :doc:`/docs/reference/markdown-config`. renderer = mistune options = strikethrough, superscript, autolink, fenced-code flags = - allowed-elements = + allowed-html-elements = allowed-attributes = renderer @@ -470,7 +470,7 @@ allowed-elements By default, only ``a``, ``blockquote``, ``br``, ``code``, ``del``, ``em``, ``h1``, ``h2``, ``h3``, ``h4``, ``h5``, ``h6``, ``hr``, ``ins``, ``li``, - ``ol``, ``p``, ``pre``, ``strong``, ``table``, ``tbody``, ``td``, ``th``, + ``ol``, ``p``, ``pre``, ``strong``, ``table``, ``tbody``, ``tr``, ``td``, ``th``, ``thead`` and ``ul`` are allowed. For a more detailed explanation, see :doc:`/docs/reference/markdown-config`. @@ -482,11 +482,29 @@ allowed-elements mean that ``br, code, del, ...`` and all other default allowed tags are still allowed. You can only add *additional* elements here. - It is planned to change this behavior, see - `this issue `_. + To specify a list of *only* allowed elements, use the + ``allowed-html-elements`` option instead. Default: (empty) + .. deprecated:: 0.14.1 + + Superseded by ``allowed-html-elements``, which gives full control over + the list of allowed elements instead of only appending to it. If both + options are set, elements listed here are still added on top of + ``allowed-html-elements`` for backwards compatibility, but a warning + is logged. Please migrate to ``allowed-html-elements``. + +allowed-html-elements + + **Only** allow the specified HTML tags in the generated output, comma-separated. + If ``allowed-elements`` is also set, its elements are added on top of this + list (with a deprecation warning logged); migrate them here instead. + + Default: (empty) + + .. versionadded:: 0.14.1 + allowed-attributes **Additional** HTML attributes (independent from elements) to allow in the generated output, comma-separated. diff --git a/isso/html/__init__.py b/isso/html/__init__.py index 540f608e0..e2a72c974 100644 --- a/isso/html/__init__.py +++ b/isso/html/__init__.py @@ -15,40 +15,7 @@ def allow_attribute_class(tag, name, value): return name == "class" and bool(Sanitizer.code_language_pattern.match(value)) def __init__(self, elements, attributes): - # attributes found in Sundown's HTML serializer [1] - # - except for tag, because images are not generated anyway. - # - sub and sup added - # - # [1] https://github.com/vmg/sundown/blob/master/html/html.c - self.elements = [ - "a", - "p", - "hr", - "br", - "ol", - "ul", - "li", - "pre", - "code", - "blockquote", - "del", - "ins", - "strong", - "em", - "h1", - "h2", - "h3", - "h4", - "h5", - "h6", - "sub", - "sup", - "table", - "thead", - "tbody", - "th", - "td", - ] + elements + self.elements = elements # allowed attributes for tags self.attributes = {"table": ["align"], "a": ["href"], "code": Sanitizer.allow_attribute_class, "*": attributes} @@ -107,13 +74,74 @@ def __init__(self, conf): ) raise ValueError("Invalid renderer value: %s. Set to either `mistune` or `misaka`." % conf_markup.get("renderer")) - # Filter out empty strings: - allowed_elements = [x for x in conf_markup.getlist("allowed-elements") if x] + # Filter out empty strings. Both options are optional and may be + # missing from the configuration file entirely. + allowed_html_elements = ( + [x for x in conf_markup.getlist("allowed-html-elements") if x] + if conf_markup.has_option("allowed-html-elements") + else [] + ) allowed_attributes = [x for x in conf_markup.getlist("allowed-attributes") if x] + # "allowed-elements" is deprecated in favor of "allowed-html-elements", + # so it may no longer be present in the configuration file. + legacy_allowed_elements = ( + [x for x in conf_markup.getlist("allowed-elements") if x] if conf_markup.has_option("allowed-elements") else [] + ) + + # if "allowed-html-elements" option is set, add "allowed-elements" on top of it + if allowed_html_elements: + allowed_elements = list(allowed_html_elements) + + new_elements = [x for x in legacy_allowed_elements if x not in allowed_elements] + if new_elements: + allowed_elements += new_elements + logger.warning( + "The `allowed-elements` configuration option is deprecated and will be removed in a " + "future release. Please migrate the following elements to `allowed-html-elements`: %s", + ", ".join(new_elements), + ) + else: + # attributes found in Sundown's HTML serializer [1] + # - except for tag, because images are not generated anyways. + # - sub and sup added + # + # [1] https://github.com/vmg/sundown/blob/master/html/html.c + allowed_elements = [ + "a", + "p", + "hr", + "br", + "ol", + "ul", + "li", + "pre", + "code", + "blockquote", + "del", + "ins", + "strong", + "em", + "h1", + "h2", + "h3", + "h4", + "h5", + "h6", + "sub", + "sup", + "table", + "thead", + "tbody", + "tr", + "th", + "td", + ] + legacy_allowed_elements + # If images are allowed, source element should be allowed as well if "img" in allowed_elements and "src" not in allowed_attributes: allowed_attributes.append("src") + self.sanitizer = Sanitizer(allowed_elements, allowed_attributes) def render(self, text): diff --git a/isso/isso.cfg b/isso/isso.cfg index c9b1ca439..3e2065f02 100644 --- a/isso/isso.cfg +++ b/isso/isso.cfg @@ -212,10 +212,10 @@ website-field = true # specific to mistune. renderer = mistune -# Additional HTML tags to allow in the generated output, comma-separated. By -# default, only a, blockquote, br, code, del, em, h1, h2, h3, h4, h5, h6, hr, -# ins, li, ol, p, pre, strong, table, tbody, td, th, thead and ul are allowed. -allowed-elements = +# Only allow the specified HTML tags in the generated output, comma-separated. +# By default, only a, blockquote, br, code, del, em, h1, h2, h3, h4, h5, h6, hr, +# ins, li, ol, p, pre, strong, table, tbody, tr, td, th, thead and ul are allowed. +allowed-html-elements = # Additional HTML attributes (independent from elements) to allow in the # generated output, comma-separated. By default, only align and href are diff --git a/isso/tests/test_html.py b/isso/tests/test_html.py index fb1a714da..32fe3f686 100644 --- a/isso/tests/test_html.py +++ b/isso/tests/test_html.py @@ -2,12 +2,13 @@ import unittest +from isso import config, html from isso.html import Sanitizer class TestHTML(unittest.TestCase): def test_sanitizer(self): - sanitizer = Sanitizer(elements=[], attributes=[]) + sanitizer = Sanitizer(elements=["p", "a", "code"], attributes=["href"]) examples = [ ('Look: ', "Look: "), ( @@ -38,3 +39,76 @@ def test_sanitizer_extensions(self): for element, expected in examples: self.assertEqual(sanitizer.sanitize(element), expected) + + @staticmethod + def _markup_conf(mistune_plugins="", **markup_options): + """Build a full config (as passed to `html.Markup`, i.e. *not* + pre-sectioned) with the given options merged into [markup]. Renders + with Mistune, which also needs a [markup.mistune] section; pass + `mistune_plugins` (e.g. "url") to enable Mistune plugins.""" + options = {"renderer": "mistune", "allowed-attributes": ""} + options.update(markup_options) + return config.new( + { + "markup": options, + "markup.mistune": {"plugins": mistune_plugins, "parameters": ""}, + } + ) + + def test_render(self): + conf = self._markup_conf(mistune_plugins="url", **{"allowed-elements": "a, p", "allowed-attributes": "href"}) + renderer = html.Markup(conf).render + self.assertIn( + renderer("http://example.org/ and sms:+1234567890"), + [ + '

http://example.org/ and sms:+1234567890

', + '

http://example.org/ and sms:+1234567890

', + ], + ) + + def test_render_with_allowed_html_elements(self): + conf = self._markup_conf(**{"allowed-elements": "a, p", "allowed-html-elements": "p", "allowed-attributes": "href"}) + renderer = html.Markup(conf).render + self.assertEqual(renderer("http://example.org/ and sms:+1234567890"), "

http://example.org/ and sms:+1234567890

") + + def test_render_allowed_elements_missing_defaults_to_builtin_list(self): + """Neither `allowed-elements` nor `allowed-html-elements` is set: the + built-in element list still applies and nothing crashes.""" + conf = self._markup_conf() + renderer = html.Markup(conf).render + self.assertEqual(renderer("plain text"), "

plain text

") + + def test_render_allowed_html_elements_missing_falls_back_to_allowed_elements(self): + """`allowed-html-elements` absent from the config file entirely (not + just empty) should not crash, and `allowed-elements` still applies.""" + conf = self._markup_conf(**{"allowed-elements": "img", "allowed-attributes": ""}) + renderer = html.Markup(conf).render + # Mistune escapes raw HTML, so exercise the `img` element through a + # Markdown image rather than a literal tag. + self.assertEqual(renderer("![](cat.gif)"), '

') + + def test_render_merges_allowed_elements_into_allowed_html_elements(self): + """When both options are set, `allowed-elements` is added on top of + `allowed-html-elements`, and a deprecation warning is logged.""" + conf = self._markup_conf( + mistune_plugins="url", **{"allowed-html-elements": "p", "allowed-elements": "a, p", "allowed-attributes": "href"} + ) + + with self.assertLogs("isso", level="WARNING") as cm: + markup = html.Markup(conf) + + self.assertTrue(any("allowed-elements" in message and "deprecated" in message for message in cm.output)) + self.assertIn( + markup.render("http://example.org/ and plain"), + [ + '

http://example.org/ and plain

', + '

http://example.org/ and plain

', + ], + ) + + def test_render_no_warning_when_allowed_elements_already_covered(self): + """No new elements are merged in, so no deprecation warning is logged.""" + conf = self._markup_conf(**{"allowed-html-elements": "a, p", "allowed-elements": "a", "allowed-attributes": "href"}) + + with self.assertNoLogs("isso", level="WARNING"): + html.Markup(conf)