From ed5a1728717386ec5be0bfc70aec2fd813435132 Mon Sep 17 00:00:00 2001
From: Jeremy Daer <jeremy@37signals.com>
Date: Mon, 15 Jun 2026 13:00:29 -0700
Subject: [PATCH] Require authentication for ActiveStorage direct-upload write
 endpoints
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ActiveStorage's direct-upload endpoints ship unauthenticated by Rails
default: ActiveStorage::DirectUploadsController and DiskController inherit
from ActionController::Base, so they bypass the app's Authentication
concern. That leaves the write path open — anyone could mint blob records
and PUT bytes to local disk storage.

Campfire never uses direct upload for legitimate attachments. Those flow
through MessagesController#create (already authenticated), and Trix file
drops are disabled in the composer. Gating the write path is therefore
pure defense-in-depth with no functional cost.

Require an authenticated session on the two write actions
(DirectUploadsController#create and DiskController#update) by including the
existing Authentication concern. Blob serving stays public —
DiskController#show keeps its auth skip, and the Blobs/Representations
controllers are untouched — so message attachments and the account logo
keep loading. Because these controllers live in ActiveStorage::Engine and
only see the engine's url helpers, also include the application route
helpers so the concern can redirect to new_session_url on failure (302,
write blocked).
---
 config/initializers/active_storage.rb         | 16 +++++++
 .../direct_uploads_controller_test.rb         | 42 +++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 test/controllers/active_storage/direct_uploads_controller_test.rb

diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb
index f2d77e1..bb0f7cb 100644
--- a/config/initializers/active_storage.rb
+++ b/config/initializers/active_storage.rb
@@ -2,4 +2,20 @@ ActiveSupport.on_load(:active_storage_blob) do
   ActiveStorage::DiskController.after_action only: :show do
     response.set_header("Cache-Control", "max-age=3600, public")
   end
+
+  # Gate the ActiveStorage write path behind app authentication. These endpoints
+  # ship unauthenticated by Rails default; Campfire never uses direct upload for
+  # legit attachments (those go through MessagesController#create, and Trix file
+  # drops are disabled in the composer). Requiring a session on the write actions
+  # blocks anonymous blob writes and disk-fill while leaving blob serving public.
+  #
+  # ActiveStorage controllers live in ActiveStorage::Engine, so they see the
+  # engine's url helpers, not the main app's. Include the application helpers
+  # first so Authentication#request_authentication can redirect to new_session_url.
+  ActiveStorage::DirectUploadsController.include Rails.application.routes.url_helpers
+  ActiveStorage::DirectUploadsController.include Authentication # only action: #create
+
+  ActiveStorage::DiskController.include Rails.application.routes.url_helpers
+  ActiveStorage::DiskController.include Authentication
+  ActiveStorage::DiskController.skip_before_action :require_authentication, :deny_bots, only: :show
 end
diff --git a/test/controllers/active_storage/direct_uploads_controller_test.rb b/test/controllers/active_storage/direct_uploads_controller_test.rb
new file mode 100644
index 0000000..0e783e4
--- /dev/null
+++ b/test/controllers/active_storage/direct_uploads_controller_test.rb
@@ -0,0 +1,42 @@
+require "test_helper"
+
+class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    host! "once.campfire.test"
+  end
+
+  test "create requires authentication" do
+    assert_no_difference -> { ActiveStorage::Blob.count } do
+      post rails_direct_uploads_url, params: { blob: blob_params }
+    end
+
+    assert_redirected_to new_session_url
+  end
+
+  test "create succeeds for an authenticated session" do
+    sign_in :david
+
+    assert_difference -> { ActiveStorage::Blob.count }, 1 do
+      post rails_direct_uploads_url, params: { blob: blob_params }
+    end
+
+    assert_response :success
+  end
+
+  test "disk show stays reachable without authentication" do
+    blob = ActiveStorage::Blob.create_and_upload! \
+      io: StringIO.new("hello"), filename: "hello.txt", content_type: "text/plain"
+    ActiveStorage::Current.url_options = { host: "once.campfire.test", protocol: "http" }
+
+    get blob.url
+
+    assert_response :success
+  end
+
+  private
+    def blob_params
+      content = "hello"
+      { filename: "hello.txt", byte_size: content.bytesize, content_type: "text/plain", \
+        checksum: Digest::MD5.base64digest(content) }
+    end
+end