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>
11 KiB
SESSION-0026.1 — AuthN/AuthZ security audit subsession (rfc-app v0.24.0)
Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md Date: 2026-05-28T13-46 → 2026-05-28T13-50 (America/Los_Angeles) Role: subsession 0026.1 of OHM security-audit driver session 0026. Mode: READ-ONLY. No edits, no deploys, no mutations. Only file written is this transcript.
Goal
Audit the authentication / authorization surface of the rfc-app codebase
(/Users/benstull/git/rfc-app, tag v0.24.0): the email/OTC login flow,
passcodes, device-trust, Gitea OAuth + core auth, session/cookie security,
the admin role model, and per-endpoint authz guards across every
api*.py write/admin/owner endpoint. Verify deployment guarantee #4
("anonymous users are off-limits for any write/mutation"). Hunt for missing
guards, IDOR, and OTC/passcode brute-force/replay.
Outcome
One High finding (OTC verify has no attempt limit / lockout — asymmetric
with the passcode path which does). Two Low/Info findings (session cookie
https_only=False; email-bounce webhook unauthenticated when env unset —
both documented dev contracts). Every enumerated write/admin/owner endpoint
enforces auth; no missing guards, no IDOR found. Admin role model is sound.
Pre-state (what I found at the start)
auth.pydefines the dependency ladder:require_user(401 if anon) →require_contributor(adds muted + permission_state='granted' gate) →require_admin(role in owner/admin). Plus per-RFC predicates (can_contribute_to_rfc,can_discuss_rfc,can_invite_to_rfc,is_rfc_owner).current_userre-reads role + permission_state from DB every request (so grants/revokes take effect next request — good).- Credentials: OTC 6-digit codes (bcrypt at rest, 10-min TTL, 60s request cooldown, single live code per email); passcodes (bcrypt, 5-fail/15-min per-account lockout, denylist, 4–20 chars); device-trust (256-bit CSPRNG, bcrypt, 30-day, HttpOnly+Secure+SameSite=Lax cookie); invite tokens (256-bit CSPRNG, bcrypt, 7-day, single-use).
Turn-by-turn arc
-
Read auth.py + config.py. Dependency ladder is clean. Noted SECRET_KEY is
_required(good — no insecure default). OAuth provisioning sets owner role only via OWNER_GITEA_LOGIN match; OTC/invite users get gitea_login="" / gitea_id=0 coalesced. -
Read otc.py + passcode.py side by side. Caught the asymmetry:
passcode.verify_passcodehas a per-account failed-attempts counter + 15-min lockout;otc.verify_codehas NOTHING — it walks the last 5 rows, bcrypt-checks, and returns "wrong" with no counter, no lockout, no per-IP throttle. The 60s cooldown lives inrequest_code, notverify_code. This is the headline finding. -
Read device_trust.py + email_otc.py. Device-trust is solid (constant-time bcrypt walk, no early-exit, token never logged, cookie flags correct, user-scoped revoke). email_otc always returns 202 / logs failures — no email enumeration via the request path.
-
Read main.py (middleware + all /auth endpoints). Confirmed:
- SessionMiddleware uses
https_only=False(line 150) — itsdangerous- signed cookie, HttpOnly by default, but Secure flag off. Dev parity comment present. Behind HTTPS+proxy in prod this is low-impact but worth flagging. /auth/otc/verifyhas NO rate-limit dependency, confirming finding #1 at the endpoint layer too.- Turnstile gates
/auth/otc/requestONLY, not verify.
- SessionMiddleware uses
-
Refutation pass on finding #1: Is there a global limiter? Grepped for RateLimit/slowapi/limiter/429 across backend/app — only tag_suggest.py (per-user, in-memory) and the OTC request cooldown 429. No global middleware, no limiter on verify. SPEC §ahead-of (line ~4100) discusses per-IP throttle for the passcode verify (which already has a per- account lockout) but never notes the OTC verify has no per-account limit at all. Quantified: single live code per email, bcrypt ~tens of ms, 10^6 keyspace, 10-min TTL. Full sweep not possible in one TTL, but attacker re-requests (60s cooldown) to keep a live code and runs parallel verifies; expected ~500k guesses. bcrypt cost + TTL + single- live-code = real friction but NO attempt cap. Downgraded Critical→High.
-
Read api_admin.py end to end. Every endpoint calls require_admin; the user-search typeahead intentionally uses require_user (no privileged data). Role escalation well-guarded: only owner grants/changes owner; self-downgrade, self-mute, self-permission-flip, self-invite all refused; admins/owners not write-mutable. No spoof/escalation path. SOLID.
-
Enumerated routes+guards across api.py, api_prs.py, api_branches.py, api_graduation.py, api_invitations.py, api_notifications.py, api_discussion.py via grep. Every write/mutation route has require_user / require_contributor / require_admin, and the per-RFC write routes additionally call can_contribute_to_rfc / can_discuss_rfc / can_invite_to_rfc. GET read routes use current_user (nullable) — anon read is by design (guarantee #4 is about writes).
-
IDOR hunt — checked every owner/id-comparing helper:
- api_prs
_can_merge/_can_withdraw/_can_edit_pr_text: compare viewer.gitea_login against RFC frontmatter owners/arbiters cache and pr_row["opened_by"] — server-side data, not user-supplied. Safe. - api_branches
_require_branch_owner/_can_contribute/_can_read_branch: branch grants keyed on grantee_user_id (session id), creator compared to gitea_login. Safe. - notification mute (
/api/users/{user_id}/notification-mute): SQL scoped to viewer.user_id; only the {user_id} target is user-supplied and it's just the mute subject, not an ownership pivot. Self-mute refused. Safe. - invitations accept: requires auth + case-insensitive email match + refuses empty email; collaborator row keyed on viewer.user_id. Safe.
- Considered: OTC/invite users have gitea_login="". Could "" match an
owners list? Only if frontmatter literally contained "" — impossible
from gitea_login frontmatter.
"" in []is False. Killed as non-issue.
- api_prs
-
Checked anonymous-reachable mutation surfaces:
/api/email/unsubscribe(GET+POST): itsdangerous URLSafeSerializer signed token (SECRET_KEY, salted) → user_id+category not forgeable; idempotent, low-stakes; no expiry but URLSafeSerializer is fine here./api/webhooks/email-bounce: optional X-Webhook-Secret (constant-time compare) when WEBHOOK_EMAIL_BOUNCE_SECRET set; UNAUTHENTICATED when unset (documented v1 dev contract). Impact: flip email opt-out flags + mark outbound rows bounced. Low./api/webhooks/...Gitea receiver: mandatory HMAC-SHA256, constant- time compare, refuses to start without secret unless RFC_APP_INSECURE_WEBHOOKS=1. SOLID./api/invites/claim,/auth/device-trust/start: anon by design (they establish the session); both gated by 256-bit bcrypt-hashed tokens.
-
Confirmed db is a single shared sqlite connection (check_same_thread= False, isolation_level=None) — no incidental concurrency limiter that would mitigate the OTC brute-force.
Cut state (final findings)
High
- F1 — OTC verify has no attempt limit / lockout / per-IP throttle.
backend/app/otc.py:211 verify_code(endpointbackend/app/main.py:279 /auth/otc/verify). The passcode path (passcode.py:262) has a 5-fail/ 15-min per-account lockout; the OTC path has none. 60s cooldown is on request only. 6-digit code (10^6), 10-min TTL, single live code per email. Remediation: add a per-account failed-OTC-attempt counter + lockout mirroring passcode.py, and/or a per-IP 429 throttle on verify; consider invalidating the live code after N failures. Confidence: HIGH the control is absent (verified across module + endpoint + global grep). Severity argued down from Critical to High because bcrypt cost + 10-min TTL + single-live-code give real friction; still a genuine gap and an inconsistency with the passcode path.
Low / Info
- F2 — Session cookie
https_only=False.backend/app/main.py:150SessionMiddleware. Cookie is itsdangerous-signed + HttpOnly, but Secure is off "for dev parity." The device-trust cookie correctly sets Secure=True. In prod behind HTTPS the session cookie should also be Secure. Remediation: drive https_only from an env flag, true in prod. Confidence: HIGH it's set false; impact Low given HTTPS + signed cookie. - F3 — Email-bounce webhook unauthenticated when env unset.
backend/app/api_notifications.py:543. Documented v1 dev contract; impact limited to email opt-out flag flips + outbound-row status. Info/ Low. Confidence HIGH on behavior, low on impact.
What's solid (controls correctly implemented)
- Three-tier dependency ladder;
current_userre-reads role + permission_state from DB every request (revokes take effect immediately). - Admin role model: owner-only owner grants, self-downgrade/self-mute/ self-permission/self-invite refusals, admins/owners not write-mutable. No escalation/spoof path. Every /api/admin/* endpoint require_admin.
- Every write/mutation endpoint across all api*.py files enforces auth; per-RFC writes additionally enforce can_*_to_rfc. No missing guard.
- No IDOR: owner/permission checks compare against server-side frontmatter cache / session user_id / pr opened_by, never a trusted user-supplied id.
- Passcode lockout (5/15min), denylist; OTC single-live-code + consume- before-provision (no replay, no double-sign-in).
- All tokens (OTC, passcode, device-trust, invite) bcrypt-hashed at rest; device-trust/invite tokens 256-bit CSPRNG; constant-time bcrypt walks, no early-exit timing leak; raw tokens never logged.
- Gitea webhook mandatory HMAC-SHA256 + constant-time compare; OAuth state param checked with secrets.compare_digest.
- SECRET_KEY is _required (no insecure default); unsubscribe tokens signed with it (salted, not forgeable).
- OTC request returns 202 regardless (no email enumeration); verify collapses wrong/unknown to one 400 (no enumeration); passcode verify collapses wrong/unknown likewise.
What the driver needs to know
- F1 is the only finding worth a remediation slice. It is a real, exploitable-with-effort gap AND an internal inconsistency (passcode has a lockout, OTC doesn't). Recommend the driver fold it in at High; if the driver judges the bcrypt+TTL friction insufficient given OHM is publicly reachable, it could argue back toward Critical — I left it at High after refutation.
- F2/F3 are documented dev contracts; flag them but they are low priority. Worth a one-line confirmation that OHM's prod overlay sets WEBHOOK_EMAIL_BOUNCE_SECRET and serves over HTTPS (the device-trust cookie is Secure-only, so if OHM weren't HTTPS, device trust would already be broken — implying HTTPS is in fact present, which de-risks F2).
- The authz guard coverage is genuinely complete — guarantee #4 holds for every write/admin/owner endpoint I enumerated. No anon-write hole found.
§19.2 candidates surfaced
- Per-account OTC-verify lockout + per-IP verify throttle (pairs with the SPEC's already-listed per-IP passcode-verify candidate). This is the remediation for F1 and is a flotilla-irrelevant, rfc-app-core change.