psa-systems/lets-chat
Watch
9
0
Fork
You've already forked lets-chat
 0
Code Issues Pull requests Projects Releases 2 Packages Wiki Activity Actions

docs(audit): codebase-wide review + live-update and search designs (LC-148) #181

Merged
nrupard merged 1 commit from chore/lc-148-codebase-audit into main 2026-05-22 19:24:35 +02:00
Conversation 0 Commits 1 Files changed 3 +219
nrupard commented 2026-05-22 19:18:03 +02:00
Owner
Copy link

What

LC-148 codebase-wide review pass: correctness bugs, security vulnerabilities, and the deep-rooted "every new surface re-derives a concept instead of inheriting it" gaps. This is a triage + design PR (docs only); fixes are spun out as their own issues. Method: four parallel read-only analysis passes (security/authZ, correctness, live-update inventory, reused-abstraction inventory).

Deliverables (in this PR)

  • docs/audit/2026-05-22-lc148-audit-report.md - severity-tagged security findings (S1-S12), correctness sweep, systemic-gap matrix, prioritized backlog.
  • docs/superpowers/specs/2026-05-22-live-updates-by-default-design.md - design for making pages live by default.
  • docs/superpowers/specs/2026-05-22-unified-search-component-design.md - design for one reusable typeahead.

Headline findings

Security (fix-first):

  • S1 / LC-149 (high) reactions.rs has no room-access check: any authed user can react in private rooms/DMs they can't see, inject a live WS fragment, fire the reaction webhook, and probe message existence. It's the one id-handler that skips the is_room_accessible gate every sibling uses.
  • S4 / LC-150 (high) unfurl.rs validates the host IP pre-flight but then follows redirects with no per-hop re-check, so an attacker page 302s to 169.254.169.254 / 127.0.0.1 and defeats the guard.
  • S5 / LC-151 (high) login and 2FA challenge endpoints have no rate limit (register does) - online brute force.

Plus med/low: webhook delivery-time SSRF (S6), no message length cap (S9), markdown javascript: link XSS (S8), token-CSPRNG/proxy-trust/og:image hardening (S10-12).

Verified clean: SQL fully parameterized, no | safe in templates, markdown strips raw HTML, cookies/sessions/API-token handling sound, role checks re-enforced server-side, most id-handlers correctly access-checked.

Systemic gaps (the main ask):

  • C1 / LC-156 Live updates are opt-in: only room/dm/voice subscribe (3 copies of the same IIFE); everything else is static until reload. Design: declarative data-lc-live + typed topics + fill the stale surfaces.
  • C2 / LC-157 6 search implementations in 3 quality tiers (4 copy-paste no-keyboard searches, 1 accessible mention combobox, 1 slash combobox with no nav). Design: one shared fragment contract + data-lc-search controller.
  • C3 / LC-158 Smaller dup: modal focus-trap stragglers, 3 confirm-dialog patterns, 2 form-error patterns, mention-popover avatar re-impl.

Follow-up issues filed

LC-149 (reactions authZ), LC-150 (unfurl SSRF), LC-151 (login/2FA rate limit), LC-152 (webhook SSRF), LC-153 (message length), LC-154 (markdown XSS), LC-155 (token/proxy/img hardening), LC-156 (live-updates epic), LC-157 (search epic), LC-158 (UI consolidation). LC-149/150/151 are the fix-first set.

Out of scope

Fixing the findings. This PR is the report + designs + backlog; each fix lands in its own issue/PR.

🤖 Generated with Claude Code

## What LC-148 codebase-wide review pass: correctness bugs, security vulnerabilities, and the deep-rooted "every new surface re-derives a concept instead of inheriting it" gaps. This is a triage + design PR (docs only); fixes are spun out as their own issues. Method: four parallel read-only analysis passes (security/authZ, correctness, live-update inventory, reused-abstraction inventory). ## Deliverables (in this PR) - **`docs/audit/2026-05-22-lc148-audit-report.md`** - severity-tagged security findings (S1-S12), correctness sweep, systemic-gap matrix, prioritized backlog. - **`docs/superpowers/specs/2026-05-22-live-updates-by-default-design.md`** - design for making pages live by default. - **`docs/superpowers/specs/2026-05-22-unified-search-component-design.md`** - design for one reusable typeahead. ## Headline findings **Security (fix-first):** - **S1 / LC-149 (high)** `reactions.rs` has no room-access check: any authed user can react in private rooms/DMs they can't see, inject a live WS fragment, fire the reaction webhook, and probe message existence. It's the one id-handler that skips the `is_room_accessible` gate every sibling uses. - **S4 / LC-150 (high)** `unfurl.rs` validates the host IP pre-flight but then follows redirects with no per-hop re-check, so an attacker page 302s to `169.254.169.254` / `127.0.0.1` and defeats the guard. - **S5 / LC-151 (high)** login and 2FA challenge endpoints have no rate limit (register does) - online brute force. Plus med/low: webhook delivery-time SSRF (S6), no message length cap (S9), markdown `javascript:` link XSS (S8), token-CSPRNG/proxy-trust/og:image hardening (S10-12). Verified clean: SQL fully parameterized, no `| safe` in templates, markdown strips raw HTML, cookies/sessions/API-token handling sound, role checks re-enforced server-side, most id-handlers correctly access-checked. **Systemic gaps (the main ask):** - **C1 / LC-156** Live updates are opt-in: only `room`/`dm`/`voice` subscribe (3 copies of the same IIFE); everything else is static until reload. Design: declarative `data-lc-live` + typed topics + fill the stale surfaces. - **C2 / LC-157** 6 search implementations in 3 quality tiers (4 copy-paste no-keyboard searches, 1 accessible mention combobox, 1 slash combobox with no nav). Design: one shared fragment contract + `data-lc-search` controller. - **C3 / LC-158** Smaller dup: modal focus-trap stragglers, 3 confirm-dialog patterns, 2 form-error patterns, mention-popover avatar re-impl. ## Follow-up issues filed LC-149 (reactions authZ), LC-150 (unfurl SSRF), LC-151 (login/2FA rate limit), LC-152 (webhook SSRF), LC-153 (message length), LC-154 (markdown XSS), LC-155 (token/proxy/img hardening), LC-156 (live-updates epic), LC-157 (search epic), LC-158 (UI consolidation). LC-149/150/151 are the fix-first set. ## Out of scope Fixing the findings. This PR is the report + designs + backlog; each fix lands in its own issue/PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
docs(audit): codebase-wide review report + live-update and search design docs (LC-148)
All checks were successful
check-secrets / TruffleHog (push) Successful in 8s
Details
check-secrets / Nosey parker (push) Successful in 8s
Details
check-secrets / Kingfisher (push) Successful in 9s
Details
check-secrets / Nosey parker (pull_request) Successful in 6s
Details
check-secrets / TruffleHog (pull_request) Successful in 8s
Details
check-secrets / Kingfisher (pull_request) Successful in 11s
Details
Check / clippy + fmt + tests (pull_request) Successful in 3m52s
Details
Create release / Create release from merged PR (pull_request) Has been skipped
Details
a38d55f1ec 
LC-148 review pass: four read-only analysis passes (security/authZ, correctness, live-update inventory, reused-abstraction inventory). This commit lands the deliverables; fixes are spun out as their own issues (LC-149 through LC-158).

- docs/audit/2026-05-22-lc148-audit-report.md: severity-tagged security findings (S1-S12), correctness sweep, systemic-gap analysis, and prioritized backlog. Headline security items: reactions IDOR (no room-access check, S1), unfurl redirect SSRF (redirects bypass the IP allowlist, S4), and unthrottled login/2FA (S5). SQL/templates/markdown-raw-HTML/session handling verified clean.
- docs/superpowers/specs/2026-05-22-live-updates-by-default-design.md: only 3 pages subscribe to WS updates (each hand-rolls the same IIFE); design for a declarative data-lc-live subscribe mechanism, typed topics (room/enclave/user/admin) with subscribe-time authorization, and the list of stale surfaces to fill.
- docs/superpowers/specs/2026-05-22-unified-search-component-design.md: 6 search implementations in 3 quality tiers; design for one shared result-fragment contract + data-lc-search controller (debounce, keyboard nav, a11y) that folds in the mention and slash comboboxes.

Follow-up issues filed: LC-149 (reactions authZ), LC-150 (unfurl redirect SSRF), LC-151 (login/2FA rate limit), LC-152 (webhook delivery SSRF), LC-153 (message length cap), LC-154 (markdown link-scheme XSS), LC-155 (token CSPRNG + proxy-trust + og:image hardening), LC-156 (live-updates epic), LC-157 (unified-search epic), LC-158 (modal/confirm/form-error/avatar consolidation).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
nrupard deleted branch chore/lc-148-codebase-audit 2026-05-22 19:24:35 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/lets-chat!181
No description provided.