isso-comments · pkvach · Apr 16, 2024
diff --git 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
@@ -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
@@ -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 <https://github.com/isso-comments/isso/issues/751>`_.
+       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
@@ -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 <img> 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 <img> 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
@@ -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
@@ -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: <img src="..." />', "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"),
+            [
+                '<p><a href="http://example.org/" rel="nofollow noopener">http://example.org/</a> and sms:+1234567890</p>',
+                '<p><a rel="nofollow noopener" href="http://example.org/">http://example.org/</a> and sms:+1234567890</p>',
+            ],
+        )
+
+    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"), "<p>http://example.org/ and sms:+1234567890</p>")
+
+    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"), "<p>plain text</p>")
+
+    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 <img> tag.
+        self.assertEqual(renderer("![](cat.gif)"), '<p><img src="cat.gif"></p>')
+
+    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"),
+            [
+                '<p><a href="http://example.org/" rel="nofollow noopener">http://example.org/</a> and plain</p>',
+                '<p><a rel="nofollow noopener" href="http://example.org/">http://example.org/</a> and plain</p>',
+            ],
+        )
+
+    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)