Fix issues as reported by bandit or mark as nosec

Nothing worrying, but good to have checked this regardless
and important to have a green pipeline.

Fix #175

theme/templatetags/code.py

@@ -1,6 +1,7 @@
 from pygments import highlight
 from pygments.lexers import get_lexer_by_name
 from pygments.formatters import HtmlFormatter
+from bugsink.utils import assert_
 
 from django import template
 
@@ -28,8 +29,8 @@ class CodeNode(template.Node):
         content = "\n".join([line.rstrip() for line in content.split("\n")])
 
         lang_identifier, code = content.split("\n", 1)
-        assert lang_identifier.startswith(":::") or lang_identifier.startswith("#!"), \
-            "Expected code block identifier ':::' or '#!' not " + lang_identifier
+        assert_(lang_identifier.startswith(":::") or lang_identifier.startswith("#!"),
+                "Expected code block identifier ':::' or '#!' not " + lang_identifier)
 
         lang = lang_identifier[3:].strip() if lang_identifier.startswith(":::") else lang_identifier[2:].strip()
         is_shebang = lang_identifier.startswith("#!")
@@ -37,4 +38,5 @@ class CodeNode(template.Node):
 
         lexer = get_lexer_by_name(lang, stripall=True)
 
-        return highlight(code, lexer, formatter).replace("highlight", "p-4 mt-4 bg-slate-50 dark:bg-slate-800 syntax-coloring")
+        return highlight(code, lexer, formatter).replace(
+            "highlight", "p-4 mt-4 bg-slate-50 dark:bg-slate-800 syntax-coloring")

theme/templatetags/issues.py

@@ -5,12 +5,12 @@ from pygments import highlight
 from pygments.formatters import HtmlFormatter
 
 from django.utils.html import escape
-from django.utils.safestring import mark_safe
+from django.utils.safestring import SafeData, mark_safe
 from django.template.defaultfilters import date
 
 from compat.timestamp import parse_timestamp
 
-
+from bugsink.utils import assert_
 from bugsink.pygments_extensions import guess_lexer_for_filename, lexer_for_platform
 
 register = template.Library()
@@ -23,7 +23,7 @@ def _split(joined, lengths):
         result.append(joined[start:start + length])
         start += length
 
-    assert [len(r) for r in result] == lengths
+    assert_([len(r) for r in result] == lengths)
     return result
 
 
@@ -52,7 +52,7 @@ def _core_pygments(code, filename=None, platform=None):
     # a line is" is not properly defined. (i.e.: is the thing after the final newline a line or not, both for the input
     # and the output?). At the level of _pygmentize_lines the idea of a line is properly defined, so we only have to
     # deal with pygments' funnyness.
-    # assert len(code.split("\n")) == result.count("\n"), "%s != %s" % (len(code.split("\n")), result.count("\n"))
+    # assert_(len(code.split("\n")) == result.count("\n"), "%s != %s" % (len(code.split("\n")), result.count("\n")))
 
     return result
 
@@ -71,7 +71,7 @@ def _pygmentize_lines(lines, filename=None, platform=None):
 
     # [:-1] to remove the last empty line, a result of split()
     result = _core_pygments(code, filename=filename, platform=platform).split('\n')[:-1]
-    assert len(lines) == len(result), "%s != %s" % (len(lines), len(result))
+    assert_(len(lines) == len(result), "%s != %s" % (len(lines), len(result)))
     return result
 
 
@@ -103,9 +103,10 @@ def pygmentize(value, platform):
 
     pre_context, context_lines, post_context = _split(lines, lengths)
 
-    value['pre_context'] = [mark_safe(s) for s in pre_context]
-    value['context_line'] = mark_safe(context_lines[0])
-    value['post_context'] = [mark_safe(s) for s in post_context]
+    # no_bandit_expl: see tests.TestPygmentizeEscape
+    value['pre_context'] = [mark_safe(s) for s in pre_context]  # nosec B703, B308
+    value['context_line'] = mark_safe(context_lines[0])  # nosec B703, B308
+    value['post_context'] = [mark_safe(s) for s in post_context]  # nosec B703, B308
 
     return value
 
@@ -141,6 +142,18 @@ def shortsha(value):
     return value[:12]
 
 
+def safe_join(sep, items, strict=False):
+    """join() that takes safe strings into account; strict=True means: I expect all inputs to be safe"""
+
+    text = sep.join(items)
+    if isinstance(sep, SafeData) and all(isinstance(i, SafeData) for i in items):
+        # no_bandit_expl: as per the check right above
+        return mark_safe(text)  # nosec B703, B308
+    if strict:
+        raise ValueError("Cannot join non-safe in strict mode")
+    return text
+
+
 @register.filter()
 def format_var(value):
     """Formats a variable for display in the template; deals with 'marked as incomplete'."""
@@ -170,23 +183,27 @@ def format_var(value):
 
     def gen_list(lst):
         for value in lst:
-            yield "", storevalue(value)
+            yield escape(""), storevalue(value)
 
         if hasattr(lst, "incomplete"):
-            yield f"<i>&lt;{lst.incomplete} items trimmed…&gt;</i>", None
+            # no_bandit_expl: constant string w/ substitution of an int (asserted)
+            assert_(isinstance(lst.incomplete, int))
+            yield mark_safe(f"<i>&lt;{lst.incomplete} items trimmed…&gt;</i>"), None  # nosec B703, B308
 
     def gen_dict(d):
         for (k, v) in d.items():
-            yield escape(repr(k)) + ": ", storevalue(v)
+            yield escape(repr(k)) + escape(": "), storevalue(v)
 
         if hasattr(d, "incomplete"):
-            yield f"<i>&lt;{d.incomplete} items trimmed…&gt;</i>", None
+            # no_bandit_expl: constant string w/ substitution of an int (asserted)
+            assert_(isinstance(d.incomplete, int))
+            yield mark_safe(f"<i>&lt;{d.incomplete} items trimmed…&gt;</i>"), None  # nosec B703, B308
 
     def gen_switch(obj):
         if isinstance(obj, list):
-            return bracket_wrap(gen_list(obj), "[", ", ", "]")
+            return bracket_wrap(gen_list(obj), escape("["), escape(", "), escape("]"))
         if isinstance(obj, dict):
-            return bracket_wrap(gen_dict(obj), "{", ", ", "}")
+            return bracket_wrap(gen_dict(obj), escape("{"), escape(", "), escape("}"))
         return gen_base(obj)
 
     result = []
@@ -209,8 +226,7 @@ def format_var(value):
             stack.append(todo)
             todo = gen_switch(recurse())
 
-    # mark_safe is OK because the only non-escaped characters are the brackets, commas, and colons.
-    return mark_safe("".join(result))
+    return safe_join(escape(""), result, strict=True)
 
 
 # recursive equivalent:
@@ -218,19 +234,17 @@ def format_var(value):
 # def format_var(value):
 #     """Formats a variable for display in the template; deals with 'marked as incomplete'.
 #     """
-#     # mark_safe is OK because the only non-escaped characters are the brackets, commas, and colons.
-#
 #     if isinstance(value, dict):
-#         parts = [(escape(repr(k)) + ": " + format_var(v)) for (k, v) in value.items()]
+#         parts = [(escape(repr(k)) + escape(": ") + format_var(v)) for (k, v) in value.items()]
 #         if hasattr(value, "incomplete"):
 #             parts.append(mark_safe(f"<i>&lt;{value.incomplete} items trimmed…&gt;</i>"))
-#         return mark_safe("{" + ", ".join(parts) + "}")
+#         return escape("{") + safe_join(escape(", "), parts, strict=True) + escape("}")
 #
 #     if isinstance(value, list):
 #         parts = [format_var(v) for v in value]
 #         if hasattr(value, "incomplete"):
 #             parts.append(mark_safe(f"<i>&lt;{value.incomplete} items trimmed…&gt;</i>"))
-#         return mark_safe("[" + ", ".join(parts) + "]")
+#         return escape("[") + safe_join(escape(", "), parts, strict=True) + escape("]")
 #
 #     return escape(value)
 
@@ -242,10 +256,12 @@ def incomplete(value):
 
 
 def _date_with_milis_html(timestamp):
-    return mark_safe(
-        '<span class="whitespace-nowrap">' +
-        date(timestamp, "j M G:i:s") + "." +
-        '<span class="text-xs">' + date(timestamp, "u")[:3] + '</span></span>')
+    # no_bandit_expl: constant string w/ substitution of an int (asserted)
+    return (
+        mark_safe('<span class="whitespace-nowrap">') +  # nosec
+        escape(date(timestamp, "j M G:i:s")) + mark_safe(".") +  # nosec
+        mark_safe('<span class="text-xs">') + escape(date(timestamp, "u")[:3]) +  # nosec
+        mark_safe('</span></span>'))  # nosec
 
 
 @register.filter

theme/tests.py

@@ -1,10 +1,11 @@
 from unittest import TestCase as RegularTestCase
 
+from django.utils.safestring import SafeString
 from bugsink.pygments_extensions import choose_lexer_for_pattern, get_all_lexers
 
 from events.utils import IncompleteList, IncompleteDict
 
-from .templatetags.issues import _pygmentize_lines as actual_pygmentize_lines, format_var
+from .templatetags.issues import _pygmentize_lines as actual_pygmentize_lines, format_var, pygmentize
 
 
 def _pygmentize_lines(lines):
@@ -109,6 +110,18 @@ class TestFormatVar(RegularTestCase):
             self._format_var(var),
         )
 
+    def test_format_var_nested_escaping(self):
+        # like format_nested, but with the focus on "does escaping happen correctly?"
+        var = {
+            "hacker": ["<script>"],
+        }
+
+        self.assertEqual(
+            '{&#x27;hacker&#x27;: [&lt;script&gt;]}',
+            format_var(var),
+        )
+        self.assertTrue(isinstance(format_var(var), SafeString))
+
     def test_format_var_deep(self):
         def _deep(level):
             result = None
@@ -138,3 +151,25 @@ class TestFormatVar(RegularTestCase):
             "{'a': 1, 'b': 2, 'c': 3, <i>&lt;9 items trimmed…&gt;</i>}",
             self._format_var(var),
         )
+
+
+class TestPygmentizeEscapeMarkSafe(RegularTestCase):
+
+    def test_escapes_html_in_all_contexts(self):
+        out = pygmentize(
+            {
+                'filename':     'test.py',
+                'pre_context':  ['<script>pre script</script>'],
+                'context_line': '<script>my script</script>',
+                'post_context': ['<script>post script</script>'],
+            },
+            platform='python',
+        )
+
+        for line in out['pre_context'] + [out['context_line']] + out['post_context']:
+            self.assertIsInstance(line, SafeString)
+
+            # we just check for the non-existance of <script> and </script> here because asserting against "whatever
+            # pygmentize does" is not very useful, as it may change in the future.
+            self.assertFalse("<script>" in line)
+            self.assertFalse("</script>" in line)