From 49f06d0b22e07f990246004d2b49a6aab52d5a30 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 15 Jun 2026 13:10:59 -0700 Subject: [PATCH] Address review: keep disk PUT CSRF-exempt, use intent helpers - Including Authentication re-arms protect_from_forgery on DiskController. Active Storage's direct-upload service PUT (#update) sends only signed service headers and no CSRF token, so a real authenticated upload would 422 storing bytes. Re-exempt #update from forgery protection; the signed URL token and session check still gate the write. - Swap the raw skip_before_action for the Authentication concern's intent-revealing allow_unauthenticated_access / allow_bot_access helpers on #show, matching the rest of the app. - Scope the test's ActiveStorage::Current.url_options override to a set { } block so it can't leak thread-local state into later tests. --- config/initializers/active_storage.rb | 12 +++++++++++- .../active_storage/direct_uploads_controller_test.rb | 5 +++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb index bb0f7cb..0de8fb8 100644 --- a/config/initializers/active_storage.rb +++ b/config/initializers/active_storage.rb @@ -17,5 +17,15 @@ ActiveSupport.on_load(:active_storage_blob) do ActiveStorage::DiskController.include Rails.application.routes.url_helpers ActiveStorage::DiskController.include Authentication - ActiveStorage::DiskController.skip_before_action :require_authentication, :deny_bots, only: :show + + # 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 diff --git a/test/controllers/active_storage/direct_uploads_controller_test.rb b/test/controllers/active_storage/direct_uploads_controller_test.rb index 0e783e4..f614f2e 100644 --- a/test/controllers/active_storage/direct_uploads_controller_test.rb +++ b/test/controllers/active_storage/direct_uploads_controller_test.rb @@ -26,9 +26,10 @@ class ActiveStorage::DirectUploadsControllerTest < ActionDispatch::IntegrationTe 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 + ActiveStorage::Current.set(url_options: { host: "once.campfire.test", protocol: "http" }) do + get blob.url + end assert_response :success end