fix: block the unused Active Storage direct-upload route (#15329)
## Description **Problem.** The default Active Storage upload route, `POST /rails/active_storage/direct_uploads`, is mounted automatically by Rails and requires no authentication. Chatwoot doesn't rely on it, our dashboard and widget uploads all use scoped, authenticated endpoints, so the route just sits there letting anyone create blobs anonymously. **Fix.** Block the built-in route so it returns `403`. Chatwoot's own upload controllers inherit from the same Rails class but are left working, an `instance_of?` check makes the block apply only to the bare route, not to the subclasses that call `super`. Fixes https://linear.app/chatwoot/issue/INF-94 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? Added a request spec asserting the bare route returns `403` and creates no blob. Existing widget and conversation direct-upload specs still pass, confirming the scoped endpoints are unaffected. 13 examples, 0 failures across the three direct-upload specs; rubocop clean. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes --------- Co-authored-by: Sony Mathew <sony@chatwoot.com>
This commit is contained in:
@@ -45,10 +45,32 @@ module ActiveStorageProxyRangeLimit
|
||||
end
|
||||
end
|
||||
|
||||
# Block the default Rails direct-upload route. Dashboard and widget uploads both go
|
||||
# through the scoped, authenticated /api/v1/... endpoints, so the bare route has no
|
||||
# legitimate caller; leaving it open allows anonymous blob creation. Scoped subclasses
|
||||
# call super and are exempt via the instance_of? check.
|
||||
module ActiveStorageBareDirectUploadGuard
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
included do
|
||||
before_action :reject_bare_direct_upload
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def reject_bare_direct_upload
|
||||
head :forbidden if instance_of?(ActiveStorage::DirectUploadsController)
|
||||
end
|
||||
end
|
||||
|
||||
Rails.application.config.to_prepare do
|
||||
unless ActiveStorage::DirectUploadsController < ActiveStorageDirectUploadMetadataFilter
|
||||
ActiveStorage::DirectUploadsController.prepend(ActiveStorageDirectUploadMetadataFilter)
|
||||
end
|
||||
|
||||
unless ActiveStorage::DirectUploadsController.include?(ActiveStorageBareDirectUploadGuard)
|
||||
ActiveStorage::DirectUploadsController.include(ActiveStorageBareDirectUploadGuard)
|
||||
end
|
||||
|
||||
ActiveStorage::Streaming.prepend(ActiveStorageProxyRangeLimit) unless ActiveStorage::Streaming < ActiveStorageProxyRangeLimit
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user