From 92059f319e320c0a7760dc34a3bfe15de0655136 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Thu, 28 May 2026 07:15:06 -0700 Subject: [PATCH] v0.18.0 Slice 1: build_envelope helper + unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- backend/app/email_envelope.py | 143 ++++++++++++++++++++++ backend/tests/test_email_envelope.py | 173 +++++++++++++++++++++++++++ 2 files changed, 316 insertions(+) create mode 100644 backend/app/email_envelope.py create mode 100644 backend/tests/test_email_envelope.py diff --git a/backend/app/email_envelope.py b/backend/app/email_envelope.py new file mode 100644 index 0000000..78d011e --- /dev/null +++ b/backend/app/email_envelope.py @@ -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"") + 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 diff --git a/backend/tests/test_email_envelope.py b/backend/tests/test_email_envelope.py new file mode 100644 index 0000000..9ad20be --- /dev/null +++ b/backend/tests/test_email_envelope.py @@ -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 ". + assert "OHM" in msg["From"] + assert "" 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: + 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"] == "" + # 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 "" in lu + assert "" 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"] == "" + 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="

Hello, world.

")) + 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 "world" in parts[1].get_content()