fix(email): give every mail an HTML alternative so its link is clickable

The bodies are text/plain only, so whether the URL in a verification or password-reset mail is a link at all was the client's decision. Gmail, Outlook and Apple Mail linkify plain text; lighter web clients render the body verbatim and leave the link as dead text — in a message whose entire purpose is that one link. _send now attaches an HTML part generated from the SAME plain body, rather than per-template HTML that would have to be kept in sync with it: one converter, all eleven templates, and text/plain stays the primary part and the source of truth. Escaping runs before linkification — bodies interpolate untrusted values (a requester's address, the operator contact) — so no caller can inject markup, and a query string's `&` is already `&` when it reaches the href, which is what an HTML attribute wants. Sentence-final punctuation is trimmed out of the href while url-safe token characters (`-`, `_`) are left alone. Inline styles only, no images, no external CSS, no tracking.
2026-09-02 03:13:23 +02:00
parent c81b110637
commit 7019e3afdf
2 changed files with 166 additions and 0 deletions
@@ -10,7 +10,9 @@ verification links, password resets and access-request notices, and the operator
 to be a person telling them nothing arrived.
 """

+import html
 import logging
+import re
 import smtplib
 import ssl
 from email.message import EmailMessage
@@ -105,6 +107,48 @@ def _admin_contact() -> str | None:
    return next(iter(sorted(settings.admin_email_set)), None)


+# A bare URL in a text/plain mail is only clickable if the CLIENT decides to linkify it. Gmail,
+# Outlook and Apple Mail do; plenty of lighter web clients render the body verbatim and leave the
+# link as dead text — which, for a verification or password-reset mail, is the whole message. So
+# every mail also carries an HTML alternative built from the SAME plain body (below), rather than
+# per-template HTML that would have to be kept in sync with it.
+#
+# `[^\s<]` stops at whitespace and at the `<` of any following markup; the trailing-punctuation trim
+# keeps a sentence-final period out of the href. `;` and `:` are deliberately NOT trimmed — escaping
+# turns `&` into `&amp;`, so a query string can legitimately end in a semicolon.
+_URL_RE = re.compile(r"https?://[^\s<]+")
+
+
+def _html_body(text: str) -> str:
+    """Render a plain-text mail body as minimal HTML: escaped, blank-line-separated paragraphs, with
+    real links. Escaping happens FIRST and the linkifier then runs over the escaped text — bodies
+    interpolate untrusted values (a requester's email address, the operator contact), so no caller
+    can inject markup, and `&` inside a URL is already `&amp;` by the time it reaches the href, which
+    is what an HTML attribute wants anyway."""
+
+    def _anchor(m: re.Match[str]) -> str:
+        url = m.group(0)
+        trail = ""
+        while url and url[-1] in ".,!?)":
+            url, trail = url[:-1], url[-1] + trail
+        if not url:
+            return trail
+        return f'<a href="{url}">{url}</a>{trail}'
+
+    paragraphs = [p for p in html.escape(text).split("\n\n") if p.strip()]
+    rendered = "".join(
+        f"<p>{_URL_RE.sub(_anchor, p).replace(chr(10), '<br>')}</p>" for p in paragraphs
+    )
+    # Inline styles only — many clients drop <style> blocks. No images, no external CSS, no tracking:
+    # the HTML part exists to make the link clickable, not to become a template to maintain.
+    return (
+        '<html><body style="margin:0;padding:16px;font-family:system-ui,-apple-system,'
+        "'Segoe UI',sans-serif;font-size:15px;line-height:1.6\">"
+        f'<div style="max-width:560px">{rendered}</div>'
+        "</body></html>"
+    )
+
+
 def _send(to: list[str], subject: str, body: str, reply_to: str | None = None) -> bool:
    recipients = [e for e in to if e]
    if not recipients:
@@ -123,7 +167,10 @@ def _send(to: list[str], subject: str, body: str, reply_to: str | None = None) -
    msg["Date"] = formatdate(localtime=True)
    if reply_to:
        msg["Reply-To"] = reply_to
+    # text/plain stays the primary part (it is the source of truth, and a text-only client still
+    # gets the full message); the HTML alternative below is what makes the link clickable everywhere.
    msg.set_content(body)
+    msg.add_alternative(_html_body(body), subtype="html")
    try:
        with smtplib.SMTP(c["host"], c["port"], timeout=20) as s:
            s.starttls(context=ssl.create_default_context())
@@ -0,0 +1,119 @@
+"""Every outbound mail carries an HTML alternative, so its link is actually clickable.
+
+The bodies are plain text and stay that way — but a bare URL in a text/plain mail is only a link if
+the CLIENT chooses to linkify it. Gmail/Outlook/Apple Mail do; lighter web clients render the body
+verbatim and leave the verification or password-reset link as dead text, which is the entire point of
+those two messages. So `_send` now attaches an HTML part generated from the same plain body.
+
+These are pure-logic tests except the last one, which drives the real `_send` against a fake SMTP so
+the wire format itself is pinned, not just the helper.
+"""
+
+import pytest
+
+from app import email as email_mod
+
+_SMTP_OK = {
+    "host": "smtp.example.com",
+    "port": 587,
+    "user": "u",
+    "password": "p",
+    "from": "Siftlode <u@example.com>",
+}
+
+
+def test_a_bare_url_becomes_a_real_link():
+    # THE regression: this is what a client that does not linkify plain text was leaving as dead text.
+    out = email_mod._html_body(
+        "Confirm your email:\n\nhttps://siftlode.example/#verify=tok123\n\n— Siftlode"
+    )
+    assert '<a href="https://siftlode.example/#verify=tok123">' in out
+
+
+def test_interpolated_content_cannot_inject_markup():
+    # Bodies carry untrusted values — a requester's address in the admin notice, the operator contact.
+    # Escaping must run BEFORE linkification, or a crafted address becomes markup in every admin's mail.
+    out = email_mod._html_body('A request from <script>alert("x")</script> & co')
+    assert "<script>" not in out
+    assert "&lt;script&gt;" in out
+    assert "&amp; co" in out
+
+
+def test_a_query_string_keeps_its_escaped_ampersand_inside_the_href():
+    # `&` is escaped to `&amp;` before the linkifier sees it, which is also what an HTML attribute
+    # wants — so the href must carry the escaped form and nothing may be trimmed off it.
+    out = email_mod._html_body("https://siftlode.example/x?a=1&b=2")
+    assert (
+        '<a href="https://siftlode.example/x?a=1&amp;b=2">https://siftlode.example/x?a=1&amp;b=2</a>'
+        in out
+    )
+
+
+@pytest.mark.parametrize(
+    "body,href,trail",
+    [
+        ("See https://siftlode.example/x.", "https://siftlode.example/x", "."),
+        ("See https://siftlode.example/x, then sign in", "https://siftlode.example/x", ","),
+        ("See (https://siftlode.example/x)", "https://siftlode.example/x", ")"),
+    ],
+)
+def test_sentence_punctuation_stays_out_of_the_href(body, href, trail):
+    out = email_mod._html_body(body)
+    assert f'<a href="{href}">{href}</a>{trail}' in out
+
+
+def test_a_token_ending_in_a_url_safe_character_is_not_trimmed():
+    # token_urlsafe output can end in `-` or `_`; trimming either would hand the user a dead link.
+    for tok in ("abc-", "abc_"):
+        out = email_mod._html_body(f"https://siftlode.example/#reset={tok}")
+        assert f'href="https://siftlode.example/#reset={tok}"' in out
+
+
+def test_blank_lines_become_paragraphs_and_single_breaks_stay_inline():
+    out = email_mod._html_body("Hi,\n\nline one\nline two\n\n— Siftlode")
+    assert out.count("<p>") == 3
+    assert "<p>line one<br>line two</p>" in out
+
+
+def test_the_sent_message_is_multipart_with_the_plain_body_intact(monkeypatch):
+    # The wire format, not just the helper: text/plain stays the primary part and the source of
+    # truth, with text/html alongside it.
+    monkeypatch.setattr(email_mod, "_smtp", lambda: _SMTP_OK)
+    sent = []
+
+    class _FakeSMTP:
+        def __init__(self, *a, **kw):
+            pass
+
+        def __enter__(self):
+            return self
+
+        def __exit__(self, *a):
+            return False
+
+        def starttls(self, *a, **kw):
+            pass
+
+        def login(self, *a, **kw):
+            pass
+
+        def send_message(self, msg):
+            sent.append(msg)
+
+    monkeypatch.setattr(email_mod.smtplib, "SMTP", _FakeSMTP)
+
+    assert (
+        email_mod.send_verify_email("someone@example.com", "https://siftlode.example/#verify=tok")
+        is True
+    )
+
+    (msg,) = sent
+    assert msg.get_content_type() == "multipart/alternative"
+    plain = msg.get_body(preferencelist=("plain",))
+    rich = msg.get_body(preferencelist=("html",))
+    assert plain is not None and rich is not None
+    # The plain part must be byte-for-byte the template's own body — the HTML part is derived from
+    # it, never a second copy that could drift.
+    assert "https://siftlode.example/#verify=tok" in plain.get_content()
+    assert "<a href=" not in plain.get_content()
+    assert '<a href="https://siftlode.example/#verify=tok">' in rich.get_content()