* Derive message DOM ids from the server id, not client_message_id
A message's DOM id was derived from the browser-chosen client_message_id
via a Message#to_key override, so dom_id(message) was
"message_<client_message_id>". Turbo's append de-dups by DOM id, so a room
member who posted a message reusing a victim's client_message_id displaced
the victim's message element in every connected member's live view; editing
the attacker's own message then broadcast onto the victim's presentation id.
Drop the to_key override so every message DOM id and broadcast target derives
from the record's primary key. Two distinct records can no longer share a DOM
id regardless of stored client_message_id, so the collision is impossible with
no data migration and no uniqueness constraint. to_param and the fragment
cache key already used the primary key, so message URLs and per-message cache
entries are unchanged.
The composer's optimistic pending message still uses client_message_id as its
placeholder DOM id, which no longer matches the server broadcast's PK-based id.
Reconcile instead by rendering data-client-message-id on the real message and
having the messages controller drop the matching pending placeholder on
connect. Only client-side placeholders (data-pending-message) are removed, so a
message another member posts reusing the same client_message_id can never
displace a real one through the reconciliation path either.
Also point the boost broadcast target at the PK-based dom_id(message, :boosts)
to match the rebuilt container id.
GHSA-3v99-4vxh-xg84
* Bust cached message fragments rendered with client_message_id DOM ids
The message fragment cache keys on the record and the template digest, and
removing the to_key override changes neither. Fragments cached by an earlier
release would keep their message_<client_message_id> ids, so edit, delete and
boost broadcasts, which now target primary-key ids, would miss those messages
in other members' live views until the cache entry expired.
* Locate messages by record id in the client_message_id collision tests
Assert on data-message-id rather than the new primary-key DOM ids, so the tests
describe the behavior instead of the fix and fail on the vulnerable code for the
real reason. Edit the attacker's message to new text and wait for it to arrive,
so the edit path is exercised rather than passing vacuously. Add a request test
that two messages sharing a client_message_id render as distinct elements.
---------
Co-authored-by: Jeremy Daer <jeremy@37signals.com>
The thumbnail or video preview is generated while the message is posted.
When ffmpeg or libvips can't decode the file (a truncated upload, an .mp4
holding only audio), the error escaped after the message had been saved:
the request failed with a 500, the message was never broadcast, and the
sender's upload stayed at 100% while the rest of the files in that drop
were never sent. Posting the message without a preview keeps the file and
lets everyone see it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVFo3Lt9T8M5NR7KxVvsZ2
Now that the unread fanout runs in a job, it can run after a member has
already opened the room and moved on to another one. Their sidebar would
then mark the room unread for a message they have seen.
The job now notifies only members who still have the room unread or are
in it now. Room#receive marks members who aren't in the room unread, and
opening the room clears it, so a member who caught up in the meantime is
skipped. When the
job runs right away this is the same set of people as before, except
members who have hidden the room, whose sidebar doesn't list it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVFo3Lt9T8M5NR7KxVvsZ2
Since the unread rooms stream was scoped per user, posting a message
publishes the room id once to each member of the room. Each publish is a
round trip to Redis, made one after another inside the request, so the time
to post a message grows with the size of the room and in a big room is far
more than the rest of the request.
The fanout now runs in Message::BroadcastUnreadRoomJob, so the poster no
longer waits for it. Who gets the notice and what it says are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NuTtwJKhb77Dv7EQqv2C3C
UnreadRoomsChannel streamed from a hardcoded global name, and every message
published its room id to it. Any authenticated user, including one with no
memberships at all, could subscribe and watch which rooms were active and exactly
when, across every closed room and direct conversation on the account. The browser
filters ids it doesn't recognise, but that happens after delivery.
Its sibling ReadRoomsChannel is already scoped per user; this mirrors it, and
message creation fans the notice out to the room's members instead of broadcasting
it to everyone. No message content was exposed either way, only the timing.
GPT on behalf of DHH
Donal McBreen
Marcello Costagliola
Jeremy Daer
Kevin McConnell