A push subscription belongs to the user, not a session, so a device
signed out by a password change kept getting notifications. Changing
the password now removes every subscription along with the sessions,
in the same transaction.
A browser still signed in keeps its own subscription in its service
worker, so the notifications controller posts it once per session: the
bell names the session by a digest of its id, never its token, and the
browser remembers the last one it posted for. That post never
unsubscribes the browser when the server refuses it.
A request that authenticated just before the change could put a
subscription back after it. The create action now checks the session
still exists and saves in one immediate transaction, after validating
the endpoint outside it.
From #371.
(cherry picked from commit 6cab924097)
Changing the password now ends every other session, so whoever holds
one could shut the owner out of their own account with a new password.
The profile asks for the current password whenever the password
changes; without it, or with a wrong one, nothing is saved and the page
says why. Other profile edits don't need it, and the admin
reset-password script still works without it.
Attempts are rate limited like signing in, 10 in 3 minutes per address,
so the current password can't be guessed here instead.
From #371. The cached-response test it touches isn't in 1.5.2.
(cherry picked from commit 44a92ff6c6)
auto_link finds URLs and email addresses with regular expressions over
the rendered html, which can't tell an attribute value from text, so it
could put a link inside an attribute. Hand it each text node on its own,
escaped the way the serializer writes text, after the final sanitize, so
links only ever land between tags. Write the result with
preserve_newline so a code block keeps its leading blank lines.
Bump the presentation cache version so nothing rendered the old way is
served again.
Backport of 7ea05e1, 4e29c6e and 9b271d0 from #364. 1.5.2 has no
plain-text fragment cache, so only the message partial's version moves.
* 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>
ActiveStorage::Preview#processed? is true as soon as ffmpeg's frame is attached, but the poster is a
variant of that frame and can fail on its own: the POST rescues a Vips::Error and leaves the frame
attached. The view then emitted the poster's URL, and every view retried the resize. It now checks the
variant too, from the variant records with_attached_attachment already preloads.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa
A video's preview and a picture's thumbnail are made inside the request that posts the message, and
nothing bounded how long either could take.
- The video preview filter also selects any frame from 5 seconds on. Rails' filter takes the second
frame it selects, which a video with a single keyframe and no scene change only gives at its end, so
ffmpeg decoded all of it.
- TimeLimitedVideoPreviewer gives ffmpeg 10 seconds of wall-clock time, kills it past that, and reports
a failed preview, so the message is posted without one.
- Pictures and videos above 250 megapixels, or whose size couldn't be read, get no preview: decoding
costs in proportion to the pixels, however small the file.
- The view shows a preview only if it was made when the message was posted. Its URL used to make it on
view, so a preview that failed or was skipped would be attempted again on every view. The cached
presentation's version goes up, so cached messages pick this up.
- A video's poster is made, when the message is posted, at the size the view shows it. The full-size WebP
made until now wasn't shown anywhere, and encoding it costs in proportion to the frame's pixels.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa
The editor keeps an attachment's content as it finds it, so a hand-written
embed with a url but no href reached the editor with its markup unvalidated,
and a mention saved under Trix's editor carried the generic octet-stream
content type the editor doesn't permit and was dropped on save. Rebuilding
each attachment from its attachable renders the hardened preview partial
and restores the mention content type.
* Bump thruster 0.1.15 → 0.1.23
* Scrub disallowed attributes on allowed tags in message rendering
SanitizeTags removes disallowed tags from message presentation, but
attributes on the tags it allows passed through untouched. Extend the
content-filter chain with a SanitizeAttributes filter that runs Rails'
safe-list sanitizer over the remaining markup, stripping event-handler
attributes and unsafe URI schemes as defense-in-depth alongside the
existing Content-Security-Policy.
Running it after SanitizeTags with the same allowed-tags list makes the
sanitizer's tag pass a no-op, preserving SanitizeTags' remove-not-unwrap
semantics while scrubbing attributes. The attribute allowlist is the
standard ActionText set (which keeps attachments intact) plus class,
which presentation styling relies on.
If deploying Campfire to a path other than the root (for example, using
Kamal's path-based routing) then the Action Cable endpoint needs to be
adjusted to include that path.
Prepending the script name to the cable path means it will be adjusted
to the path in the same way that the application routes are. When script
name is the default, the behaviour is unchanged.
Jeremy Daer
GPT on behalf of DHH
Donal McBreenandJeremy Daer
Marcello CostagliolaandClaude Opus 5.5
Stanko K.R.
Kevin McConnell
Keshav Kk
Dmitri Kondratjev
sjdonado