diff --git a/ohm/0019/SESSION-0019.1-TRANSCRIPT-2026-05-28T11-29--2026-05-28T11-32.md b/ohm/0019/SESSION-0019.1-TRANSCRIPT-2026-05-28T11-29--2026-05-28T11-32.md new file mode 100644 index 0000000..37297a8 --- /dev/null +++ b/ohm/0019/SESSION-0019.1-TRANSCRIPT-2026-05-28T11-29--2026-05-28T11-32.md @@ -0,0 +1,158 @@ +# SESSION-0019.1 — App.css + index.css token sweep + +Parent: SESSION-0019.0-TRANSCRIPT-… (UX-polish wave, rfc-app v0.21.0, roadmap item #31) + +## Goal + +Sweep `frontend/src/App.css` (2434 lines) and `frontend/src/index.css` +onto the design-token system established in `frontend/src/styles/tokens.css`: +hardcoded hex → nearest semantic/primitive token, font-size literals → +`--text-*` scale, border-radius literals → `--radius-*` scale, safe +spacing where it maps cleanly, a coherent transition vocabulary + a +consistent `:focus-visible` ring, and base rules pointed at semantic +tokens. Hard constraint: never rename/remove a selector (parallel +siblings reference these class names); values + additive rules only. + +## Pre-state + +- Worktree: `/Users/benstull/git/rfc-app-v0.21.0-appcss` +- Branch `feature/v0.21.0-appcss-sweep`, based on `5be2c48` (token-module + foundation). +- index.css used literal `#1a1a1a` / `#fafaf8` / inline font stack. +- App.css had ~98 distinct hex (66× `#fff`, 51× `#1a1a1a`, plus a long + tail of near-duplicate grays: #888/#999/#777/#666/#555 etc.), 16 + distinct font-size px values, 12 distinct radius px values. + +## Turn-by-turn arc + +1. Read tokens.css (the foundation) + both target files in full + (App.css needed two reads — 2434 lines). Confirmed branch/base. +2. Extracted the full distinct-hex inventory via grep+uniq across both + files to build the mapping deterministically rather than guessing. +3. Round-1 script (`/tmp/sweep.py`): regex word-boundary, + case-insensitive, longest-key-first replacement of all hex that map + cleanly to a ramp step or status/accent token. 615 replacements. + - One dict-key collision worth noting: `#ddd` appears as both a + gray-300 candidate and an on-dark-muted candidate. Resolved to + `--c-gray-300` (the consolidation target); the header user-name + `#ddd`-on-dark shifts imperceptibly to #d1d5db. +4. Re-grepped remainder; 35 distinct hex left — all genuinely off-token + (true blues, violet tints/shades, functional state dots, deep + diff-contrast shades, warm oranges with no token). +5. Round-2 script (`/tmp/sweep2.py`): conservative — only the 7 hex that + are unambiguously the same ROLE as an existing token and visually + near it (light-red error bgs/borders → danger-bg/-border, warm soft + bgs → warning-bg-soft). 15 replacements. Left everything else. +6. index.css base `:root` → `var(--font-sans)`, `var(--color-text)`, + `var(--color-bg)`. +7. Round-3 script (`/tmp/sweep3.py`): font-size px → `--text-*` (280), + border-radius px → `--radius-*` (128), `rgba(255,255,255,.15/.25)` → + `--color-on-dark-soft/-hover` (5), the three 48px header-band values + → `--header-height` (3). Note: script rewrite invalidated an Edit I + had queued for `.app-header`; re-read + re-applied. +8. Promoted `.app-header` to semantic `--color-header-bg` / + `--color-text-inverse`. +9. Appended (not interleaved) the interaction-polish layer: a + transition vocab on hover-reactive selectors + one `:focus-visible` + ring using `--color-focus-ring`. Additive only — no resting-state + change, reduced-motion handled by the token module's media query. +10. Build: first attempt failed on `VITE_APP_NAME is required` (env + gate, not CSS). Re-ran with `VITE_APP_NAME=OHM` → built in 249ms, + only the expected ~500kB chunk-size warning. CSS is sound. +11. Committed; wrote this transcript. + +## Hex → token MAPPING TABLE + +Grays (→ neutral ramp): +| literal(s) | token | note | +|---|---|---| +| #ffffff, #fff | --c-white | | +| #fafafa, #fafaf8, #fafaf9, #fafbfc, #f9f9f9, #f9fafb | --c-gray-50 | | +| #f3f4f6, #f5f5f5, #f6f6f4, #f7f7f5, #f7f6f0 | --c-gray-100 | | +| #f0f0ee, #fcfcfb | --c-gray-150 | warm canvas | +| #e5e5e5, #e5e7eb, #e5e5e0, #e7e5e4, #eee | --c-gray-200 | | +| #d1d5db, #d4d4d4, #ddd, #ccc | --c-gray-300 | **#ccc→#d1d5db slightly lighter; #ddd→#d1d5db imperceptible** | +| #9ca3af, #aaa, #b0b0b0, #94a3b8 | --c-gray-400 | #94a3b8 is slightly cooler→neutral, minor | +| #6b7280, #888, #999, #777, #666, #555 | --c-gray-500 | **#555→#6b7280 and #999→#6b7280 are the most noticeable gray shifts (the desired consolidation)** | +| #4b5563, #444, #475569 | --c-gray-600 | **#444→#4b5563 noticeable; #475569→#4b5563 slightly warmer** | +| #374151, #333 | --c-gray-700 | **#333→#374151 slightly cooler, noticeable** | +| #1f2937, #222 | --c-gray-800 | **#222→#1f2937 noticeable** | +| #111 | --c-gray-900 | | +| #1a1a1a | --c-ink (header→--color-header-bg, base text→--color-text) | | + +Accent / status (→ family tokens): +| literal | token | +|---|---| +| #5b5bd6 | --c-accent | +| #4338ca | --c-accent-strong | +| #7c3aed | --c-violet | +| #166534 | --c-success-fg | +| #dcfce7 | --c-success-bg | +| #991b1b | --c-danger-fg | +| #b91c1c | --c-danger-fg-strong | +| #fef2f2, #fff5f5 | --c-danger-bg | +| #fecaca, #fca5a5, #fcc | --c-danger-border | +| #fee2e2, #fee | --c-danger-bg (light red error bg; **slightly pinker→#fef2f2**) | +| #92400e | --c-warning-fg | +| #b45309 | --c-warning-accent | +| #fef3c7 | --c-warning-bg | +| #fffbeb, #fff7ed, #fff8e0 | --c-warning-bg-soft (**#fff7ed warm→#fffbeb, minor**) | + +Font-size (→ --text-* scale; round-to-nearest flagged): +10→2xs · 11→xs · 12→sm · 13→base · 14→md · **15→lg(16)** · +16→lg · **17→xl(18)** · 18→xl · **19→xl(18)** · 22→2xl · +**24→2xl(22)** · **26→3xl(28)** · 28→3xl. 9px left as literal +(below the 10px scale floor — the .beta-chip micro-tag). + +Border-radius (→ --radius-* scale): +2→xs · **3→sm(4)** · 4→sm · **5→md(6)** · 6→md · 8→lg · +**10→xl(12)** · 12→xl · **14→xl(12)** · **20→pill** (model-pill, +already pill-shaped at its height) · 99→pill · 999→pill. `50%` +(circle dots) left untouched. + +On-dark + layout: +rgba(255,255,255,0.15)→--color-on-dark-soft; +rgba(255,255,255,0.25)→--color-on-dark-hover; +48px header band (×3)→--header-height. +rgba(255,255,255,0.08) (header link hover) LEFT — no token. + +Left as literals (no clean token, intentional): blues #3b82f6 #2563eb +#1e40af #dbeafe #bfdbfe #eff6ff #eef2ff #c7d2fe; violet tints/shades +#c4b5fd #ede9fe #ddd6fe #faf5ff #4c1d95 #3737a0 #3730a3; status dots / +functional #ef4444 #dc2626 #fbbf24 #f59e0b #d97706 #10b981; deep +diff-contrast #14532d #7f1d1d #78350f #bbf7d0 #f0fdf4; warm/misc +#9a3412 #c2410c #4a3f00 #cb6a6a #cfc8a8 #e6dca0 #fde68a #fdba74 +#fed7aa #f1f5f9. Spacing (--space-*) left untouched this pass — +prioritized color/type/radius per the brief; no spacing literal was +both clearly mappable and zero-layout-risk enough to be worth it. + +## Cut state + +- Branch: `feature/v0.21.0-appcss-sweep` +- Final commit: `cbf02d5507f91cc5bf16a68b3f8a8b6b52990849` +- Files changed: `frontend/src/App.css`, `frontend/src/index.css` (only). +- Replacements: ~630 color, 280 font-size, 128 radius, 5 on-dark, 3 + header-height. Build green (`VITE_APP_NAME=OHM npm run build`, 249ms, + expected chunk-size warning only). + +## What the driver needs to know + +- No selector renamed/removed — safe for parallel sibling subagents. +- Gray consolidation deliberately shifts a handful of mid-grays by a + visually-noticeable amount (#555/#999→gray-500, #444→gray-600, + #333→gray-700, #222→gray-800). This IS the intended polish; operator + should eyeball muted-text and secondary-button text before deploy. +- 15px reading-body → 16px (lg): markdown/entry/landing body text gets + 1px larger. Intentional rounding; check it reads well. +- The appended polish layer adds a global `:focus-visible` ring and + transitions to broad element selectors (button/a/input/…). Low risk + (focus-visible never fires on pointer), but it is the one place that + touches elements beyond the named classes. + +## §19.2 candidates + +- The on-dark translucent-white pattern (`--color-on-dark-soft/-hover`) + and the focus-ring/transition vocabulary are deployment-agnostic and + would belong in a future `flotilla-core`-adjacent shared UI layer if + rfc-app's frontend ever extracts one — but that's an rfc-app concern, + not flotilla. No flotilla-side extraction implied by this work. diff --git a/ohm/0019/SESSION-0019.2-TRANSCRIPT-2026-05-28T11-27--2026-05-28T11-31.md b/ohm/0019/SESSION-0019.2-TRANSCRIPT-2026-05-28T11-27--2026-05-28T11-31.md new file mode 100644 index 0000000..3163c62 --- /dev/null +++ b/ohm/0019/SESSION-0019.2-TRANSCRIPT-2026-05-28T11-27--2026-05-28T11-31.md @@ -0,0 +1,96 @@ +# SESSION-0019.2 — header rename + inbox icon + light inbox UX + +Parent: SESSION-0019.0-TRANSCRIPT-… (UX-polish wave, rfc-app v0.21.0) +Subsession: 0019.2 +Window: 2026-05-28T11-27 → 2026-05-28T11-31 (PST) + +## Goal + +Roadmap #24 (rename), #25 (inbox icon + light inbox UX), and the +header's share of #31 polish — confined to the App.jsx header region, +Inbox.jsx, and a new Inbox.css. No version/changelog/tag/merge/push. + +## Pre-state + +- Branch `feature/v0.21.0-header-inbox` at the foundation commit (token + module `frontend/src/styles/tokens.css` present). +- Header link text was "About" (already routing to /philosophy). +- Inbox trigger used a 📮 emoji with title "Notifications inbox (§15.2)". +- Inbox.jsx: working panel; row click marks-read + navigates; one + "Mark all read (under filter)" button; terse empty copy; no per-row + mark-read; unread indicated only by `.inbox-row.unread` bg tint. +- No Amplitude `track()` calls inside Inbox.jsx (only api.js calls: + listNotifications, markNotificationRead, markNotificationsReadByFilter). + So "preserve §15 tracking" = don't break the data path; nothing to add. + +## Turn-by-turn arc (honest) + +1. Read tokens.css, the App.jsx header block (~150–238), Inbox.jsx full, + and the §15 inbox block of App.css (1573–1641) to learn existing class + names + property values before touching anything. +2. #24: one-word change "About" → "Philosophy" (kept the title attr). +3. #25 icon: replaced `📮` with a hand-rolled inline SVG + envelope (18×18, viewBox 0 0 24 24, stroke=currentColor, ~1.75 stroke, + rounded). Set `aria-label="Inbox"`, retitled "Inbox (§15.2)". Badge + block untouched. +4. #25 UX: refactored Inbox.jsx. Extracted `markOneRead(item)` (used by + both row-click and the new per-row affordance) which also decrements + the local unreadCount. Added an `inbox-unread-dot` span + `read`/`unread` + row class, a per-row "Mark as read" button (inline check SVG) that sits + OUTSIDE the Link and `preventDefault/stopPropagation`s so it marks + without navigating, shown only for unread rows. Simplified the + mark-all label to "Mark all read" with an explanatory title. Replaced + empty copy with a "You're all caught up." state that branches on + whether filters are active. +5. Wrote Inbox.css (tokenized). Discovered via import order that Inbox.jsx + is imported at App.jsx line 6, BEFORE App.css at line 30 — so under + ESM depth-first eval Inbox.css injects FIRST and App.css wins ties. + Wrote override rules one notch more specific (`.inbox-list .inbox-row…`) + and `.inbox-empty .muted` to beat global `.muted`. New classes need no + guard. All values via var(--space/-text/-color/-radius/-motion/-ease). +6. Build: first `npm run build` failed — VITE_APP_NAME required (config + guard, unrelated to my change; no .env in worktree). Re-ran with + `VITE_APP_NAME="OHM RFC" npm run build` → green (expected ~971kB + chunk warning only). +7. Verified `git status` shows only App.jsx + Inbox.jsx modified and + Inbox.css new (node_modules symlink left untracked). Committed exactly + those three. + +## Cut state + +- Branch: `feature/v0.21.0-header-inbox` +- Final commit: 317738ed7998a7af5abe0b64769941cb11808ab2 +- Files changed: frontend/src/App.jsx (header region only), + frontend/src/components/Inbox.jsx, frontend/src/components/Inbox.css (new) +- Build: green (chunk-size warning only). + +## What the driver needs to know (inbox UX review pointers) + +The inbox pass is LIGHT — same layout, panel, filters, bundle toggle, +deep-links, and API calls as before. Exactly what changed for review: + +- Unread/read: unread rows now get an accent left-bar (inset box-shadow) + + warm tint + a filled accent dot + medium-weight summary; read rows + drop the dot (space reserved, stays aligned) and mute the summary. +- Per-row affordance: a check-icon button appears on row hover/focus + (always visible on touch via `@media (hover:none)`) that marks the row + read without navigating. Uses the same markOneRead path. +- Mark-all: relabeled "Mark all read" (+ title clarifying it's + filter-scoped). Same `markNotificationsReadByFilter` call, same + disabled-when-all-read guard. +- Empty state: "You're all caught up." with filter-aware sub-copy. +- Behavior preserved: row-click still marks-read then navigates + + closes; unreadCount now also decremented locally on single marks (was + only reset to 0 on mark-all before — minor, improves badge accuracy, + SSE still authoritative per §15.3). + +## §19.2 candidates + +- Full inbox redesign deferred to a #25-followup: the operator's + reference screenshot did not transmit, so this stayed a polish pass + within the existing structure rather than a re-layout. Revisit with + the screenshot. +- The inbox-trigger / inbox-* base rules still live in App.css with + hardcoded hex (a sibling is tokenizing App.css this wave). If that + sweep lands, the few overrides in Inbox.css could shed their + extra-specificity guards once import order is normalized. diff --git a/ohm/0019/SESSION-0019.3-TRANSCRIPT-2026-05-28T11-28--2026-05-28T11-33.md b/ohm/0019/SESSION-0019.3-TRANSCRIPT-2026-05-28T11-28--2026-05-28T11-33.md new file mode 100644 index 0000000..e80b7d5 --- /dev/null +++ b/ohm/0019/SESSION-0019.3-TRANSCRIPT-2026-05-28T11-28--2026-05-28T11-33.md @@ -0,0 +1,141 @@ +# Session 0019.3 — Transcript + +Parent: SESSION-0019.0-TRANSCRIPT-… (UX-polish wave, rfc-app v0.21.0) +Subagent ordinal: .3 — roadmap item #32 (session/transcript page +polish) + the /docs surfaces' share of #31. + +## Goal + +In the isolated worktree `/Users/benstull/git/rfc-app-v0.21.0-docs` +on branch `feature/v0.21.0-docs-32`: + +(a) Collapse single-transcript session roots — render the transcript + inline at `/docs/sessions/:nnnn` instead of a dead-end placeholder; + multi-transcript sessions inline the `.0` driver transcript and + list the siblings. +(b) Add a transcript metadata header (title, started/ended parsed from + the filename ISO segments, derived duration, optional `tldr` from + the manifest, external view-source link). +(c) Docs-surface token polish + consistent loading/empty/error states + + aria-labels on icon-only controls. + +## Pre-state + +- Foundation commit carried `frontend/src/styles/tokens.css` (the #31 + design-token module). Build requires `VITE_APP_NAME` (no default). +- `/docs/sessions/:nnnn` (DocsSessionIndex.jsx) rendered a + `.docs-session-overview` placeholder: "N transcript(s) in this + session. Select one from the navigation." +- DocsSessionTranscript.jsx rendered body via MarkdownPreview with a + breadcrumb but no metadata header. +- Backend `/api/docs/sessions/manifest` (api.py:177-178) returns + `result["manifest"]` verbatim — the full `sessions.json` object — + so arbitrary per-entry fields already reach the frontend. + +## Turn-by-turn arc (honest) + +1. Read tokens.css, SESSION-PROTOCOL.md (§1 naming + the 0017.0 + folder-layout amendment + the sessions.json shape), all seven owned + docs components, api.js docs helpers, docs_sessions.py, and the + api.py manifest route. +2. Confirmed the manifest endpoint is a pass-through → **no backend + change needed** for `tldr`. The decision branch in the task was + resolved in favor of "already passes through". +3. Read the existing docs CSS block in App.css (read-only) to learn + which classes a sibling owns (.docs-article, .docs-empty, + .docs-error, .docs-source-link, .docs-session-overview, etc.) so + Docs.css would not collide. +4. Rewrote DocsSessionTranscript.jsx: exported `gitSourceUrl`, + `transcriptOrdinal`, `parseTranscriptMeta`, and `TranscriptMetaHeader`; + added a manifest fetch for title+tldr; rendered the header above the + body. +5. Rewrote DocsSessionIndex.jsx to inline-render the primary transcript + (driver `.0` preferred, else `files[0]`) and list siblings; reused + the imported header + ordinal helpers. +6. Created Docs.css (new classes only, all token-valued). +7. Brought DocsUserGuide's bare `

` onto the shared + `.docs-error` + retry convention. +8. Build failed first run (VITE_APP_NAME unset — env requirement, not + my change); re-ran with `VITE_APP_NAME="OHM Verify" npm run build` + → green (974 kB index chunk; >500 kB warning expected/benign). +9. Staged the four files explicitly (avoided the `frontend/node_modules` + symlink), committed. + +## Design decisions + +**Inline-collapse (a).** URL stays stable to the session number — the +session root fetches the index, picks a primary file, fetches its body, +and renders the same `TranscriptMetaHeader` + `philosophy-body` the +standalone route uses. No ``, no 301. Primary selection: +`SESSION-NNNN.0-TRANSCRIPT…` if present, else `files[0]` (backend +returns the list pre-sorted). For multi-transcript sessions, a +`.docs-session-siblings` strip lists every transcript ordinal — the +primary marked "(shown below)" with `aria-current`, the rest linking to +`/docs/sessions/:nnnn/:filename`. No empty placeholder remains on any +path that has at least one transcript. + +**Metadata header (b).** `parseTranscriptMeta` regexes the +`-TRANSCRIPT---.md` ISO segment (`YYYY-MM-DDTHH-MM` x2, +colons-as-dashes per protocol). Times are built as **local** Dates and +rendered as wall-clock via `toLocaleString` — the filename encodes no +timezone (PST implied), and duration is a difference of two same-basis +Dates so the implied-TZ ambiguity cancels. Legacy renamed-letter +transcripts (no segment) → all derived fields null → header shows just +the title + source link (graceful degrade). Duration omitted when +start==end or unparseable. + +**tldr schema (b).** Consumed as a **string** field named `tldr` on the +per-session entry of `sessions.json`, i.e. keyed by the 4-digit session +number alongside `title`: +```json +{ "0019": { "title": "UX-polish wave …", "tldr": "One-line summary." } } +``` +Read via `getSessionsManifest()` → `payload[nnnn].tldr`; rendered only +when `typeof === 'string'` and truthy. Absent ⇒ no TL;DR line. + +**Polish (c).** Docs.css styles ONLY the two new element families +(`.docs-transcript-meta*`, `.docs-session-siblings*`), every value a +tokens.css custom property, to avoid racing the #31 App.css sweep for +the pre-existing docs selectors. DocsUserGuide normalized to the shared +error convention. All icon-only controls already carried aria-labels +(verified the drawer toggle `☰`, scrim) — no aria gaps found in owned +files. + +## Cut state + +- Branch: `feature/v0.21.0-docs-32` +- Final commit SHA: `adb5d2571532ad5992900791d295ef214ddfba0d` +- Files changed (4): + - `frontend/src/components/DocsSessionIndex.jsx` (rewrite — inline collapse) + - `frontend/src/components/DocsSessionTranscript.jsx` (rewrite — meta header + exports) + - `frontend/src/components/DocsUserGuide.jsx` (error-state normalize) + - `frontend/src/components/Docs.css` (new) +- **Backend: NOT changed.** docs_sessions.py untouched — manifest + pass-through already exposes `tldr`. (No py_compile needed.) +- Build: GREEN (`VITE_APP_NAME=… npm run build`; 974 kB chunk warning benign). + +## What the driver needs to know + +- **No backend coordination required** for this item — frontend renders + `tldr` when present; the field's value-backfill is the documented + out-of-scope follow-up. +- **sessions.json `tldr` schema to backfill** (driver/operator task in + `wiggleverse/ohm-session-history`): add an optional `"tldr": ""` string to any per-session entry next to `"title"`. The + frontend already consumes it on both the inline session-root view and + the standalone transcript view. Absent ⇒ no-op. +- Inline-collapse changes the `/docs/sessions/:nnnn` semantics: it is no + longer nav-only. The left-nav transcript rows + sibling links still + point at the standalone `/docs/sessions/:nnnn/:filename` route, which + is unchanged and now also carries the metadata header. +- Build needs `VITE_APP_NAME` set (env requirement, not from this item). + +## §19.2 candidates + +- `parseTranscriptMeta` / `transcriptOrdinal` are filename-shape logic + that mirrors the backend `_TRANSCRIPT_FILENAME_RE` and the nav's own + `transcriptOrdinal` in DocsLayout.jsx — there are now two frontend + copies of the ordinal regex (DocsLayout owns one; I export one). A + future consolidation could lift a single `lib/transcripts.js` and have + DocsLayout import it (DocsLayout is sibling-owned, so I did not touch + it). Worth flagging for a follow-up de-dup. diff --git a/ohm/0026/SESSION-0026.1-TRANSCRIPT-2026-05-28T13-46--2026-05-28T13-50.md b/ohm/0026/SESSION-0026.1-TRANSCRIPT-2026-05-28T13-46--2026-05-28T13-50.md new file mode 100644 index 0000000..36639ad --- /dev/null +++ b/ohm/0026/SESSION-0026.1-TRANSCRIPT-2026-05-28T13-46--2026-05-28T13-50.md @@ -0,0 +1,200 @@ +# 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, 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 + +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. diff --git a/ohm/0026/SESSION-0026.2-TRANSCRIPT-2026-05-28T13-46--2026-05-28T13-51.md b/ohm/0026/SESSION-0026.2-TRANSCRIPT-2026-05-28T13-46--2026-05-28T13-51.md new file mode 100644 index 0000000..41caedf --- /dev/null +++ b/ohm/0026/SESSION-0026.2-TRANSCRIPT-2026-05-28T13-46--2026-05-28T13-51.md @@ -0,0 +1,136 @@ +# SESSION-0026.2 — rfc-app Injection & Input-Handling Security Audit + +Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md +Subsession: 0026.2 (read-only audit leg of security-audit driver session 0026) +Date: 2026-05-28 (PST) +Target: rfc-app at /Users/benstull/git/rfc-app, tag v0.24.0 (HEAD 28015ed) +Scope: SQL injection, XSS, SSRF, path traversal, email/markdown sanitization. +Mode: READ-ONLY. No edits, no deploys, no git mutations. No secret bytes recorded. + +## Pre-state + +Driver assigned the injection & input-handling surface. Repo confirmed at +tag v0.24.0. Backend is FastAPI + SQLite (backend/app/*.py); frontend is +React 19 + Vite (frontend/src). Markdown via `marked` v18; no sanitizer +library in frontend/package.json dependencies. + +## Turn-by-turn arc + +1. Enumerated backend modules and grepped all 321 `execute*` call sites for + f-string / `.format` / `%` / concat SQL. Pulled the ~16 dynamic-SQL hits. + +2. SQL injection — examined every dynamic-SQL site: + - api_notifications.py:274 (`UPDATE users SET {', '.join(sets)}`) — `sets` + contains only hardcoded column-name literals; values parameterized via + `args`. Safe. + - api_notifications.py:479/483 (`UPDATE users SET {column} = ...`) — + `column` from static `_CATEGORY_COLUMN` allowlist. Safe. + - api_admin.py:413, notify.py:535, digest.py:201, email.py:610, + notify.py:942/964/1000 — all IN-clauses built via `",".join("?" * len)`, + the correct parameterized-IN idiom; values in the tuple. Safe. + - notify.py:638/643 — `{now}` is the literal `datetime('now')`, not input. + - api_admin.py:629/711/754 — `WHERE` built from `clauses` lists holding + only fixed `col = ?` strings; user values parameterized; `limit` is + FastAPI `int` with ge/le bounds. Safe. + - db.py — connection/migration layer; migrations read trusted on-disk .sql. + - Swept for dynamic ORDER BY / LIMIT / table-name injection: none found. + Verdict: parameterization is consistent and correct across the backend. + +3. XSS — found the markdown pipeline: + - frontend/package.json has NO DOMPurify / rehype-sanitize / any sanitizer. + - MarkdownPreview.jsx:101-102 — `previewMarked.parse(content)` → + `hostRef.current.innerHTML = html`. The custom `Marked` instance only + overrides the ```mermaid code-fence renderer; everything else is marked + default (raw HTML passthrough). `escapeHtml` is applied ONLY to the + mermaid placeholder text and error text, NOT the document body. + - ProposalView.jsx:164 & 172 — `dangerouslySetInnerHTML` with the GLOBAL + unconfigured `marked.parse(data.entry?.body)` / `proposed_use_case`. + - Content sources: RFCView passes RFC body (previewContent/editorContent) + into MarkdownPreview; DocsSessionTranscript / DocsSpec / DocsUserGuide / + Philosophy / DocsSessionsAbout/Index also feed body through it. RFC body + and proposal body/use-case are free-text authored by any invited user. + - Backend: no sanitization on write (api.py — only Field max_length caps). + - Self-refutation step: ran `marked.parse` on + `