Compare commits

...

5 Commits

Author SHA1 Message Date
Ben Stull 019c8a9185 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 2026-05-29 02:42:05 +00:00
Ben Stull 79a447c77b 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>
2026-05-28 19:12:16 -07:00
Ben Stull fe044ed3db Merge pull request 'Release v0.27.0: security hardening (audit 0026)' (#1) from feature/v0.27.0-security-hardening into main 2026-05-29 01:28:42 +00:00
Ben Stull bd3ef269d4 Release v0.27.0: security hardening (audit 0026)
Remediates the rfc-app application + deploy-config findings from the
Session 0026 security audit. Cut as the "v0.25.0-security-hardening"
branch (from v0.24.0); reversioned to 0.27.0 on rebase onto main since
v0.26.0 (#28) shipped while this was in flight.

- C1 (Critical): single sanitizeHtml.js chokepoint (DOMPurify) for every
  marked→innerHTML / dangerouslySetInnerHTML sink (MarkdownPreview,
  ProposalView x2, Editor); rel=noopener hook on target=_blank links.
- H1: per-account OTC-verify lockout (migration 023, auto-applied) +
  per-IP throttle via new ratelimit.py; wired on otc verify/request +
  passcode check/verify.
- M1: device_trust.lookup() single indexed-row read — cookie value is now
  "<row_id>.<raw_token>"; bcrypt-checks one row, not a global table scan.
  (Behavior change: existing device-trust cookies re-prompt once.)
- M2: HTTP security headers (CSP/HSTS/XFO/XCTO/Referrer-Policy) at nginx.
- M4: session cookie Secure-by-default (SESSION_COOKIE_SECURE opt-out).
- M5: bounce webhook fails CLOSED (503) when secret unset, instead of open;
  RFC_APP_INSECURE_BOUNCE_WEBHOOK=1 dev opt-in.
- L2/L3: per-IP cooldown + check-endpoint throttle.
- L4: systemd sandbox knobs. L8/I1: nginx server_tokens off + TLS1.0/1.1 out.

VERSION + frontend/package.json → 0.27.0; CHANGELOG documents the upgrade
steps (incl. the out-of-band nginx + systemd apply, which the flotilla
deploy gesture does not perform).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-05-28 18:28:10 -07:00
Ben Stull 698821f065 Merge feature/v0.26.0-pr-rfc-links (#28 Part 1: auto-link accepted RFCs in PR text + comments) 2026-05-28 16:21:26 -07:00
22 changed files with 668 additions and 89 deletions
+96
View File
@@ -23,6 +23,102 @@ 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).
One auto-applied migration (023); one behavior change that re-prompts
device-trust; deployments MUST re-apply the nginx + systemd files.**
This is the work cut as the "v0.25.0 security-hardening" branch; it
reversioned to 0.27.0 because v0.26.0 (#28) took the next slot while it
was in flight. Shipped from driver session 0030.0.
- **C1 (Critical) — stored-XSS closed.** Every markdown→HTML sink now
routes through one chokepoint, `frontend/src/lib/sanitizeHtml.js`
(DOMPurify), before any `innerHTML` / `dangerouslySetInnerHTML` write:
`MarkdownPreview`, both `ProposalView` sinks (entry body +
`proposed_use_case`), and `Editor`. A hook adds
`rel="noopener noreferrer"` to `target=_blank` links. `marked` no
longer passes raw HTML / `javascript:` URIs to the DOM, so a
contributor can no longer plant a payload that runs in an admin/owner
session during review.
- **H1 — OTC verify is rate-limited.** New `backend/app/ratelimit.py`
(per-IP token buckets) gates `/auth/otc/verify`, `/auth/otc/request`,
and the passcode check/verify paths; a per-account OTC-verify lockout
(migration `023_otc_verify_lockout.sql`) mirrors the passcode lockout.
- **M1 — device-trust lookup no longer table-scans.** The device-trust
cookie value is now `"<row_id>.<raw_token>"`; `device_trust.lookup`
reads the one indexed row and bcrypt-checks only it, instead of
bcrypt-checking every row in the table on each unauthenticated
`/auth/device-trust/start`.
- **M2 — HTTP security headers** (CSP, HSTS, X-Frame-Options,
X-Content-Type-Options, Referrer-Policy) added to the nginx server
block. **L8/I1**: `server_tokens off` + legacy TLS1.0/1.1 removed.
- **M4 — session cookie `Secure` by default** (`SESSION_COOKIE_SECURE`,
defaults on; a dev box on plain http sets it `false`).
- **M5 — bounce webhook fails closed.** An unset
`WEBHOOK_EMAIL_BOUNCE_SECRET` now **disables** `/api/webhooks/email-bounce`
(503) instead of leaving it open; a dev opts back in with
`RFC_APP_INSECURE_BOUNCE_WEBHOOK=1`.
- **L2/L3** per-IP cooldown + check-endpoint throttle. **L4** systemd
sandbox knobs (`CapabilityBoundingSet=`, `ProtectKernel*`,
`RestrictAddressFamilies`, `SystemCallFilter`, …).
Upgrade steps:
1. **Migration** — none manual; `023_otc_verify_lockout.sql` auto-applies
at startup via `db.run_migrations`.
2. **Device trust (MUST expect re-prompt)** — the cookie format changed,
so existing "trusted device" cookies no longer match; affected users
are re-prompted for device verification once. No data migration; old
rows are simply never matched and age out.
3. **nginx + systemd (MUST apply out-of-band)** — the deploy gesture does
**not** install `deploy/nginx/ohm.wiggleverse.org.conf` or
`deploy/systemd/rfc-app.service`. After deploying the code, copy both
to their system locations, then `nginx -t && systemctl reload nginx`
and `systemctl daemon-reload && systemctl restart <unit>`. (M2 headers
and L4 sandboxing do not take effect until this is done.)
4. **Bounce webhook (SHOULD)** — bind `WEBHOOK_EMAIL_BOUNCE_SECRET` (or
set `RFC_APP_INSECURE_BOUNCE_WEBHOOK=1` for dev). Unset → the endpoint
returns 503 (closed). No legitimate bounce source is wired today, so
503 is the safe default.
5. **Session cookie (SHOULD, dev only)** — a deployment served over plain
http MUST set `SESSION_COOKIE_SECURE=false` or the session cookie
won't be sent. Production over HTTPS leaves it unset (Secure on).
## 0.26.0 — 2026-05-28
**Minor — no schema migration, no new secret, no config, no upgrade
+1 -1
View File
@@ -1 +1 @@
0.26.0
0.28.0
+20 -1
View File
@@ -557,7 +557,26 @@ def make_router(config: Config) -> APIRouter:
# stays unauthenticated for dev (the v1 contract).
import os as _os
expected = _os.environ.get("WEBHOOK_EMAIL_BOUNCE_SECRET", "").strip()
if expected:
# v0.25.0 (audit 0026 M5): fail closed. An unset secret used to
# leave this endpoint fully unauthenticated — anyone could suppress
# any user's mail by POSTing their address (email_opt_out_all flip
# below). Now an unset secret DISABLES the endpoint (503) instead
# of opening it. A dev that genuinely wants it open opts in
# explicitly with RFC_APP_INSECURE_BOUNCE_WEBHOOK=1, mirroring the
# RFC_APP_INSECURE_WEBHOOKS dev-bypass on the Gitea hook.
if not expected:
if _os.environ.get("RFC_APP_INSECURE_BOUNCE_WEBHOOK", "").strip() == "1":
log.warning(
"email-bounce webhook running UNAUTHENTICATED "
"(RFC_APP_INSECURE_BOUNCE_WEBHOOK=1) — never set this in production"
)
else:
log.error(
"email-bounce webhook refused: WEBHOOK_EMAIL_BOUNCE_SECRET is unset "
"(set the secret to enable, or RFC_APP_INSECURE_BOUNCE_WEBHOOK=1 for dev)"
)
raise HTTPException(503, "Bounce webhook not configured")
else:
received = request.headers.get("X-Webhook-Secret", "")
import hmac as _hmac
if not received or not _hmac.compare_digest(expected, received):
+32 -21
View File
@@ -97,6 +97,13 @@ class IssueOutcome:
raw_token: str
row_id: int
@property
def cookie_value(self) -> str:
"""The value to put in the `rfc_device_trust` cookie: the row-id
selector joined to the raw token (v0.25.0 / audit 0026 M1). The
selector lets `lookup` read one indexed row instead of scanning."""
return f"{self.row_id}.{self.raw_token}"
def _new_token() -> str:
return secrets.token_urlsafe(TOKEN_BYTES)
@@ -173,33 +180,37 @@ def lookup(raw_token: str) -> LookupOutcome:
if not raw:
return LookupOutcome(ok=False, user=None, reason="invalid")
# The unique index on `device_token_hash` would let us SELECT by
# hash if bcrypt were a stable hash, but bcrypt incorporates a
# per-row salt — equal tokens produce different hashes. We walk
# the candidate set instead. In practice the set is small (a
# human has a handful of trusted devices) and bcrypt is cheap on
# the order of milliseconds; the walk is bounded by the user's
# active device count.
# v0.25.0 (audit 0026 M1): the cookie is "<row_id>.<raw_token>". We
# parse the row-id selector and read exactly ONE row by its indexed
# primary key, then bcrypt-check the token against that single row.
#
# We don't pre-filter by `revoked_at IS NULL` here so that a
# token presented for a recently-revoked row produces a
# 'revoked' outcome (the endpoint surfaces a different shape).
# Same for expired: we let the walk hit and classify after.
rows = db.conn().execute(
# The previous shape read EVERY device_trust row (all users, including
# revoked/expired) and bcrypt-checked each — an unauthenticated
# CPU-amplification DoS reachable at /auth/device-trust/start that
# grew without bound as the table accumulated. bcrypt's per-row salt
# is why we can't SELECT by hash; carrying the row-id in the cookie is
# the standard fix (the id is not secret; the token still is).
selector, sep, token = raw.partition(".")
if not sep or not selector.isdigit() or not token:
# Legacy bare-token cookies (pre-v0.25.0) and malformed values land
# here. We refuse rather than fall back to a full-table scan, so
# the amplification path is fully closed; affected users simply
# re-authenticate once via OTC/passcode and get a new cookie.
return LookupOutcome(ok=False, user=None, reason="invalid")
matched = db.conn().execute(
"""
SELECT id, user_id, device_token_hash, expires_at, revoked_at
FROM device_trust
ORDER BY id DESC
WHERE id = ?
""",
).fetchall()
(int(selector),),
).fetchone()
matched = None
for row in rows:
if _check(raw, row["device_token_hash"]):
matched = row
break
if matched is None:
# One bcrypt check, against the selected row only. A wrong/forged token
# for a real id reads as 'unknown' (cookie cleared), same as a missing
# row — a probing client can't distinguish the two.
if matched is None or not _check(token, matched["device_token_hash"]):
return LookupOutcome(ok=False, user=None, reason="unknown")
if matched["revoked_at"] is not None:
+27 -15
View File
@@ -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
+61 -19
View File
@@ -7,6 +7,7 @@ no need for a separate worker.
from __future__ import annotations
import logging
import os
import secrets
from contextlib import asynccontextmanager
@@ -28,6 +29,7 @@ from . import (
otc,
passcode as passcode_mod,
providers as providers_mod,
ratelimit,
turnstile,
webhooks,
)
@@ -142,12 +144,20 @@ def create_app() -> FastAPI:
# eagerly via load_config(). Everything else waits for lifespan.
config = load_config()
app = FastAPI(lifespan=lifespan)
# v0.25.0 (audit 0026 M4): the session cookie is the primary 30-day
# auth credential and must carry `Secure` in production so it never
# travels cleartext. Default to Secure; a dev box serving over plain
# http opts out with SESSION_COOKIE_SECURE=false. Production (OHM is
# HTTPS-only with an HTTP->HTTPS 301) leaves this unset → Secure on.
session_secure = os.environ.get("SESSION_COOKIE_SECURE", "true").strip().lower() not in (
"0", "false", "no", "off",
)
app.add_middleware(
SessionMiddleware,
secret_key=config.secret_key,
session_cookie="rfc_session",
max_age=60 * 60 * 24 * 30,
https_only=False,
https_only=session_secure,
)
return app
@@ -155,24 +165,25 @@ def create_app() -> FastAPI:
app = create_app()
def _set_device_trust_cookie(response: Response, raw_token: str) -> None:
def _set_device_trust_cookie(response: Response, cookie_value: str) -> None:
"""Attach the v0.11.0 device-trust cookie to the response.
HttpOnly + Secure + SameSite=Lax + 30-day Max-Age + Path=/. The
cookie value is the raw token; server-side storage is the hash.
The cookie is "essential" per the v0.13.0 cookie-consent contract
(it is part of authentication), so we set it regardless of the
user's analytics / other-cookies choice.
HttpOnly + Secure + SameSite=Lax + 30-day Max-Age + Path=/. As of
v0.25.0 (audit 0026 M1) the value is `IssueOutcome.cookie_value` —
"<row_id>.<raw_token>" — so `device_trust.lookup` can read one indexed
row instead of scanning; server-side storage remains the bcrypt hash
of the token half only. The cookie is "essential" per the v0.13.0
cookie-consent contract (it is part of authentication), so we set it
regardless of the user's analytics / other-cookies choice.
Secure=True means the cookie is only ever sent over HTTPS. The
SessionMiddleware in `create_app` keeps `https_only=False` for
dev parity, but the device-trust cookie holds a 30-day credential
and must not travel cleartext — production deployments serve over
HTTPS, so Secure on the device-trust cookie is non-negotiable.
Secure=True means the cookie is only ever sent over HTTPS — the
device-trust cookie holds a 30-day credential and must never travel
cleartext. (The session cookie now also defaults to Secure; see M4 in
`create_app`.)
"""
response.set_cookie(
key=device_trust_mod.COOKIE_NAME,
value=raw_token,
value=cookie_value,
max_age=device_trust_mod.COOKIE_MAX_AGE_SECONDS,
path="/",
secure=True,
@@ -245,6 +256,10 @@ def _oauth_router(config) -> APIRouter:
@router.post("/auth/otc/request")
async def otc_request(body: OtcRequestBody, request: Request):
# v0.25.0 (audit 0026 H1/L2): per-IP brake at the cheapest point,
# before the Turnstile network call or any bcrypt/SMTP work.
if not ratelimit.otc_request_limiter.allow(ratelimit.client_key(request)):
raise HTTPException(429, "Too many requests; please wait a few minutes")
# v0.12.0 / roadmap item #10: gate the request on a successful
# Turnstile siteverify before the bcrypt hash + SMTP send. The
# check runs first so a failed challenge spends no rate budget
@@ -252,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
@@ -278,9 +293,25 @@ def _oauth_router(config) -> APIRouter:
@router.post("/auth/otc/verify")
async def otc_verify(body: OtcVerifyBody, request: Request, response: Response):
# v0.25.0 (audit 0026 H1): per-IP brake against fan-out guessing,
# plus the per-email lockout enforced inside otc.verify_code.
ip = ratelimit.client_key(request)
if not ratelimit.verify_limiter.allow(ip):
raise HTTPException(429, "Too many attempts; please wait a few minutes")
result = otc.verify_code(body.email, body.code)
if result.reason == "locked":
raise HTTPException(
423,
{
"detail": "Too many failed attempts; wait a few minutes or request a new code",
"locked_until": result.locked_until,
},
)
if not result.ok or result.user is None:
raise HTTPException(400, "Invalid or expired code")
# Legit sign-in: clear this IP's window so a user who fat-fingered
# a couple of codes isn't left throttled.
ratelimit.verify_limiter.reset(ip)
auth.store_session(request, result.user)
# v0.8.0: surface `needs_profile` so the Login.jsx surface can
# decide whether to advance to the first/last/why capture step
@@ -313,7 +344,7 @@ def _oauth_router(config) -> APIRouter:
if body.trust_device:
ua = request.headers.get("user-agent", "")
outcome = device_trust_mod.issue(result.user.user_id, ua)
_set_device_trust_cookie(response, outcome.raw_token)
_set_device_trust_cookie(response, outcome.cookie_value)
return {
"ok": True,
"user": {
@@ -337,12 +368,17 @@ def _oauth_router(config) -> APIRouter:
# ---------------------------------------------------------------
@router.get("/auth/passcode/check")
async def passcode_check(email: str = ""):
async def passcode_check(request: Request, email: str = ""):
"""Does this email have a passcode set? Anonymous endpoint —
the Login.jsx flow calls this after the user types their email
to decide whether to render a passcode input or fall back to
OTC. We surface only the boolean; lockout state, the hash, and
the set-at stamp are not leaked here."""
the set-at stamp are not leaked here.
v0.25.0 (audit 0026 L3): per-IP rate limit so the has-passcode
boolean can't be bulk-harvested to enumerate accounts."""
if not ratelimit.check_limiter.allow(ratelimit.client_key(request)):
raise HTTPException(429, "Too many requests; please wait a few minutes")
status = passcode_mod.passcode_status(email)
return {"has_passcode": status.has_passcode}
@@ -376,6 +412,11 @@ def _oauth_router(config) -> APIRouter:
v0.11.0: the body's `trust_device` flag, if true, mints a
fresh device-trust row and sets the long-lived cookie. Same
opt-in contract as `/auth/otc/verify`."""
# v0.25.0 (audit 0026 H1): per-IP brake in front of the per-account
# passcode lockout, so fan-out across emails is throttled too.
ip = ratelimit.client_key(request)
if not ratelimit.verify_limiter.allow(ip):
raise HTTPException(429, "Too many attempts; please wait a few minutes")
result = passcode_mod.verify_passcode(body.email, body.passcode)
if result.reason == "locked":
raise HTTPException(
@@ -387,11 +428,12 @@ def _oauth_router(config) -> APIRouter:
)
if not result.ok or result.user is None:
raise HTTPException(400, "Invalid passcode")
ratelimit.verify_limiter.reset(ip)
auth.store_session(request, result.user)
if body.trust_device:
ua = request.headers.get("user-agent", "")
outcome = device_trust_mod.issue(result.user.user_id, ua)
_set_device_trust_cookie(response, outcome.raw_token)
_set_device_trust_cookie(response, outcome.cookie_value)
return {
"ok": True,
"user": {
@@ -459,7 +501,7 @@ def _oauth_router(config) -> APIRouter:
if body.trust_device:
ua = request.headers.get("user-agent", "")
outcome = device_trust_mod.issue(result.user.user_id, ua)
_set_device_trust_cookie(response, outcome.raw_token)
_set_device_trust_cookie(response, outcome.cookie_value)
# Has the user already set a passcode? (Could only happen via
# an admin pre-population path that doesn't exist yet, but
+76
View File
@@ -85,6 +85,15 @@ def _cooldown_seconds() -> int:
return 60
# v0.25.0 / security audit 0026 (H1): per-email OTC verify lockout,
# mirroring the passcode path (passcode.py). Five consecutive wrong codes
# for an email lock its OTC verify for 15 minutes. The per-IP limiter in
# ratelimit.py is the primary brute-force brake; this is the durable,
# passcode-parity layer.
LOCKOUT_AFTER_FAILED_ATTEMPTS = 5
LOCKOUT_DURATION_MINUTES = 15
# ---------------------------------------------------------------------------
# Code generation + hashing
# ---------------------------------------------------------------------------
@@ -206,6 +215,61 @@ class VerifyOutcome:
ok: bool
user: SessionUser | None
reason: str
# v0.25.0 (H1): ISO-8601 stamp when reason == 'locked'.
locked_until: str | None = None
def _verify_lockout_until(email: str) -> str | None:
"""Return the active lockout stamp for `email`, or None if not locked.
Clears an elapsed lockout (and resets the counter) as a side effect so
the next failure starts a fresh budget — mirrors passcode.verify_passcode.
"""
row = db.conn().execute(
"SELECT failed_attempts, locked_until FROM otc_verify_state WHERE email = ?",
(email,),
).fetchone()
if row is None or not row["locked_until"]:
return None
still_locked = db.conn().execute(
"SELECT datetime(?) > datetime('now') AS locked", (row["locked_until"],),
).fetchone()["locked"]
if still_locked:
return row["locked_until"]
db.conn().execute(
"UPDATE otc_verify_state SET failed_attempts = 0, locked_until = NULL WHERE email = ?",
(email,),
)
return None
def _record_verify_failure(email: str) -> None:
"""Increment the per-email failure counter; stamp a lockout once it
crosses the threshold. Mirrors the passcode lockout shape."""
db.conn().execute(
"""
INSERT INTO otc_verify_state (email, failed_attempts)
VALUES (?, 1)
ON CONFLICT(email) DO UPDATE SET failed_attempts = failed_attempts + 1
""",
(email,),
)
count = db.conn().execute(
"SELECT failed_attempts FROM otc_verify_state WHERE email = ?", (email,),
).fetchone()["failed_attempts"]
if count >= LOCKOUT_AFTER_FAILED_ATTEMPTS:
db.conn().execute(
f"""
UPDATE otc_verify_state
SET locked_until = datetime('now', '+{LOCKOUT_DURATION_MINUTES} minutes')
WHERE email = ?
""",
(email,),
)
def _clear_verify_state(email: str) -> None:
db.conn().execute("DELETE FROM otc_verify_state WHERE email = ?", (email,))
def verify_code(email: str, code: str) -> VerifyOutcome:
@@ -214,6 +278,13 @@ def verify_code(email: str, code: str) -> VerifyOutcome:
if not email or not code:
return VerifyOutcome(ok=False, user=None, reason="invalid")
# v0.25.0 (H1): refuse before spending any bcrypt if this email is in
# its OTC-verify lockout window. The passcode path is unaffected — a
# locked-out OTC user can still set/use a passcode, and vice versa.
locked_until = _verify_lockout_until(email)
if locked_until:
return VerifyOutcome(ok=False, user=None, reason="locked", locked_until=locked_until)
rows = db.conn().execute(
"""
SELECT id, code_hash, expires_at, consumed_at
@@ -237,6 +308,9 @@ def verify_code(email: str, code: str) -> VerifyOutcome:
break
if matched is None:
# A genuine wrong guess against this email — the brute-force
# signal. Count it toward the lockout threshold (H1).
_record_verify_failure(email)
return VerifyOutcome(ok=False, user=None, reason="wrong")
if matched["consumed_at"] is not None:
@@ -255,6 +329,8 @@ def verify_code(email: str, code: str) -> VerifyOutcome:
"UPDATE otc_codes SET consumed_at = datetime('now') WHERE id = ?",
(matched["id"],),
)
# Success wipes the per-email failure counter (H1).
_clear_verify_state(email)
user = provision_or_link_user(email)
return VerifyOutcome(ok=True, user=user, reason="ok")
+92
View File
@@ -0,0 +1,92 @@
"""In-process per-IP sliding-window rate limiter (security audit 0026, H1).
The auth verify endpoints (`/auth/otc/verify`, `/auth/passcode/verify`)
had no per-IP brake, so an attacker could fan out guesses against a
target identity bounded only by bcrypt cost. This module is the brake.
It is deliberately tiny: §4.2 says the app is a single process with a
colocated SQLite file, so an in-memory dict of `key -> deque[timestamps]`
is sufficient and needs no shared store. State resets on restart, which
fails *open* for a brief window — acceptable because the per-email OTC
lockout (`otc_verify_state`) and the passcode lockout both persist in the
database and carry the durable guarantee; this limiter is the
anti-fan-out layer on top.
Chosen over a per-identity lockout *as the primary control* because a
per-IP window throttles the attacker without letting them grief a victim
by locking that victim's account (the known downside of identity
lockouts). Both layers run together.
"""
from __future__ import annotations
import threading
import time
from collections import defaultdict, deque
class SlidingWindowLimiter:
"""Allow at most `max_events` per `window_seconds` per key.
`allow(key)` records an event and returns True if the key is still
within budget, False if it has exceeded it. Timestamps use a
monotonic clock so the limiter is immune to wall-clock jumps.
"""
def __init__(self, max_events: int, window_seconds: float) -> None:
self.max_events = max_events
self.window_seconds = window_seconds
self._events: dict[str, deque[float]] = defaultdict(deque)
self._lock = threading.Lock()
def allow(self, key: str) -> bool:
now = time.monotonic()
cutoff = now - self.window_seconds
with self._lock:
q = self._events[key]
while q and q[0] < cutoff:
q.popleft()
if len(q) >= self.max_events:
return False
q.append(now)
# Opportunistic cleanup so idle keys don't accumulate forever.
if not q:
self._events.pop(key, None)
return True
def reset(self, key: str) -> None:
"""Drop a key's window — e.g. after a successful sign-in so a
legitimate user who fat-fingered a few times isn't throttled."""
with self._lock:
self._events.pop(key, None)
# Module-level limiters shared across requests (one process, so module
# state is the natural home). Tunables are intentionally generous enough
# not to bother a human retyping a code, tight enough to kill fan-out:
# * verify: 10 attempts / 5 min / IP across the auth verify surfaces.
# * otc request: 5 sends / 5 min / IP (Turnstile is the primary gate;
# this is defense in depth against a solved-challenge replay loop).
verify_limiter = SlidingWindowLimiter(max_events=10, window_seconds=300)
otc_request_limiter = SlidingWindowLimiter(max_events=5, window_seconds=300)
# /auth/passcode/check is an anonymous has-passcode oracle (audit 0026 L3).
# It's a legitimate Login-flow affordance, so the budget is generous —
# enough for a human typing emails, tight enough to stop bulk scraping.
check_limiter = SlidingWindowLimiter(max_events=30, window_seconds=300)
def _reset_all_for_tests() -> None:
"""Clear every module-level limiter's window. Test support only — the
limiters are process-global singletons, so without a per-test reset
one test's requests bleed into the next and later tests trip the
budget (429). Not called in production."""
for lim in (verify_limiter, otc_request_limiter, check_limiter):
with lim._lock:
lim._events.clear()
def client_key(request) -> str:
"""Best-effort client identity for limiting. Behind nginx the app is
started with `--forwarded-allow-ips 127.0.0.1`, so `request.client.host`
reflects the real client IP via Uvicorn's ProxyHeaders handling."""
client = getattr(request, "client", None)
return client.host if client and client.host else "unknown"
+23 -5
View File
@@ -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)
@@ -0,0 +1,18 @@
-- v0.25.0 / security audit 0026, finding H1.
--
-- The OTC verify path had no attempt-limit or lockout, unlike the
-- passcode path (015_passcode.sql gave users.passcode_failed_attempts +
-- passcode_locked_until). This table gives the OTC verify endpoint the
-- same per-identity lockout shape. It is keyed by email rather than
-- user_id because an OTC sign-in may not have a users row yet — the row
-- is provisioned only on a *successful* verify, so the lockout state has
-- to survive independently of it.
--
-- The per-IP rate limiter (app/ratelimit.py) is the primary brute-force
-- defense; this table is the parity layer that mirrors the passcode
-- lockout and persists across restarts.
CREATE TABLE IF NOT EXISTS otc_verify_state (
email TEXT PRIMARY KEY,
failed_attempts INTEGER NOT NULL DEFAULT 0,
locked_until TEXT
);
+19
View File
@@ -0,0 +1,19 @@
"""Shared pytest fixtures for the backend suite.
Added in v0.27.0 (security audit 0026) alongside the new per-IP rate
limiter. The limiters in `app.ratelimit` are process-global singletons,
so their state survives across tests within a run; without a reset, the
accumulated requests from earlier tests exhaust the budget and later
tests see spurious 429s. This autouse fixture gives every test a clean
limiter window.
"""
import pytest
from app import ratelimit
@pytest.fixture(autouse=True)
def _reset_rate_limiters():
ratelimit._reset_all_for_tests()
yield
ratelimit._reset_all_for_tests()
+17 -11
View File
@@ -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"
+12
View File
@@ -444,6 +444,18 @@ def tmp_env(monkeypatch):
# the dev-bypass path monkeypatch `RFC_APP_INSECURE_WEBHOOKS=1`.
"GITEA_WEBHOOK_SECRET": "test-webhook-secret-for-signature-verification",
"ENABLED_MODELS": "claude",
# v0.27.0 (audit 0026 M4): the session cookie now defaults to
# Secure. The TestClient talks plain http://testserver, so a
# Secure cookie is never sent back and every authenticated flow
# would fail. Tests opt out explicitly, exactly as a dev box on
# plain http does.
"SESSION_COOKIE_SECURE": "false",
# v0.27.0 (audit 0026 M5): the bounce webhook fails closed (503)
# when its secret is unset. Tests exercise the legacy behavioral
# path via the documented dev opt-in, mirroring the
# RFC_APP_INSECURE_WEBHOOKS bypass above. Tests that assert the
# fail-closed default delenv this key themselves.
"RFC_APP_INSECURE_BOUNCE_WEBHOOK": "1",
}
for k, v in env.items():
monkeypatch.setenv(k, v)
+38 -7
View File
@@ -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"
+50
View File
@@ -18,6 +18,56 @@ server {
listen [::]:80;
server_name ohm.wiggleverse.org;
# v0.25.0 security hardening (audit 0026 M2/L8)
#
# NOTE: certbot promotes THIS server block to the HTTPS listener
# (`listen 443 ssl`) and adds a separate port-80 → 443 redirect
# block (see the install comment above). These response headers
# therefore ride into the HTTPS server block on the VM. They use
# `add_header ... always` so they also apply to nginx-generated
# error responses (4xx/5xx), not just 200s.
#
# `server_tokens off` (L8) — suppress the nginx version in the
# Server header and on error pages so we don't advertise the
# build to scanners.
server_tokens off;
add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always;
add_header X-Frame-Options "DENY" always;
add_header X-Content-Type-Options "nosniff" always;
add_header Referrer-Policy "strict-origin-when-cross-origin" always;
# Content-Security-Policy (M2). Tuned to what the SPA actually loads:
# - default-src 'self': everything not called out below is same-origin.
# - script-src 'self' + challenges.cloudflare.com: the only external
# <script> tag the app injects is the CloudFlare Turnstile widget
# (frontend/src/components/TurnstileWidget.jsx). Amplitude and
# mermaid are BUNDLED (dynamic `import()` from node_modules, served
# from 'self'), so they need no extra script origin — *.amplitude.com
# is listed defensively in case a future SDK build script-injects.
# script-src DELIBERATELY OMITS 'unsafe-inline' — no inline <script>
# is used, so we keep XSS-via-inline-script blocked.
# - style-src 'unsafe-inline' IS REQUIRED by the current build: the
# JSX uses inline `style={...}` attributes throughout and mermaid
# injects <style> blocks at render time. Removing it would break
# layout; tightening this is a future build-side change (nonce/hash).
# - img-src 'self' data: https: — markdown/RFC bodies may embed remote
# images and data: URIs; svg/mermaid output uses data: too.
# - font-src 'self' data: — bundled fonts plus data: webfonts.
# - connect-src 'self' + *.amplitude.com + challenges.cloudflare.com:
# the app's API/auth/SSE are same-origin (nginx proxy); Amplitude
# Analytics + Session Replay (shipped at sampleRate 1) POST to
# *.amplitude.com; Turnstile verifies via challenges.cloudflare.com.
# - worker-src 'self' blob: — Amplitude Session Replay spins up a
# Web Worker from a blob: URL for capture/compression; without
# blob: here session replay breaks for every consenting user.
# - frame-src challenges.cloudflare.com — the Turnstile challenge
# renders in an iframe from that origin.
# - frame-ancestors 'none' — clickjacking defense, pairs with
# X-Frame-Options DENY for older agents.
# - base-uri 'self'; object-src 'none' — lock down <base>/<object>.
add_header Content-Security-Policy "default-src 'self'; script-src 'self' https://challenges.cloudflare.com https://*.amplitude.com; style-src 'self' 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data:; connect-src 'self' https://*.amplitude.com https://challenges.cloudflare.com; worker-src 'self' blob:; frame-src https://challenges.cloudflare.com; frame-ancestors 'none'; base-uri 'self'; object-src 'none'" always;
# Static SPA assets live in the Vite build output. The systemd unit
# runs as user `rfc-app`; make sure nginx (usually `www-data`) can
# read this path. Either group-add www-data into rfc-app's group, or
+27
View File
@@ -42,5 +42,32 @@ ProtectHome=true
PrivateTmp=true
ReadWritePaths=/opt/rfc-app/backend/data
# v0.25.0 security hardening (audit 0026 L4) — defense-in-depth.
# The service binds 127.0.0.1:8000 and runs plain CPython
# (FastAPI/uvicorn + sqlite + bcrypt + httpx), so it needs no
# capabilities and no exotic syscalls.
CapabilityBoundingSet=
AmbientCapabilities=
PrivateDevices=true
ProtectKernelTunables=true
ProtectKernelModules=true
ProtectKernelLogs=true
ProtectControlGroups=true
RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX
RestrictNamespaces=true
LockPersonality=true
# MemoryDenyWriteExecute=true blocks W^X memory — safe for stock
# CPython (no JIT) and the pure-Python/C-extension stack here, but
# would break a JIT or a C-ext that mmaps W+X. Watch the first
# restart's journal for a crash; if uvicorn fails to come up,
# comment this one line out and reload.
MemoryDenyWriteExecute=true
RestrictRealtime=true
RestrictSUIDSGID=true
SystemCallFilter=@system-service
SystemCallErrorNumber=EPERM
SystemCallArchitectures=native
UMask=0077
[Install]
WantedBy=multi-user.target
+3 -2
View File
@@ -1,12 +1,12 @@
{
"name": "rfc-app-frontend",
"version": "0.21.0",
"version": "0.24.0",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "rfc-app-frontend",
"version": "0.21.0",
"version": "0.24.0",
"dependencies": {
"@amplitude/unified": "^1.1.9",
"@codemirror/commands": "^6.10.3",
@@ -18,6 +18,7 @@
"@tiptap/pm": "^3.5.0",
"@tiptap/react": "^3.5.0",
"@tiptap/starter-kit": "^3.5.0",
"dompurify": "^3.2.4",
"marked": "^18.0.4",
"mermaid": "^11.15.0",
"react": "^19.2.6",
+2 -1
View File
@@ -1,7 +1,7 @@
{
"name": "rfc-app-frontend",
"private": true,
"version": "0.26.0",
"version": "0.28.0",
"type": "module",
"scripts": {
"dev": "vite",
@@ -19,6 +19,7 @@
"@tiptap/pm": "^3.5.0",
"@tiptap/react": "^3.5.0",
"@tiptap/starter-kit": "^3.5.0",
"dompurify": "^3.2.4",
"marked": "^18.0.4",
"mermaid": "^11.15.0",
"react": "^19.2.6",
+2 -2
View File
@@ -17,7 +17,7 @@
import { useEditor, EditorContent, Extension } from '@tiptap/react'
import StarterKit from '@tiptap/starter-kit'
import { useEffect, useRef, useCallback } from 'react'
import { marked } from 'marked'
import { renderMarkdown } from '../lib/sanitizeHtml'
import { Plugin, PluginKey } from 'prosemirror-state'
import { Decoration, DecorationSet } from 'prosemirror-view'
@@ -122,7 +122,7 @@ export default function Editor({
useEffect(() => {
if (!editor || content == null) return
const html = marked.parse(content)
const html = renderMarkdown(content)
editor.commands.setContent(html, false)
}, [content, editor])
+2 -1
View File
@@ -21,6 +21,7 @@
import { useEffect, useRef, useState, useCallback } from 'react'
import { Marked } from 'marked'
import { sanitizeHtml } from '../lib/sanitizeHtml'
import { decorateAcceptedChanges } from './trackedOverlay.js'
import ChangeTooltip from './ChangeTooltip.jsx'
@@ -98,7 +99,7 @@ export default function MarkdownPreview({
// synchronously with the body itself — no flash of un-decorated text.
useEffect(() => {
if (!hostRef.current) return
const html = previewMarked.parse(content || '')
const html = sanitizeHtml(previewMarked.parse(content || ''))
hostRef.current.innerHTML = html
const token = ++renderTokenRef.current
// Reset memo so the new block set re-renders from scratch.
+3 -3
View File
@@ -10,7 +10,7 @@
import { useEffect, useState } from 'react'
import { useParams, useNavigate } from 'react-router-dom'
import { marked } from 'marked'
import { renderMarkdown } from '../lib/sanitizeHtml'
import { getProposal, mergeProposal, declineProposal, withdrawProposal } from '../api'
export default function ProposalView({ viewer, onChange }) {
@@ -161,7 +161,7 @@ export default function ProposalView({ viewer, onChange }) {
</h3>
<div
className="entry-body"
dangerouslySetInnerHTML={{ __html: marked.parse(data.entry?.body || '') }}
dangerouslySetInnerHTML={{ __html: renderMarkdown(data.entry?.body || '') }}
/>
{/* #26: the optional ground-truth use case the proposer supplied. */}
@@ -169,7 +169,7 @@ export default function ProposalView({ viewer, onChange }) {
Intended use case
</h3>
{data.proposed_use_case
? <div className="entry-body" dangerouslySetInnerHTML={{ __html: marked.parse(data.proposed_use_case) }} />
? <div className="entry-body" dangerouslySetInnerHTML={{ __html: renderMarkdown(data.proposed_use_case) }} />
: <p style={{ color: '#999', fontStyle: 'italic' }}>Left blank by the proposer.</p>}
</article>
)
+47
View File
@@ -0,0 +1,47 @@
// sanitizeHtml.js — the single chokepoint for turning user-authored
// markdown into DOM-bound HTML.
//
// Security audit 0026 (finding C1, Critical): every `marked.parse(...)`
// result that reaches an `innerHTML` / `dangerouslySetInnerHTML` sink was
// previously written raw. `marked` passes through embedded HTML and
// `javascript:`/event-handler attributes verbatim, so any user-authored
// document (RFC body, proposal body, proposed_use_case, transcript) was a
// stored-XSS vector — a contributor's payload executed in the session of
// whoever viewed it, including an admin/owner during review.
//
// Fix: route EVERY markdown render through `renderMarkdown` (or, for
// already-rendered HTML, `sanitizeHtml`). DOMPurify's defaults already
// strip <script>, on* event handlers, and javascript:/unsafe-data: URIs;
// we add a hook so any link opening a new tab carries rel="noopener
// noreferrer". The html profile keeps the standard markdown tag set plus
// class + data-* attributes (the latter is what MarkdownPreview's mermaid
// placeholder relies on); mermaid renders its SVG into the DOM *after*
// sanitization and is itself locked down with securityLevel:'strict'.
import DOMPurify from 'dompurify'
import { marked } from 'marked'
let _hookInstalled = false
function ensureHook() {
if (_hookInstalled) return
DOMPurify.addHook('afterSanitizeAttributes', (node) => {
if (node.tagName === 'A' && node.getAttribute('target') === '_blank') {
node.setAttribute('rel', 'noopener noreferrer')
}
})
_hookInstalled = true
}
// Sanitize an already-rendered HTML string. Use when the HTML did not come
// from `marked` (rare) or when a caller parses markdown with a bespoke
// `Marked` instance and only needs the sanitize step.
export function sanitizeHtml(html) {
ensureHook()
return DOMPurify.sanitize(html || '', { USE_PROFILES: { html: true } })
}
// Parse markdown with the shared `marked` and sanitize the result. This is
// the drop-in replacement for `marked.parse(src)` at any HTML sink.
export function renderMarkdown(src) {
return sanitizeHtml(marked.parse(src || ''))
}