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 release's steps in order — no A-to-B path is pre-computed beyond
that. 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 ## 0.26.0 — 2026-05-28
**Minor — no schema migration, no new secret, no config, no upgrade **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). # stays unauthenticated for dev (the v1 contract).
import os as _os import os as _os
expected = _os.environ.get("WEBHOOK_EMAIL_BOUNCE_SECRET", "").strip() 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", "") received = request.headers.get("X-Webhook-Secret", "")
import hmac as _hmac import hmac as _hmac
if not received or not _hmac.compare_digest(expected, received): if not received or not _hmac.compare_digest(expected, received):
+32 -21
View File
@@ -97,6 +97,13 @@ class IssueOutcome:
raw_token: str raw_token: str
row_id: int 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: def _new_token() -> str:
return secrets.token_urlsafe(TOKEN_BYTES) return secrets.token_urlsafe(TOKEN_BYTES)
@@ -173,33 +180,37 @@ def lookup(raw_token: str) -> LookupOutcome:
if not raw: if not raw:
return LookupOutcome(ok=False, user=None, reason="invalid") return LookupOutcome(ok=False, user=None, reason="invalid")
# The unique index on `device_token_hash` would let us SELECT by # v0.25.0 (audit 0026 M1): the cookie is "<row_id>.<raw_token>". We
# hash if bcrypt were a stable hash, but bcrypt incorporates a # parse the row-id selector and read exactly ONE row by its indexed
# per-row salt — equal tokens produce different hashes. We walk # primary key, then bcrypt-check the token against that single row.
# 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.
# #
# We don't pre-filter by `revoked_at IS NULL` here so that a # The previous shape read EVERY device_trust row (all users, including
# token presented for a recently-revoked row produces a # revoked/expired) and bcrypt-checked each — an unauthenticated
# 'revoked' outcome (the endpoint surfaces a different shape). # CPU-amplification DoS reachable at /auth/device-trust/start that
# Same for expired: we let the walk hit and classify after. # grew without bound as the table accumulated. bcrypt's per-row salt
rows = db.conn().execute( # 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 SELECT id, user_id, device_token_hash, expires_at, revoked_at
FROM device_trust FROM device_trust
ORDER BY id DESC WHERE id = ?
""", """,
).fetchall() (int(selector),),
).fetchone()
matched = None # One bcrypt check, against the selected row only. A wrong/forged token
for row in rows: # for a real id reads as 'unknown' (cookie cleared), same as a missing
if _check(raw, row["device_token_hash"]): # row — a probing client can't distinguish the two.
matched = row if matched is None or not _check(token, matched["device_token_hash"]):
break
if matched is None:
return LookupOutcome(ok=False, user=None, reason="unknown") return LookupOutcome(ok=False, user=None, reason="unknown")
if matched["revoked_at"] is not None: if matched["revoked_at"] is not None:
+26 -14
View File
@@ -60,12 +60,17 @@ def build_envelope(
`from_name` is the display label that goes through `formataddr` `from_name` is the display label that goes through `formataddr`
so spaces / commas in the display string are encoded correctly. 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 `reply_to`, when set, lets a send path point replies at a
different mailbox than the From line (e.g., a watcher different mailbox than the From line (e.g., a watcher
@@ -131,13 +136,20 @@ def build_envelope(
# idempotent and not require auth. See # idempotent and not require auth. See
# `api_notifications.py` for the receiver. # `api_notifications.py` for the receiver.
msg["List-Unsubscribe-Post"] = "List-Unsubscribe=One-Click" 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) msg.set_content(body_plain)
return msg return msg
+61 -19
View File
@@ -7,6 +7,7 @@ no need for a separate worker.
from __future__ import annotations from __future__ import annotations
import logging import logging
import os
import secrets import secrets
from contextlib import asynccontextmanager from contextlib import asynccontextmanager
@@ -28,6 +29,7 @@ from . import (
otc, otc,
passcode as passcode_mod, passcode as passcode_mod,
providers as providers_mod, providers as providers_mod,
ratelimit,
turnstile, turnstile,
webhooks, webhooks,
) )
@@ -142,12 +144,20 @@ def create_app() -> FastAPI:
# eagerly via load_config(). Everything else waits for lifespan. # eagerly via load_config(). Everything else waits for lifespan.
config = load_config() config = load_config()
app = FastAPI(lifespan=lifespan) 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( app.add_middleware(
SessionMiddleware, SessionMiddleware,
secret_key=config.secret_key, secret_key=config.secret_key,
session_cookie="rfc_session", session_cookie="rfc_session",
max_age=60 * 60 * 24 * 30, max_age=60 * 60 * 24 * 30,
https_only=False, https_only=session_secure,
) )
return app return app
@@ -155,24 +165,25 @@ def create_app() -> FastAPI:
app = create_app() 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. """Attach the v0.11.0 device-trust cookie to the response.
HttpOnly + Secure + SameSite=Lax + 30-day Max-Age + Path=/. The HttpOnly + Secure + SameSite=Lax + 30-day Max-Age + Path=/. As of
cookie value is the raw token; server-side storage is the hash. v0.25.0 (audit 0026 M1) the value is `IssueOutcome.cookie_value`
The cookie is "essential" per the v0.13.0 cookie-consent contract "<row_id>.<raw_token>" so `device_trust.lookup` can read one indexed
(it is part of authentication), so we set it regardless of the row instead of scanning; server-side storage remains the bcrypt hash
user's analytics / other-cookies choice. 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 Secure=True means the cookie is only ever sent over HTTPS the
SessionMiddleware in `create_app` keeps `https_only=False` for device-trust cookie holds a 30-day credential and must never travel
dev parity, but the device-trust cookie holds a 30-day credential cleartext. (The session cookie now also defaults to Secure; see M4 in
and must not travel cleartext production deployments serve over `create_app`.)
HTTPS, so Secure on the device-trust cookie is non-negotiable.
""" """
response.set_cookie( response.set_cookie(
key=device_trust_mod.COOKIE_NAME, key=device_trust_mod.COOKIE_NAME,
value=raw_token, value=cookie_value,
max_age=device_trust_mod.COOKIE_MAX_AGE_SECONDS, max_age=device_trust_mod.COOKIE_MAX_AGE_SECONDS,
path="/", path="/",
secure=True, secure=True,
@@ -245,6 +256,10 @@ def _oauth_router(config) -> APIRouter:
@router.post("/auth/otc/request") @router.post("/auth/otc/request")
async def otc_request(body: OtcRequestBody, request: 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 # v0.12.0 / roadmap item #10: gate the request on a successful
# Turnstile siteverify before the bcrypt hash + SMTP send. The # Turnstile siteverify before the bcrypt hash + SMTP send. The
# check runs first so a failed challenge spends no rate budget # 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 # secret AND TURNSTILE_REQUIRED=false (the default), the gate
# opens — see `backend/app/turnstile.py` for the full matrix. # opens — see `backend/app/turnstile.py` for the full matrix.
client_ip = request.client.host if request.client else None 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 not ts.ok:
if ts.reason == "misconfigured": if ts.reason == "misconfigured":
# TURNSTILE_REQUIRED=true but the secret is unset. This # TURNSTILE_REQUIRED=true but the secret is unset. This
@@ -278,9 +293,25 @@ def _oauth_router(config) -> APIRouter:
@router.post("/auth/otc/verify") @router.post("/auth/otc/verify")
async def otc_verify(body: OtcVerifyBody, request: Request, response: Response): 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) 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: if not result.ok or result.user is None:
raise HTTPException(400, "Invalid or expired code") 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) auth.store_session(request, result.user)
# v0.8.0: surface `needs_profile` so the Login.jsx surface can # v0.8.0: surface `needs_profile` so the Login.jsx surface can
# decide whether to advance to the first/last/why capture step # decide whether to advance to the first/last/why capture step
@@ -313,7 +344,7 @@ def _oauth_router(config) -> APIRouter:
if body.trust_device: if body.trust_device:
ua = request.headers.get("user-agent", "") ua = request.headers.get("user-agent", "")
outcome = device_trust_mod.issue(result.user.user_id, ua) 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 { return {
"ok": True, "ok": True,
"user": { "user": {
@@ -337,12 +368,17 @@ def _oauth_router(config) -> APIRouter:
# --------------------------------------------------------------- # ---------------------------------------------------------------
@router.get("/auth/passcode/check") @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 — """Does this email have a passcode set? Anonymous endpoint —
the Login.jsx flow calls this after the user types their email the Login.jsx flow calls this after the user types their email
to decide whether to render a passcode input or fall back to to decide whether to render a passcode input or fall back to
OTC. We surface only the boolean; lockout state, the hash, and 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) status = passcode_mod.passcode_status(email)
return {"has_passcode": status.has_passcode} 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 v0.11.0: the body's `trust_device` flag, if true, mints a
fresh device-trust row and sets the long-lived cookie. Same fresh device-trust row and sets the long-lived cookie. Same
opt-in contract as `/auth/otc/verify`.""" 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) result = passcode_mod.verify_passcode(body.email, body.passcode)
if result.reason == "locked": if result.reason == "locked":
raise HTTPException( raise HTTPException(
@@ -387,11 +428,12 @@ def _oauth_router(config) -> APIRouter:
) )
if not result.ok or result.user is None: if not result.ok or result.user is None:
raise HTTPException(400, "Invalid passcode") raise HTTPException(400, "Invalid passcode")
ratelimit.verify_limiter.reset(ip)
auth.store_session(request, result.user) auth.store_session(request, result.user)
if body.trust_device: if body.trust_device:
ua = request.headers.get("user-agent", "") ua = request.headers.get("user-agent", "")
outcome = device_trust_mod.issue(result.user.user_id, ua) 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 { return {
"ok": True, "ok": True,
"user": { "user": {
@@ -459,7 +501,7 @@ def _oauth_router(config) -> APIRouter:
if body.trust_device: if body.trust_device:
ua = request.headers.get("user-agent", "") ua = request.headers.get("user-agent", "")
outcome = device_trust_mod.issue(result.user.user_id, ua) 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 # Has the user already set a passcode? (Could only happen via
# an admin pre-population path that doesn't exist yet, but # an admin pre-population path that doesn't exist yet, but
+76
View File
@@ -85,6 +85,15 @@ def _cooldown_seconds() -> int:
return 60 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 # Code generation + hashing
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
@@ -206,6 +215,61 @@ class VerifyOutcome:
ok: bool ok: bool
user: SessionUser | None user: SessionUser | None
reason: str 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: 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: if not email or not code:
return VerifyOutcome(ok=False, user=None, reason="invalid") 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( rows = db.conn().execute(
""" """
SELECT id, code_hash, expires_at, consumed_at SELECT id, code_hash, expires_at, consumed_at
@@ -237,6 +308,9 @@ def verify_code(email: str, code: str) -> VerifyOutcome:
break break
if matched is None: 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") return VerifyOutcome(ok=False, user=None, reason="wrong")
if matched["consumed_at"] is not None: 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 = ?", "UPDATE otc_codes SET consumed_at = datetime('now') WHERE id = ?",
(matched["id"],), (matched["id"],),
) )
# Success wipes the per-email failure counter (H1).
_clear_verify_state(email)
user = provision_or_link_user(email) user = provision_or_link_user(email)
return VerifyOutcome(ok=True, user=user, reason="ok") 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 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 @dataclass
class VerifyOutcome: class VerifyOutcome:
"""Result of a Turnstile siteverify call. """Result of a Turnstile siteverify call.
@@ -98,7 +111,7 @@ class VerifyOutcome:
reason: str 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. """Validate a Turnstile token against CloudFlare's siteverify endpoint.
Returns a VerifyOutcome describing whether the calling 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" * 'misconfigured' 500 "auth misconfigured"
* 'missing-token' / 'failed' / 'network' 400 "verification failed" * '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() secret = _secret()
required = _required() required = _required()
@@ -133,7 +151,7 @@ def verify_token(token: str | None, *, client_ip: str | None = None) -> VerifyOu
data["remoteip"] = client_ip data["remoteip"] = client_ip
try: try:
response = httpx.post(_siteverify_url(), data=data, timeout=10.0) response = await _siteverify_post(_siteverify_url(), data)
payload = response.json() payload = response.json()
except Exception as exc: # network, JSON parse, etc. except Exception as exc: # network, JSON parse, etc.
log.warning("Turnstile siteverify call failed: %s", exc) 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 from email.utils import parsedate_to_datetime
import pytest
from app.email_envelope import build_envelope 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." 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"
+12
View File
@@ -444,6 +444,18 @@ def tmp_env(monkeypatch):
# the dev-bypass path monkeypatch `RFC_APP_INSECURE_WEBHOOKS=1`. # the dev-bypass path monkeypatch `RFC_APP_INSECURE_WEBHOOKS=1`.
"GITEA_WEBHOOK_SECRET": "test-webhook-secret-for-signature-verification", "GITEA_WEBHOOK_SECRET": "test-webhook-secret-for-signature-verification",
"ENABLED_MODELS": "claude", "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(): for k, v in env.items():
monkeypatch.setenv(k, v) 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): 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 returns the requested success shape. The stub does not touch the
real CloudFlare endpoint and never sees a real secret. 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 = {} captured = {}
def fake_post(url, *, data=None, timeout=None, **kwargs): async def fake_post(url, data):
captured["url"] = url captured["url"] = url
captured["data"] = data captured["data"] = data
body = {"success": bool(success)} body = {"success": bool(success)}
@@ -70,7 +76,7 @@ def _patch_siteverify(monkeypatch, *, success: bool, error_codes: list[str] | No
return SimpleNamespace(json=lambda: body) return SimpleNamespace(json=lambda: body)
from app import turnstile as turnstile_mod 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 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("CLOUDFLARE_TURNSTILE_SECRET", raising=False)
monkeypatch.delenv("TURNSTILE_REQUIRED", 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 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") 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 app, _fake = app_with_fake_gitea
with TestClient(app) as client: 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 r.status_code == 500, r.text
assert _outbound_otc_envelopes("alice@example.com") == [] 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; listen [::]:80;
server_name ohm.wiggleverse.org; 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 # Static SPA assets live in the Vite build output. The systemd unit
# runs as user `rfc-app`; make sure nginx (usually `www-data`) can # 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 # 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 PrivateTmp=true
ReadWritePaths=/opt/rfc-app/backend/data 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] [Install]
WantedBy=multi-user.target WantedBy=multi-user.target
+3 -2
View File
@@ -1,12 +1,12 @@
{ {
"name": "rfc-app-frontend", "name": "rfc-app-frontend",
"version": "0.21.0", "version": "0.24.0",
"lockfileVersion": 3, "lockfileVersion": 3,
"requires": true, "requires": true,
"packages": { "packages": {
"": { "": {
"name": "rfc-app-frontend", "name": "rfc-app-frontend",
"version": "0.21.0", "version": "0.24.0",
"dependencies": { "dependencies": {
"@amplitude/unified": "^1.1.9", "@amplitude/unified": "^1.1.9",
"@codemirror/commands": "^6.10.3", "@codemirror/commands": "^6.10.3",
@@ -18,6 +18,7 @@
"@tiptap/pm": "^3.5.0", "@tiptap/pm": "^3.5.0",
"@tiptap/react": "^3.5.0", "@tiptap/react": "^3.5.0",
"@tiptap/starter-kit": "^3.5.0", "@tiptap/starter-kit": "^3.5.0",
"dompurify": "^3.2.4",
"marked": "^18.0.4", "marked": "^18.0.4",
"mermaid": "^11.15.0", "mermaid": "^11.15.0",
"react": "^19.2.6", "react": "^19.2.6",
+2 -1
View File
@@ -1,7 +1,7 @@
{ {
"name": "rfc-app-frontend", "name": "rfc-app-frontend",
"private": true, "private": true,
"version": "0.26.0", "version": "0.28.0",
"type": "module", "type": "module",
"scripts": { "scripts": {
"dev": "vite", "dev": "vite",
@@ -19,6 +19,7 @@
"@tiptap/pm": "^3.5.0", "@tiptap/pm": "^3.5.0",
"@tiptap/react": "^3.5.0", "@tiptap/react": "^3.5.0",
"@tiptap/starter-kit": "^3.5.0", "@tiptap/starter-kit": "^3.5.0",
"dompurify": "^3.2.4",
"marked": "^18.0.4", "marked": "^18.0.4",
"mermaid": "^11.15.0", "mermaid": "^11.15.0",
"react": "^19.2.6", "react": "^19.2.6",
+2 -2
View File
@@ -17,7 +17,7 @@
import { useEditor, EditorContent, Extension } from '@tiptap/react' import { useEditor, EditorContent, Extension } from '@tiptap/react'
import StarterKit from '@tiptap/starter-kit' import StarterKit from '@tiptap/starter-kit'
import { useEffect, useRef, useCallback } from 'react' import { useEffect, useRef, useCallback } from 'react'
import { marked } from 'marked' import { renderMarkdown } from '../lib/sanitizeHtml'
import { Plugin, PluginKey } from 'prosemirror-state' import { Plugin, PluginKey } from 'prosemirror-state'
import { Decoration, DecorationSet } from 'prosemirror-view' import { Decoration, DecorationSet } from 'prosemirror-view'
@@ -122,7 +122,7 @@ export default function Editor({
useEffect(() => { useEffect(() => {
if (!editor || content == null) return if (!editor || content == null) return
const html = marked.parse(content) const html = renderMarkdown(content)
editor.commands.setContent(html, false) editor.commands.setContent(html, false)
}, [content, editor]) }, [content, editor])
+2 -1
View File
@@ -21,6 +21,7 @@
import { useEffect, useRef, useState, useCallback } from 'react' import { useEffect, useRef, useState, useCallback } from 'react'
import { Marked } from 'marked' import { Marked } from 'marked'
import { sanitizeHtml } from '../lib/sanitizeHtml'
import { decorateAcceptedChanges } from './trackedOverlay.js' import { decorateAcceptedChanges } from './trackedOverlay.js'
import ChangeTooltip from './ChangeTooltip.jsx' import ChangeTooltip from './ChangeTooltip.jsx'
@@ -98,7 +99,7 @@ export default function MarkdownPreview({
// synchronously with the body itself no flash of un-decorated text. // synchronously with the body itself no flash of un-decorated text.
useEffect(() => { useEffect(() => {
if (!hostRef.current) return if (!hostRef.current) return
const html = previewMarked.parse(content || '') const html = sanitizeHtml(previewMarked.parse(content || ''))
hostRef.current.innerHTML = html hostRef.current.innerHTML = html
const token = ++renderTokenRef.current const token = ++renderTokenRef.current
// Reset memo so the new block set re-renders from scratch. // 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 { useEffect, useState } from 'react'
import { useParams, useNavigate } from 'react-router-dom' import { useParams, useNavigate } from 'react-router-dom'
import { marked } from 'marked' import { renderMarkdown } from '../lib/sanitizeHtml'
import { getProposal, mergeProposal, declineProposal, withdrawProposal } from '../api' import { getProposal, mergeProposal, declineProposal, withdrawProposal } from '../api'
export default function ProposalView({ viewer, onChange }) { export default function ProposalView({ viewer, onChange }) {
@@ -161,7 +161,7 @@ export default function ProposalView({ viewer, onChange }) {
</h3> </h3>
<div <div
className="entry-body" 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. */} {/* #26: the optional ground-truth use case the proposer supplied. */}
@@ -169,7 +169,7 @@ export default function ProposalView({ viewer, onChange }) {
Intended use case Intended use case
</h3> </h3>
{data.proposed_use_case {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>} : <p style={{ color: '#999', fontStyle: 'italic' }}>Left blank by the proposer.</p>}
</article> </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 || ''))
}