9e6756f678
Backfills transcripts that existed only locally in ohm-infra: - 0019.1/.2/.3 - UX-polish wave subagent transcripts (driver 0019.0 never finalized) - 0026.1-.9 - security-audit-0026 subagent transcripts (driver abandoned/closed-out by 0068; audit drove published 0030 remediation) - 0073.0 - PPE/progressive-delivery + engineering-handbook session, originally drafted as a duplicate 0046.0; reassigned next free number (0072 taken by a concurrent session) sessions.json: add 0019/0073 titles, update 0026 title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
173 lines
9.8 KiB
Markdown
173 lines
9.8 KiB
Markdown
# SESSION-0026.4 — OHM security-audit subsession (newest endpoints, rate-limiting/abuse, frontend posture)
|
|
|
|
- **Role:** subsession 0026.4 of OHM security-audit driver session 0026.
|
|
- **Parent transcript:** SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
|
- **Mode:** READ-ONLY. No mutations. No secret bytes printed.
|
|
- **Target:** rfc-app @ /Users/benstull/git/rfc-app, tag v0.24.0 (HEAD 28015ed).
|
|
- **Window:** 2026-05-28T13-46 → 13-50 (America/Los_Angeles).
|
|
|
|
## Surface assigned
|
|
Newest endpoints from session 0022 work (proposed_use_cases writes #26,
|
|
PUT /api/me/last-state #29, Claude Haiku tag suggestions #27); rate
|
|
limiting / abuse on OTC + beta-request + suggest-tags + propose; frontend
|
|
token/secret handling, VITE_ public-vs-secret discipline, token storage,
|
|
CSP/security headers, npm audit.
|
|
|
|
## Pre-state
|
|
Fresh read-only checkout at v0.24.0. No prior findings in this subsession.
|
|
|
|
## Turn-by-turn arc
|
|
|
|
1. Enumerated backend/app + frontend/src layout, confirmed v0.24.0 tag.
|
|
2. Read tag_suggest.py in full, useLastState.js, main.py (full).
|
|
- tag_suggest: output strictly constrained to the corpus tag universe
|
|
(parse_reply maps replies back to canonical spellings, drops invented
|
|
tags). Prompt-injection into the Haiku call can't escape the tag set;
|
|
worst case is a junk/empty suggestion list. Per-user in-process rate
|
|
limit (TAG_SUGGEST_RATE_MAX default 30/60s) + require_contributor gate
|
|
+ draft truncation (title 300, pitch/use_case 4000) + universe cap 200
|
|
bound the LLM cost surface. SOLID.
|
|
3. Located + read the #26 use_case writes (api.py propose, api_prs.py
|
|
open-pr), the #29 last-state route, the suggest-tags route.
|
|
- last-state: keyed on user.user_id from the session in BOTH read
|
|
(/api/auth/me) and write (PUT /api/me/last-state). No user-supplied
|
|
id anywhere → no cross-user read/write (IDOR). require_user gate.
|
|
resume_enabled opt-out respected. route bounded 2048, state is a
|
|
dict serialized as JSON. SOLID on authz.
|
|
4. Checked propose + open-pr authz: both require_contributor (granted,
|
|
non-muted, authenticated). use_case bounded max_length=8000. SOLID.
|
|
5. Frontend rendering of proposed_use_case: grep found
|
|
ProposalView.jsx:172 renders it via marked.parse() into
|
|
dangerouslySetInnerHTML. No DOMPurify anywhere in the project
|
|
(confirmed by grep across frontend/src + package.json).
|
|
6. KILLED a refutation attempt: verified marked default does NOT sanitize.
|
|
Ran node one-liner: marked.parse('<img src=x onerror=alert(1)>') emits
|
|
the raw <img onerror> tag, and [a](javascript:alert(2)) emits a live
|
|
javascript: href. So this is a real stored-XSS sink, not theoretical.
|
|
7. Confirmed the XSS audience: GET /api/proposals/{pr_number} is
|
|
ANONYMOUS-readable (no require_ gate; uses current_user only for
|
|
affordances). Admins reviewing a proposal (who hold merge/decline)
|
|
render the payload → privileged-victim XSS.
|
|
8. Frontend token storage: grep for localStorage/sessionStorage/
|
|
document.cookie/Authorization/Bearer in api.js → ZERO hits. Auth is
|
|
server cookie-session (credentials:'include'); device-trust + session
|
|
cookies are HttpOnly+Secure+SameSite=Lax (main.py). No token in JS
|
|
reach → XSS can't exfil an auth token directly. SOLID.
|
|
9. VITE_ discipline: all VITE_ vars are genuinely public — APP_NAME,
|
|
BETA_CONTACT, TURNSTILE_SITE_KEY (public by design), AMPLITUDE_API_KEY
|
|
(Amplitude browser ingestion key, public by design; .env.example
|
|
explicitly documents it as `flotilla overlay set`, NOT secret), policy
|
|
URLs. No secret shipped as a VITE_ var. SOLID.
|
|
10. npm audit (frontend): 0 vulnerabilities (info/low/moderate/high/
|
|
critical all 0). SOLID.
|
|
11. OTC abuse: request_code has a per-email cooldown
|
|
(OTC_REQUEST_COOLDOWN_SECONDS default 60) AND the route runs
|
|
turnstile.verify_token BEFORE request_code (so a failed challenge
|
|
spends no budget / sends no mail). Email-bomb + LLM-cost surfaces
|
|
mitigated. Cooldown is per-email not per-IP, but Turnstile covers the
|
|
bot/IP dimension. SOLID with one caveat (below).
|
|
12. beta-request: require_user gated (not anon), admin notification fires
|
|
once via the was_already_complete guard. Not anon-abusable.
|
|
13. Security headers: app has only SessionMiddleware — no CSP / X-Frame /
|
|
X-Content-Type / HSTS. nginx conf (deploy/nginx/ohm.wiggleverse.org.conf)
|
|
sets only Cache-Control — NO security headers either. So nothing
|
|
backstops the XSS sinks at the header layer.
|
|
|
|
## Cut state — findings
|
|
|
|
### F1 — Stored XSS via proposed_use_case rendered through marked + dangerouslySetInnerHTML (no sanitizer)
|
|
- **Location:** frontend/src/components/ProposalView.jsx:172 (sink);
|
|
source written at backend/app/api.py:841-854 (#26 propose) and
|
|
backend/app/api_prs.py:186-199 (#26 open-pr); served unsanitized via
|
|
GET /api/proposals/{pr_number} (api.py:716, anon-readable);
|
|
proposed_use_case max_length=8000.
|
|
- **Issue:** marked does not sanitize by default (verified live: raw
|
|
`<img onerror>` and `javascript:` hrefs pass through), and the project
|
|
has NO DOMPurify. A granted contributor's use_case text renders as
|
|
live HTML in any viewer's browser, including admins who hold
|
|
merge/decline on that proposal.
|
|
- **Exploit:** contributor proposes an RFC with
|
|
`<img src=x onerror=...>` in the use case; an admin opens the proposal
|
|
to review it; script runs in the admin's authenticated session
|
|
(CSRF-style writes via the admin's cookie, UI defacement). Auth-token
|
|
exfil is blocked (HttpOnly cookies), but cookie-authenticated state-
|
|
changing requests from the victim's browser are not.
|
|
- **Remediation:** sanitize all marked output with DOMPurify before
|
|
dangerouslySetInnerHTML (or configure a safe renderer), at minimum on
|
|
the new proposed_use_case sink. Same pattern should cover the
|
|
pre-existing entry-body sink (F2).
|
|
- **Severity:** High. **Confidence:** High (live-verified marked
|
|
passthrough; confirmed no sanitizer; confirmed privileged victim).
|
|
|
|
### F2 — Pre-existing same-sink XSS on entry body (in-scope by adjacency)
|
|
- **Location:** ProposalView.jsx:164 (also Editor.jsx:125 preview path);
|
|
marked.parse(data.entry?.body) → dangerouslySetInnerHTML.
|
|
- **Issue:** identical unsanitized sink; the proposed entry body is
|
|
proposer-controlled markdown. Predates v0.24.0 but shares the root
|
|
cause and remediation with F1.
|
|
- **Severity:** High. **Confidence:** High. (Flagged as the root-cause
|
|
class so a fix covers both, not just the #26 field.)
|
|
|
|
### F3 — No security headers at app or nginx layer (defense-in-depth gap)
|
|
- **Location:** backend/app/main.py (only SessionMiddleware);
|
|
deploy/nginx/ohm.wiggleverse.org.conf (only Cache-Control).
|
|
- **Issue:** no Content-Security-Policy, X-Frame-Options/frame-ancestors,
|
|
X-Content-Type-Options, or HSTS. A CSP would meaningfully blunt F1/F2
|
|
(block inline/onerror script execution). Absence means the XSS sinks
|
|
have no header-layer backstop, and the app is clickjackable.
|
|
- **Remediation:** add CSP (script-src self, no unsafe-inline),
|
|
X-Frame-Options DENY / frame-ancestors none, X-Content-Type-Options
|
|
nosniff, HSTS — at the nginx layer is simplest.
|
|
- **Severity:** Medium. **Confidence:** High.
|
|
|
|
### F4 (minor / needs-verification) — OTC cooldown is per-email, not per-IP
|
|
- **Location:** backend/app/otc.py:152-164.
|
|
- **Issue:** the cooldown keys on email, so one attacker can rotate
|
|
through many target emails (mail-bomb across recipients) without
|
|
tripping a single email's cooldown. Mitigated by the mandatory
|
|
Turnstile gate on /auth/otc/request (main.py:255) when TURNSTILE is
|
|
wired; if TURNSTILE_REQUIRED=false and no secret is set (the default
|
|
dev posture), the gate opens and only the per-email cooldown remains.
|
|
- **Remediation:** keep Turnstile required in production; consider an
|
|
IP-scoped request ceiling as belt-and-suspenders.
|
|
- **Severity:** Low (prod-config-dependent). **Confidence:** Medium —
|
|
hinges on the deployment's TURNSTILE_REQUIRED + secret state, which
|
|
this subsession did not inspect (operator/§13 record, not code).
|
|
|
|
## What's solid
|
|
- last-state #29: no IDOR (server-session user_id only), no open-redirect
|
|
(resume only navigates client-side react-router routes from "/", with a
|
|
NON_RESUMABLE_PREFIXES skip list; no external URL navigation), state
|
|
blob is bounded + JSON-parsed defensively. Resume redirect is
|
|
one-shot. SOLID.
|
|
- tag_suggest #27: prompt-injection contained (output constrained to
|
|
canonical tag universe), cost bounded (per-user rate limit + input
|
|
truncation + universe cap + require_contributor gate), degrades to
|
|
silence on any failure. SOLID.
|
|
- Frontend secrets: no auth token in JS-reachable storage; cookies are
|
|
HttpOnly+Secure+SameSite=Lax; no secret shipped as a VITE_ var
|
|
(AMPLITUDE/TURNSTILE keys are public by design). SOLID.
|
|
- npm audit: 0 advisories. SOLID.
|
|
- OTC/beta-request authz + cost: Turnstile-gated OTC with per-email
|
|
cooldown; beta-request require_user + once-only admin notify. SOLID
|
|
(modulo F4 caveat).
|
|
|
|
## Driver-needs-to-know
|
|
- F1 is the headline: a real, live-verified stored XSS in the #26
|
|
use-case field that can land on an admin reviewer. F2 is the same sink
|
|
on the entry body — fix the sanitizer once, cover both. F3 (no CSP)
|
|
removes the only header-layer backstop. Recommend a single remediation
|
|
wave: add DOMPurify around every marked→dangerouslySetInnerHTML sink +
|
|
add a CSP at nginx.
|
|
- F4 depends on the OHM deployment's TURNSTILE_REQUIRED + secret state
|
|
(a §13 record / operator question, not in this code), so verify that
|
|
before downgrading or upgrading it.
|
|
|
|
## §19.2 candidates (flotilla-core / cross-cutting)
|
|
- A shared "render-trusted-markdown" frontend helper (marked + DOMPurify
|
|
in one place) so no future sink reintroduces F1. Not flotilla-core
|
|
(it's rfc-app frontend), but a cross-cutting rfc-app refactor worth a
|
|
roadmap item.
|
|
- A security-headers block as a reusable nginx snippet shipped with the
|
|
deploy/ templates — could become a flotilla overlay default.
|