feat: sanitized preview for HTML email bodies

Restores the ability to read a formatted email, which the previous commit gave
up when it stopped serving the inbound HTML body as .html, and ends up safer
than the original behaviour rather than merely as safe.

The stored file keeps its .txt extension, so the direct MEDIA_ROOT link stays
inert whatever the web server is configured to do. The mime_type recorded in the
database goes back to text/html, because that is what the bytes are: it is only
used for display and to decide what can be rendered, never to serve anything.
A new staff-only view renders those bytes, and it is defended three times over:

* nh3 strips scripts, event handlers and dangerous URL schemes. bleach was
  retired in June 2026 with no further security releases, and lxml.html.clean
  was split out of lxml precisely because its blocklist design made lxml
  problematic for security-sensitive use, so nh3 is the remaining choice.
* Content-Security-Policy: default-src 'none' stops the document loading or
  executing anything.
* The CSP sandbox directive gives it an opaque origin with scripting disabled,
  so a sanitizer bypass still cannot reach the helpdesk origin or the session.

Sanitizing at render time rather than on upload is deliberate. The stored bytes
stay verbatim, which is what you want when working out what a customer actually
sent, and upgrading nh3 retroactively protects every attachment ever stored
instead of leaving files cleaned by an older version to be trusted forever.

Remote images are blocked by default, so tracking pixels in a customer's email
no longer fire when a staff member reads it. Set
HELPDESK_HTML_PREVIEW_ALLOW_REMOTE_IMAGES to True to prefer fidelity.

Routing through a view also adds the queue and ticket permission checks that the
raw media link never had.

pyproject.toml

@@ -25,6 +25,7 @@ dependencies = [
     "akismet",
     "markdown",
     "beautifulsoup4",
+    "nh3",
     "lxml",
     "pytz",
     "djangorestframework",

src/helpdesk/email.py

@@ -58,13 +58,18 @@ STRIPPED_SUBJECT_STRINGS = [
 
 # Name of the attachment holding the raw HTML body of an inbound email.
 #
-# Stored with a .txt extension and a text/plain content type on purpose. The
-# body is attacker controlled: anyone able to email a queue can put arbitrary
-# markup and script in it, and attachments are linked straight from the ticket
-# page and served inline by the web server, which derives the content type from
-# the extension. Naming this .html therefore executed the sender's JavaScript
-# in the staff member's session as soon as they opened the attachment while
-# triaging the ticket.
+# Stored with a .txt extension on purpose. The body is attacker controlled:
+# anyone able to email a queue can put arbitrary markup and script in it, and
+# attachments are linked straight from the ticket page and served inline by the
+# web server, which derives the content type from the extension. Naming this
+# .html therefore executed the sender's JavaScript in the staff member's session
+# as soon as they opened the attachment while triaging the ticket.
+#
+# The stored extension is what the web server keys off, so it governs safety.
+# The mime_type recorded in the database is a separate field used only for
+# display and to decide what can be rendered, so it stays text/html: that is
+# what the bytes actually are, and it is what drives the sanitized preview in
+# helpdesk.views.staff.attachment_preview.
 #
 # Deliberately not wrapped in gettext: a translated filename can change the
 # extension (es_CO rendered this as "email_html_body.html.") and so change what
@@ -895,8 +900,10 @@ def extract_email_message_content(
                 (mime_content if formatted_body is None else formatted_body).encode(
                     "utf-8"
                 ),
-                # text/plain, not text/html: see HTML_EMAIL_ATTACHMENT_FILENAME.
-                "text/plain",
+                # Recorded in the database, never used to serve the file: the
+                # stored .txt extension is what keeps this inert. See
+                # HTML_EMAIL_ATTACHMENT_FILENAME.
+                "text/html",
             )
         )
         # Try to get a plain part message

src/helpdesk/sanitize.py

@@ -0,0 +1,82 @@
+"""
+Sanitization for the one place where django-helpdesk deliberately turns
+third-party markup back into rendered HTML: the preview of an inbound email's
+HTML body.
+
+The body is attacker controlled. Anyone able to email a queue, or to use the
+public ticket form, chooses its content, and the person who ends up rendering it
+is a staff member with access to every ticket in their queues. So the preview
+never relies on sanitization alone. Three independent layers have to fail before
+that becomes an XSS:
+
+  1. nh3 removes scripts, event handler attributes and dangerous URL schemes.
+  2. `Content-Security-Policy: default-src 'none'` stops the document from
+     loading or executing anything at all.
+  3. The CSP `sandbox` directive gives the document an opaque origin with
+     scripting disabled, so even a sanitizer bypass cannot reach the helpdesk
+     origin, the session cookie, or any helpdesk URL.
+
+Sanitizing happens here, at render time, rather than when the attachment is
+stored. That matters: the stored file keeps the sender's original bytes, which is
+what you want when debugging what a customer actually sent, and upgrading nh3
+retroactively protects every attachment ever stored instead of leaving already
+cleaned files on disk to be trusted forever.
+"""
+
+from helpdesk import settings as helpdesk_settings
+
+try:
+    import nh3
+except ImportError:  # pragma: no cover - nh3 is a declared dependency
+    nh3 = None
+
+
+def _allowed_attributes():
+    """nh3's defaults, plus `style` on every tag.
+
+    HTML email leans almost entirely on inline styles for layout, and dropping
+    them turns a formatted message into an unreadable wall of text, which is the
+    thing this preview exists to avoid. Inline CSS is safe enough here because
+    the CSP forbids the document from loading any external resource, so a
+    `url()` cannot be used to phone home, and scripting is off entirely.
+    """
+    attributes = {tag: set(attrs) for tag, attrs in nh3.ALLOWED_ATTRIBUTES.items()}
+    attributes.setdefault("*", set()).add("style")
+    return attributes
+
+
+def sanitizer_available() -> bool:
+    return nh3 is not None
+
+
+def sanitize_email_html(html: str) -> str:
+    """Strip everything executable out of an inbound email's HTML body.
+
+    Raises RuntimeError rather than returning the input untouched if nh3 is
+    missing: failing closed is the only acceptable behaviour here.
+    """
+    if nh3 is None:  # pragma: no cover - nh3 is a declared dependency
+        raise RuntimeError(
+            "nh3 is required to render HTML previews and is not installed."
+        )
+    return nh3.clean(html, attributes=_allowed_attributes())
+
+
+def preview_csp() -> str:
+    """The Content-Security-Policy sent with a rendered preview.
+
+    `sandbox` with no allow-tokens is what actually makes this safe: the
+    document gets an opaque origin and scripting is disabled by the browser,
+    independently of whether the sanitizer missed something.
+
+    Remote images are blocked by default, which also means tracking pixels in a
+    customer's email do not fire when a staff member previews it. Operators who
+    would rather have full fidelity can set
+    HELPDESK_HTML_PREVIEW_ALLOW_REMOTE_IMAGES to True.
+    """
+    img_src = (
+        "https: http:"
+        if helpdesk_settings.HELPDESK_HTML_PREVIEW_ALLOW_REMOTE_IMAGES
+        else "'none'"
+    )
+    return f"default-src 'none'; img-src {img_src}; style-src 'unsafe-inline'; sandbox"

src/helpdesk/settings.py

@@ -532,6 +532,15 @@ HELPDESK_VALIDATE_ATTACHMENT_TYPES = getattr(
     settings, "HELPDESK_VALIDATE_ATTACHMENT_TYPES", True
 )
 
+# Let the rendered preview of an inbound email's HTML body load remote images.
+#
+# Off by default, which also stops tracking pixels in a customer's email from
+# firing when a staff member previews it. Most email clients block remote images
+# by default for the same reason. Turn it on if fidelity matters more than that.
+HELPDESK_HTML_PREVIEW_ALLOW_REMOTE_IMAGES = getattr(
+    settings, "HELPDESK_HTML_PREVIEW_ALLOW_REMOTE_IMAGES", False
+)
+
 
 def get_followup_webhook_urls():
     urls = os.environ.get("HELPDESK_FOLLOWUP_WEBHOOK_URLS", None)

src/helpdesk/templates/helpdesk/include/attachment_list_item.html

@@ -2,6 +2,15 @@
 <li class="list-group-item ps-0">
   <a class="d-block" href="{{ attachment.file.url }}" target="_blank">{{ attachment.filename|truncatechars:40 }}</a>
   <small class="text-secondary">({{ attachment.mime_type }}, {{ attachment.size|filesizeformat }})</small>
+  {% if attachment.mime_type == "text/html" %}
+    {# The link above serves the original source inertly. This one renders it, sanitized. #}
+    <a href="{% url 'helpdesk:attachment_preview' ticket.id attachment.id %}"
+       target="_blank"
+       class="text-secondary"
+       title="{% trans 'View formatted' %}">
+      <i class="fa-regular fa-eye fa-sm ms-2"></i>
+    </a>
+  {% endif %}
   {% if attachment.followup.user and request.user == attachment.followup.user %}
     <a href="{% url 'helpdesk:attachment_del' ticket.id attachment.id %}"
        class="text-secondary">

src/helpdesk/urls.py

@@ -101,6 +101,11 @@ if helpdesk_settings.HELPDESK_UI_ENABLED:
             staff.attachment_del,
             name="attachment_del",
         ),
+        path(
+            "tickets/<int:ticket_id>/attachment_preview/<int:attachment_id>/",
+            staff.attachment_preview,
+            name="attachment_preview",
+        ),
         path(
             "tickets/<int:ticket_id>/checklists/<int:checklist_id>/",
             staff.edit_ticket_checklist,

src/helpdesk/views/staff.py

@@ -93,6 +93,7 @@ from helpdesk.models import (
     UserSettings,
 )
 from helpdesk.query import get_query_class, query_from_base64, query_to_base64
+from helpdesk.sanitize import preview_csp, sanitize_email_html, sanitizer_available
 from helpdesk.update_ticket import (
     return_ticketccstring_and_show_subscribe,
     subscribe_to_ticket_updates,
@@ -2105,6 +2106,56 @@ def ticket_resolves_del(request, ticket_id, dependency_id):
 ticket_resolves_del = staff_member_required(ticket_resolves_del)
 
 
+@helpdesk_staff_member_required
+def attachment_preview(request, ticket_id, attachment_id):
+    """Render an attachment that holds HTML, with the markup neutralised.
+
+    Attachments are otherwise linked straight into MEDIA_ROOT and served by the
+    web server, which picks the content type from the file extension. That is why
+    an inbound email's HTML body is stored as .txt: it makes the direct link
+    inert. This view is the only place that turns those bytes back into a
+    rendered document, so it is also the only place that has to be defended, and
+    it is defended three times over. See helpdesk.sanitize for what each layer
+    does and why sanitizing happens here rather than at upload time.
+
+    Going through a view has a second benefit the raw media link never had: the
+    queue and ticket permission checks below actually run.
+    """
+    ticket = get_object_or_404(Ticket, id=ticket_id)
+    ticket_perm_check(request, ticket)
+
+    attachment = get_object_or_404(
+        FollowUpAttachment, id=attachment_id, followup__ticket=ticket
+    )
+    if attachment.mime_type != "text/html":
+        raise Http404("This attachment has no HTML rendering.")
+
+    if not sanitizer_available():
+        # Fail closed. Serving the raw bytes as text/html here would hand the
+        # sender exactly the XSS this view exists to prevent.
+        return HttpResponse(
+            "HTML previews are unavailable because the nh3 package is not "
+            "installed. Download the attachment to read its source instead.",
+            content_type="text/plain; charset=utf-8",
+            status=503,
+        )
+
+    attachment.file.open("rb")
+    try:
+        raw = attachment.file.read()
+    finally:
+        attachment.file.close()
+
+    response = HttpResponse(
+        sanitize_email_html(raw.decode("utf-8", errors="replace")),
+        content_type="text/html; charset=utf-8",
+    )
+    response["Content-Security-Policy"] = preview_csp()
+    response["X-Content-Type-Options"] = "nosniff"
+    response["Referrer-Policy"] = "no-referrer"
+    return response
+
+
 @helpdesk_staff_member_required
 def attachment_del(request, ticket_id, attachment_id):
     ticket = get_object_or_404(Ticket, id=ticket_id)

tests/test_attachment_html_preview.py

@@ -0,0 +1,157 @@
+"""
+The sanitized preview that gives staff back a readable rendering of an HTML
+email without serving the sender's markup from the helpdesk origin.
+
+What matters here is that all three layers described in helpdesk.sanitize are
+actually present on the response, not just the sanitizer: the first one is a
+blocklist race we should expect to lose eventually, the other two are what make
+losing it survivable.
+"""
+
+import logging
+
+from django.test import TestCase, override_settings
+from django.urls import reverse
+
+from helpdesk.email import extract_email_metadata
+from helpdesk.models import FollowUpAttachment, Queue, Ticket
+
+from .helpers import get_staff_user, get_user
+
+logger = logging.getLogger("helpdesk")
+
+PAYLOAD = (
+    "<html><body>"
+    "<p style='color:red'>Dear customer</p>"
+    "<table><tr><td>Invoice</td><td>42 EUR</td></tr></table>"
+    "<a href='https://example.com/ok'>legitimate link</a>"
+    "<a href='javascript:alert(1)'>bad link</a>"
+    "<div onmouseover='alert(1)'>hover me</div>"
+    "<script>alert(document.cookie)</script>"
+    "<iframe src='//evil.example'></iframe>"
+    "</body></html>"
+)
+
+
+def build_email(html_body):
+    from email.message import EmailMessage
+
+    message = EmailMessage()
+    message["Subject"] = "Formatted email"
+    message["From"] = "customer@example.com"
+    message["To"] = "helpdesk@example.com"
+    message.set_content("plain text body")
+    message.add_alternative(html_body, subtype="html")
+    return message.as_string()
+
+
+class HTMLPreviewTestCase(TestCase):
+    def setUp(self):
+        self.queue = Queue.objects.create(
+            title="Email queue",
+            slug="mail",
+            email_address="helpdesk@example.com",
+            allow_email_submission=True,
+        )
+        extract_email_metadata(build_email(PAYLOAD), self.queue, logger=logger)
+        self.ticket = Ticket.objects.get()
+        self.attachment = FollowUpAttachment.objects.get()
+        self.user = get_staff_user()
+        self.client.login(username=self.user.username, password="password")
+
+    def preview_url(self, ticket=None, attachment=None):
+        return reverse(
+            "helpdesk:attachment_preview",
+            args=[(ticket or self.ticket).id, (attachment or self.attachment).id],
+        )
+
+    def test_stored_file_stays_inert_while_mime_type_records_the_truth(self):
+        """The extension governs what the web server serves, the mime_type only
+        records what the bytes are and drives the preview."""
+        self.assertTrue(self.attachment.file.name.endswith(".txt"))
+        self.assertEqual(self.attachment.mime_type, "text/html")
+
+    def test_preview_strips_everything_executable(self):
+        response = self.client.get(self.preview_url())
+        self.assertEqual(response.status_code, 200)
+        body = response.content.decode("utf-8")
+        self.assertNotIn("<script", body.lower())
+        self.assertNotIn("onmouseover", body.lower())
+        self.assertNotIn("javascript:", body.lower())
+        self.assertNotIn("<iframe", body.lower())
+
+    def test_preview_keeps_the_message_readable(self):
+        """The whole point of the feature: a non-technical reader still gets
+        their formatted email."""
+        body = self.client.get(self.preview_url()).content.decode("utf-8")
+        self.assertIn("Dear customer", body)
+        self.assertIn("42 EUR", body)
+        self.assertIn("<table", body)
+        self.assertIn("color:red", body)
+        self.assertIn("https://example.com/ok", body)
+
+    def test_preview_is_sandboxed_and_locked_down(self):
+        """Sanitizing is the layer most likely to fail, so assert the two that
+        hold even when it does."""
+        response = self.client.get(self.preview_url())
+        csp = response["Content-Security-Policy"]
+        self.assertIn("default-src 'none'", csp)
+        self.assertIn("sandbox", csp)
+        self.assertIn("img-src 'none'", csp)
+        self.assertEqual(response["X-Content-Type-Options"], "nosniff")
+        self.assertEqual(response["Referrer-Policy"], "no-referrer")
+
+    @override_settings(HELPDESK_HTML_PREVIEW_ALLOW_REMOTE_IMAGES=True)
+    def test_remote_images_can_be_opted_into(self):
+        # The setting is read at import time by helpdesk.settings, so reload the
+        # module the way the project's other settings tests do.
+        from importlib import reload
+
+        from helpdesk import sanitize
+        from helpdesk import settings as helpdesk_settings
+
+        reload(helpdesk_settings)
+        reload(sanitize)
+        try:
+            self.assertIn("img-src https: http:", sanitize.preview_csp())
+        finally:
+            reload(helpdesk_settings)
+            reload(sanitize)
+
+    def test_preview_refuses_non_html_attachments(self):
+        self.attachment.mime_type = "application/pdf"
+        self.attachment.save()
+        response = self.client.get(self.preview_url())
+        self.assertEqual(response.status_code, 404)
+
+    def test_preview_requires_staff(self):
+        self.client.logout()
+        response = self.client.get(self.preview_url())
+        self.assertNotEqual(response.status_code, 200)
+
+    def test_preview_enforces_queue_permissions(self):
+        """Going through a view is what makes this check possible at all: the
+        raw media link never had one.
+
+        Only meaningful with per-queue permissions turned on, since by default
+        any staff member has full access to every queue.
+        """
+        from helpdesk import settings as helpdesk_settings
+
+        original = helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION
+        helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = True
+        try:
+            get_user(username="outsider", is_staff=True)
+            self.client.logout()
+            self.client.login(username="outsider", password="password")
+            response = self.client.get(self.preview_url())
+            self.assertIn(response.status_code, (403, 404))
+        finally:
+            helpdesk_settings.HELPDESK_ENABLE_PER_QUEUE_STAFF_PERMISSION = original
+
+    def test_attachment_id_must_belong_to_the_ticket(self):
+        other_ticket = Ticket.objects.create(
+            title="Unrelated", queue=self.queue, description="x"
+        )
+        response = self.client.get(self.preview_url(ticket=other_ticket))
+        self.assertEqual(response.status_code, 404)

tests/test_attachment_html_xss.py

@@ -125,8 +125,11 @@ class InboundEmailAttachmentTestCase(TestCase):
         attachment = FollowUpAttachment.objects.get()
         self.assertEqual(attachment.filename, HTML_EMAIL_ATTACHMENT_FILENAME)
         self.assertTrue(attachment.filename.endswith(".txt"))
-        self.assertEqual(attachment.mime_type, "text/plain")
+        # The stored extension is what the web server keys off, so that is what
+        # has to be inert. mime_type is only a record of what the bytes are, and
+        # what the sanitized preview keys off.
         self.assertFalse(attachment.file.name.lower().endswith((".htm", ".html")))
+        self.assertEqual(attachment.mime_type, "text/html")
         # The body is kept verbatim, it just is not served as markup any more.
         attachment.file.open("rb")
         content = attachment.file.read().decode("utf-8")