Address review: assert uploaded bytes in disk update test

direct_upload_url_for now returns [blob, url] so the authenticated update
test can assert blob.download matches the uploaded payload, not just the
204 status — guarding against a write landing at the wrong key or with the
wrong content. Both update call sites destructure the tuple.

Gemfile.lock

@@ -137,7 +137,7 @@ GEM
     bcrypt (3.1.22)
     benchmark (0.5.0)
     bigdecimal (3.3.1)
-    brakeman (8.0.5)
+    brakeman (8.0.4)
       racc
     builder (3.3.0)
     bundler-audit (0.9.3)
@@ -153,13 +153,13 @@ GEM
       regexp_parser (>= 1.5, < 3.0)
       xpath (~> 3.2)
     chunky_png (1.4.0)
-    concurrent-ruby (1.3.7)
+    concurrent-ruby (1.3.5)
     connection_pool (2.5.5)
     crack (1.0.0)
       bigdecimal
       rexml
-    crass (1.0.7)
-    date (3.5.1)
+    crass (1.0.6)
+    date (3.5.0)
     debug (1.11.0)
       irb (~> 1.10)
       reline (>= 0.3.8)
@@ -191,7 +191,7 @@ GEM
     jbuilder (2.14.1)
       actionview (>= 7.0.0)
       activesupport (>= 7.0.0)
-    json (2.20.0)
+    json (2.13.2)
     jwt (3.2.0)
       base64
     kredis (1.8.0)
@@ -224,7 +224,7 @@ GEM
       ruby2_keywords (~> 0.0.1)
     net-http-persistent (4.0.6)
       connection_pool (~> 2.2, >= 2.2.4)
-    net-imap (0.6.4.1)
+    net-imap (0.6.4)
       date
       net-protocol
     net-pop (0.1.2)
@@ -234,13 +234,13 @@ GEM
     net-smtp (0.5.1)
       net-protocol
     nio4r (2.7.5)
-    nokogiri (1.19.4-aarch64-linux-gnu)
+    nokogiri (1.19.3-aarch64-linux-gnu)
       racc (~> 1.4)
-    nokogiri (1.19.4-arm64-darwin)
+    nokogiri (1.19.3-arm64-darwin)
       racc (~> 1.4)
-    nokogiri (1.19.4-x86_64-darwin)
+    nokogiri (1.19.3-x86_64-darwin)
       racc (~> 1.4)
-    nokogiri (1.19.4-x86_64-linux-gnu)
+    nokogiri (1.19.3-x86_64-linux-gnu)
       racc (~> 1.4)
     openssl (3.3.0)
     ostruct (0.6.3)
@@ -380,7 +380,7 @@ GEM
     thruster (0.1.15-x86_64-darwin)
     thruster (0.1.15-x86_64-linux)
     tilt (2.6.1)
-    timeout (0.6.1)
+    timeout (0.4.4)
     tsort (0.2.0)
     tzinfo (2.0.6)
       concurrent-ruby (~> 1.0)

config/initializers/active_storage.rb

@@ -2,4 +2,30 @@ 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
+
+  # Blob serving (#show) stays public so signed-token attachment URLs keep
+  # resolving for unauthenticated and bot clients alike.
+  ActiveStorage::DiskController.allow_unauthenticated_access only: :show
+  ActiveStorage::DiskController.allow_bot_access only: :show
+
+  # Including Authentication re-adds protect_from_forgery, but Active Storage's
+  # direct-upload service PUT (#update) carries only signed service headers and
+  # no authenticity token. Re-exempt it from CSRF so authenticated uploads can
+  # still store bytes; the signed URL token and the session check remain.
+  ActiveStorage::DiskController.skip_forgery_protection only: :update
 end

test/controllers/active_storage/direct_uploads_controller_test.rb

@@ -0,0 +1,83 @@
+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.set(url_options: { host: "once.campfire.test", protocol: "http" }) do
+      get blob.url
+    end
+
+    assert_response :success
+  end
+
+  test "disk update requires authentication" do
+    _blob, url = direct_upload_url_for("hello")
+
+    put url, params: "hello", headers: { "Content-Type" => "text/plain" }
+
+    assert_redirected_to new_session_url
+  end
+
+  test "disk update stores bytes for an authenticated session without a CSRF token" do
+    sign_in :david
+    blob, url = direct_upload_url_for("hello")
+
+    # Forgery protection is off in test by default; turn it on so this proves the
+    # signed-token service PUT stays CSRF-exempt even when the concern re-arms it.
+    with_forgery_protection do
+      put url, params: "hello", headers: { "Content-Type" => "text/plain" }
+    end
+
+    assert_response :no_content
+    assert_equal "hello", blob.download
+  end
+
+  private
+    def blob_params
+      content = "hello"
+      { filename: "hello.txt", byte_size: content.bytesize, content_type: "text/plain", \
+        checksum: Digest::MD5.base64digest(content) }
+    end
+
+    def direct_upload_url_for(content)
+      blob = ActiveStorage::Blob.create_before_direct_upload! \
+        filename: "hello.txt", byte_size: content.bytesize, \
+        checksum: Digest::MD5.base64digest(content), content_type: "text/plain"
+      url = ActiveStorage::Current.set(url_options: { host: "once.campfire.test", protocol: "http" }) do
+        blob.service_url_for_direct_upload
+      end
+      [ blob, url ]
+    end
+
+    def with_forgery_protection
+      original = ActionController::Base.allow_forgery_protection
+      ActionController::Base.allow_forgery_protection = true
+      yield
+    ensure
+      ActionController::Base.allow_forgery_protection = original
+    end
+end