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
 

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,14 +378,13 @@ 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)
+    thor (1.5.0)
     thruster (0.1.23-aarch64-linux)
     thruster (0.1.23-arm64-darwin)
     thruster (0.1.23-x86_64-darwin)
@@ -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/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/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/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")

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

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/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/fixtures/boosts.yml

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

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?