v0.18.0 Slice 1: build_envelope helper + unit tests

Per the v0.18.0 email + webhook hygiene proposal (~/git/ohm-infra/RFC-APP-EMAIL-HYGIENE-PROPOSAL.md §1), this is the shared envelope builder every send path will call in Slice 2. No call-site changes yet — Slice 2 migrates email_otc / email_invite / email._deliver / email._send_bundle to use it. The helper centralizes the deliverability-critical headers (Date, Message-ID, Auto-Submitted) and exposes per-kind unsubscribe semantics (none for OTC, mailto: for invites, full one-click for watcher notifications) as explicit kwargs rather than buried in each call site. 15 new unit tests covering: always-present headers, Auto-Submitted toggle, the three List-Unsubscribe shapes, plain-only vs multipart/alternative body. Full suite: 267 passed (was 252).
2026-05-28 07:15:06 -07:00
parent 1456c8b73f
commit 92059f319e
2 changed files with 316 additions and 0 deletions
@@ -0,0 +1,143 @@
+"""v0.18.0 / roadmap items #18 + #20: a shared envelope builder.
+
+Every outbound mail in rfc-app today (OTC, admin-invite, watcher
+notification, "while you were away" bundle, per-RFC invite) constructs
+its own `email.message.EmailMessage` ad-hoc. The four sites diverged
+just enough to be a deliverability hazard: missing `Date`, missing
+`Message-ID`, no `Auto-Submitted`, no `List-Unsubscribe` on the
+bulk-adjacent paths, no `multipart/alternative` body.
+
+This module is the one place an `EmailMessage` is constructed. Every
+send path imports `build_envelope` and calls it; the headers that
+matter for inbox placement (Date, Message-ID, Auto-Submitted) land
+uniformly, and the per-kind variations (unsubscribe semantics,
+HTML alternative) are explicit arguments rather than buried in
+each call site.
+
+Per the v0.18.0 proposal at `~/git/ohm-infra/RFC-APP-EMAIL-HYGIENE-PROPOSAL.md`,
+the per-kind unsubscribe matrix is:
+
+  * OTC: no `List-Unsubscribe` (the recipient explicitly requested
+    the code; advertising an unsubscribe header would imply OHM has
+    them on a list, which it doesn't).
+  * Admin invite / per-RFC invite: `mailto:` form only (the
+    recipient isn't a user yet, so there's no per-user opt-out row
+    to flip; the operator handles ad-hoc opt-outs manually).
+  * Watcher notification / bundle: full `mailto:` + signed-URL
+    `List-Unsubscribe` plus `List-Unsubscribe-Post:
+    List-Unsubscribe=One-Click` per RFC 8058 (Gmail and Yahoo
+    enforce this for bulk-adjacent senders).
+
+The `is_transactional` flag governs `Auto-Submitted: auto-generated`,
+which prevents auto-responder loops on every kind of mail we send.
+All five mail kinds today are transactional in the SMTP sense (no
+human is at the From mailbox watching for replies), so the default
+is True; the argument is exposed for future symmetry.
+"""
+from __future__ import annotations
+
+from email.message import EmailMessage
+from email.utils import formataddr, formatdate, make_msgid
+
+
+def build_envelope(
+    *,
+    to_address: str,
+    from_address: str,
+    from_name: str,
+    subject: str,
+    body_plain: str,
+    body_html: str | None = None,
+    reply_to: str | None = None,
+    unsubscribe_mailto: str | None = None,
+    unsubscribe_url: str | None = None,
+    is_transactional: bool = True,
+    msgid_domain: str | None = None,
+) -> EmailMessage:
+    """Compose an `EmailMessage` with hardened headers.
+
+    `to_address` / `from_address` are bare RFC 5322 addresses;
+    `from_name` is the display label that goes through `formataddr`
+    so spaces / commas in the display string are encoded correctly.
+
+    `body_plain` is mandatory. `body_html`, if supplied, lands as the
+    second part of a `multipart/alternative` body — mail clients
+    that prefer HTML render it; clients that don't fall back to the
+    plain part. The text/plain part comes first per RFC 2046, so a
+    plain-text client that picks the first body gets the readable
+    text.
+
+    `reply_to`, when set, lets a send path point replies at a
+    different mailbox than the From line (e.g., a watcher
+    notification with From=notifications@... but Reply-To=
+    ohm@... so a confused recipient who hits Reply lands at a
+    monitored mailbox).
+
+    `unsubscribe_mailto` / `unsubscribe_url` populate
+    `List-Unsubscribe`. If `unsubscribe_url` is set, the helper also
+    emits `List-Unsubscribe-Post: List-Unsubscribe=One-Click` per
+    RFC 8058 — Gmail and Yahoo POST that payload on the user's
+    one-click action. (Send paths that wire `unsubscribe_url`
+    therefore MUST also expose a matching POST endpoint that accepts
+    the same token; see `api_notifications.py:email_unsubscribe`.)
+
+    `msgid_domain` defaults to the @-domain of `from_address` so
+    Message-IDs are aligned with the sending domain by default. A
+    deployment that wants the Message-ID domain to track a different
+    surface (e.g., a tracking-domain that's separate from the From
+    domain) can override.
+
+    `Date` is RFC 5322 formatted via `email.utils.formatdate`; the
+    `localtime=True` setting picks the running process's local
+    timezone, which is what every popular MUA does too. (A
+    deployment running in UTC stamps UTC; that's correct, not a
+    bug.)
+    """
+    msg = EmailMessage()
+    msg["From"] = formataddr((from_name, from_address))
+    msg["To"] = to_address
+    msg["Subject"] = subject
+    msg["Date"] = formatdate(localtime=True)
+    # If the caller didn't pin a Message-ID domain, derive it from the
+    # From address. `make_msgid` accepts None and falls back to the
+    # local hostname, which is the wrong shape for a deliverable
+    # message (the hostname might be `gke-pool-xxx`); a deployment
+    # without a configured From would surface that as a build-time
+    # config error elsewhere, so the fallback here is just defensive.
+    if msgid_domain is None:
+        if "@" in from_address:
+            msgid_domain = from_address.split("@", 1)[1]
+        else:
+            msgid_domain = "localhost"
+    msg["Message-ID"] = make_msgid(domain=msgid_domain)
+    if reply_to:
+        msg["Reply-To"] = reply_to
+    if is_transactional:
+        # RFC 3834: prevents auto-responders (vacation replies, etc.)
+        # from triggering on this message. Every kind of mail rfc-app
+        # sends today is transactional in this sense.
+        msg["Auto-Submitted"] = "auto-generated"
+    if unsubscribe_mailto or unsubscribe_url:
+        parts: list[str] = []
+        if unsubscribe_mailto:
+            parts.append(f"<mailto:{unsubscribe_mailto}>")
+        if unsubscribe_url:
+            parts.append(f"<{unsubscribe_url}>")
+        msg["List-Unsubscribe"] = ", ".join(parts)
+        if unsubscribe_url:
+            # RFC 8058 one-click. Gmail and Yahoo POST the payload
+            # `List-Unsubscribe=One-Click` to the URL on the user's
+            # one-click action; the matching POST endpoint must be
+            # idempotent and not require auth. See
+            # `api_notifications.py` for the receiver.
+            msg["List-Unsubscribe-Post"] = "List-Unsubscribe=One-Click"
+    if body_html:
+        # multipart/alternative: text/plain first, text/html second.
+        # `set_content` sets the first part (and the message's main
+        # body); `add_alternative` adds the second part and
+        # restructures the message as multipart/alternative.
+        msg.set_content(body_plain)
+        msg.add_alternative(body_html, subtype="html")
+    else:
+        msg.set_content(body_plain)
+    return msg
@@ -0,0 +1,173 @@
+"""Unit tests for `app.email_envelope.build_envelope` (v0.18.0 Slice 1).
+
+These tests don't spin up the FastAPI app or touch the DB — they
+exercise the helper directly. The integration tests in
+test_otc_vertical / test_admin_create_user_invite_vertical /
+test_notifications_vertical exercise the helper's *use* via the
+shared `_SENT` buffer (the send path appends the envelope dict
+before invoking the helper).
+"""
+from __future__ import annotations
+
+from email.utils import parsedate_to_datetime
+
+from app.email_envelope import build_envelope
+
+
+def _base_kwargs(**overrides):
+    base = dict(
+        to_address="recipient@example.com",
+        from_address="notifications@ohm.wiggleverse.org",
+        from_name="OHM",
+        subject="A test subject",
+        body_plain="Hello, world.\n",
+    )
+    base.update(overrides)
+    return base
+
+
+# ---------------------------------------------------------------------------
+# Always-present headers
+# ---------------------------------------------------------------------------
+
+
+def test_envelope_sets_from_to_subject():
+    msg = build_envelope(**_base_kwargs())
+    assert msg["To"] == "recipient@example.com"
+    assert msg["Subject"] == "A test subject"
+    # `From` is the display-form: "OHM <notifications@ohm.wiggleverse.org>".
+    assert "OHM" in msg["From"]
+    assert "<notifications@ohm.wiggleverse.org>" in msg["From"]
+
+
+def test_envelope_sets_date_header_parseable():
+    msg = build_envelope(**_base_kwargs())
+    raw = msg["Date"]
+    assert raw, "Date header must be set"
+    # parsedate_to_datetime raises ValueError on malformed input.
+    dt = parsedate_to_datetime(raw)
+    assert dt is not None
+
+
+def test_envelope_sets_message_id_with_from_domain_by_default():
+    msg = build_envelope(**_base_kwargs())
+    mid = msg["Message-ID"]
+    assert mid, "Message-ID must be set"
+    # Shape per RFC 5322 / make_msgid: <random@domain>
+    assert mid.startswith("<") and mid.endswith(">")
+    assert "@ohm.wiggleverse.org>" in mid
+
+
+def test_envelope_message_id_domain_override():
+    msg = build_envelope(**_base_kwargs(msgid_domain="example.test"))
+    assert "@example.test>" in msg["Message-ID"]
+
+
+def test_envelope_message_id_falls_back_to_localhost_if_from_has_no_at():
+    # Defensive: a malformed from_address shouldn't crash the helper.
+    msg = build_envelope(**_base_kwargs(from_address="bare-no-at-sign"))
+    assert "@localhost>" in msg["Message-ID"]
+
+
+# ---------------------------------------------------------------------------
+# Auto-Submitted (RFC 3834)
+# ---------------------------------------------------------------------------
+
+
+def test_envelope_sets_auto_submitted_for_transactional_default():
+    msg = build_envelope(**_base_kwargs())
+    assert msg["Auto-Submitted"] == "auto-generated"
+
+
+def test_envelope_omits_auto_submitted_when_transactional_is_false():
+    msg = build_envelope(**_base_kwargs(is_transactional=False))
+    assert msg["Auto-Submitted"] is None
+
+
+# ---------------------------------------------------------------------------
+# Reply-To
+# ---------------------------------------------------------------------------
+
+
+def test_envelope_sets_reply_to_when_provided():
+    msg = build_envelope(**_base_kwargs(reply_to="ohm@wiggleverse.org"))
+    assert msg["Reply-To"] == "ohm@wiggleverse.org"
+
+
+def test_envelope_omits_reply_to_when_absent():
+    msg = build_envelope(**_base_kwargs())
+    assert msg["Reply-To"] is None
+
+
+# ---------------------------------------------------------------------------
+# List-Unsubscribe (the headers RFC 8058 / Gmail-Yahoo care about)
+# ---------------------------------------------------------------------------
+
+
+def test_envelope_no_list_unsubscribe_when_neither_given():
+    """OTC mail: the recipient explicitly requested the code; no
+    unsubscribe semantics. The header MUST be absent (presence would
+    imply OHM has the recipient on a list, which it doesn't)."""
+    msg = build_envelope(**_base_kwargs())
+    assert msg["List-Unsubscribe"] is None
+    assert msg["List-Unsubscribe-Post"] is None
+
+
+def test_envelope_mailto_only_list_unsubscribe():
+    """Admin invite / per-RFC invite: `mailto:` form only, no URL.
+    The recipient isn't a user yet, so there's no per-user opt-out
+    URL to flip; the operator handles ad-hoc opt-outs manually."""
+    msg = build_envelope(**_base_kwargs(
+        unsubscribe_mailto="ohm@wiggleverse.org?subject=remove",
+    ))
+    assert msg["List-Unsubscribe"] == "<mailto:ohm@wiggleverse.org?subject=remove>"
+    # NO List-Unsubscribe-Post when only a mailto is present — the
+    # one-click semantic requires a URL the MUA can POST to.
+    assert msg["List-Unsubscribe-Post"] is None
+
+
+def test_envelope_full_one_click_list_unsubscribe():
+    """Watcher notification / bundle: `mailto:` + signed-URL +
+    `List-Unsubscribe-Post: List-Unsubscribe=One-Click`. Gmail and
+    Yahoo enforce this for bulk-adjacent mail per RFC 8058."""
+    msg = build_envelope(**_base_kwargs(
+        unsubscribe_mailto="ohm@wiggleverse.org?subject=remove",
+        unsubscribe_url="https://ohm.wiggleverse.org/api/email/unsubscribe?t=abc",
+    ))
+    lu = msg["List-Unsubscribe"]
+    assert "<mailto:ohm@wiggleverse.org?subject=remove>" in lu
+    assert "<https://ohm.wiggleverse.org/api/email/unsubscribe?t=abc>" in lu
+    assert msg["List-Unsubscribe-Post"] == "List-Unsubscribe=One-Click"
+
+
+def test_envelope_url_only_list_unsubscribe_still_sets_post():
+    msg = build_envelope(**_base_kwargs(
+        unsubscribe_url="https://ohm.wiggleverse.org/api/email/unsubscribe?t=abc",
+    ))
+    assert msg["List-Unsubscribe"] == "<https://ohm.wiggleverse.org/api/email/unsubscribe?t=abc>"
+    assert msg["List-Unsubscribe-Post"] == "List-Unsubscribe=One-Click"
+
+
+# ---------------------------------------------------------------------------
+# Body shape — plain-only vs multipart/alternative
+# ---------------------------------------------------------------------------
+
+
+def test_envelope_plain_only_body_is_text_plain():
+    msg = build_envelope(**_base_kwargs())
+    # No HTML alternative -> single-part text/plain.
+    assert msg.get_content_type() == "text/plain"
+    assert msg.get_content().strip() == "Hello, world."
+
+
+def test_envelope_with_html_is_multipart_alternative():
+    msg = build_envelope(**_base_kwargs(body_html="<p>Hello, <b>world</b>.</p>"))
+    assert msg.get_content_type() == "multipart/alternative"
+    # Two parts: text/plain first (so plain-text clients picking the
+    # first part get the readable text), text/html second.
+    parts = list(msg.iter_parts())
+    assert len(parts) == 2
+    assert parts[0].get_content_type() == "text/plain"
+    assert parts[1].get_content_type() == "text/html"
+    assert "Hello, world." in parts[0].get_content()
+    assert "<b>world</b>" in parts[1].get_content()