Allow bots to update and destroy their own messages

Bots can only create. A lifecycle notification — an alert that fires and then
resolves, a deploy that starts and finishes, a backup that runs — therefore has
to post a second message, and the room becomes an append-only log of states
rather than a view of the current one.

Adds PATCH and DELETE inside the existing bot_key scope, routed to
Messages::ByBotsController. The body is read the way create reads it, so
updating a message is the same request shape as posting one.

No new authorization: both actions already run through ensure_can_administer,
and can_administer? grants access only to a record the user created, so a bot
key reaches that bot's own messages and no others. set_room narrows it again by
looking the room up through the bot's own memberships. A leaked bot key gains
what it could already do by posting: write to rooms that bot belongs to.

update answers head :ok rather than the redirect, which meant extracting the
update and its broadcast into update_message — calling super and then head
would double render, since the parent redirects inside the action. destroy
needs no split, because the parent renders implicitly like create does.

app/controllers/messages/by_bots_controller.rb

@@ -1,9 +1,11 @@
 class Messages::ByBotsController < MessagesController
   include RawRequestBody
 
-  allow_bot_access only: %i[ index create ]
+  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
@@ -16,6 +18,18 @@ class Messages::ByBotsController < MessagesController
     head :created, location: message_url(@message)
   end
 
+  # ensure_can_administer still applies, and can_administer? only grants access to
+  # a record the user created, so a bot key reaches that bot's own messages and no others.
+  def update
+    update_message
+    head :ok
+  end
+
+  def destroy
+    super
+    head :no_content
+  end
+
   private
     def set_room
       @room = Current.user.rooms.find_by(id: params[:room_id])

app/controllers/messages_controller.rb

@@ -34,9 +34,7 @@ class MessagesController < ApplicationController
   end
 
   def update
-    @message.update!(message_params)
-
-    @message.broadcast_replace_to @room, :messages, target: [ @message, :presentation ], partial: "messages/presentation", attributes: { maintain_scroll: true }
+    update_message
     redirect_to room_message_url(@room, @message)
   end
 
@@ -50,6 +48,13 @@ class MessagesController < ApplicationController
       @message = @room.messages.find(params[:id])
     end
 
+    # Extracted so bots can reuse the update and its broadcast while answering with
+    # a status code instead of a redirect.
+    def update_message
+      @message.update!(message_params)
+      @message.broadcast_replace_to @room, :messages, target: [ @message, :presentation ], partial: "messages/presentation", attributes: { maintain_scroll: true }
+    end
+
     def ensure_can_administer
       head :forbidden unless Current.user.can_administer?(@message)
     end

config/routes.rb

@@ -64,7 +64,7 @@ Rails.application.routes.draw do
 
     nested do
       scope path: ":bot_key", as: :bot, defaults: { format: :json } do
-        resources :messages, controller: "messages/by_bots", only: %i[ index create ] do
+        resources :messages, controller: "messages/by_bots", only: %i[ index create update destroy ] do
           resources :boosts, controller: "messages/boosts/by_bots", only: :create
         end
       end

test/controllers/messages/by_bots_controller_test.rb

@@ -148,4 +148,81 @@ class Messages::ByBotsControllerTest < ActionDispatch::IntegrationTest
     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
+  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
+  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