build(deps): bump turbo-rails from 30cd8fc to 37530c0

Bumps [turbo-rails](https://github.com/hotwired/turbo-rails) from `30cd8fc` to `37530c0`.
- [Release notes](https://github.com/hotwired/turbo-rails/releases)
- [Commits](https://github.com/hotwired/turbo-rails/compare/30cd8fcc6f82c1ad4edd1ed6069ba878f21f02b3...37530c08780fa6f6dbb56a633de4b81169bdd174)

---
updated-dependencies:
- dependency-name: turbo-rails
  dependency-version: 37530c08780fa6f6dbb56a633de4b81169bdd174
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

.github/dependabot.yml

@@ -20,13 +20,12 @@ updates:
       semver-minor-days: 3
       semver-patch-days: 2
       default-days: 7
+      exclude:
+        - "brakeman"
 
   - package-ecosystem: docker
     directory: "/"
     schedule:
       interval: weekly
     cooldown:
-      semver-major-days: 7
-      semver-minor-days: 3
-      semver-patch-days: 2
       default-days: 7

.github/workflows/ci.yml

@@ -60,10 +60,10 @@ jobs:
           persist-credentials: false
 
       - name: Run actionlint
-        uses: rhysd/actionlint@393031adb9afb225ee52ae2ccd7a5af5525e03e8 # v1.7.11
+        uses: rhysd/actionlint@914e7df21a07ef503a81201c76d2b11c789d3fca # v1.7.12
 
       - name: Run zizmor
-        uses: zizmorcore/zizmor-action@71321a20a9ded102f6e9ce5718a2fcec2c4f70d8 # v0.5.2
+        uses: zizmorcore/zizmor-action@3dc1ecc9bcb9e94e9b2c709687979e1298497054 # v0.6.2
         with:
           advanced-security: false
 

Dockerfile

@@ -66,6 +66,9 @@ ENV HTTP_WRITE_TIMEOUT=300
 COPY --from=build --chown=rails:rails /usr/local/bundle /usr/local/bundle
 COPY --from=build --chown=rails:rails /rails /rails
 
+# Install ONCE backup/restore hooks
+COPY --chmod=755 hooks /hooks
+
 # Set version and revision
 ARG APP_VERSION
 ENV APP_VERSION=$APP_VERSION

Gemfile

@@ -41,6 +41,7 @@ gem "rails_autolink"
 gem "geared_pagination"
 gem "jbuilder"
 gem "net-http-persistent"
+gem "surfguard", github: "basecamp/surfguard" # The SSRF address policy behind RestrictedHTTP
 gem "kredis"
 gem "platform_agent"
 gem "thruster"

Gemfile.lock

@@ -1,8 +1,14 @@
+GIT
+  remote: https://github.com/basecamp/surfguard.git
+  revision: 910be917fd0ab782c5ea939698d44f26694a501d
+  specs:
+    surfguard (0.1.0)
+
 GIT
   remote: https://github.com/hotwired/turbo-rails.git
-  revision: 30cd8fcc6f82c1ad4edd1ed6069ba878f21f02b3
+  revision: 37530c08780fa6f6dbb56a633de4b81169bdd174
   specs:
-    turbo-rails (2.0.16)
+    turbo-rails (2.0.23)
       actionpack (>= 7.1.0)
       railties (>= 7.1.0)
 
@@ -136,8 +142,8 @@ GEM
     base64 (0.3.0)
     bcrypt (3.1.22)
     benchmark (0.5.0)
-    bigdecimal (3.3.1)
-    brakeman (8.0.5)
+    bigdecimal (4.1.2)
+    brakeman (8.0.6)
       racc
     builder (3.3.0)
     bundler-audit (0.9.3)
@@ -153,7 +159,7 @@ GEM
       regexp_parser (>= 1.5, < 3.0)
       xpath (~> 3.2)
     chunky_png (1.4.0)
-    concurrent-ruby (1.3.7)
+    concurrent-ruby (1.3.8)
     connection_pool (2.5.5)
     crack (1.0.0)
       bigdecimal
@@ -164,7 +170,7 @@ GEM
       irb (~> 1.10)
       reline (>= 0.3.8)
     drb (2.2.3)
-    erb (6.0.4)
+    erb (6.0.7)
     erubi (1.13.1)
     faker (3.5.2)
       i18n (>= 1.8.11, < 2)
@@ -178,20 +184,21 @@ GEM
     globalid (1.3.0)
       activesupport (>= 6.1)
     hashdiff (1.2.0)
-    i18n (1.14.7)
+    i18n (1.15.2)
       concurrent-ruby (~> 1.0)
     image_processing (1.14.0)
       mini_magick (>= 4.9.5, < 6)
       ruby-vips (>= 2.0.17, < 3)
-    io-console (0.8.1)
-    irb (1.15.3)
+    io-console (0.9.2)
+    irb (1.18.0)
       pp (>= 0.6.0)
+      prism (>= 1.3.0)
       rdoc (>= 4.0.0)
       reline (>= 0.4.2)
     jbuilder (2.14.1)
       actionview (>= 7.0.0)
       activesupport (>= 7.0.0)
-    json (2.20.0)
+    json (2.21.2)
     jwt (3.2.0)
       base64
     kredis (1.8.0)
@@ -204,7 +211,7 @@ GEM
     loofah (2.25.2)
       crass (~> 1.0.2)
       nokogiri (>= 1.12.0)
-    mail (2.9.0)
+    mail (2.9.1)
       logger
       mini_mime (>= 0.1.1)
       net-imap
@@ -215,7 +222,9 @@ GEM
     mini_magick (5.3.1)
       logger
     mini_mime (1.1.5)
-    minitest (5.26.2)
+    minitest (6.0.6)
+      drb (~> 2.0)
+      prism (~> 1.5)
     mocha (2.7.1)
       ruby2_keywords (>= 0.0.5)
     mono_logger (1.1.2)
@@ -251,18 +260,15 @@ GEM
     platform_agent (1.0.1)
       activesupport (>= 5.2.0)
       useragent (~> 0.16.3)
-    pp (0.6.3)
+    pp (0.6.4)
       prettyprint
     prettyprint (0.2.0)
-    prism (1.4.0)
-    psych (5.2.6)
-      date
-      stringio
+    prism (1.9.0)
     public_suffix (6.0.2)
     puma (7.2.1)
       nio4r (~> 2.0)
     racc (1.8.1)
-    rack (3.2.6)
+    rack (3.2.7)
     rack-protection (4.2.1)
       base64 (>= 0.1.0)
       logger (>= 1.6.0)
@@ -272,7 +278,7 @@ GEM
       rack (>= 3.0.0)
     rack-test (2.2.0)
       rack (>= 1.3)
-    rackup (2.2.1)
+    rackup (2.3.1)
       rack (>= 3)
     rails-dom-testing (2.3.0)
       activesupport (>= 5.0.0)
@@ -286,10 +292,15 @@ GEM
       activesupport (> 3.1)
       railties (> 3.1)
     rainbow (3.1.1)
-    rake (13.3.1)
-    rdoc (6.16.1)
+    rake (13.4.2)
+    rbs (4.1.3)
+      logger
+      prism (>= 1.6.0)
+      tsort
+    rdoc (8.0.0)
       erb
-      psych (>= 4.0.0)
+      prism (>= 1.6.0)
+      rbs (>= 4.0.0)
       tsort
     redis (5.4.1)
       redis-client (>= 0.22.0)
@@ -298,7 +309,7 @@ GEM
     redis-namespace (1.11.0)
       redis (>= 4)
     regexp_parser (2.11.2)
-    reline (0.6.3)
+    reline (0.7.0)
       io-console (~> 0.5)
     resque (2.7.0)
       mono_logger (~> 1)
@@ -367,18 +378,17 @@ GEM
       rack-protection (= 4.2.1)
       rack-session (>= 2.0.0, < 3)
       tilt (~> 2.0)
-    sqlite3 (2.9.5-aarch64-linux-gnu)
-    sqlite3 (2.9.5-arm64-darwin)
-    sqlite3 (2.9.5-x86_64-darwin)
-    sqlite3 (2.9.5-x86_64-linux-gnu)
+    sqlite3 (2.9.6-aarch64-linux-gnu)
+    sqlite3 (2.9.6-arm64-darwin)
+    sqlite3 (2.9.6-x86_64-darwin)
+    sqlite3 (2.9.6-x86_64-linux-gnu)
     stimulus-rails (1.3.4)
       railties (>= 6.0.0)
-    stringio (3.1.8)
-    thor (1.4.0)
-    thruster (0.1.15-aarch64-linux)
-    thruster (0.1.15-arm64-darwin)
-    thruster (0.1.15-x86_64-darwin)
-    thruster (0.1.15-x86_64-linux)
+    thor (1.5.0)
+    thruster (0.1.23-aarch64-linux)
+    thruster (0.1.23-arm64-darwin)
+    thruster (0.1.23-x86_64-darwin)
+    thruster (0.1.23-x86_64-linux)
     tilt (2.6.1)
     timeout (0.6.1)
     tsort (0.2.0)
@@ -403,7 +413,7 @@ GEM
     websocket-extensions (0.1.5)
     xpath (3.2.0)
       nokogiri (~> 1.8)
-    zeitwerk (2.7.3)
+    zeitwerk (2.8.3)
 
 PLATFORMS
   aarch64-linux
@@ -442,6 +452,7 @@ DEPENDENCIES
   sentry-ruby
   sqlite3
   stimulus-rails
+  surfguard!
   thruster
   turbo-rails!
   web-push

app/channels/concerns/room_streams_are_authorized.rb

@@ -0,0 +1,13 @@
+# Prepended onto Turbo::StreamsChannel. The subscriber names the channel it wants, so
+# authorizing room messages only in RoomMessagesChannel would leave the stock channel as
+# a way around it: same signed stream name, no membership check. Turn those names away
+# here and RoomMessagesChannel becomes the only door.
+module RoomStreamsAreAuthorized
+  def subscribed
+    if RoomMessagesChannel.guarded_stream?(verified_stream_name_from_params)
+      reject
+    else
+      super
+    end
+  end
+end

app/channels/room_messages_channel.rb

@@ -0,0 +1,55 @@
+# Authorizes the room message stream when the subscription is made, so that revoking a
+# membership actually stops delivery.
+#
+# Turbo's stock channel verifies only the signature on the stream name. That name carries
+# no expiry and no binding to a user, so one harvested while a member keeps working after
+# the membership is gone. Reconnecting makes it worse rather than better: revoking a
+# membership disconnects the user with reconnect: true, and the client then replays its
+# subscriptions on the fresh socket.
+#
+# The room is derived from the verified stream name rather than taken as a parameter, so
+# there's nothing for a subscriber to point somewhere else. The subscriber also doesn't
+# get to choose the channel: Turbo::StreamsChannel turns these stream names away, so this
+# is the only way onto them. See config/initializers/turbo_streams_authorization.rb.
+class RoomMessagesChannel < ApplicationCable::Channel
+  extend  Turbo::Streams::StreamName
+  include Turbo::Streams::StreamName::ClassMethods
+
+  STREAM_SUFFIX = "messages"
+
+  class << self
+    # True for the stream names this channel exists to guard, whoever is asking.
+    def guarded_stream?(stream_name)
+      stream_name.to_s.split(":", 2).second == STREAM_SUFFIX
+    end
+
+    def subscribable_room(user, stream_name)
+      gid_param, suffix = stream_name.to_s.split(":", 2)
+
+      if suffix == STREAM_SUFFIX && room = room_from(gid_param)
+        user.rooms.find_by(id: room.id)
+      end
+    end
+
+    private
+      def room_from(gid_param)
+        GlobalID::Locator.locate gid_param, only: Room
+      rescue ActiveRecord::RecordNotFound
+        nil
+      end
+  end
+
+  def subscribed
+    if stream_name = authorized_stream_name
+      stream_from stream_name
+    else
+      reject
+    end
+  end
+
+  private
+    def authorized_stream_name
+      stream_name = verified_stream_name_from_params
+      stream_name if stream_name.present? && self.class.subscribable_room(current_user, stream_name)
+    end
+end

app/channels/unread_rooms_channel.rb

@@ -1,5 +1,12 @@
 class UnreadRoomsChannel < ApplicationCable::Channel
+  # Scoped per user, like ReadRoomsChannel. A single global stream would tell every
+  # authenticated connection when any room on the account is active, including closed
+  # rooms and direct conversations they're not part of.
+  def self.stream_name_for(user_id)
+    "user_#{user_id}_unreads"
+  end
+
   def subscribed
-    stream_from "unread_rooms"
+    stream_from self.class.stream_name_for(current_user.id)
   end
 end

app/controllers/concerns/raw_request_body.rb

@@ -0,0 +1,12 @@
+module RawRequestBody
+  extend ActiveSupport::Concern
+
+  private
+
+  def raw_request_body
+    request.body.rewind
+    request.body.read.force_encoding("UTF-8")
+  ensure
+    request.body.rewind
+  end
+end

app/controllers/messages/boosts/by_bots_controller.rb

@@ -0,0 +1,39 @@
+class Messages::Boosts::ByBotsController < Messages::BoostsController
+  include RawRequestBody
+
+  allow_bot_access only: %i[ create destroy ]
+
+  before_action :ensure_content_present, only: :create
+
+  def create
+    @boost = @message.boosts.create!(boost_params)
+
+    broadcast_create
+    render :show, status: :created
+  end
+
+  private
+    def set_message
+      if room = Current.user.rooms.find_by(id: params[:room_id])
+        @message = room.messages.find_by(id: params[:message_id])
+      end
+
+      head :not_found unless @message
+    end
+
+    def set_boost
+      super
+    rescue ActiveRecord::RecordNotFound
+      head :not_found
+    end
+
+    def ensure_content_present
+      if raw_request_body.blank?
+        head :unprocessable_content
+      end
+    end
+
+    def boost_params
+      { content: raw_request_body }
+    end
+end

app/controllers/messages/boosts_controller.rb

@@ -1,5 +1,6 @@
 class Messages::BoostsController < ApplicationController
   before_action :set_message
+  before_action :set_boost, only: :destroy
 
   def index
   end
@@ -15,7 +16,6 @@ class Messages::BoostsController < ApplicationController
   end
 
   def destroy
-    @boost = Current.user.boosts.find(params[:id])
     @boost.destroy!
 
     broadcast_remove
@@ -26,6 +26,10 @@ class Messages::BoostsController < ApplicationController
       @message = Current.user.reachable_messages.find(params[:message_id])
     end
 
+    def set_boost
+      @boost = @message.boosts.find_by!(id: params[:id], booster: Current.user)
+    end
+
     def boost_params
       params.require(:boost).permit(:content)
     end

app/controllers/messages/by_bots_controller.rb

@@ -1,24 +1,64 @@
 class Messages::ByBotsController < MessagesController
-  allow_bot_access only: :create
+  include RawRequestBody
+
+  allow_bot_access only: %i[ index create update destroy ]
+
+  before_action :set_room
+  before_action :set_message, only: %i[ update destroy ]
+  before_action :ensure_can_administer, only: %i[ update destroy ]
+  before_action :ensure_body_or_attachment_present, only: :create
+
+  def index
+    @messages = find_paged_messages
+    set_pagination_headers
+  end
 
   def create
     super
     head :created, location: message_url(@message)
   end
 
+  def destroy
+    super
+    head :no_content
+  end
+
   private
+    def set_room
+      @room = Current.user.rooms.find_by(id: params[:room_id])
+
+      head :not_found unless @room
+    end
+
+    def ensure_body_or_attachment_present
+      if params[:attachment].blank? && raw_request_body.blank?
+        head :unprocessable_content
+      end
+    end
+
+    def set_pagination_headers
+      headers["X-Total-Count"] = @room.messages.count.to_s
+
+      if next_page = next_page_params
+        headers["Link"] = %(<#{room_bot_messages_url(@room, params[:bot_key], **next_page)}>; rel="next")
+      end
+    end
+
+    def next_page_params
+      if @messages.any?
+        if params[:after].present?
+          { after: @messages.last.id } if @room.messages.after(@messages.last).exists?
+        else
+          { before: @messages.first.id } if @room.messages.before(@messages.first).exists?
+        end
+      end
+    end
+
     def message_params
       if params[:attachment]
         params.permit(:attachment)
       else
-        reading(request.body) { |body| { body: body } }
+        { body: raw_request_body }
       end
     end
-
-    def reading(io)
-      io.rewind
-      yield io.read.force_encoding("UTF-8")
-    ensure
-      io.rewind
-    end
 end

app/controllers/messages_controller.rb

@@ -37,7 +37,11 @@ class MessagesController < ApplicationController
     @message.update!(message_params)
 
     @message.broadcast_replace_to @room, :messages, target: [ @message, :presentation ], partial: "messages/presentation", attributes: { maintain_scroll: true }
-    redirect_to room_message_url(@room, @message)
+
+    respond_to do |format|
+      format.html { redirect_to room_message_url(@room, @message) }
+      format.json { render :show }
+    end
   end
 
   def destroy

app/controllers/rooms/closeds_controller.rb

@@ -42,6 +42,12 @@ class Rooms::ClosedsController < RoomsController
       @room = @room.becomes!(Rooms::Closed)
     end
 
+    # Open and closed rooms convert into each other, so both are in reach here. Direct
+    # rooms never are: converting one would let its creator revise who's in it.
+    def room_scope
+      Current.user.rooms.without_directs
+    end
+
     def grantees
       User.where(id: grantee_ids)
     end

app/controllers/rooms/directs_controller.rb

@@ -29,8 +29,13 @@ class Rooms::DirectsController < RoomsController
       end
     end
 
-    # All users in a direct room can administer it
+    # All users in a direct room can administer it. Only direct rooms, though: this
+    # relaxation is why room_scope below has to keep every other type out of reach.
     def ensure_can_administer
       true
     end
+
+    def room_scope
+      Current.user.rooms.directs
+    end
 end

app/controllers/rooms/opens_controller.rb

@@ -40,6 +40,12 @@ class Rooms::OpensController < RoomsController
       @room = @room.becomes!(Rooms::Open)
     end
 
+    # Open and closed rooms convert into each other, so both are in reach here. Direct
+    # rooms never are: promoting one would republish its history to the whole account.
+    def room_scope
+      Current.user.rooms.without_directs
+    end
+
     def broadcast_create_room(room)
       broadcast_prepend_to :rooms, target: :shared_rooms, partial: "users/sidebars/rooms/shared", locals: { room: room }
     end

app/controllers/rooms_controller.rb

@@ -20,13 +20,19 @@ class RoomsController < ApplicationController
 
   private
     def set_room
-      if room = Current.user.rooms.find_by(id: params[:room_id] || params[:id])
+      if room = room_scope.find_by(id: params[:room_id] || params[:id])
         @room = room
       else
         redirect_to root_url, alert: "Room not found or inaccessible"
       end
     end
 
+    # Subclasses narrow this to the room types they're allowed to act on, so that one
+    # room namespace can't be used to reach another's rooms.
+    def room_scope
+      Current.user.rooms
+    end
+
     def ensure_can_administer
       head :forbidden unless Current.user.can_administer?(@room)
     end

app/helpers/content_filters.rb

@@ -1,3 +1,3 @@
 module ContentFilters
-  TextMessagePresentationFilters = ActionText::Content::Filters.new(RemoveSoloUnfurledLinkText, StyleUnfurledTwitterAvatars, SanitizeTags)
+  TextMessagePresentationFilters = ActionText::Content::Filters.new(RemoveSoloUnfurledLinkText, StyleUnfurledTwitterAvatars, SanitizeTags, SanitizeAttributes)
 end

app/helpers/content_filters/sanitize_attributes.rb

@@ -0,0 +1,47 @@
+class ContentFilters::SanitizeAttributes < ActionText::Content::Filter
+  def applicable?
+    true
+  end
+
+  # Scrub attributes on the tags that SanitizeTags allows, using Rails' safe-list
+  # sanitizer so unsafe URI schemes and event-handler attributes are stripped.
+  # Runs after SanitizeTags, so passing the same allowed tags makes the tag pass
+  # a no-op and only attributes are scrubbed.
+  def apply
+    sanitizer.sanitize fragment.to_html, tags: allowed_tags, attributes: allowed_attributes
+  end
+
+  private
+    # Presentation styling relies on class attributes (e.g. unfurled link embeds),
+    # which the standard ActionText set doesn't include.
+    EXTRA_ALLOWED_ATTRIBUTES = %w[ class ]
+
+    # ActionText::ContentHelper.sanitizer is a single process-wide instance, and
+    # rails-html-sanitizer reuses one mutable permit scrubber that it reconfigures
+    # from the tags/attributes on every #sanitize call. Sharing it here would race
+    # our stricter tag set against ActionText's default rendering on concurrent
+    # requests, so use a dedicated instance of the same sanitizer class to keep the
+    # scrubber isolated.
+    def sanitizer
+      sanitizer_class.new
+    end
+
+    def sanitizer_class
+      ActionText::ContentHelper.sanitizer.class
+    end
+
+    def allowed_tags
+      ContentFilters::SanitizeTags::ALLOWED_TAGS
+    end
+
+    def allowed_attributes
+      standard_allowed_attributes + EXTRA_ALLOWED_ATTRIBUTES
+    end
+
+    # Mirrors ActionText::ContentHelper#sanitizer_allowed_attributes, which isn't
+    # exposed at the module level.
+    def standard_allowed_attributes
+      ActionText::ContentHelper.allowed_attributes ||
+        (sanitizer_class.allowed_attributes + ActionText::Attachment::ATTRIBUTES).to_a
+    end
+end

app/models/message/broadcasts.rb

@@ -1,10 +1,19 @@
 module Message::Broadcasts
   def broadcast_create
     broadcast_append_to room, :messages, target: [ room, :messages ]
-    ActionCable.server.broadcast("unread_rooms", { roomId: room.id })
+    broadcast_unread_room
   end
 
   def broadcast_remove
     broadcast_remove_to room, :messages
   end
+
+  private
+    # Fanned out to the room's members rather than published on one global stream, so
+    # that the timing of activity in a room only reaches people who are in it.
+    def broadcast_unread_room
+      room.memberships.pluck(:user_id).each do |user_id|
+        ActionCable.server.broadcast UnreadRoomsChannel.stream_name_for(user_id), { roomId: room.id }
+      end
+    end
 end

app/models/room.rb

@@ -22,6 +22,8 @@ class Room < ApplicationRecord
 
   belongs_to :creator, class_name: "User", default: -> { Current.user }
 
+  validate :direct_rooms_keep_their_type, on: :update
+
   scope :opens,           -> { where(type: "Rooms::Open") }
   scope :closeds,         -> { where(type: "Rooms::Closed") }
   scope :directs,         -> { where(type: "Rooms::Direct") }
@@ -65,6 +67,15 @@ class Room < ApplicationRecord
   end
 
   private
+    # Open and closed rooms convert into each other freely. A direct room can't become
+    # either: its participants agreed to a private conversation, not to one whose
+    # audience someone else gets to widen afterwards.
+    def direct_rooms_keep_their_type
+      if type_changed? && type_was == "Rooms::Direct"
+        errors.add :type, "can't be changed for a direct room"
+      end
+    end
+
     def unread_memberships(message)
       memberships.visible.disconnected.where.not(user: message.creator).update_all(unread_at: message.created_at, updated_at: Time.current)
     end

app/views/messages/_message.html.erb

@@ -1,6 +1,7 @@
 <%# Be sure to check/update messages/_template.html.erb when changing this file %>
 
-<% cache message do %>
+<%# Bump this version when the message presentation filters change what they emit. Editing this line changes the template digest, which busts BOTH this fragment cache and the collection cache that keys on this partial's digest (helper Ruby changes alone don't). %>
+<% cache [ message, "presentation-v2" ] do %>
   <%= message_tag message do %>
     <h2 class="message__day-separator"><%= local_datetime_tag message.created_at, style: :date %></h2>
 

app/views/messages/_message.json.jbuilder

@@ -0,0 +1,18 @@
+json.cache! message do
+  json.(message, :id)
+
+  json.created_at message.created_at.utc
+
+  json.body do
+    json.plain_text message.plain_text_body
+    json.html message.body.to_s
+  end
+
+  json.creator message.creator, partial: "users/user", as: :user
+
+  json.room do
+    json.id message.room_id
+  end
+
+  json.url room_message_url(message.room, message)
+end

app/views/messages/boosts/_boost.json.jbuilder

@@ -0,0 +1,12 @@
+json.cache! boost do
+  json.(boost, :id, :content)
+
+  json.created_at boost.created_at.utc
+
+  json.booster boost.booster, partial: "users/user", as: :user
+
+  json.message do
+    json.id boost.message_id
+    json.url room_message_url(boost.message.room, boost.message)
+  end
+end

app/views/messages/boosts/by_bots/show.json.jbuilder

@@ -0,0 +1 @@
+json.partial! "messages/boosts/boost", boost: @boost

app/views/messages/by_bots/index.json.jbuilder

@@ -0,0 +1 @@
+json.array! @messages, partial: "messages/message", as: :message

app/views/messages/by_bots/show.json.jbuilder

@@ -0,0 +1 @@
+json.partial! "messages/message", message: @message

app/views/rooms/show.html.erb

@@ -17,7 +17,7 @@
     <%= render partial: "messages/message", collection: @messages, cached: true %>
   <% end %>
 
-  <%= turbo_stream_from @room, :messages %>
+  <%= turbo_stream_from @room, :messages, channel: "RoomMessagesChannel" %>
   <%= button_to_jump_to_newest_message %>
 <% end %>
 

app/views/users/_user.json.jbuilder

@@ -0,0 +1,5 @@
+json.cache! user do
+  json.(user, :id, :name, :role)
+
+  json.avatar_url fresh_user_avatar_url(user)
+end

bin/brakeman

@@ -2,6 +2,6 @@
 require "rubygems"
 require "bundler/setup"
 
-ARGV.unshift("--ensure-latest")
+ARGV.unshift("--ensure-latest", "15")
 
 load Gem.bin_path("brakeman", "brakeman")

bin/release

@@ -15,6 +15,7 @@ include SSHKit::DSL
 ARCH = "linux/amd64,linux/arm64"
 BUILDER_NAME = "campfire-builder"
 GIT_SHA = `git rev-parse HEAD`.strip
+MANUAL_NOTES = ARGV.delete("--notes")
 VERSION = ARGV[0]
 
 DOCKER_IMAGE_NAME = "registry.once.com/campfire"
@@ -28,7 +29,7 @@ EXPORT_REMOTE_USER = "app"
 EXPORT_DEPLOY_HOSTS = %w[ once-store-app-101 once-store-app-102 ]
 EXPORT_DEPLOY_LOCATION = "/local/app/product_releases/"
 
-abort "You must specify a version, e.g. `bin/release 1.0.0`" unless VERSION
+abort "You must specify a version, e.g. `bin/release 1.0.0` (pass --notes to write release notes manually)" unless VERSION
 
 current_branch = `git rev-parse --abbrev-ref HEAD`.strip
 if current_branch != "main"
@@ -57,19 +58,25 @@ on(:local) do
 end
 
 announcing "Creating GitHub release v#{VERSION}..." do
-  notes_file = "tmp/release_notes.md"
-
   on(:local) do
-    execute :mkdir, "-p", "tmp"
+    if MANUAL_NOTES
+      notes_file = "tmp/release_notes.md"
 
-    File.write(notes_file, "# What's Changed\n\n- \n")
-    system ENV["EDITOR"] || "vim", notes_file
+      execute :mkdir, "-p", "tmp"
 
-    unless system "gh", "release", "create", "v#{VERSION}", "--title", "v#{VERSION}", "--notes-file", notes_file
-      abort "Failed to create release; check that the version doesn't already exist"
+      File.write(notes_file, "# What's Changed\n\n- \n")
+      system ENV["EDITOR"] || "vim", notes_file
+
+      unless system "gh", "release", "create", "v#{VERSION}", "--title", "v#{VERSION}", "--notes-file", notes_file
+        abort "Failed to create release; check that the version doesn't already exist"
+      end
+
+      execute :rm, notes_file
+    else
+      unless system "gh", "release", "create", "v#{VERSION}", "--title", "v#{VERSION}", "--generate-notes"
+        abort "Failed to create release; check that the version doesn't already exist"
+      end
     end
-
-    execute :rm, notes_file
   end
 end
 

config/initializers/turbo_streams_authorization.rb

@@ -0,0 +1,3 @@
+Rails.application.config.to_prepare do
+  Turbo::StreamsChannel.prepend RoomStreamsAreAuthorized
+end

config/routes.rb

@@ -62,7 +62,13 @@ Rails.application.routes.draw do
   resources :rooms do
     resources :messages
 
-    post ":bot_key/messages", to: "messages/by_bots#create", as: :bot_messages
+    nested do
+      scope path: ":bot_key", as: :bot, defaults: { format: :json } do
+        resources :messages, controller: "messages/by_bots", only: %i[ index create update destroy ] do
+          resources :boosts, controller: "messages/boosts/by_bots", only: %i[ create destroy ]
+        end
+      end
+    end
 
     scope module: "rooms" do
       resource :refresh, only: :show

hooks/post-restore

@@ -0,0 +1,12 @@
+#!/bin/bash
+set -euo pipefail
+
+db_name="${RAILS_ENV:-production}.sqlite3"
+snapshot="/rails/storage/backups/$db_name"
+db="/rails/storage/db/$db_name"
+
+if [ -f "$snapshot" ]; then
+  mkdir -p "$(dirname "$db")"
+  cp "$snapshot" "$db"
+  rm -f "$db-wal" "$db-shm"
+fi

hooks/pre-backup

@@ -0,0 +1,2 @@
+#!/bin/bash
+exec /rails/script/admin/prepare-backup

lib/restricted_http/private_network_guard.rb

@@ -1,76 +1,29 @@
-require "ipaddr"
-require "resolv"
+require "surfguard"
 
 module RestrictedHTTP
   class Violation < StandardError; end
 
+  # The address policy lives in the surfguard gem so this app, Basecamp, HEY and
+  # Fizzy classify "internal" the same way instead of each keeping a copy that
+  # drifts. The callers here hand a bare hostname in and pin the address that
+  # comes back, so this stays a hostname-in, address-out shim.
   module PrivateNetworkGuard
     extend self
 
-    # IPv4 special-use ranges (RFC 5735/6890) not already covered by the
-    # private?/loopback?/link_local? predicates in #disallowed_ipv4?.
-    DISALLOWED_IPV4 = %w[
-      0.0.0.0/8 100.64.0.0/10 192.0.0.0/24 192.0.2.0/24 192.88.99.0/24
-      198.18.0.0/15 198.51.100.0/24 203.0.113.0/24 224.0.0.0/4 240.0.0.0/4
-    ].map { |cidr| IPAddr.new(cidr) }.freeze
-
-    # IPv6 special-use ranges not caught by the predicates. 6to4 (2002::/16) and
-    # Teredo (2001::/32) are deprecated transition mechanisms with no legitimate
-    # fetch target, so they are blocked outright. ULA (fc00::/7, incl. the AWS
-    # IMDSv6 address fd00:ec2::254), link-local, and loopback are covered by the
-    # predicates in #disallowed_ipv6?.
-    DISALLOWED_IPV6 = %w[
-      ::/128 100::/64 2001::/32 2001:2::/48 2001:db8::/32 2002::/16
-      fec0::/10 ff00::/8
-    ].map { |cidr| IPAddr.new(cidr) }.freeze
-
-    # NAT64 prefixes: the well-known prefix (RFC 6052/6146) and the local-use
-    # prefix (RFC 8215). An address here embeds an IPv4 target in its low 32
-    # bits; we extract it and re-check against the IPv4 rules so NAT64 to a
-    # public address still resolves while NAT64 to an internal address is
-    # blocked.
-    NAT64_PREFIXES = [
-      IPAddr.new("64:ff9b::/96"),
-      IPAddr.new("64:ff9b:1::/48")
-    ].freeze
-
+    # A hostname that resolves to nothing (NXDOMAIN, timeout, empty answer)
+    # raises Surfguard::Unresolvable, which we let propagate as a lookup failure
+    # -- the same way the old Resolv.getaddress guard raised Resolv::ResolvError,
+    # and the callers here already treat it as a fetch failure. The Violation is
+    # reserved for a host that resolves to a blocked address (an empty list back
+    # from resolve_public_ips), so a transient DNS miss is never misreported as
+    # an SSRF attempt.
     def resolve(hostname)
-      Resolv.getaddress(hostname).tap do |ip|
-        raise Violation.new("Attempt to access private IP via #{hostname}") if ip && private_ip?(ip)
-      end
+      Surfguard.resolve_public_ips(hostname).first or
+        raise Violation.new("Attempt to access private IP via #{hostname}")
     end
 
     def private_ip?(ip)
-      ipaddr = IPAddr.new(ip)
-
-      # DNS never legitimately returns these embedded forms, so block them all
-      # regardless of the address they wrap.
-      if ipaddr.ipv4_mapped? || ipaddr.ipv4_compat?
-        true
-      elsif ipaddr.ipv4?
-        disallowed_ipv4?(ipaddr)
-      elsif NAT64_PREFIXES.any? { |prefix| prefix.include?(ipaddr) }
-        disallowed_ipv4?(embedded_ipv4(ipaddr))
-      else
-        disallowed_ipv6?(ipaddr)
-      end
-    rescue IPAddr::InvalidAddressError
-      true
+      Surfguard.blocked_address?(ip)
     end
-
-    private
-      def disallowed_ipv4?(ipaddr)
-        ipaddr.private? || ipaddr.loopback? || ipaddr.link_local? ||
-          DISALLOWED_IPV4.any? { |range| range.include?(ipaddr) }
-      end
-
-      def disallowed_ipv6?(ipaddr)
-        ipaddr.private? || ipaddr.loopback? || ipaddr.link_local? ||
-          DISALLOWED_IPV6.any? { |range| range.include?(ipaddr) }
-      end
-
-      def embedded_ipv4(ipaddr)
-        IPAddr.new([ ipaddr.to_i & 0xffffffff ].pack("N").unpack("C4").join("."))
-      end
   end
 end

test/channels/room_messages_channel_test.rb

@@ -0,0 +1,86 @@
+require "test_helper"
+
+class RoomMessagesChannelTest < ActionCable::Channel::TestCase
+  tests RoomMessagesChannel
+
+  setup do
+    @room = rooms(:designers)
+    @signed_stream_name = Turbo::StreamsChannel.signed_stream_name [ @room, :messages ]
+  end
+
+  test "a member may subscribe to a room's message stream" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: @signed_stream_name
+
+    assert subscription.confirmed?
+    assert_has_stream Turbo.signed_stream_verifier.verified(@signed_stream_name)
+  end
+
+  test "a user who was never a member may not subscribe" do
+    stub_connection(current_user: users(:bender))
+
+    subscribe signed_stream_name: @signed_stream_name
+
+    assert subscription.rejected?
+  end
+
+  test "a revoked member may not re-subscribe with a stream name harvested while a member" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: @signed_stream_name
+    assert subscription.confirmed?, "kevin must start out able to subscribe"
+
+    @room.memberships.revoke_from users(:kevin)
+
+    subscribe signed_stream_name: @signed_stream_name
+
+    assert subscription.rejected?
+  end
+
+  test "an unsigned stream name is rejected" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: Turbo.signed_stream_verifier.verified(@signed_stream_name)
+
+    assert subscription.rejected?
+  end
+
+  test "a missing stream name is rejected" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe
+
+    assert subscription.rejected?
+  end
+
+  test "a validly signed stream name for another room the user isn't in is rejected" do
+    stub_connection(current_user: users(:bender))
+
+    subscribe signed_stream_name: Turbo::StreamsChannel.signed_stream_name([ rooms(:hq), :messages ])
+
+    assert subscription.rejected?
+  end
+end
+
+class RoomMessagesViaStockTurboChannelTest < ActionCable::Channel::TestCase
+  tests Turbo::StreamsChannel
+
+  # The subscriber picks the channel, so the stock channel has to turn these names away
+  # too. Otherwise a revoked member just names Turbo::StreamsChannel instead.
+  test "the stock turbo channel refuses to serve a room message stream" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: Turbo::StreamsChannel.signed_stream_name([ rooms(:designers), :messages ])
+
+    assert subscription.rejected?
+  end
+
+  test "the stock turbo channel still serves the room list stream" do
+    stub_connection(current_user: users(:kevin))
+
+    subscribe signed_stream_name: Turbo::StreamsChannel.signed_stream_name([ :rooms ])
+
+    assert subscription.confirmed?
+  end
+end

test/channels/unread_rooms_channel_test.rb

@@ -0,0 +1,47 @@
+require "test_helper"
+
+class UnreadRoomsChannelTest < ActionCable::Channel::TestCase
+  test "streams only the subscriber's own unread stream" do
+    stub_connection(current_user: users(:jz))
+
+    subscribe
+
+    assert subscription.confirmed?
+    assert_has_stream "user_#{users(:jz).id}_unreads"
+    assert_not_includes subscription.streams, "unread_rooms"
+  end
+
+  test "an outsider is not told about activity in a room they can't see" do
+    direct = rooms(:bender_and_kevin)
+    assert_not direct.users.include?(users(:jz)), "jz must be an outsider for this test to mean anything"
+
+    broadcasts = capture_unread_broadcasts_for(users(:jz)) do
+      direct.messages.create!(body: "Private", creator: users(:kevin), client_message_id: "outsider").broadcast_create
+    end
+
+    assert_empty broadcasts
+  end
+
+  test "a member is told about activity in their own room" do
+    direct = rooms(:bender_and_kevin)
+
+    broadcasts = capture_unread_broadcasts_for(users(:kevin)) do
+      direct.messages.create!(body: "Private", creator: users(:bender), client_message_id: "member").broadcast_create
+    end
+
+    assert_equal [ direct.id ], broadcasts.collect { |broadcast| broadcast["roomId"] }
+  end
+
+  private
+    def capture_unread_broadcasts_for(user)
+      stub_connection(current_user: user)
+      subscribe
+
+      stream = subscription.streams.sole
+      before = ActionCable.server.pubsub.broadcasts(stream).size
+
+      yield
+
+      ActionCable.server.pubsub.broadcasts(stream).drop(before).collect { |broadcast| JSON.parse(broadcast) }
+    end
+end

test/controllers/messages/boosts/by_bots_controller_test.rb

@@ -0,0 +1,112 @@
+require "test_helper"
+
+class Messages::Boosts::ByBotsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @room = rooms(:watercooler)
+    @message = messages(:fourth)
+    @bot = users(:bender)
+  end
+
+  test "create adds a boost to the message and returns it" do
+    assert_difference -> { @message.boosts.count }, +1 do
+      post room_bot_message_boosts_url(@room, @bot.bot_key, @message), params: +"👀"
+      assert_response :created
+    end
+
+    boost = @message.boosts.last
+    assert_equal "👀", boost.content
+    assert_equal @bot, boost.booster
+
+    json = JSON.parse(response.body)
+    assert_equal boost.id, json["id"]
+    assert_equal "👀", json["content"]
+    assert_equal @bot.id, json["booster"]["id"]
+    assert_equal @message.id, json["message"]["id"]
+    assert_equal room_message_url(@room, @message), json["message"]["url"]
+  end
+
+  test "create with text content" do
+    assert_difference -> { Boost.count }, +1 do
+      post room_bot_message_boosts_url(@room, @bot.bot_key, @message), params: +"Nice!"
+      assert_response :created
+    end
+
+    assert_equal "Nice!", @message.boosts.last.content
+  end
+
+  test "create broadcasts the boost" do
+    assert_turbo_stream_broadcasts [ @message.room, :messages ], count: 1 do
+      post room_bot_message_boosts_url(@room, @bot.bot_key, @message), params: +"👍"
+    end
+  end
+
+  test "create without content" do
+    assert_no_difference -> { Boost.count } do
+      post room_bot_message_boosts_url(@room, @bot.bot_key, @message)
+      assert_response :unprocessable_content
+
+      post room_bot_message_boosts_url(@room, @bot.bot_key, @message), params: +"   "
+      assert_response :unprocessable_content
+    end
+  end
+
+  test "create requires a valid bot key" do
+    assert_no_difference -> { Boost.count } do
+      post room_bot_message_boosts_url(@room, "invalid-bot-key", @message), params: +"👀"
+    end
+    assert_response :redirect
+  end
+
+  test "create is not found for a room the bot is not a member of" do
+    assert_no_difference -> { Boost.count } do
+      post room_bot_message_boosts_url(rooms(:designers), @bot.bot_key, messages(:first)), params: +"👀"
+    end
+    assert_response :not_found
+  end
+
+  test "create is not found for a message outside the room" do
+    assert_no_difference -> { Boost.count } do
+      post room_bot_message_boosts_url(@room, @bot.bot_key, messages(:first)), params: +"👀"
+    end
+    assert_response :not_found
+  end
+
+  test "create can't be abused to post boosts as a regular user" do
+    bot_key = "#{users(:kevin).id}-"
+
+    assert_no_difference -> { Boost.count } do
+      post room_bot_message_boosts_url(@room, bot_key, @message), params: +"👀"
+    end
+    assert_response :redirect
+  end
+
+  test "destroy removes the bot's own boost" do
+    assert_difference -> { Boost.count }, -1 do
+      delete room_bot_message_boost_url(@room, @bot.bot_key, @message, boosts(:fourth_by_bender))
+    end
+
+    assert_response :no_content
+  end
+
+  test "destroy broadcasts the removal" do
+    assert_turbo_stream_broadcasts [ @message.room, :messages ], count: 1 do
+      delete room_bot_message_boost_url(@room, @bot.bot_key, @message, boosts(:fourth_by_bender))
+    end
+  end
+
+  test "destroy can't touch a boost the bot did not make" do
+    assert_no_difference -> { Boost.count } do
+      delete room_bot_message_boost_url(@room, @bot.bot_key, messages(:thirteenth), boosts(:thirteenth))
+    end
+
+    assert_response :not_found
+  end
+
+  test "destroy requires a valid bot key" do
+    assert_no_difference -> { Boost.count } do
+      delete room_bot_message_boost_url(@room, "invalid-bot-key", @message, boosts(:fourth_by_bender))
+    end
+
+    assert_response :redirect
+  end
+end

test/controllers/messages/by_bots_controller_test.rb

@@ -1,6 +1,6 @@
 require "test_helper"
 
-class Messages::ByBotsControlleTest < ActionDispatch::IntegrationTest
+class Messages::ByBotsControllerTest < ActionDispatch::IntegrationTest
   setup do
     @room = rooms(:watercooler)
   end
@@ -40,6 +40,16 @@ class Messages::ByBotsControlleTest < ActionDispatch::IntegrationTest
     end
   end
 
+  test "create without a body or attachment" do
+    assert_no_difference -> { Message.count } do
+      post room_bot_messages_url(@room, users(:bender).bot_key)
+      assert_response :unprocessable_content
+
+      post room_bot_messages_url(@room, users(:bender).bot_key), params: +"   "
+      assert_response :unprocessable_content
+    end
+  end
+
   test "create can't be abused to post messages as any user" do
     user = users(:kevin)
     bot_key = "#{user.id}-"
@@ -51,8 +61,175 @@ class Messages::ByBotsControlleTest < ActionDispatch::IntegrationTest
     assert_response :redirect
   end
 
-  test "denied index" do
-    get room_messages_url(@room, bot_key: users(:bender).bot_key, format: :json)
+  test "index returns the room's messages in the order they were sent" do
+    get room_bot_messages_url(@room, users(:bender).bot_key)
+    assert_response :success
+
+    json = JSON.parse(response.body)
+    assert_equal @room.messages.ordered.map(&:id), json.map { it["id"] }
+  end
+
+  test "index includes message details" do
+    post room_bot_messages_url(@room, users(:bender).bot_key), params: +"Hello from Bender!"
+
+    get room_bot_messages_url(@room, users(:bender).bot_key)
+    assert_response :success
+
+    message = Message.last
+    json_message = JSON.parse(response.body).last
+    assert_equal message.id, json_message["id"]
+    assert_equal "Hello from Bender!", json_message["body"]["plain_text"]
+    assert_includes json_message["body"]["html"], "Hello from Bender!"
+    assert_equal message.created_at.utc.iso8601(3), json_message["created_at"]
+    assert_equal users(:bender).id, json_message["creator"]["id"]
+    assert_equal "Bender Bot", json_message["creator"]["name"]
+    assert_equal "bot", json_message["creator"]["role"]
+    assert_equal @room.id, json_message["room"]["id"]
+    assert_equal room_message_url(@room, message), json_message["url"]
+  end
+
+  test "index pages through older messages with the Link header" do
+    (Message::PAGE_SIZE - @room.messages.count + 1).times do |i|
+      @room.messages.create!(body: "Filler #{i}", creator: users(:jason), client_message_id: "filler-#{i}")
+    end
+
+    get room_bot_messages_url(@room, users(:bender).bot_key)
+    assert_response :success
+
+    json = JSON.parse(response.body)
+    assert_equal Message::PAGE_SIZE, json.size
+    assert_equal "41", response.headers["X-Total-Count"]
+    assert_not_includes json.map { it["id"] }, messages(:fourth).id
+
+    get response.headers["Link"][/<(.*)>/, 1]
+    assert_response :success
+
+    json = JSON.parse(response.body)
+    assert_equal [ messages(:fourth).id ], json.map { it["id"] }
+    assert_nil response.headers["Link"]
+  end
+
+  test "index pages newer messages with after" do
+    get room_bot_messages_url(@room, users(:bender).bot_key, after: messages(:tenth).id)
+    assert_response :success
+
+    json = JSON.parse(response.body)
+    assert_equal %i[ eleventh twelfth thirteenth ].map { messages(it).id }, json.map { it["id"] }
+    assert_nil response.headers["Link"]
+  end
+
+  test "index in a room with no messages" do
+    get room_bot_messages_url(rooms(:bender_and_kevin), users(:bender).bot_key)
+    assert_response :success
+
+    assert_equal [], JSON.parse(response.body)
+    assert_equal "0", response.headers["X-Total-Count"]
+    assert_nil response.headers["Link"]
+  end
+
+  test "index requires a valid bot key" do
+    get room_bot_messages_url(@room, "invalid-bot-key")
+    assert_response :redirect
+  end
+
+  test "index is not found for a room the bot is not a member of" do
+    get room_bot_messages_url(rooms(:designers), users(:bender).bot_key)
+    assert_response :not_found
+  end
+
+  test "create is not found for a room the bot is not a member of" do
+    assert_no_difference -> { Message.count } do
+      post room_bot_messages_url(rooms(:designers), users(:bender).bot_key), params: +"Hello!"
+    end
+    assert_response :not_found
+  end
+
+  test "regular messages index remains denied for bots" do
+    get room_messages_url(@room, bot_key: users(:bender).bot_key)
     assert_response :forbidden
   end
+
+  test "update" do
+    message = post_bot_message "Deploying..."
+
+    assert_no_difference -> { Message.count } do
+      patch room_bot_message_url(@room, users(:bender).bot_key, message), params: +"Deployed."
+    end
+
+    assert_response :ok
+    assert_equal "Deployed.", message.reload.plain_text_body
+
+    json = JSON.parse(response.body)
+    assert_equal message.id, json["id"]
+    assert_equal "Deployed.", json["body"]["plain_text"]
+    assert_equal users(:bender).id, json["creator"]["id"]
+    assert_equal room_message_url(@room, message), json["url"]
+  end
+
+  test "update with UTF-8 content" do
+    message = post_bot_message "Deploying..."
+
+    patch room_bot_message_url(@room, users(:bender).bot_key, message), params: +"Deployed 🚀!"
+
+    assert_response :ok
+    assert_equal "Deployed 🚀!", message.reload.plain_text_body
+    assert_equal "Deployed 🚀!", JSON.parse(response.body)["body"]["plain_text"]
+  end
+
+  test "update can't touch a message the bot did not create" do
+    message = messages(:fourth)
+    original = message.plain_text_body
+
+    patch room_bot_message_url(@room, users(:bender).bot_key, message), params: +"Hijacked!"
+
+    assert_response :forbidden
+    assert_equal original, message.reload.plain_text_body
+  end
+
+  test "update is not found for a room the bot is not a member of" do
+    message = messages(:first)
+    original = message.plain_text_body
+
+    patch room_bot_message_url(rooms(:designers), users(:bender).bot_key, message), params: +"Hijacked!"
+
+    assert_response :not_found
+    assert_equal original, message.reload.plain_text_body
+  end
+
+  test "update can't be abused to edit messages as any user" do
+    message = messages(:fourth)
+    bot_key = "#{users(:jz).id}-"
+    original = message.plain_text_body
+
+    patch room_bot_message_url(@room, bot_key, message), params: +"Hijacked!"
+
+    assert_response :redirect
+    assert_equal original, message.reload.plain_text_body
+  end
+
+  test "destroy" do
+    message = post_bot_message "Deploying..."
+
+    assert_difference -> { Message.count }, -1 do
+      delete room_bot_message_url(@room, users(:bender).bot_key, message)
+    end
+
+    assert_response :no_content
+  end
+
+  test "destroy can't touch a message the bot did not create" do
+    message = messages(:fourth)
+
+    assert_no_difference -> { Message.count } do
+      delete room_bot_message_url(@room, users(:bender).bot_key, message)
+    end
+
+    assert_response :forbidden
+  end
+
+  private
+    def post_bot_message(body)
+      post room_bot_messages_url(@room, users(:bender).bot_key), params: +body
+      Message.last
+    end
 end

test/controllers/messages_controller_test.rb

@@ -57,9 +57,22 @@ class MessagesControllerTest < ActionDispatch::IntegrationTest
     end
   end
 
-  test "creating a message broadcasts unread room" do
-    assert_broadcasts "unread_rooms", 1 do
-      post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one", client_message_id: 999 } }
+  test "creating a message broadcasts unread room to each member" do
+    @room.users.each do |member|
+      assert_broadcasts UnreadRoomsChannel.stream_name_for(member.id), 1 do
+        post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one #{member.id}", client_message_id: member.id } }
+      end
+    end
+  end
+
+  test "creating a message doesn't broadcast unread room to non-members" do
+    outsiders = User.where.not(id: @room.users.map(&:id))
+    assert outsiders.any?, "need someone outside the room for this test to mean anything"
+
+    outsiders.each do |outsider|
+      assert_no_broadcasts UnreadRoomsChannel.stream_name_for(outsider.id) do
+        post room_messages_url(@room, format: :turbo_stream), params: { message: { body: "New one", client_message_id: 999 } }
+      end
     end
   end
 

test/controllers/rooms/closeds_controller_test.rb

@@ -66,6 +66,18 @@ class Rooms::ClosedsControllerTest < ActionDispatch::IntegrationTest
     assert rooms(:designers).reload.name, "Designers"
   end
 
+  test "a direct room can't be converted to closed and have its participants revised" do
+    sign_in :kevin
+    direct = rooms(:bender_and_kevin)
+
+    put rooms_closed_url(direct), params: {
+      room: { name: "Watercooler" }, user_ids: [ users(:kevin).id, users(:jz).id ]
+    }
+
+    assert_equal "Rooms::Direct", Room.find(direct.id).type
+    assert_equal [ users(:bender).id, users(:kevin).id ].sort, Room.find(direct.id).user_ids.sort
+  end
+
   test "remove yourself" do
     assert_difference -> { users(:david).rooms.count }, -1 do
       put rooms_closed_url(rooms(:designers), params: { room: { name: "Designers" }, user_ids: [ users(:jason).id, users(:jz).id ] })

test/controllers/rooms/directs_controller_test.rb

@@ -29,4 +29,34 @@ class Rooms::DirectsControllerTest < ActionDispatch::IntegrationTest
       assert_redirected_to root_url
     end
   end
+
+  test "destroy can't reach a closed room the member didn't create" do
+    sign_in :kevin
+
+    assert_no_difference -> { Room.count } do
+      delete rooms_direct_url(rooms(:designers))
+    end
+
+    assert rooms(:designers).reload.persisted?
+  end
+
+  test "destroy can't reach an open room the member didn't create" do
+    sign_in :kevin
+
+    assert_no_difference -> { Room.count } do
+      delete rooms_direct_url(rooms(:hq))
+    end
+
+    assert rooms(:hq).reload.persisted?
+  end
+
+  test "destroy can't reach a room the member isn't in at all" do
+    sign_in :jz
+
+    assert_no_difference -> { Room.count } do
+      delete rooms_direct_url(rooms(:david_and_kevin))
+    end
+
+    assert rooms(:david_and_kevin).reload.persisted?
+  end
 end

test/controllers/rooms/opens_controller_test.rb

@@ -57,4 +57,23 @@ class Rooms::OpensControllerTest < ActionDispatch::IntegrationTest
     put rooms_open_url(rooms(:designers)), params: { room: { name: "Doesn't matter" } }
     assert_equal rooms(:designers).memberships.count, User.count
   end
+
+  test "a direct room can't be promoted to open by its creator" do
+    sign_in :kevin
+    direct = rooms(:bender_and_kevin)
+
+    put rooms_open_url(direct), params: { room: { name: "Watercooler" } }
+
+    assert_equal "Rooms::Direct", Room.find(direct.id).type
+    assert_equal [ users(:bender).id, users(:kevin).id ].sort, Room.find(direct.id).user_ids.sort
+  end
+
+  test "a direct room can't be promoted to open by an administrator either" do
+    direct = rooms(:david_and_kevin)
+
+    put rooms_open_url(direct), params: { room: { name: "Watercooler" } }
+
+    assert_equal "Rooms::Direct", Room.find(direct.id).type
+    assert_equal [ users(:david).id, users(:kevin).id ].sort, Room.find(direct.id).user_ids.sort
+  end
 end

test/fixtures/boosts.yml

@@ -6,3 +6,7 @@ thirteenth:
   message: thirteenth
   booster: jason
   content: 💯
+fourth_by_bender:
+  message: fourth
+  booster: bender
+  content: 👀

test/helpers/content_filters_test.rb

@@ -61,6 +61,75 @@ class ContentFiltersTest < ActionView::TestCase
     assert_equal "Hello World", filtered.to_html
   end
 
+  test "message with a link using an unsafe URI scheme" do
+    message = Message.create! room: rooms(:pets), body: '<div><a href="javascript:alert(1)">x</a></div>', client_message_id: "0015", creator: users(:jason)
+
+    filtered = ContentFilters::TextMessagePresentationFilters.apply(message.body.body)
+    assert_no_match /javascript:/, filtered.to_html
+    assert_match /<a>x<\/a>/, filtered.to_html
+  end
+
+  test "message with an event handler attribute on an allowed tag" do
+    message = Message.create! room: rooms(:pets), body: '<div><a href="/x" onmouseover="alert(1)">x</a> <span onclick="alert(2)">y</span></div>', client_message_id: "0015", creator: users(:jason)
+
+    filtered = ContentFilters::TextMessagePresentationFilters.apply(message.body.body)
+    assert_no_match /onmouseover/, filtered.to_html
+    assert_no_match /onclick/, filtered.to_html
+    assert_match /<a href="\/x">x<\/a>/, filtered.to_html
+    assert_match /<span>y<\/span>/, filtered.to_html
+  end
+
+  test "message with a data URI link" do
+    message = Message.create! room: rooms(:pets), body: '<div><a href="data:text/html,pwned">x</a></div>', client_message_id: "0015", creator: users(:jason)
+
+    filtered = ContentFilters::TextMessagePresentationFilters.apply(message.body.body)
+    assert_no_match /data:/, filtered.to_html
+    assert_match /<a>x<\/a>/, filtered.to_html
+  end
+
+  test "message with a safe link and formatting is preserved" do
+    body = '<div><a href="https://example.com">example</a> <strong>bold</strong> <code>code</code><ul><li>one</li><li>two</li></ul></div>'
+    message = Message.create! room: rooms(:pets), body: body, client_message_id: "0015", creator: users(:jason)
+
+    filtered = ContentFilters::TextMessagePresentationFilters.apply(message.body.body)
+    assert_match /<a href="https:\/\/example\.com">example<\/a>/, filtered.to_html
+    assert_match /<strong>bold<\/strong>/, filtered.to_html
+    assert_match /<code>code<\/code>/, filtered.to_html
+    assert_match /<ul>\s*<li>one<\/li>\s*<li>two<\/li>\s*<\/ul>/, filtered.to_html
+  end
+
+  test "SanitizeAttributes uses an isolated sanitizer rather than the shared ActionText instance" do
+    filter = ContentFilters::SanitizeAttributes.new(ActionText::Content.new("<div>x</div>"))
+
+    isolated = filter.send(:sanitizer)
+    assert_not_same ActionText::ContentHelper.sanitizer, isolated,
+      "must not reuse the process-wide ActionText sanitizer, whose permit scrubber is mutated per call"
+    assert_not_same isolated, filter.send(:sanitizer),
+      "each call must build a fresh sanitizer so concurrent requests never share scrubber state"
+    assert_kind_of ActionText::ContentHelper.sanitizer.class, isolated
+  end
+
+  test "SanitizeAttributes neutralizes unsafe input and preserves benign content on its isolated sanitizer" do
+    body = <<~HTML.squish
+      <div><a href="javascript:alert(1)" onclick="x()">link</a>
+      <a href="data:text/html,pwned">data</a>
+      <span class="cf-twitter-avatar" onmouseover="y()">avatar</span>
+      <img src="https://evil.example/x.svg"> Hey #{mention_attachment_for(:david)}</div>
+    HTML
+    message = Message.create! room: rooms(:pets), body: body, client_message_id: "0015", creator: users(:jason)
+
+    filtered = ContentFilters::SanitizeAttributes.apply(message.body.body).to_html
+
+    assert_no_match /javascript:/, filtered
+    assert_no_match /data:text\/html/, filtered
+    assert_no_match /onclick/, filtered
+    assert_no_match /onmouseover/, filtered
+    assert_no_match /evil\.example/, filtered
+    assert_match /<span class="cf-twitter-avatar">avatar<\/span>/, filtered
+    assert_match />link</, filtered
+    assert_match /<action-text-attachment sgid="#{users(:david).attachable_sgid}"/, filtered
+  end
+
   test "message with a mention attachment" do
     message = Message.create! room: rooms(:pets), body: "<div>Hey #{mention_attachment_for(:david)}</div>", creator: users(:jason)
 

test/helpers/messages_helper_test.rb

@@ -0,0 +1,27 @@
+require "test_helper"
+
+class MessagesHelperTest < ActionView::TestCase
+  test "message_presentation neutralizes unsafe URI schemes in links" do
+    message = Message.create! room: rooms(:pets), body: '<div><a href="javascript:alert(1)">x</a></div>', client_message_id: "0015", creator: users(:jason)
+
+    presentation = view.message_presentation(message)
+    assert_no_match /javascript:/, presentation
+    assert_match /<a>x<\/a>/, presentation
+  end
+
+  test "message_presentation strips event handler attributes from allowed tags" do
+    message = Message.create! room: rooms(:pets), body: '<div><a href="/x" onmouseover="alert(1)">x</a></div>', client_message_id: "0015", creator: users(:jason)
+
+    presentation = view.message_presentation(message)
+    assert_no_match /onmouseover/, presentation
+    assert_match /<a href="\/x">x<\/a>/, presentation
+  end
+
+  test "message_presentation preserves safe links and formatting" do
+    message = Message.create! room: rooms(:pets), body: '<div><a href="https://example.com">example</a> <strong>bold</strong></div>', client_message_id: "0015", creator: users(:jason)
+
+    presentation = view.message_presentation(message)
+    assert_match /<a href="https:\/\/example\.com"[^>]*>example<\/a>/, presentation
+    assert_match /<strong>bold<\/strong>/, presentation
+  end
+end

test/lib/restricted_http/private_network_guard_test.rb

@@ -74,12 +74,27 @@ class RestrictedHTTP::PrivateNetworkGuardTest < ActiveSupport::TestCase
     assert_private_ip "64:ff9b::a00:5"       # NAT64 -> 10.0.0.5
   end
 
-  test "private_ip? returns true for local-use NAT64 addresses embedding a private IPv4 (RFC8215)" do
-    assert_private_ip "64:ff9b:1::a00:1"  # local-use NAT64 -> 10.0.0.1
+  # The local-use block is refused whole rather than decoded. A Pref64 inside it
+  # can be any length from /32 to /96 and the position is not recoverable from
+  # the address alone (RFC 6052 §2.2), so reading the low 32 bits reads the
+  # wrong octets -- 64:ff9b:1::808:808 only looks like 8.8.8.8 under a /96
+  # reading. The block is never globally routed, so nothing legitimate is lost.
+  test "private_ip? returns true for the whole local-use NAT64 block (RFC8215)" do
+    assert_private_ip "64:ff9b:1::a00:1"    # reads as 10.0.0.1 under a /96
+    assert_private_ip "64:ff9b:1::808:808"  # reads as 8.8.8.8 under a /96
+    assert_private_ip "64:ff9b:1:ffff::1"
   end
 
-  test "private_ip? returns false for local-use NAT64 addresses embedding a public IPv4 (RFC8215)" do
-    assert_not RestrictedHTTP::PrivateNetworkGuard.private_ip?("64:ff9b:1::808:808")  # -> 8.8.8.8
+  # SIIT is the third way an IPv4 address rides inside an IPv6 one and the only
+  # one Ruby has no predicate for: ipv4_mapped?, ipv4_compat?, private?,
+  # loopback? and link_local? are all false here. Note the extra group --
+  # ::ffff:0:0:0/96 is not the IPv4-mapped ::ffff:0:0/96, and they do not
+  # overlap -- so this reached the metadata endpoint unrecognised.
+  test "private_ip? returns true for SIIT IPv4-translated addresses (RFC2765)" do
+    assert_private_ip "::ffff:0:169.254.169.254"
+    assert_private_ip "::ffff:0:a9fe:a9fe"        # the same address, hex spelling
+    assert_private_ip "::ffff:0:127.0.0.1"
+    assert_private_ip "::ffff:0:192.168.0.1"
   end
 
   test "private_ip? returns false for NAT64 addresses embedding a public IPv4" do
@@ -114,17 +129,24 @@ class RestrictedHTTP::PrivateNetworkGuardTest < ActiveSupport::TestCase
   end
 
   test "resolve raises Violation for private hostname" do
-    Resolv.stubs(:getaddress).returns("192.168.1.1")
+    Resolv.stubs(:getaddresses).returns([ "192.168.1.1" ])
     assert_raises RestrictedHTTP::Violation do
       RestrictedHTTP::PrivateNetworkGuard.resolve("private.example.com")
     end
   end
 
   test "resolve returns IP for public hostname" do
-    Resolv.stubs(:getaddress).returns("93.184.216.34")
+    Resolv.stubs(:getaddresses).returns([ "93.184.216.34" ])
     assert_equal "93.184.216.34", RestrictedHTTP::PrivateNetworkGuard.resolve("example.com")
   end
 
+  test "resolve raises Unresolvable, not Violation, when the host resolves to nothing" do
+    Resolv.stubs(:getaddresses).returns([])
+    assert_raises Surfguard::Unresolvable do
+      RestrictedHTTP::PrivateNetworkGuard.resolve("nxdomain.example.com")
+    end
+  end
+
   private
     def assert_private_ip(address)
       assert RestrictedHTTP::PrivateNetworkGuard.private_ip?(address),

test/models/opengraph/fetch_test.rb

@@ -37,7 +37,7 @@ class Opengraph::FetchTest < ActiveSupport::TestCase
 
     WebMock.stub_request(:get, "https://www.other.com/")
       .to_return(status: 200, body: "<body>ok<body>", headers: { content_type: "text/html" })
-    Resolv.stubs(:getaddress).with("www.other.com").returns("127.0.0.1")
+    Resolv.stubs(:getaddresses).with("www.other.com").returns([ "127.0.0.1" ])
 
     assert_raises RestrictedHTTP::Violation do
       @fetch.fetch_document(@url, ip: "1.2.3.4")
@@ -48,7 +48,7 @@ class Opengraph::FetchTest < ActiveSupport::TestCase
     # Allow but interrupt a real connection to demonstrate that we connect
     # to a resolved IP, not a hostname to re-resolve.
     WebMock.disable_net_connect! allow: [ @url.host ]
-    Resolv.stubs(:getaddress).with(@url.host).returns("1.2.3.4", "127.0.0.1")
+    Resolv.stubs(:getaddresses).with(@url.host).returns([ "1.2.3.4" ], [ "127.0.0.1" ])
     TCPSocket.expects(:open).with { |*args, **| args.first == @url.host }.never
     TCPSocket.expects(:open).with { |*args, **| args.first == "1.2.3.4" && args[1] == 443 }.throws(:dns_not_rebound)
 
@@ -65,7 +65,7 @@ class Opengraph::FetchTest < ActiveSupport::TestCase
     # Allow but interrupt a real connection to demonstrate that we connect
     # to a resolved IP, not a hostname to re-resolve.
     WebMock.disable_net_connect! allow: [ @url.host ]
-    Resolv.stubs(:getaddress).with(@url.host).returns("1.2.3.4", "127.0.0.1")
+    Resolv.stubs(:getaddresses).with(@url.host).returns([ "1.2.3.4" ], [ "127.0.0.1" ])
     TCPSocket.expects(:open).with { |*args, **| args.first == @url.host }.never
     TCPSocket.expects(:open).with { |*args, **| args.first == "1.2.3.4" && args[1] == 443 }.throws(:dns_not_rebound)
 

test/models/opengraph/location_test.rb

@@ -14,7 +14,7 @@ class Opengraph::LocationTest < ActiveSupport::TestCase
   end
 
   test "private network urls" do
-    Resolv.stubs(:getaddress).with("www.example.com").returns("172.16.0.0")
+    Resolv.stubs(:getaddresses).with("www.example.com").returns([ "172.16.0.0" ])
 
     location = Opengraph::Location.new("https://www.example.com")
     assert_not location.valid?
@@ -22,7 +22,7 @@ class Opengraph::LocationTest < ActiveSupport::TestCase
   end
 
   test "link-local addresses are blocked" do
-    Resolv.stubs(:getaddress).with("metadata.internal").returns("169.254.169.254")
+    Resolv.stubs(:getaddresses).with("metadata.internal").returns([ "169.254.169.254" ])
 
     location = Opengraph::Location.new("https://metadata.internal")
     assert_not location.valid?
@@ -30,13 +30,13 @@ class Opengraph::LocationTest < ActiveSupport::TestCase
   end
 
   test "IPv6 addresses mapped to IPv4 addresses are blocked" do
-    Resolv.stubs(:getaddress).with("metadata.internal").returns("::ffff:192.168.1.1")
+    Resolv.stubs(:getaddresses).with("metadata.internal").returns([ "::ffff:192.168.1.1" ])
 
     location = Opengraph::Location.new("https://metadata.internal")
     assert_not location.valid?
     assert_equal [ "is not public" ], location.errors[:url]
 
-    Resolv.stubs(:getaddress).with("metadata.internal").returns("::ffff:c0a8:0101")
+    Resolv.stubs(:getaddresses).with("metadata.internal").returns([ "::ffff:c0a8:0101" ])
 
     location = Opengraph::Location.new("https://metadata.internal")
     assert_not location.valid?