fix(security): enforce room access on reaction handlers (LC-149) #182

Merged

nrupard merged 1 commit from feat/lc-149-reactions-authz into main

2026-05-22 19:37:59 +02:00

nrupard commented

2026-05-22 19:35:31 +02:00

Owner

What

Closes the reaction-handler authorization gap from the LC-148 audit (findings S1-S3). routes/reactions.rs was the only id-based handler that did not re-check room access the way pinned/bookmarks/polls/room do.

Fixes

S1 (high, IDOR) toggle_reaction: fetched a message by attacker-supplied id, toggled a reaction, broadcast ReactionAdded/Removed into the room, and fired the reaction.added outgoing webhook, all with no access check. Any authenticated user could react in a private room / DM / enclave they cannot see, inject a live WS fragment into that room, and probe message existence. Now gated by require_room_access.
S2 (med) get_picker: had no auth extractor at all; leaked a room's custom-emoji inventory and was an unauthenticated message-existence oracle. Now requires AuthUser + access check, and 404s the message before the access check so a non-member cannot distinguish "no such message" from "no access".
S3 (med) the {emoji} path segment was inserted unbounded/unvalidated. Now bounded by is_valid_reaction (non-empty, <= 64 bytes, no control chars).

require_room_access mirrors pinned.rs (admin-aware is_room_accessible).

Test

New routes_reactions_authz integration test: non-member gets 403 toggling and opening the picker on a private-room message; a General member gets 200 for both; an oversized emoji token gets 400. just check, just test, just test-saas green (the routes_uploads upload-pipeline flake is pre-existing and passes in isolation).

Part of the LC-159 post-audit story.

🤖 Generated with Claude Code

## What Closes the reaction-handler authorization gap from the LC-148 audit (findings S1-S3). `routes/reactions.rs` was the only id-based handler that did not re-check room access the way `pinned`/`bookmarks`/`polls`/`room` do. ## Fixes - **S1 (high, IDOR)** `toggle_reaction`: fetched a message by attacker-supplied id, toggled a reaction, broadcast `ReactionAdded`/`Removed` into the room, and fired the `reaction.added` outgoing webhook, all with no access check. Any authenticated user could react in a private room / DM / enclave they cannot see, inject a live WS fragment into that room, and probe message existence. Now gated by `require_room_access`. - **S2 (med)** `get_picker`: had no auth extractor at all; leaked a room's custom-emoji inventory and was an unauthenticated message-existence oracle. Now requires `AuthUser` + access check, and 404s the message before the access check so a non-member cannot distinguish "no such message" from "no access". - **S3 (med)** the `{emoji}` path segment was inserted unbounded/unvalidated. Now bounded by `is_valid_reaction` (non-empty, <= 64 bytes, no control chars). `require_room_access` mirrors `pinned.rs` (admin-aware `is_room_accessible`). ## Test New `routes_reactions_authz` integration test: non-member gets **403** toggling and opening the picker on a private-room message; a General member gets **200** for both; an oversized emoji token gets **400**. `just check`, `just test`, `just test-saas` green (the `routes_uploads` upload-pipeline flake is pre-existing and passes in isolation). Part of the LC-159 post-audit story. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

nrupard added 1 commit

2026-05-22 19:35:32 +02:00

fix(security): enforce room access on reaction handlers (LC-149)

check-secrets / Kingfisher (push) Successful in 4s

Details

check-secrets / TruffleHog (push) Successful in 5s

Details

check-secrets / Nosey parker (push) Successful in 4s

Details

check-secrets / Nosey parker (pull_request) Successful in 7s

Details

check-secrets / Kingfisher (pull_request) Successful in 8s

Details

check-secrets / TruffleHog (pull_request) Successful in 11s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

Check / clippy + fmt + tests (pull_request) Successful in 3m14s

Details

d2473af5d0

reactions.rs was the one id-based handler that did not re-check room access, unlike pinned/bookmarks/polls/room. Three gaps (audit S1-S3):

- toggle_reaction (high, IDOR): fetched a message by attacker-supplied id and toggled a reaction + broadcast ReactionAdded/Removed + fired the reaction.added outgoing webhook with no access check. Any authenticated user could react in private rooms / DMs / enclaves they cannot see, inject a live WS fragment into that room, and probe message existence.
- get_picker (med): had no auth extractor at all; leaked a room's custom-emoji inventory and served as an unauthenticated message-existence oracle.
- {emoji} path segment (med): inserted into message_reactions unbounded and unvalidated.

Adds a require_room_access helper (admin-aware is_room_accessible, mirroring pinned.rs) called by both handlers, an AuthUser extractor on get_picker, and an is_valid_reaction bound (non-empty, <= 64 bytes, no control chars) before the toggle. get_picker now 404s the message before the access check so a non-member cannot distinguish "no such message" from "no access".

New routes_reactions_authz integration test: a non-member gets 403 toggling and opening the picker on a private-room message; a General member gets 200 for both; an oversized emoji token gets 400.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>