Files
session-history/ohm/0026/SESSION-0026.1-TRANSCRIPT-2026-05-28T13-46--2026-05-28T13-50.md
T
Ben Stull 9e6756f678 ohm: migrate orphaned subagent transcripts (0019.x, 0026.x) + renumber dup 0046.0 -> 0073
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>
2026-06-05 03:28:59 -07:00

11 KiB
Raw Blame History

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.py defines 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_user re-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, 420 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

  1. 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.

  2. Read otc.py + passcode.py side by side. Caught the asymmetry: passcode.verify_passcode has a per-account failed-attempts counter + 15-min lockout; otc.verify_code has 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 in request_code, not verify_code. This is the headline finding.

  3. 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.

  4. 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/verify has NO rate-limit dependency, confirming finding #1 at the endpoint layer too.
    • Turnstile gates /auth/otc/request ONLY, not verify.
  5. 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.

  6. 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.

  7. 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).

  8. 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.
  9. 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.
  10. 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 (endpoint backend/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:150 SessionMiddleware. 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_user re-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.