Ben Stull
79a447c77b
Two informational findings from the Session-0026 audit, both framework-internal defense-in-depth. No operator action: no migration, no schema/config/overlay change, no deployment-facing surface. - I3: guard the dead text/html branch in email_envelope.build_envelope. No send path passes body_html; the unused branch would emit HTML built from possibly-unescaped user content (C1 stored-XSS class in the mail channel). Passing body_html now raises NotImplementedError; the arg is kept for documented future symmetry, enabling HTML mail becomes a deliberate escape-then-unguard change. - I4: make turnstile.verify_token async. The sync httpx.post ran inside the async /auth/otc/request handler, blocking the event loop up to the 10s timeout on a slow CloudFlare call. It now awaits httpx.AsyncClient via a narrow _siteverify_post seam (tests patch the seam, not the shared AsyncClient). The sole caller (main.py) now awaits it. Tests: full backend suite 365 passed. Added a coroutine-contract unit test for verify_token and flipped the email_envelope HTML test to assert the guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
156 lines
7.3 KiB
Python
156 lines
7.3 KiB
Python
"""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` is **reserved and not yet
|
|
enabled** (security-audit-0026 I3): no send path supplies it today —
|
|
every rfc-app mail is plain text — and passing it raises
|
|
`NotImplementedError`. The parameter is kept in the signature for
|
|
documented future symmetry: when HTML mail is enabled it will land
|
|
as the second part of a `multipart/alternative` body (text/plain
|
|
first per RFC 2046, so a plain-text client picking the first part
|
|
still gets the readable text). Enabling it is a deliberate act — the
|
|
caller MUST HTML-escape any user content into `body_html` first (cf.
|
|
the C1 stored-XSS class: a mail client renders the HTML) and remove
|
|
the guard below in the same change.
|
|
|
|
`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 is not None:
|
|
# I3 (security-audit-0026): the multipart/alternative HTML path
|
|
# is intentionally NOT enabled. No send path passes `body_html`
|
|
# today, and emitting an HTML body built from user-supplied
|
|
# content without escaping it first would reintroduce the C1
|
|
# stored-XSS class in the mail channel (the recipient's client
|
|
# renders the HTML). Fail loudly here rather than silently
|
|
# shipping HTML: enabling HTML mail is a deliberate change that
|
|
# MUST HTML-escape user content at the call site and remove this
|
|
# guard together. The text/plain path below is the only live one.
|
|
raise NotImplementedError(
|
|
"HTML email is not enabled (security-audit-0026 I3): do not "
|
|
"pass body_html until user content is HTML-escaped at the "
|
|
"call site and this guard is intentionally removed."
|
|
)
|
|
msg.set_content(body_plain)
|
|
return msg
|