X-Total-Count on GET /rooms/:id/:bot_key/messages was COUNT(*) of the
room on every page. Serve it from rooms.messages_count updated by SQLite
triggers so Rails, bulk SQL, and foreign writers stay in step — without
ActiveRecord counter_cache callbacks those paths skip.
Fixes#309.
Co-authored-by: Thomas Klemm <github@tklemm.eu>
* 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>
Bots can only create. A lifecycle notification — an alert that fires and then
resolves, a deploy that starts and finishes, a backup that runs — therefore has
to post a second message, and the room becomes an append-only log of states
rather than a view of the current one.
Adds PATCH and DELETE inside the existing bot_key scope, routed to
Messages::ByBotsController. The body is read the way create reads it, so
updating a message is the same request shape as posting one.
No new authorization: both actions already run through ensure_can_administer,
and can_administer? grants access only to a record the user created, so a bot
key reaches that bot's own messages and no others. set_room narrows it again by
looking the room up through the bot's own memberships. A leaked bot key gains
what it could already do by posting: write to rooms that bot belongs to.
update answers head :ok rather than the redirect, which meant extracting the
update and its broadcast into update_message — calling super and then head
would double render, since the parent redirects inside the action. destroy
needs no split, because the parent renders implicitly like create does.
- Use jbuilder instead of hashes
- Use resource instead of direct HTTP verbs
Verbs only make sense if you have one or two routes, if there are
multiple that emulate what resource does then it's better to use resource.
- Paginate using link headers
- Cache responses
Adds a new endpoint that allows bots to add emoji reactions (boosts) to messages:
POST /rooms/:room_id/:bot_key/messages/:message_id/boosts
This enables bots to acknowledge messages with reactions like 👀 (eyes) when
mentioned, providing immediate feedback to users before generating a full response.
The endpoint:
- Validates the bot is a member of the room
- Validates the message exists in the room
- Broadcasts the boost to connected clients via Turbo Streams
- Returns 201 Created on success, 404 if room/message not found
Co-authored-by: openhands <openhands@all-hands.dev>
Changed `message.creator.role == "bot"` to `message.creator.bot?` per
Copilot code review. This is the idiomatic Rails pattern for enum checks
and consistent with how other role checks are done in the codebase.
Co-authored-by: openhands <openhands@all-hands.dev>
Both create and index now return HTTP 404 Not Found when a bot tries to
access a room it's not a member of. This is consistent with REST API
security best practices (not revealing resource existence) and ensures
read and write permissions are handled identically.
Changed create action to no longer call super (which rendered HTML) and
instead directly handle the request with proper JSON API error responses.
Added test to verify create returns 404 for non-member rooms.
Co-authored-by: openhands <openhands@all-hands.dev>
Added explicit RecordNotFound handling to return 404 when a bot tries to
read messages from a room it's not a member of. This matches the security
model used by the create action.
Added tests to verify:
- Bot gets 404 when trying to read from room it's not a member of
- Bot can successfully read from room it IS a member of
Co-authored-by: openhands <openhands@all-hands.dev>
Adds a new GET endpoint at /rooms/:room_id/:bot_key/messages that allows
bots to read messages from rooms they are members of.
The endpoint returns JSON with:
- Room info (id, name)
- Messages array with body (plain/html), created_at, and creator info
- Pagination info (oldest_id, newest_id, has_more)
Supports pagination via ?before=:id and ?after=:id query parameters,
consistent with the existing pagination in the messages controller.
This enables AI bots and other automated agents to understand conversation
context when responding to messages, rather than only receiving the single
message that triggered the webhook.
Co-authored-by: openhands <openhands@all-hands.dev>
Cursor Agent
Donal McBreen
Stanko Krtalić
Ronald Lokers
John-Mason Shackelford
Kevin McConnell