From 45c4119c7d35a98050d99528805fa757702a3f5e Mon Sep 17 00:00:00 2001
From: Sony Mathew <2040199+sony-mathew@users.noreply.github.com>
Date: Thu, 23 Apr 2026 14:50:30 +0530
Subject: [PATCH] fix(ssrf): close safe fetch tempfiles explicitly

---
 app/builders/messages/messenger/message_builder.rb  | 12 +++++++++---
 .../api/v1/accounts/upload_controller.rb            |  2 ++
 app/jobs/avatar/avatar_from_url_job.rb              |  2 ++
 app/services/sms/incoming_message_service.rb        | 12 ++++++++++++
 app/services/telegram/incoming_message_service.rb   | 12 ++++++++++++
 app/services/tiktok/message_service.rb              | 12 ++++++++++++
 app/services/twilio/incoming_message_service.rb     | 12 ++++++++++++
 .../whatsapp/incoming_message_base_service.rb       | 13 +++++++++++++
 enterprise/lib/captain/tools/http_tool.rb           |  2 ++
 9 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/app/builders/messages/messenger/message_builder.rb b/app/builders/messages/messenger/message_builder.rb
index 4500ca705..de23af619 100644
--- a/app/builders/messages/messenger/message_builder.rb
+++ b/app/builders/messages/messenger/message_builder.rb
@@ -8,11 +8,13 @@ class Messages::Messenger::MessageBuilder
 
     params = attachment_params(attachment)
     attachment_obj = @message.attachments.new(params.except(:remote_file_url))
+    downloaded_file = nil
     if facebook_reel?(attachment)
       attachment_obj.save!
       update_facebook_reel_content(attachment)
     elsif params[:remote_file_url]
-      return unless attach_file(attachment_obj, params[:remote_file_url])
+      downloaded_file = attach_file(attachment_obj, params[:remote_file_url])
+      return unless downloaded_file
 
       attachment_obj.save!
     else
@@ -22,24 +24,28 @@ class Messages::Messenger::MessageBuilder
     fetch_ig_story_link(attachment_obj) if attachment_obj.file_type == 'ig_story'
     fetch_ig_post_link(attachment_obj) if attachment_obj.file_type == 'ig_post'
     update_attachment_file_type(attachment_obj)
+  ensure
+    downloaded_file&.close!
   end
   # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
 
   def attach_file(attachment, file_url)
+    downloaded_file = nil
     SafeFetch.fetch(
       file_url,
       allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES
     ) do |attachment_file|
+      downloaded_file = attachment_file
       attachment.file.attach(
         io: attachment_file.tempfile,
         filename: attachment_file.original_filename,
         content_type: attachment_file.content_type
       )
     end
-    true
+    downloaded_file
   rescue SafeFetch::Error => e
     Rails.logger.info "Error downloading Messenger attachment from #{file_url}: #{e.message}: Skipping"
-    false
+    nil
   end
 
   def attachment_params(attachment)
diff --git a/app/controllers/api/v1/accounts/upload_controller.rb b/app/controllers/api/v1/accounts/upload_controller.rb
index bf20bc6ff..77af555a1 100644
--- a/app/controllers/api/v1/accounts/upload_controller.rb
+++ b/app/controllers/api/v1/accounts/upload_controller.rb
@@ -21,6 +21,8 @@ class Api::V1::Accounts::UploadController < Api::V1::Accounts::BaseController
   def create_from_url
     SafeFetch.fetch(params[:external_url].to_s) do |result|
       create_and_save_blob(result.tempfile, result.filename, result.content_type)
+    ensure
+      result.close!
     end
   rescue SafeFetch::HttpError => e
     render_error(I18n.t('errors.upload.fetch_failed_with_message', message: e.message), :unprocessable_entity)
diff --git a/app/jobs/avatar/avatar_from_url_job.rb b/app/jobs/avatar/avatar_from_url_job.rb
index 16da29490..9632f3f3c 100644
--- a/app/jobs/avatar/avatar_from_url_job.rb
+++ b/app/jobs/avatar/avatar_from_url_job.rb
@@ -40,6 +40,8 @@ class Avatar::AvatarFromUrlJob < ApplicationJob
       allowed_content_types: %w[image/jpeg image/png image/gif]
     ) do |avatar_file|
       attach_avatar(avatarable, avatar_file)
+    ensure
+      avatar_file.close!
     end
   end
 
diff --git a/app/services/sms/incoming_message_service.rb b/app/services/sms/incoming_message_service.rb
index 791699a6a..4143d32a3 100644
--- a/app/services/sms/incoming_message_service.rb
+++ b/app/services/sms/incoming_message_service.rb
@@ -4,6 +4,7 @@ class Sms::IncomingMessageService
   pattr_initialize [:inbox!, :params!]
 
   def perform
+    @downloaded_files = []
     set_contact
     set_conversation
     @message = @conversation.messages.create!(
@@ -16,6 +17,8 @@ class Sms::IncomingMessageService
     )
     attach_files
     @message.save!
+  ensure
+    close_downloaded_files
   end
 
   private
@@ -84,6 +87,7 @@ class Sms::IncomingMessageService
       next if media_url.end_with?('.smil', '.xml')
 
       download_attachment_file(media_url) do |attachment_file|
+        track_downloaded_file(attachment_file)
         @message.attachments.new(
           account_id: @message.account_id,
           file_type: file_type(attachment_file.content_type),
@@ -107,4 +111,12 @@ class Sms::IncomingMessageService
   rescue SafeFetch::Error => e
     Rails.logger.info "Error downloading SMS attachment from #{media_url}: #{e.message}: Skipping"
   end
+
+  def track_downloaded_file(attachment_file)
+    @downloaded_files << attachment_file
+  end
+
+  def close_downloaded_files
+    Array(@downloaded_files).each(&:close!)
+  end
 end
diff --git a/app/services/telegram/incoming_message_service.rb b/app/services/telegram/incoming_message_service.rb
index 6ba0b2d37..4b18d5ee2 100644
--- a/app/services/telegram/incoming_message_service.rb
+++ b/app/services/telegram/incoming_message_service.rb
@@ -8,6 +8,7 @@ class Telegram::IncomingMessageService
   pattr_initialize [:inbox!, :params!]
 
   def perform
+    @downloaded_files = []
     # chatwoot doesn't support group conversations at the moment
     transform_business_message!
     return unless private_message?
@@ -34,6 +35,8 @@ class Telegram::IncomingMessageService
 
     process_message_attachments if message_params?
     @message.save!
+  ensure
+    close_downloaded_files
   end
 
   private
@@ -167,6 +170,7 @@ class Telegram::IncomingMessageService
   end
 
   def build_file_attachment(attachment_file)
+    track_downloaded_file(attachment_file)
     @message.attachments.new(
       account_id: @message.account_id,
       file_type: file_content_type,
@@ -233,6 +237,14 @@ class Telegram::IncomingMessageService
       params[:message][:video_note].presence
   end
 
+  def track_downloaded_file(attachment_file)
+    @downloaded_files << attachment_file
+  end
+
+  def close_downloaded_files
+    Array(@downloaded_files).each(&:close!)
+  end
+
   def transform_business_message!
     params[:message] = params[:business_message] if params[:business_message] && !params[:message]
   end
diff --git a/app/services/tiktok/message_service.rb b/app/services/tiktok/message_service.rb
index 2c76e0dbf..1f1b0ad15 100644
--- a/app/services/tiktok/message_service.rb
+++ b/app/services/tiktok/message_service.rb
@@ -32,6 +32,7 @@ class Tiktok::MessageService
   end
 
   def create_message
+    @downloaded_files = []
     message = conversation.messages.build(
       content: message_content,
       account_id: channel.inbox.account_id,
@@ -48,6 +49,8 @@ class Tiktok::MessageService
 
     create_message_attachments(message)
     message.save!
+  ensure
+    close_downloaded_files
   end
 
   def message_content
@@ -65,6 +68,7 @@ class Tiktok::MessageService
     return unless image_message?
 
     fetch_attachment(channel, tt_conversation_id, tt_message_id, tt_image_media_id) do |attachment_file|
+      track_downloaded_file(attachment_file)
       message.attachments.new(
         account_id: message.account_id,
         file_type: :image,
@@ -176,4 +180,12 @@ class Tiktok::MessageService
   def outgoing_message?
     !incoming_message?
   end
+
+  def track_downloaded_file(attachment_file)
+    @downloaded_files << attachment_file
+  end
+
+  def close_downloaded_files
+    Array(@downloaded_files).each(&:close!)
+  end
 end
diff --git a/app/services/twilio/incoming_message_service.rb b/app/services/twilio/incoming_message_service.rb
index 7981558b2..61c16d599 100644
--- a/app/services/twilio/incoming_message_service.rb
+++ b/app/services/twilio/incoming_message_service.rb
@@ -5,6 +5,7 @@ class Twilio::IncomingMessageService
   pattr_initialize [:params!]
 
   def perform
+    @downloaded_files = []
     return if twilio_channel.blank?
 
     set_contact
@@ -20,6 +21,8 @@ class Twilio::IncomingMessageService
     attach_files
     attach_location if location_message?
     @message.save!
+  ensure
+    close_downloaded_files
   end
 
   private
@@ -145,6 +148,7 @@ class Twilio::IncomingMessageService
 
   def attach_single_file(media_url)
     download_attachment_file(media_url) do |attachment_file|
+      track_downloaded_file(attachment_file)
       @message.attachments.new(
         account_id: @message.account_id,
         file_type: file_type(attachment_file.content_type),
@@ -218,5 +222,13 @@ class Twilio::IncomingMessageService
   def contact_name_matches_phone_number?
     @contact.name == phone_number || @contact.name == formatted_phone_number
   end
+
+  def track_downloaded_file(attachment_file)
+    @downloaded_files << attachment_file
+  end
+
+  def close_downloaded_files
+    Array(@downloaded_files).each(&:close!)
+  end
 end
 # rubocop:enable Metrics/ClassLength
diff --git a/app/services/whatsapp/incoming_message_base_service.rb b/app/services/whatsapp/incoming_message_base_service.rb
index 78f8f45b5..b03ddc09f 100644
--- a/app/services/whatsapp/incoming_message_base_service.rb
+++ b/app/services/whatsapp/incoming_message_base_service.rb
@@ -24,6 +24,8 @@ class Whatsapp::IncomingMessageBaseService
   private
 
   def process_messages
+    @downloaded_files = []
+
     # We don't support reactions & ephemeral message now, we need to skip processing the message
     # if the webhook event is a reaction or an ephermal message or an unsupported message.
     return if unprocessable_message_type?(message_type)
@@ -43,6 +45,8 @@ class Whatsapp::IncomingMessageBaseService
       set_conversation
       create_messages
     end
+  ensure
+    close_downloaded_files
   end
 
   def process_statuses
@@ -149,6 +153,7 @@ class Whatsapp::IncomingMessageBaseService
     @message.content ||= attachment_payload[:caption]
 
     download_attachment_file(attachment_payload) do |attachment_file|
+      track_downloaded_file(attachment_file)
       @message.attachments.new(
         account_id: @message.account_id,
         file_type: file_content_type(message_type),
@@ -229,4 +234,12 @@ class Whatsapp::IncomingMessageBaseService
     formatted_phone_number = TelephoneNumber.parse(phone_number).international_number
     @contact.name == phone_number || @contact.name == formatted_phone_number
   end
+
+  def track_downloaded_file(attachment_file)
+    @downloaded_files << attachment_file
+  end
+
+  def close_downloaded_files
+    Array(@downloaded_files).each(&:close!)
+  end
 end
diff --git a/enterprise/lib/captain/tools/http_tool.rb b/enterprise/lib/captain/tools/http_tool.rb
index 84725033a..d4833e1ed 100644
--- a/enterprise/lib/captain/tools/http_tool.rb
+++ b/enterprise/lib/captain/tools/http_tool.rb
@@ -43,6 +43,8 @@ class Captain::Tools::HttpTool < Agents::Tool
       validate_content_type: false
     ) do |response|
       response_body = response.tempfile.read
+    ensure
+      response.close!
     end
 
     response_body