Merge pull request 'Release v0.28.0: security-audit-0026 I3 + I4 (HTML-email guard + async Turnstile siteverify)' (#2) from feature/v0.28.0-email-turnstile-async into main

CHANGELOG.md

@@ -23,6 +23,40 @@ skip versions are the composition of each intervening adjacent
 release's steps in order — no A-to-B path is pre-computed beyond
 that.
 
+## 0.28.0 — 2026-05-28
+
+**Minor — security-hardening follow-up (Session-0026 audit, informational
+findings I3 + I4). No operator action required: no migration, no schema
+change, no config/overlay change, no API/behavior change for any caller.
+A plain code deploy picks it up. Shipped from driver session 0032.0.**
+
+Two informational findings from the Session-0026 audit, both
+framework-internal defense-in-depth:
+
+- **I3 — dead HTML-email branch guarded.** `email_envelope.build_envelope`
+  accepted a `body_html=` argument that built a `multipart/alternative`
+  body, but no send path ever passed it — every rfc-app mail is plain
+  text. An unused branch that would emit HTML built from (potentially
+  unescaped) user content is the C1 stored-XSS class waiting in the mail
+  channel. The branch is now a loud guard: passing `body_html` raises
+  `NotImplementedError`. The argument is kept in the signature for
+  documented future symmetry; enabling HTML mail becomes a deliberate
+  change that MUST HTML-escape user content at the call site and remove
+  the guard in the same commit.
+- **I4 — Turnstile siteverify no longer blocks the event loop.**
+  `turnstile.verify_token` was a synchronous function issuing a blocking
+  `httpx.post` from inside the async `/auth/otc/request` handler, so a
+  slow CloudFlare response stalled the single worker for up to the 10s
+  timeout. It is now `async` and awaits the call on an
+  `httpx.AsyncClient` (matching the codebase's existing async-httpx
+  pattern), isolated behind a narrow `_siteverify_post` seam. The sole
+  caller (`main.py`) now `await`s it.
+
+Upgrade steps: **none.** Both changes are internal. The `verify_token`
+signature changed from sync to `async` (callers must `await`), but the
+only caller is in-tree (`main.py`) and is updated in this release; no
+deployment-facing surface, config key, or migration is affected.
+
 ## 0.27.0 — 2026-05-28
 
 **Minor — security-hardening release (Session-0026 audit remediation).

VERSION

@@ -1 +1 @@
-0.27.0
+0.28.0

backend/app/email_envelope.py

@@ -60,12 +60,17 @@ def build_envelope(
     `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
+    `body_plain` is mandatory. `body_html` is **reserved and not yet
-    second part of a `multipart/alternative` body — mail clients
+    enabled** (security-audit-0026 I3): no send path supplies it today —
-    that prefer HTML render it; clients that don't fall back to the
+    every rfc-app mail is plain text — and passing it raises
-    plain part. The text/plain part comes first per RFC 2046, so a
+    `NotImplementedError`. The parameter is kept in the signature for
-    plain-text client that picks the first body gets the readable
+    documented future symmetry: when HTML mail is enabled it will land
-    text.
+    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
@@ -131,13 +136,20 @@ def build_envelope(
             # idempotent and not require auth. See
             # `api_notifications.py` for the receiver.
             msg["List-Unsubscribe-Post"] = "List-Unsubscribe=One-Click"
-    if body_html:
+    if body_html is not None:
-        # multipart/alternative: text/plain first, text/html second.
+        # I3 (security-audit-0026): the multipart/alternative HTML path
-        # `set_content` sets the first part (and the message's main
+        # is intentionally NOT enabled. No send path passes `body_html`
-        # body); `add_alternative` adds the second part and
+        # today, and emitting an HTML body built from user-supplied
-        # restructures the message as multipart/alternative.
+        # content without escaping it first would reintroduce the C1
-        msg.set_content(body_plain)
+        # stored-XSS class in the mail channel (the recipient's client
-        msg.add_alternative(body_html, subtype="html")
+        # renders the HTML). Fail loudly here rather than silently
-    else:
+        # 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

backend/app/main.py

@@ -267,7 +267,7 @@ def _oauth_router(config) -> APIRouter:
         # secret AND TURNSTILE_REQUIRED=false (the default), the gate
         # opens — see `backend/app/turnstile.py` for the full matrix.
         client_ip = request.client.host if request.client else None
-        ts = turnstile.verify_token(body.turnstile_token, client_ip=client_ip)
+        ts = await turnstile.verify_token(body.turnstile_token, client_ip=client_ip)
         if not ts.ok:
             if ts.reason == "misconfigured":
                 # TURNSTILE_REQUIRED=true but the secret is unset. This

backend/app/turnstile.py

@@ -73,6 +73,19 @@ def _siteverify_url() -> str:
     return os.environ.get("TURNSTILE_SITEVERIFY_URL", "").strip() or SITEVERIFY_URL
 
 
+async def _siteverify_post(url: str, data: dict) -> httpx.Response:
+    """Perform the siteverify POST on an `httpx.AsyncClient`.
+
+    Isolated as a narrow seam (I4, security-audit-0026): the call is
+    awaited so a slow CloudFlare response can't block the event loop,
+    and tests patch *this function* rather than the shared
+    `httpx.AsyncClient` (which other modules — gitea, docs — also
+    construct, so a global patch would break app boot).
+    """
+    async with httpx.AsyncClient(timeout=10.0) as client:
+        return await client.post(url, data=data)
+
+
 @dataclass
 class VerifyOutcome:
     """Result of a Turnstile siteverify call.
@@ -98,7 +111,7 @@ class VerifyOutcome:
     reason: str
 
 
-def verify_token(token: str | None, *, client_ip: str | None = None) -> VerifyOutcome:
+async def verify_token(token: str | None, *, client_ip: str | None = None) -> VerifyOutcome:
     """Validate a Turnstile token against CloudFlare's siteverify endpoint.
 
     Returns a VerifyOutcome describing whether the calling endpoint
@@ -108,9 +121,14 @@ def verify_token(token: str | None, *, client_ip: str | None = None) -> VerifyOu
       * 'misconfigured' → 500 "auth misconfigured"
       * 'missing-token' / 'failed' / 'network' → 400 "verification failed"
 
-    Tests monkeypatch `httpx.post` (or set `TURNSTILE_SITEVERIFY_URL`
+    Async (I4, security-audit-0026): the siteverify call is awaited on an
-    + a MockTransport client) to avoid touching the real CloudFlare
+    `httpx.AsyncClient` so a slow CloudFlare response can't block the
-    endpoint. No real keys are ever embedded in tests.
+    event loop (the prior synchronous `httpx.post` stalled the single
+    worker for up to the 10s timeout). Callers must `await` it.
+
+    Tests monkeypatch `_siteverify_post` (the narrow async seam) to avoid
+    touching the real CloudFlare endpoint and to keep the patch off the
+    shared `httpx.AsyncClient`. No real keys are ever embedded in tests.
     """
     secret = _secret()
     required = _required()
@@ -133,7 +151,7 @@ def verify_token(token: str | None, *, client_ip: str | None = None) -> VerifyOu
         data["remoteip"] = client_ip
 
     try:
-        response = httpx.post(_siteverify_url(), data=data, timeout=10.0)
+        response = await _siteverify_post(_siteverify_url(), data)
         payload = response.json()
     except Exception as exc:  # network, JSON parse, etc.
         log.warning("Turnstile siteverify call failed: %s", exc)

backend/tests/test_email_envelope.py

@@ -11,6 +11,8 @@ from __future__ import annotations
 
 from email.utils import parsedate_to_datetime
 
+import pytest
+
 from app.email_envelope import build_envelope
 
 
@@ -160,14 +162,18 @@ def test_envelope_plain_only_body_is_text_plain():
     assert msg.get_content().strip() == "Hello, world."
 
 
-def test_envelope_with_html_is_multipart_alternative():
+def test_envelope_html_body_is_guarded_not_enabled():
-    msg = build_envelope(**_base_kwargs(body_html="<p>Hello, <b>world</b>.</p>"))
+    # I3 (security-audit-0026): the HTML/multipart-alternative path is
-    assert msg.get_content_type() == "multipart/alternative"
+    # intentionally not enabled — passing body_html must fail loudly so
-    # Two parts: text/plain first (so plain-text clients picking the
+    # a future caller can't silently ship unescaped user HTML (the C1
-    # first part get the readable text), text/html second.
+    # stored-XSS class in the mail channel). When HTML mail is enabled
-    parts = list(msg.iter_parts())
+    # deliberately, this test flips to assert the multipart shape.
-    assert len(parts) == 2
+    with pytest.raises(NotImplementedError):
-    assert parts[0].get_content_type() == "text/plain"
+        build_envelope(**_base_kwargs(body_html="<p>Hello, <b>world</b>.</p>"))
-    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()
+def test_envelope_html_none_is_plain_only():
+    # The guard keys on `is not None`, so the default (None) stays the
+    # live plain-text path — exercised here to lock the boundary.
+    msg = build_envelope(**_base_kwargs(body_html=None))
+    assert msg.get_content_type() == "text/plain"

backend/tests/test_turnstile_vertical.py

@@ -55,13 +55,19 @@ def _outbound_otc_envelopes(to_address: str | None = None) -> list[dict]:
 
 
 def _patch_siteverify(monkeypatch, *, success: bool, error_codes: list[str] | None = None):
-    """Replace `httpx.post` inside `app.turnstile` with a stub that
+    """Replace `turnstile._siteverify_post` with an async stub that
     returns the requested success shape. The stub does not touch the
     real CloudFlare endpoint and never sees a real secret.
+
+    I4 (security-audit-0026): the siteverify call is now awaited on an
+    `httpx.AsyncClient`, isolated behind the `_siteverify_post` seam.
+    Patching that narrow function (rather than the shared
+    `httpx.AsyncClient`, which gitea/docs also construct) keeps app boot
+    intact.
     """
     captured = {}
 
-    def fake_post(url, *, data=None, timeout=None, **kwargs):
+    async def fake_post(url, data):
         captured["url"] = url
         captured["data"] = data
         body = {"success": bool(success)}
@@ -70,7 +76,7 @@ def _patch_siteverify(monkeypatch, *, success: bool, error_codes: list[str] | No
         return SimpleNamespace(json=lambda: body)
 
     from app import turnstile as turnstile_mod
-    monkeypatch.setattr(turnstile_mod.httpx, "post", fake_post)
+    monkeypatch.setattr(turnstile_mod, "_siteverify_post", fake_post)
     return captured
 
 
@@ -170,14 +176,15 @@ def test_otc_request_admits_when_secret_unset_and_not_required(app_with_fake_git
 
     monkeypatch.delenv("CLOUDFLARE_TURNSTILE_SECRET", raising=False)
     monkeypatch.delenv("TURNSTILE_REQUIRED", raising=False)
-    # The httpx.post inside turnstile must not be called in this path —
+    # The siteverify call inside turnstile must not be made in this path —
-    # patch it to a sentinel that explodes if it ever runs.
+    # patch the seam to a sentinel that explodes if it ever runs
+    # (I4: the seam is now `_siteverify_post`, not module-level httpx.post).
     from app import turnstile as turnstile_mod
 
-    def must_not_be_called(*a, **kw):
+    async def must_not_be_called(*a, **kw):
         raise AssertionError("siteverify should not run when no secret is configured")
 
-    monkeypatch.setattr(turnstile_mod.httpx, "post", must_not_be_called)
+    monkeypatch.setattr(turnstile_mod, "_siteverify_post", must_not_be_called)
 
     app, _fake = app_with_fake_gitea
     with TestClient(app) as client:
@@ -218,3 +225,27 @@ def test_otc_request_refuses_when_required_but_secret_unset(app_with_fake_gitea,
         )
         assert r.status_code == 500, r.text
         assert _outbound_otc_envelopes("alice@example.com") == []
+
+
+# ---------------------------------------------------------------------------
+# I4 (security-audit-0026): verify_token is a coroutine — calling it returns
+# an awaitable, not a VerifyOutcome. Locks the async contract so a revert to
+# the synchronous event-loop-blocking shape fails here, not just in the
+# integration paths.
+# ---------------------------------------------------------------------------
+
+
+def test_verify_token_is_async_and_soft_skips_without_secret(monkeypatch):
+    import asyncio
+
+    from app import turnstile as turnstile_mod
+
+    monkeypatch.delenv("CLOUDFLARE_TURNSTILE_SECRET", raising=False)
+    monkeypatch.delenv("TURNSTILE_REQUIRED", raising=False)
+
+    coro = turnstile_mod.verify_token("any-token")
+    assert asyncio.iscoroutine(coro), "verify_token must be a coroutine (I4)"
+    outcome = asyncio.run(coro)
+    # No secret + not required → the gate stays open without any network call.
+    assert outcome.ok is True
+    assert outcome.reason == "skipped"

frontend/package.json

@@ -1,7 +1,7 @@
 {
   "name": "rfc-app-frontend",
   "private": true,
-  "version": "0.27.0",
+  "version": "0.28.0",
   "type": "module",
   "scripts": {
     "dev": "vite",