Release v0.28.0: security-audit-0026 I3 + I4 (HTML-email guard + async Turnstile)
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>
This commit is contained in:
@@ -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).
|
||||
|
||||
@@ -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
|
||||
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.
|
||||
`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
|
||||
@@ -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:
|
||||
# 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)
|
||||
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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
@@ -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`
|
||||
+ a MockTransport client) to avoid touching the real CloudFlare
|
||||
endpoint. No real keys are ever embedded in tests.
|
||||
Async (I4, security-audit-0026): the siteverify call is awaited on an
|
||||
`httpx.AsyncClient` so a slow CloudFlare response can't block the
|
||||
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)
|
||||
|
||||
@@ -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():
|
||||
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()
|
||||
def test_envelope_html_body_is_guarded_not_enabled():
|
||||
# I3 (security-audit-0026): the HTML/multipart-alternative path is
|
||||
# intentionally not enabled — passing body_html must fail loudly so
|
||||
# a future caller can't silently ship unescaped user HTML (the C1
|
||||
# stored-XSS class in the mail channel). When HTML mail is enabled
|
||||
# deliberately, this test flips to assert the multipart shape.
|
||||
with pytest.raises(NotImplementedError):
|
||||
build_envelope(**_base_kwargs(body_html="<p>Hello, <b>world</b>.</p>"))
|
||||
|
||||
|
||||
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"
|
||||
|
||||
@@ -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 —
|
||||
# patch it to a sentinel that explodes if it ever runs.
|
||||
# The siteverify call inside turnstile must not be made in this path —
|
||||
# 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"
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
{
|
||||
"name": "rfc-app-frontend",
|
||||
"private": true,
|
||||
"version": "0.27.0",
|
||||
"version": "0.28.0",
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"dev": "vite",
|
||||
|
||||
Reference in New Issue
Block a user