From ffbf40c72039bacb0ec0a9d4a4df73af233396ce Mon Sep 17 00:00:00 2001
From: Sojan Jose <sojan@pepalo.com>
Date: Thu, 14 May 2026 14:50:29 +0530
Subject: [PATCH] fix: harden Active Storage direct uploads and proxy streaming
 (#14440)

Hardens Active Storage handling on Rails 7.1 by filtering internal
direct-upload metadata keys and limiting proxy range requests, while
keeping audio playback on redirect URLs so large recordings are not
routed through the proxy limiter.

Closes
- CVE-2026-33173
- CVE-2026-33174
- CVE-2026-33658

Why
Rails 7.1 does not currently have patched releases for these Active
Storage advisories, and Chatwoot exposes Active Storage direct-upload
endpoints and media URLs. This keeps the Rails dependency unchanged
while adding small local mitigations until Rails can be upgraded to
7.2.3.1+.

What changed
- Filters `identified`, `analyzed`, and `composed` from direct-upload
blob metadata.
- Limits Active Storage proxy range requests to one range under 100 MB.
- Uses redirect URLs for inline audio attachments so normal playback of
large recordings avoids the proxy streaming path.
- Adds scoped bundle-audit ignores for the locally mitigated Active
Storage advisories and the remaining Rails advisories that are not
reachable through current Chatwoot usage.

How to test
- Upload an attachment from the dashboard reply composer and confirm it
sends successfully.
- Upload an attachment from the website widget and confirm it appears in
the conversation.
- POST a direct-upload request with `blob.metadata.identified`,
`blob.metadata.analyzed`, and `blob.metadata.composed`; confirm those
keys are not persisted while custom metadata remains.
- Play an audio/call-recording attachment and confirm the audio URL
loads through Active Storage redirect rather than proxy.
- Run `bundle exec bundle audit check -v`.

---------

Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
---
 .bundler-audit.yml                    | 19 ++++++++++---
 app/models/attachment.rb              |  8 ++----
 config/initializers/active_storage.rb | 41 +++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/.bundler-audit.yml b/.bundler-audit.yml
index 908d97175c9..0a6c574abf8 100644
--- a/.bundler-audit.yml
+++ b/.bundler-audit.yml
@@ -2,8 +2,19 @@
 ignore:
   - CVE-2021-41098 # https://github.com/chatwoot/chatwoot/issues/3097 (update once azure blob storage is updated)
   - GHSA-57hq-95w6-v4fc # Devise confirmable race condition — patched locally in User model (remove once on Devise 5+)
-  # Chatwoot defaults to Active Storage redirect-style URLs, and its recommended
-  # storage setup uses local/cloud storage with optional direct uploads to the
-  # storage provider rather than Rails proxy mode. Revisit if we enable
-  # rails_storage_proxy or other app-served Active Storage proxy routes.
+  # Rails 7.1 has no patched release for the Active Storage proxy range
+  # advisories. Chatwoot limits proxy range requests locally.
   - CVE-2026-33658
+  # Rails 7.1 has no patched release for this Active Storage direct-upload
+  # advisory. Chatwoot filters internal metadata keys locally.
+  - CVE-2026-33173
+  - CVE-2026-33174
+  # Rails 7.1 has no patched release for these Rails advisories. These are not
+  # reachable through Chatwoot's current usage patterns and should be removed
+  # once we upgrade to Rails 7.2.3.1+.
+  - CVE-2026-33168
+  - CVE-2026-33169
+  - CVE-2026-33170
+  - CVE-2026-33176
+  - CVE-2026-33195
+  - CVE-2026-33202
diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 8c87ff43320..2d46f3b7e6f 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -104,9 +104,7 @@ class Attachment < ApplicationRecord
     audio_file_data = base_data.merge(file_metadata)
     audio_file_data.merge(
       {
-        # ActiveStorage's redirect endpoint defaults to Content-Disposition: attachment,
-        # which makes <audio> elements download instead of play. Force inline so the
-        # call-recording chip (and any other audio bubble) can stream directly.
+        # Keep audio playback inline while avoiding the ActiveStorage proxy path.
         data_url: inline_audio_url,
         transcribed_text: meta&.[]('transcribed_text') || ''
       }
@@ -116,9 +114,7 @@ class Attachment < ApplicationRecord
   def inline_audio_url
     return '' unless file.attached?
 
-    # Proxy endpoint streams through Rails and honours `disposition: 'inline'`,
-    # unlike the redirect endpoint which always sends Content-Disposition: attachment.
-    Rails.application.routes.url_helpers.rails_storage_proxy_url(file, disposition: 'inline')
+    Rails.application.routes.url_helpers.rails_storage_redirect_url(file, disposition: 'inline')
   end
 
   def file_metadata
diff --git a/config/initializers/active_storage.rb b/config/initializers/active_storage.rb
index d927e1472b7..653b0c53572 100644
--- a/config/initializers/active_storage.rb
+++ b/config/initializers/active_storage.rb
@@ -11,3 +11,44 @@ Rails.application.config.active_storage.content_types_allowed_inline += %w[
   audio/wav
   audio/x-wav
 ]
+
+module ActiveStorageDirectUploadMetadataFilter
+  INTERNAL_METADATA_KEYS = %w[identified analyzed composed].freeze
+
+  private
+
+  def blob_args
+    super.tap do |args|
+      args[:metadata]&.except!(*INTERNAL_METADATA_KEYS, *INTERNAL_METADATA_KEYS.map(&:to_sym))
+    end
+  end
+end
+
+module ActiveStorageProxyRangeLimit
+  STREAMING_MAX_RANGES = 1
+  STREAMING_CHUNK_MAX_SIZE = 100.megabytes
+
+  private
+
+  def send_blob_byte_range_data(blob, range_header, disposition: nil)
+    ranges = Rack::Utils.get_byte_ranges(range_header, blob.byte_size)
+    return head(:range_not_satisfiable) unless valid_ranges?(ranges)
+
+    super
+  end
+
+  def valid_ranges?(ranges)
+    ranges.present? &&
+      ranges.any?(&:present?) &&
+      ranges.length <= STREAMING_MAX_RANGES &&
+      ranges.sum { |range| range.end - range.begin } < STREAMING_CHUNK_MAX_SIZE
+  end
+end
+
+Rails.application.config.to_prepare do
+  unless ActiveStorage::DirectUploadsController < ActiveStorageDirectUploadMetadataFilter
+    ActiveStorage::DirectUploadsController.prepend(ActiveStorageDirectUploadMetadataFilter)
+  end
+
+  ActiveStorage::Streaming.prepend(ActiveStorageProxyRangeLimit) unless ActiveStorage::Streaming < ActiveStorageProxyRangeLimit
+end