From c7da5bd5e64dc7052ca8ab36502f46db955b7b1a Mon Sep 17 00:00:00 2001
From: Sony Mathew <2040199+sony-mathew@users.noreply.github.com>
Date: Thu, 23 Apr 2026 01:00:56 +0530
Subject: [PATCH] fix(ssrf): harden external downloads
---
.../instagram/base_message_builder.rb | 7 +-
.../messages/messenger/message_builder.rb | 31 ++++--
app/jobs/avatar/avatar_from_url_job.rb | 31 ++++--
app/services/sms/incoming_message_service.rb | 36 ++++---
.../telegram/incoming_message_service.rb | 30 +++---
app/services/tiktok/message_service.rb | 22 ++--
app/services/tiktok/messaging_helpers.rb | 11 +-
.../twilio/incoming_message_service.rb | 50 +++++----
.../whatsapp/incoming_message_base_service.rb | 25 ++---
.../incoming_message_service_helpers.rb | 9 +-
...incoming_message_whatsapp_cloud_service.rb | 11 +-
enterprise/lib/captain/tools/http_tool.rb | 100 ++++--------------
lib/safe_fetch.rb | 87 +++++++++++++--
.../instagram/message_builder_spec.rb | 19 +++-
.../messenger/message_builder_spec.rb | 30 ++++++
.../lib/captain/tools/http_tool_spec.rb | 15 +++
spec/jobs/avatar/avatar_from_url_job_spec.rb | 99 ++++++++++++-----
spec/lib/safe_fetch_spec.rb | 95 ++++++++++++++++-
.../sms/incoming_message_service_spec.rb | 22 +++-
.../telegram/incoming_message_service_spec.rb | 35 +++++-
spec/services/tiktok/message_service_spec.rb | 45 ++++++--
.../twilio/incoming_message_service_spec.rb | 43 +++++++-
.../whatsapp/incoming_message_service_spec.rb | 24 ++++-
...ing_message_whatsapp_cloud_service_spec.rb | 34 +++++-
24 files changed, 664 insertions(+), 247 deletions(-)
diff --git a/app/builders/messages/instagram/base_message_builder.rb b/app/builders/messages/instagram/base_message_builder.rb
index 8045e84c9..241dceed0 100644
--- a/app/builders/messages/instagram/base_message_builder.rb
+++ b/app/builders/messages/instagram/base_message_builder.rb
@@ -123,12 +123,9 @@ class Messages::Instagram::BaseMessageBuilder < Messages::Messenger::MessageBuil
account_id: @message.account_id,
external_url: story_url
)
+ return unless attach_file(attachment, story_url)
+
attachment.save!
- begin
- attach_file(attachment, story_url)
- rescue Down::Error, StandardError => e
- Rails.logger.warn "Failed to download Instagram story attachment: #{e.message}"
- end
@message.content_attributes[:image_type] = 'ig_story_reply'
@message.save!
end
diff --git a/app/builders/messages/messenger/message_builder.rb b/app/builders/messages/messenger/message_builder.rb
index be1f98b43..97ccfade7 100644
--- a/app/builders/messages/messenger/message_builder.rb
+++ b/app/builders/messages/messenger/message_builder.rb
@@ -7,11 +7,15 @@ class Messages::Messenger::MessageBuilder
params = attachment_params(attachment)
attachment_obj = @message.attachments.new(params.except(:remote_file_url))
- attachment_obj.save!
if facebook_reel?(attachment)
+ attachment_obj.save!
update_facebook_reel_content(attachment)
elsif params[:remote_file_url]
- attach_file(attachment_obj, params[:remote_file_url])
+ return unless attach_file(attachment_obj, params[:remote_file_url])
+
+ attachment_obj.save!
+ else
+ attachment_obj.save!
end
fetch_story_link(attachment_obj) if attachment_obj.file_type == 'story_mention'
fetch_ig_story_link(attachment_obj) if attachment_obj.file_type == 'ig_story'
@@ -20,14 +24,21 @@ class Messages::Messenger::MessageBuilder
end
def attach_file(attachment, file_url)
- attachment_file = Down.download(
- file_url
- )
- attachment.file.attach(
- io: attachment_file,
- filename: attachment_file.original_filename,
- content_type: attachment_file.content_type
- )
+ SafeFetch.fetch(
+ file_url,
+ allowed_content_type_prefixes: %w[image/ video/ audio/],
+ allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES
+ ) do |attachment_file|
+ attachment.file.attach(
+ io: attachment_file.tempfile,
+ filename: attachment_file.original_filename,
+ content_type: attachment_file.content_type
+ )
+ end
+ true
+ rescue SafeFetch::Error => e
+ Rails.logger.info "Error downloading Messenger attachment from #{file_url}: #{e.message}: Skipping"
+ false
end
def attachment_params(attachment)
diff --git a/app/jobs/avatar/avatar_from_url_job.rb b/app/jobs/avatar/avatar_from_url_job.rb
index 929e76597..f810e9af8 100644
--- a/app/jobs/avatar/avatar_from_url_job.rb
+++ b/app/jobs/avatar/avatar_from_url_job.rb
@@ -18,18 +18,27 @@ class Avatar::AvatarFromUrlJob < ApplicationJob
return unless should_sync_avatar?(avatarable, avatar_url)
- avatar_file = Down.download(avatar_url, max_size: MAX_DOWNLOAD_SIZE)
- raise Down::Error, 'Invalid file' unless valid_file?(avatar_file)
+ SafeFetch.fetch(
+ avatar_url,
+ max_bytes: MAX_DOWNLOAD_SIZE,
+ allowed_content_type_prefixes: [],
+ allowed_content_types: %w[image/jpeg image/png image/gif]
+ ) do |avatar_file|
+ raise SafeFetch::FetchError, 'Invalid file' unless valid_file?(avatar_file)
- avatarable.avatar.attach(
- io: avatar_file,
- filename: avatar_file.original_filename,
- content_type: avatar_file.content_type
- )
-
- rescue Down::NotFound
- Rails.logger.info "AvatarFromUrlJob: avatar not found at #{avatar_url}"
- rescue Down::Error => e
+ avatarable.avatar.attach(
+ io: avatar_file.tempfile,
+ filename: avatar_file.original_filename,
+ content_type: avatar_file.content_type
+ )
+ end
+ rescue SafeFetch::HttpError => e
+ if e.message.start_with?('404')
+ Rails.logger.info "AvatarFromUrlJob: avatar not found at #{avatar_url}"
+ else
+ Rails.logger.error "AvatarFromUrlJob error for #{avatar_url}: #{e.class} - #{e.message}"
+ end
+ rescue SafeFetch::Error => e
Rails.logger.error "AvatarFromUrlJob error for #{avatar_url}: #{e.class} - #{e.message}"
ensure
update_avatar_sync_attributes(avatarable, avatar_url)
diff --git a/app/services/sms/incoming_message_service.rb b/app/services/sms/incoming_message_service.rb
index 1a39d799a..791699a6a 100644
--- a/app/services/sms/incoming_message_service.rb
+++ b/app/services/sms/incoming_message_service.rb
@@ -83,20 +83,28 @@ class Sms::IncomingMessageService
# we don't need to process this files since chatwoot doesn't support it
next if media_url.end_with?('.smil', '.xml')
- attachment_file = Down.download(
- media_url,
- http_basic_authentication: [channel.provider_config['api_key'], channel.provider_config['api_secret']]
- )
-
- @message.attachments.new(
- account_id: @message.account_id,
- file_type: file_type(attachment_file.content_type),
- file: {
- io: attachment_file,
- filename: attachment_file.original_filename,
- content_type: attachment_file.content_type
- }
- )
+ download_attachment_file(media_url) do |attachment_file|
+ @message.attachments.new(
+ account_id: @message.account_id,
+ file_type: file_type(attachment_file.content_type),
+ file: {
+ io: attachment_file.tempfile,
+ filename: attachment_file.original_filename,
+ content_type: attachment_file.content_type
+ }
+ )
+ end
end
end
+
+ def download_attachment_file(media_url, &)
+ SafeFetch.fetch(
+ media_url,
+ http_basic_authentication: [channel.provider_config['api_key'], channel.provider_config['api_secret']],
+ allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES,
+ &
+ )
+ rescue SafeFetch::Error => e
+ Rails.logger.info "Error downloading SMS attachment from #{media_url}: #{e.message}: Skipping"
+ end
end
diff --git a/app/services/telegram/incoming_message_service.rb b/app/services/telegram/incoming_message_service.rb
index 897db29c3..99d57e167 100644
--- a/app/services/telegram/incoming_message_service.rb
+++ b/app/services/telegram/incoming_message_service.rb
@@ -150,19 +150,23 @@ class Telegram::IncomingMessageService
return
end
- attachment_file = Down.download(
- inbox.channel.get_telegram_file_path(file[:file_id])
- )
-
- @message.attachments.new(
- account_id: @message.account_id,
- file_type: file_content_type,
- file: {
- io: attachment_file,
- filename: attachment_file.original_filename,
- content_type: attachment_file.content_type
- }
- )
+ SafeFetch.fetch(
+ file_download_path,
+ allowed_content_type_prefixes: %w[image/ video/ audio/],
+ allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES
+ ) do |attachment_file|
+ @message.attachments.new(
+ account_id: @message.account_id,
+ file_type: file_content_type,
+ file: {
+ io: attachment_file.tempfile,
+ filename: attachment_file.original_filename,
+ content_type: attachment_file.content_type
+ }
+ )
+ end
+ rescue SafeFetch::Error => e
+ Rails.logger.info "Error downloading Telegram attachment from #{file_download_path}: #{e.message}: Skipping"
end
def attach_location
diff --git a/app/services/tiktok/message_service.rb b/app/services/tiktok/message_service.rb
index 7acbb9486..2c76e0dbf 100644
--- a/app/services/tiktok/message_service.rb
+++ b/app/services/tiktok/message_service.rb
@@ -64,17 +64,17 @@ class Tiktok::MessageService
def create_image_message_attachment(message)
return unless image_message?
- attachment_file = fetch_attachment(channel, tt_conversation_id, tt_message_id, tt_image_media_id)
-
- message.attachments.new(
- account_id: message.account_id,
- file_type: :image,
- file: {
- io: attachment_file,
- filename: attachment_file.original_filename,
- content_type: attachment_file.content_type
- }
- )
+ fetch_attachment(channel, tt_conversation_id, tt_message_id, tt_image_media_id) do |attachment_file|
+ message.attachments.new(
+ account_id: message.account_id,
+ file_type: :image,
+ file: {
+ io: attachment_file.tempfile,
+ filename: attachment_file.original_filename,
+ content_type: attachment_file.content_type
+ }
+ )
+ end
end
def create_share_post_message_attachment(message)
diff --git a/app/services/tiktok/messaging_helpers.rb b/app/services/tiktok/messaging_helpers.rb
index 0f9d9e0b3..ad978d20f 100644
--- a/app/services/tiktok/messaging_helpers.rb
+++ b/app/services/tiktok/messaging_helpers.rb
@@ -66,9 +66,16 @@ module Tiktok::MessagingHelpers
message
end
- def fetch_attachment(channel, tt_conversation_id, tt_message_id, tt_image_media_id)
+ def fetch_attachment(channel, tt_conversation_id, tt_message_id, tt_image_media_id, &)
file_download_url = tiktok_client(channel).file_download_url(tt_conversation_id, tt_message_id, tt_image_media_id)
- Down.download(file_download_url, headers: { 'x-user' => channel.validated_access_token })
+ SafeFetch.fetch(
+ file_download_url,
+ headers: { 'x-user' => channel.validated_access_token },
+ allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES,
+ &
+ )
+ rescue SafeFetch::Error => e
+ Rails.logger.info "Error downloading TikTok attachment from #{file_download_url}: #{e.message}: Skipping"
end
def tiktok_client(channel)
diff --git a/app/services/twilio/incoming_message_service.rb b/app/services/twilio/incoming_message_service.rb
index d67b6d515..72b9ee1e8 100644
--- a/app/services/twilio/incoming_message_service.rb
+++ b/app/services/twilio/incoming_message_service.rb
@@ -143,27 +143,26 @@ class Twilio::IncomingMessageService
end
def attach_single_file(media_url)
- attachment_file = download_attachment_file(media_url)
- return if attachment_file.blank?
-
- @message.attachments.new(
- account_id: @message.account_id,
- file_type: file_type(attachment_file.content_type),
- file: {
- io: attachment_file,
- filename: attachment_file.original_filename,
- content_type: attachment_file.content_type
- }
- )
+ download_attachment_file(media_url) do |attachment_file|
+ @message.attachments.new(
+ account_id: @message.account_id,
+ file_type: file_type(attachment_file.content_type),
+ file: {
+ io: attachment_file.tempfile,
+ filename: attachment_file.original_filename,
+ content_type: attachment_file.content_type
+ }
+ )
+ end
end
- def download_attachment_file(media_url)
- download_with_auth(media_url)
- rescue Down::Error, Down::ClientError => e
- handle_download_attachment_error(e, media_url)
+ def download_attachment_file(media_url, &)
+ download_with_auth(media_url, &)
+ rescue SafeFetch::Error => e
+ handle_download_attachment_error(e, media_url, &)
end
- def download_with_auth(media_url)
+ def download_with_auth(media_url, &)
auth_credentials = if twilio_channel.api_key_sid.present?
# When using api_key_sid, the auth token should be the api_secret_key
[twilio_channel.api_key_sid, twilio_channel.auth_token]
@@ -172,13 +171,22 @@ class Twilio::IncomingMessageService
[twilio_channel.account_sid, twilio_channel.auth_token]
end
- Down.download(media_url, http_basic_authentication: auth_credentials)
+ SafeFetch.fetch(
+ media_url,
+ http_basic_authentication: auth_credentials,
+ allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES,
+ &
+ )
end
- def handle_download_attachment_error(error, media_url)
+ def handle_download_attachment_error(error, media_url, &)
Rails.logger.info "Error downloading attachment from Twilio: #{error.message}: Retrying without auth"
- Down.download(media_url)
- rescue StandardError => e
+ SafeFetch.fetch(
+ media_url,
+ allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES,
+ &
+ )
+ rescue SafeFetch::Error => e
Rails.logger.info "Error downloading attachment from Twilio: #{e.message}: Skipping"
nil
end
diff --git a/app/services/whatsapp/incoming_message_base_service.rb b/app/services/whatsapp/incoming_message_base_service.rb
index 5449c4740..78f8f45b5 100644
--- a/app/services/whatsapp/incoming_message_base_service.rb
+++ b/app/services/whatsapp/incoming_message_base_service.rb
@@ -148,18 +148,19 @@ class Whatsapp::IncomingMessageBaseService
attachment_payload = messages_data.first[message_type.to_sym]
@message.content ||= attachment_payload[:caption]
- attachment_file = download_attachment_file(attachment_payload)
- return if attachment_file.blank?
-
- @message.attachments.new(
- account_id: @message.account_id,
- file_type: file_content_type(message_type),
- file: {
- io: attachment_file,
- filename: attachment_file.original_filename,
- content_type: attachment_file.content_type
- }
- )
+ download_attachment_file(attachment_payload) do |attachment_file|
+ @message.attachments.new(
+ account_id: @message.account_id,
+ file_type: file_content_type(message_type),
+ file: {
+ io: attachment_file.tempfile,
+ filename: attachment_file.original_filename,
+ content_type: attachment_file.content_type
+ }
+ )
+ end
+ rescue SafeFetch::Error => e
+ Rails.logger.info "Error downloading WhatsApp attachment: #{e.message}: Skipping"
end
def attach_location
diff --git a/app/services/whatsapp/incoming_message_service_helpers.rb b/app/services/whatsapp/incoming_message_service_helpers.rb
index bac6f6222..217767221 100644
--- a/app/services/whatsapp/incoming_message_service_helpers.rb
+++ b/app/services/whatsapp/incoming_message_service_helpers.rb
@@ -1,6 +1,11 @@
module Whatsapp::IncomingMessageServiceHelpers
- def download_attachment_file(attachment_payload)
- Down.download(inbox.channel.media_url(attachment_payload[:id]), headers: inbox.channel.api_headers)
+ def download_attachment_file(attachment_payload, &)
+ SafeFetch.fetch(
+ inbox.channel.media_url(attachment_payload[:id]),
+ headers: inbox.channel.api_headers,
+ allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES,
+ &
+ )
end
def conversation_params
diff --git a/app/services/whatsapp/incoming_message_whatsapp_cloud_service.rb b/app/services/whatsapp/incoming_message_whatsapp_cloud_service.rb
index 164c3ac12..7d13a284a 100644
--- a/app/services/whatsapp/incoming_message_whatsapp_cloud_service.rb
+++ b/app/services/whatsapp/incoming_message_whatsapp_cloud_service.rb
@@ -8,13 +8,20 @@ class Whatsapp::IncomingMessageWhatsappCloudService < Whatsapp::IncomingMessageB
@processed_params ||= params[:entry].try(:first).try(:[], 'changes').try(:first).try(:[], 'value')
end
- def download_attachment_file(attachment_payload)
+ def download_attachment_file(attachment_payload, &)
url_response = HTTParty.get(
inbox.channel.media_url(attachment_payload[:id]),
headers: inbox.channel.api_headers
)
# This url response will be failure if the access token has expired.
inbox.channel.authorization_error! if url_response.unauthorized?
- Down.download(url_response.parsed_response['url'], headers: inbox.channel.api_headers) if url_response.success?
+ return unless url_response.success?
+
+ SafeFetch.fetch(
+ url_response.parsed_response['url'],
+ headers: inbox.channel.api_headers,
+ allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES,
+ &
+ )
end
end
diff --git a/enterprise/lib/captain/tools/http_tool.rb b/enterprise/lib/captain/tools/http_tool.rb
index b4593d27f..b37d3b815 100644
--- a/enterprise/lib/captain/tools/http_tool.rb
+++ b/enterprise/lib/captain/tools/http_tool.rb
@@ -1,6 +1,8 @@
require 'agents'
class Captain::Tools::HttpTool < Agents::Tool
+ MAX_RESPONSE_SIZE = 1.megabyte
+
def initialize(assistant, custom_tool)
@assistant = assistant
@custom_tool = custom_tool
@@ -15,8 +17,8 @@ class Captain::Tools::HttpTool < Agents::Tool
url = @custom_tool.build_request_url(params)
body = @custom_tool.build_request_body(params)
- response = execute_http_request(url, body, tool_context)
- @custom_tool.format_response(response.body)
+ response_body = execute_http_request(url, body, tool_context)
+ @custom_tool.format_response(response_body)
rescue StandardError => e
Rails.logger.error("HttpTool execution error for #{@custom_tool.slug}: #{e.class} - #{e.message}")
'An error occurred while executing the request'
@@ -24,89 +26,27 @@ class Captain::Tools::HttpTool < Agents::Tool
private
- PRIVATE_IP_RANGES = [
- IPAddr.new('127.0.0.0/8'), # IPv4 Loopback
- IPAddr.new('10.0.0.0/8'), # IPv4 Private network
- IPAddr.new('172.16.0.0/12'), # IPv4 Private network
- IPAddr.new('192.168.0.0/16'), # IPv4 Private network
- IPAddr.new('169.254.0.0/16'), # IPv4 Link-local
- IPAddr.new('::1'), # IPv6 Loopback
- IPAddr.new('fc00::/7'), # IPv6 Unique local addresses
- IPAddr.new('fe80::/10') # IPv6 Link-local
- ].freeze
-
- # Limit response size to prevent memory exhaustion and match LLM token limits
- # 1MB of text ≈ 250K tokens, which exceeds most LLM context windows
- MAX_RESPONSE_SIZE = 1.megabyte
-
def execute_http_request(url, body, tool_context)
- uri = URI.parse(url)
+ response_body = nil
- # Check if resolved IP is private
- check_private_ip!(uri.host)
-
- http = Net::HTTP.new(uri.host, uri.port)
- http.use_ssl = uri.scheme == 'https'
- http.read_timeout = 30
- http.open_timeout = 10
- http.max_retries = 0 # Disable redirects
-
- request = build_http_request(uri, body)
- apply_authentication(request)
- apply_metadata_headers(request, tool_context)
-
- response = http.request(request)
-
- raise "HTTP request failed with status #{response.code}" unless response.is_a?(Net::HTTPSuccess)
-
- validate_response!(response)
-
- response
- end
-
- def check_private_ip!(hostname)
- ip_address = IPAddr.new(Resolv.getaddress(hostname))
-
- raise 'Request blocked: hostname resolves to private IP address' if PRIVATE_IP_RANGES.any? { |range| range.include?(ip_address) }
- rescue Resolv::ResolvError, SocketError => e
- raise "DNS resolution failed: #{e.message}"
- end
-
- def validate_response!(response)
- content_length = response['content-length']&.to_i
- if content_length && content_length > MAX_RESPONSE_SIZE
- raise "Response size #{content_length} bytes exceeds maximum allowed #{MAX_RESPONSE_SIZE} bytes"
+ SafeFetch.fetch(
+ url,
+ method: @custom_tool.http_method,
+ body: body,
+ max_bytes: MAX_RESPONSE_SIZE,
+ headers: request_headers(tool_context, body),
+ http_basic_authentication: @custom_tool.build_basic_auth_credentials,
+ validate_content_type: false
+ ) do |response|
+ response_body = response.tempfile.read
end
- return unless response.body && response.body.bytesize > MAX_RESPONSE_SIZE
-
- raise "Response body size #{response.body.bytesize} bytes exceeds maximum allowed #{MAX_RESPONSE_SIZE} bytes"
+ response_body
end
- def build_http_request(uri, body)
- if @custom_tool.http_method == 'POST'
- request = Net::HTTP::Post.new(uri.request_uri)
- if body
- request.body = body
- request['Content-Type'] = 'application/json'
- end
- else
- request = Net::HTTP::Get.new(uri.request_uri)
- end
- request
- end
-
- def apply_authentication(request)
- headers = @custom_tool.build_auth_headers
- headers.each { |key, value| request[key] = value }
-
- credentials = @custom_tool.build_basic_auth_credentials
- request.basic_auth(*credentials) if credentials
- end
-
- def apply_metadata_headers(request, tool_context)
- state = tool_context&.state || {}
- metadata_headers = @custom_tool.build_metadata_headers(state)
- metadata_headers.each { |key, value| request[key] = value }
+ def request_headers(tool_context, body)
+ headers = @custom_tool.build_auth_headers.merge(@custom_tool.build_metadata_headers(tool_context&.state || {}))
+ headers['Content-Type'] = 'application/json' if @custom_tool.http_method == 'POST' && body.present?
+ headers
end
end
diff --git a/lib/safe_fetch.rb b/lib/safe_fetch.rb
index e6635c9c3..6cad5ef7c 100644
--- a/lib/safe_fetch.rb
+++ b/lib/safe_fetch.rb
@@ -2,11 +2,21 @@ require 'ssrf_filter'
module SafeFetch
DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES = %w[image/ video/].freeze
+ DEFAULT_ALLOWED_CONTENT_TYPES = [].freeze
+ DEFAULT_SENSITIVE_HEADERS = %w[authorization cookie].freeze
DEFAULT_OPEN_TIMEOUT = 2
DEFAULT_READ_TIMEOUT = 20
DEFAULT_MAX_BYTES_FALLBACK_MB = 40
- Result = Data.define(:tempfile, :filename, :content_type)
+ Result = Data.define(:tempfile, :filename, :content_type) do
+ def original_filename
+ filename
+ end
+
+ def close!
+ tempfile.close! if tempfile.respond_to?(:close!)
+ end
+ end
class Error < StandardError; end
class InvalidUrlError < Error; end
@@ -15,10 +25,17 @@ module SafeFetch
class HttpError < Error; end
class FileTooLargeError < Error; end
class UnsupportedContentTypeError < Error; end
+ class UnsupportedMethodError < Error; end
def self.fetch(url,
+ method: :get,
+ body: nil,
max_bytes: nil,
- allowed_content_type_prefixes: DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES)
+ headers: nil,
+ http_basic_authentication: nil,
+ allowed_content_type_prefixes: DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES,
+ allowed_content_types: DEFAULT_ALLOWED_CONTENT_TYPES,
+ validate_content_type: true)
raise ArgumentError, 'block required' unless block_given?
effective_max_bytes = max_bytes || default_max_bytes
@@ -26,11 +43,26 @@ module SafeFetch
filename = filename_for(uri)
tempfile = Tempfile.new('chatwoot-safe-fetch', binmode: true)
- response = stream_to_tempfile(url, tempfile, effective_max_bytes, allowed_content_type_prefixes)
+ response = stream_to_tempfile(
+ url,
+ method,
+ body,
+ tempfile,
+ effective_max_bytes,
+ headers,
+ http_basic_authentication,
+ allowed_content_type_prefixes,
+ allowed_content_types,
+ validate_content_type
+ )
raise HttpError, "#{response.code} #{response.message}" unless response.is_a?(Net::HTTPSuccess)
tempfile.rewind
- yield Result.new(tempfile: tempfile, filename: filename, content_type: response['content-type'])
+ yield Result.new(
+ tempfile: duplicate_tempfile(tempfile),
+ filename: filename,
+ content_type: normalize_content_type(response['content-type'])
+ )
rescue SsrfFilter::InvalidUriScheme, URI::InvalidURIError => e
raise InvalidUrlError, e.message
rescue SsrfFilter::Error, Resolv::ResolvError => e
@@ -44,18 +76,26 @@ module SafeFetch
class << self
private
- def stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes)
+ def stream_to_tempfile(url, method, body, tempfile, max_bytes, headers, http_basic_authentication,
+ allowed_content_type_prefixes, allowed_content_types, validate_content_type)
response = nil
bytes_written = 0
- SsrfFilter.get(
+ http_method = normalize_method(method)
+
+ SsrfFilter.public_send(
+ http_method,
url,
+ headers: headers,
+ body: body,
+ request_proc: request_proc(http_basic_authentication),
+ sensitive_headers: sensitive_headers(headers),
http_options: { open_timeout: DEFAULT_OPEN_TIMEOUT, read_timeout: DEFAULT_READ_TIMEOUT }
) do |res|
response = res
next unless res.is_a?(Net::HTTPSuccess)
- unless allowed_content_type?(res['content-type'], allowed_content_type_prefixes)
+ if validate_content_type && !allowed_content_type?(res['content-type'], allowed_content_type_prefixes, allowed_content_types)
raise UnsupportedContentTypeError, "content-type not allowed: #{res['content-type']}"
end
@@ -74,12 +114,28 @@ module SafeFetch
File.basename(uri.path).presence || "download-#{Time.current.to_i}-#{SecureRandom.hex(4)}"
end
+ def duplicate_tempfile(tempfile)
+ duplicated = tempfile.dup
+ duplicated.rewind
+ duplicated
+ end
+
def default_max_bytes
limit_mb = GlobalConfigService.load('MAXIMUM_FILE_UPLOAD_SIZE', DEFAULT_MAX_BYTES_FALLBACK_MB).to_i
limit_mb = DEFAULT_MAX_BYTES_FALLBACK_MB if limit_mb <= 0
limit_mb.megabytes
end
+ def request_proc(http_basic_authentication)
+ return unless http_basic_authentication.present?
+
+ proc { |request| request.basic_auth(*http_basic_authentication) }
+ end
+
+ def sensitive_headers(headers)
+ DEFAULT_SENSITIVE_HEADERS | Array(headers).map { |header, _| header.to_s }
+ end
+
def parse_and_validate_url!(url)
uri = URI.parse(url)
raise InvalidUrlError, 'scheme must be http or https' unless uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS)
@@ -88,11 +144,22 @@ module SafeFetch
uri
end
- def allowed_content_type?(value, prefixes)
- mime = value.to_s.split(';').first&.strip&.downcase
+ def normalize_method(method)
+ http_method = method.to_s.downcase.to_sym
+ return http_method if SsrfFilter::VERB_MAP.key?(http_method)
+
+ raise UnsupportedMethodError, "unsupported method: #{method}"
+ end
+
+ def normalize_content_type(value)
+ value.to_s.split(';').first&.strip&.downcase
+ end
+
+ def allowed_content_type?(value, prefixes, content_types)
+ mime = normalize_content_type(value)
return false if mime.blank?
- prefixes.any? { |prefix| mime.start_with?(prefix) }
+ Array(prefixes).any? { |prefix| mime.start_with?(prefix) } || Array(content_types).include?(mime)
end
end
end
diff --git a/spec/builders/messages/instagram/message_builder_spec.rb b/spec/builders/messages/instagram/message_builder_spec.rb
index 1ea5cd7af..e6a2822ce 100644
--- a/spec/builders/messages/instagram/message_builder_spec.rb
+++ b/spec/builders/messages/instagram/message_builder_spec.rb
@@ -4,9 +4,12 @@ describe Messages::Instagram::MessageBuilder do
subject(:instagram_direct_message_builder) { described_class }
before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('www.example.com').and_return(['93.184.216.34'])
+ allow(Resolv).to receive(:getaddresses).with('chatwoot-assets.local').and_return(['93.184.216.35'])
stub_request(:post, /graph\.instagram\.com/)
stub_request(:get, 'https://www.example.com/test.jpeg')
- .to_return(status: 200, body: '', headers: {})
+ .to_return(status: 200, body: 'image-data', headers: { 'Content-Type' => 'image/jpeg' })
end
let!(:account) { create(:account) }
@@ -120,6 +123,20 @@ describe Messages::Instagram::MessageBuilder do
expect(message.attachments.first.external_url).to eq(story_url)
end
+ it 'skips story reply attachments for blocked URLs' do
+ messaging = instagram_story_reply_event[:entry][0]['messaging'][0]
+ messaging['message']['reply_to']['story']['url'] = 'http://127.0.0.1/blocked.png'
+ create_instagram_contact_for_sender(messaging['sender']['id'], instagram_inbox)
+
+ expect do
+ described_class.new(messaging, instagram_inbox).perform
+ end.not_to raise_error
+
+ message = instagram_inbox.messages.first
+ expect(message.content).to eq('This is the story reply')
+ expect(message.attachments).to be_empty
+ end
+
it 'creates message with reply to mid' do
# Create first message to ensure reply to is valid
first_messaging = dm_params[:entry][0]['messaging'][0]
diff --git a/spec/builders/messages/instagram/messenger/message_builder_spec.rb b/spec/builders/messages/instagram/messenger/message_builder_spec.rb
index 07170d789..bd7694859 100644
--- a/spec/builders/messages/instagram/messenger/message_builder_spec.rb
+++ b/spec/builders/messages/instagram/messenger/message_builder_spec.rb
@@ -4,8 +4,12 @@ describe Messages::Instagram::Messenger::MessageBuilder do
subject(:instagram_message_builder) { described_class }
before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('www.example.com').and_return(['93.184.216.34'])
+ allow(Resolv).to receive(:getaddresses).with('chatwoot-assets.local').and_return(['93.184.216.35'])
stub_request(:post, /graph\.facebook\.com/)
stub_request(:get, 'https://www.example.com/test.jpeg')
+ .to_return(status: 200, body: 'image-data', headers: { 'Content-Type' => 'image/jpeg' })
end
let!(:account) { create(:account) }
@@ -132,6 +136,32 @@ describe Messages::Instagram::Messenger::MessageBuilder do
expect(message.attachments.first.external_url).to eq(story_url)
end
+ it 'skips story reply attachments for blocked URLs' do
+ messaging = instagram_story_reply_event[:entry][0]['messaging'][0]
+ sender_id = messaging['sender']['id']
+ messaging['message']['reply_to']['story']['url'] = 'http://127.0.0.1/blocked.png'
+
+ allow(Koala::Facebook::API).to receive(:new).and_return(fb_object)
+ allow(fb_object).to receive(:get_object).and_return(
+ {
+ name: 'Jane',
+ id: sender_id,
+ account_id: instagram_messenger_inbox.account_id,
+ profile_pic: 'https://chatwoot-assets.local/sample.png'
+ }.with_indifferent_access
+ )
+
+ create_instagram_contact_for_sender(sender_id, instagram_messenger_inbox)
+
+ expect do
+ described_class.new(messaging, instagram_messenger_inbox).perform
+ end.not_to raise_error
+
+ message = instagram_messenger_channel.inbox.messages.first
+ expect(message.content).to eq('This is the story reply')
+ expect(message.attachments).to be_empty
+ end
+
it 'creates message with for reply with mid' do
# create first message to ensure reply to is valid
first_message_data = dm_params[:entry][0]['messaging'][0]
diff --git a/spec/enterprise/lib/captain/tools/http_tool_spec.rb b/spec/enterprise/lib/captain/tools/http_tool_spec.rb
index e05308a7d..6a2b1aa5e 100644
--- a/spec/enterprise/lib/captain/tools/http_tool_spec.rb
+++ b/spec/enterprise/lib/captain/tools/http_tool_spec.rb
@@ -7,6 +7,12 @@ RSpec.describe Captain::Tools::HttpTool, type: :model do
let(:tool) { described_class.new(assistant, custom_tool) }
let(:tool_context) { Struct.new(:state).new({}) }
+ before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('example.com').and_return(['93.184.216.34'])
+ allow(Resolv).to receive(:getaddresses).with('internal.example.com').and_return(['10.0.0.1'])
+ end
+
describe '#active?' do
it 'returns true when custom tool is enabled' do
custom_tool.update!(enabled: true)
@@ -165,6 +171,15 @@ RSpec.describe Captain::Tools::HttpTool, type: :model do
end
context 'when handling errors' do
+ it 'returns generic error message when hostname resolves to a private IP' do
+ custom_tool.update!(endpoint_url: 'https://internal.example.com/data')
+
+ result = tool.perform(tool_context)
+
+ expect(result).to eq('An error occurred while executing the request')
+ expect(WebMock).not_to have_requested(:any, 'https://internal.example.com/data')
+ end
+
it 'returns generic error message on network failure' do
custom_tool.update!(endpoint_url: 'https://example.com/data')
stub_request(:get, 'https://example.com/data').to_raise(SocketError.new('Failed to connect'))
diff --git a/spec/jobs/avatar/avatar_from_url_job_spec.rb b/spec/jobs/avatar/avatar_from_url_job_spec.rb
index 8db3769ad..1086cffaa 100644
--- a/spec/jobs/avatar/avatar_from_url_job_spec.rb
+++ b/spec/jobs/avatar/avatar_from_url_job_spec.rb
@@ -1,9 +1,13 @@
require 'rails_helper'
RSpec.describe Avatar::AvatarFromUrlJob do
- let(:file) { fixture_file_upload(Rails.root.join('spec/assets/avatar.png'), 'image/png') }
let(:valid_url) { 'https://example.com/avatar.png' }
+ before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('example.com').and_return(['93.184.216.34'])
+ end
+
it 'enqueues the job' do
contact = create(:contact)
expect { described_class.perform_later(contact, 'https://example.com/avatar.png') }
@@ -14,7 +18,13 @@ RSpec.describe Avatar::AvatarFromUrlJob do
let(:avatarable) { create(:contact) }
it 'attaches and updates sync attributes' do
- expect(Down).to receive(:download).with(valid_url, max_size: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE).and_return(file)
+ stub_request(:get, valid_url)
+ .to_return(
+ status: 200,
+ body: File.read(Rails.root.join('spec/assets/avatar.png')),
+ headers: { 'Content-Type' => 'image/png' }
+ )
+
described_class.perform_now(avatarable, valid_url)
avatarable.reload
expect(avatarable.avatar).to be_attached
@@ -25,7 +35,13 @@ RSpec.describe Avatar::AvatarFromUrlJob do
it 'returns early when rate limited' do
ts = 30.seconds.ago.iso8601
avatarable.update(additional_attributes: { 'last_avatar_sync_at' => ts })
- expect(Down).not_to receive(:download)
+ stub_request(:get, valid_url)
+ .to_return(
+ status: 200,
+ body: File.read(Rails.root.join('spec/assets/avatar.png')),
+ headers: { 'Content-Type' => 'image/png' }
+ )
+
described_class.perform_now(avatarable, valid_url)
avatarable.reload
expect(avatarable.avatar).not_to be_attached
@@ -33,21 +49,28 @@ RSpec.describe Avatar::AvatarFromUrlJob do
expect(Time.zone.parse(avatarable.additional_attributes['last_avatar_sync_at']))
.to be > Time.zone.parse(ts)
expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(valid_url))
+ expect(WebMock).not_to have_requested(:get, valid_url)
end
it 'returns early when hash unchanged' do
avatarable.update(additional_attributes: { 'avatar_url_hash' => Digest::SHA256.hexdigest(valid_url) })
- expect(Down).not_to receive(:download)
+ stub_request(:get, valid_url)
+ .to_return(
+ status: 200,
+ body: File.read(Rails.root.join('spec/assets/avatar.png')),
+ headers: { 'Content-Type' => 'image/png' }
+ )
+
described_class.perform_now(avatarable, valid_url)
expect(avatarable.avatar).not_to be_attached
avatarable.reload
expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present
expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(valid_url))
+ expect(WebMock).not_to have_requested(:get, valid_url)
end
it 'updates sync attributes even when URL is invalid' do
invalid_url = 'invalid_url'
- expect(Down).not_to receive(:download)
described_class.perform_now(avatarable, invalid_url)
avatarable.reload
expect(avatarable.avatar).not_to be_attached
@@ -56,17 +79,12 @@ RSpec.describe Avatar::AvatarFromUrlJob do
end
it 'updates sync attributes when file download is valid but content type is unsupported' do
- temp_file = Tempfile.new(['invalid', '.xml'])
- temp_file.write('content')
- temp_file.rewind
-
- uploaded = ActionDispatch::Http::UploadedFile.new(
- tempfile: temp_file,
- filename: 'invalid.xml',
- type: 'application/xml'
- )
-
- expect(Down).to receive(:download).with(valid_url, max_size: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE).and_return(uploaded)
+ stub_request(:get, valid_url)
+ .to_return(
+ status: 200,
+ body: 'content',
+ headers: { 'Content-Type' => 'application/xml' }
+ )
described_class.perform_now(avatarable, valid_url)
avatarable.reload
@@ -74,9 +92,19 @@ RSpec.describe Avatar::AvatarFromUrlJob do
expect(avatarable.avatar).not_to be_attached
expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present
expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(valid_url))
+ end
- temp_file.close
- temp_file.unlink
+ it 'updates sync attributes when the avatar URL is blocked by SSRF protection' do
+ blocked_url = 'http://127.0.0.1/avatar.png'
+
+ expect do
+ described_class.perform_now(avatarable, blocked_url)
+ end.not_to raise_error
+
+ avatarable.reload
+ expect(avatarable.avatar).not_to be_attached
+ expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present
+ expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(blocked_url))
end
end
@@ -84,7 +112,13 @@ RSpec.describe Avatar::AvatarFromUrlJob do
let(:avatarable) { create(:agent_bot) }
it 'downloads and attaches avatar' do
- expect(Down).to receive(:download).with(valid_url, max_size: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE).and_return(file)
+ stub_request(:get, valid_url)
+ .to_return(
+ status: 200,
+ body: File.read(Rails.root.join('spec/assets/avatar.png')),
+ headers: { 'Content-Type' => 'image/png' }
+ )
+
described_class.perform_now(avatarable, valid_url)
expect(avatarable.avatar).to be_attached
end
@@ -93,23 +127,30 @@ RSpec.describe Avatar::AvatarFromUrlJob do
# ref: https://github.com/chatwoot/chatwoot/issues/10449
it 'does not raise error when downloaded file has no filename (invalid content)' do
contact = create(:contact)
- temp_file = Tempfile.new(['invalid', '.xml'])
- temp_file.write('content')
- temp_file.rewind
+ invalid_file = Tempfile.new('avatar-without-name')
- expect(Down).to receive(:download).with(valid_url, max_size: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE)
- .and_return(ActionDispatch::Http::UploadedFile.new(tempfile: temp_file, type: 'application/xml'))
+ allow(SafeFetch).to receive(:fetch)
+ .with(
+ valid_url,
+ max_bytes: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE,
+ allowed_content_type_prefixes: [],
+ allowed_content_types: %w[image/jpeg image/png image/gif]
+ ).and_yield(
+ SafeFetch::Result.new(
+ tempfile: invalid_file,
+ filename: nil,
+ content_type: 'image/png'
+ )
+ )
expect { described_class.perform_now(contact, valid_url) }.not_to raise_error
-
- temp_file.close
- temp_file.unlink
+ expect(contact.reload.avatar).not_to be_attached
+ ensure
+ invalid_file.close!
end
it 'skips sync attribute updates when URL is nil' do
contact = create(:contact)
- expect(Down).not_to receive(:download)
-
expect { described_class.perform_now(contact, nil) }.not_to raise_error
contact.reload
diff --git a/spec/lib/safe_fetch_spec.rb b/spec/lib/safe_fetch_spec.rb
index 70f9b05de..012dd09cf 100644
--- a/spec/lib/safe_fetch_spec.rb
+++ b/spec/lib/safe_fetch_spec.rb
@@ -9,6 +9,8 @@ RSpec.describe SafeFetch do
before do
allow(Resolv).to receive(:getaddresses).and_call_original
allow(Resolv).to receive(:getaddresses).with('example.com').and_return(['93.184.216.34'])
+ allow(Resolv).to receive(:getaddresses).with('redirect.example.com').and_return(['93.184.216.35'])
+ allow(Resolv).to receive(:getaddresses).with('cdn.example.com').and_return(['93.184.216.36'])
end
describe '.fetch' do
@@ -30,13 +32,17 @@ RSpec.describe SafeFetch do
end
end
- it 'closes the tempfile after the block returns' do
+ it 'keeps the yielded tempfile readable after the block returns' do
captured = nil
described_class.fetch(url) { |result| captured = result.tempfile }
- expect(captured.closed?).to be true
+ expect(captured.closed?).to be false
+ captured.rewind
+ expect(captured.read.bytesize).to be > 0
+ ensure
+ captured&.close!
end
- it 'closes the tempfile even when the block raises' do
+ it 'keeps the yielded tempfile readable even when the block raises' do
captured = nil
expect do
described_class.fetch(url) do |result|
@@ -44,7 +50,11 @@ RSpec.describe SafeFetch do
raise 'boom'
end
end.to raise_error('boom')
- expect(captured.closed?).to be true
+ expect(captured.closed?).to be false
+ captured.rewind
+ expect(captured.read.bytesize).to be > 0
+ ensure
+ captured&.close!
end
it 'defaults the filename to a unique "download--" when the URL has no path' do
@@ -63,6 +73,38 @@ RSpec.describe SafeFetch do
it 'requires a block' do
expect { described_class.fetch(url) }.to raise_error(ArgumentError, /block required/)
end
+
+ it 'forwards custom headers to the upstream request' do
+ described_class.fetch(url, headers: { 'x-user' => 'secret-token' }) { nil }
+
+ expect(WebMock).to(have_requested(:get, url)
+ .with { |request| request.headers['X-User'] == 'secret-token' })
+ end
+
+ it 'supports basic authentication' do
+ described_class.fetch(url, http_basic_authentication: %w[user password]) { nil }
+
+ expect(WebMock).to(have_requested(:get, url)
+ .with { |request| request.headers['Authorization']&.start_with?('Basic ') })
+ end
+
+ it 'supports POST requests with a request body' do
+ post_url = 'http://example.com/orders'
+ stub_request(:post, post_url)
+ .with(body: '{"order_id":"123"}', headers: { 'Content-Type' => 'application/json' })
+ .to_return(status: 200, body: '{"created":true}', headers: { 'Content-Type' => 'application/json' })
+
+ described_class.fetch(
+ post_url,
+ method: :post,
+ body: '{"order_id":"123"}',
+ headers: { 'Content-Type' => 'application/json' },
+ allowed_content_types: ['application/json']
+ ) { nil }
+
+ expect(WebMock).to have_requested(:post, post_url)
+ .with(body: '{"order_id":"123"}', headers: { 'Content-Type' => 'application/json' })
+ end
end
context 'with URL validation' do
@@ -169,6 +211,51 @@ RSpec.describe SafeFetch do
expect { described_class.fetch(url) { nil } }
.to raise_error(SafeFetch::UnsupportedContentTypeError)
end
+
+ it 'allows exact content-type matches when prefixes are empty' do
+ pdf_url = 'http://example.com/file.pdf'
+ stub_request(:get, pdf_url).to_return(
+ status: 200,
+ body: 'pdf-data',
+ headers: { 'Content-Type' => 'application/pdf' }
+ )
+
+ expect do
+ described_class.fetch(
+ pdf_url,
+ allowed_content_type_prefixes: [],
+ allowed_content_types: ['application/pdf']
+ ) { nil }
+ end.not_to raise_error
+ end
+
+ it 'skips content-type validation when disabled' do
+ stub_request(:get, url).to_return(status: 200, body: 'archive-data', headers: {})
+
+ expect { described_class.fetch(url, validate_content_type: false) { nil } }.not_to raise_error
+ end
+ end
+
+ context 'with redirects and sensitive headers' do
+ it 'strips custom headers on cross-origin redirects' do
+ redirect_url = 'http://redirect.example.com/image.png'
+ final_url = 'http://cdn.example.com/image.png'
+
+ stub_request(:get, redirect_url)
+ .with { |request| request.headers['X-User'] == 'secret-token' }
+ .to_return(status: 302, headers: { 'Location' => final_url })
+
+ stub_request(:get, final_url)
+ .with { |request| request.headers['X-User'].blank? }
+ .to_return(status: 200, body: 'image-data', headers: { 'Content-Type' => 'image/png' })
+
+ expect do
+ described_class.fetch(redirect_url, headers: { 'x-user' => 'secret-token' }) { nil }
+ end.not_to raise_error
+
+ expect(WebMock).to(have_requested(:get, final_url)
+ .with { |request| request.headers['X-User'].blank? })
+ end
end
context 'with body size cap' do
diff --git a/spec/services/sms/incoming_message_service_spec.rb b/spec/services/sms/incoming_message_service_spec.rb
index 5da56ff61..3b0887663 100644
--- a/spec/services/sms/incoming_message_service_spec.rb
+++ b/spec/services/sms/incoming_message_service_spec.rb
@@ -2,6 +2,11 @@ require 'rails_helper'
describe Sms::IncomingMessageService do
describe '#perform' do
+ before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('test.com').and_return(['93.184.216.34'])
+ end
+
let!(:sms_channel) { create(:channel_sms) }
let(:params) do
{
@@ -77,8 +82,10 @@ describe Sms::IncomingMessageService do
end
it 'creates attachment messages and ignores .smil files' do
- stub_request(:get, 'http://test.com/test.png').to_return(status: 200, body: File.read('spec/assets/sample.png'), headers: {})
- stub_request(:get, 'http://test.com/test2.png').to_return(status: 200, body: File.read('spec/assets/sample.png'), headers: {})
+ stub_request(:get, 'http://test.com/test.png')
+ .to_return(status: 200, body: File.read('spec/assets/sample.png'), headers: { 'Content-Type' => 'image/png' })
+ stub_request(:get, 'http://test.com/test2.png')
+ .to_return(status: 200, body: File.read('spec/assets/sample.png'), headers: { 'Content-Type' => 'image/png' })
media_params = { 'media': [
'http://test.com/test.smil',
@@ -92,6 +99,17 @@ describe Sms::IncomingMessageService do
expect(sms_channel.inbox.messages.first.content).to eq('test message')
expect(sms_channel.inbox.messages.first.attachments.present?).to be true
end
+
+ it 'skips blocked attachment URLs' do
+ media_params = { 'media': ['http://127.0.0.1/blocked.png'] }.with_indifferent_access
+
+ expect do
+ described_class.new(inbox: sms_channel.inbox, params: params.merge(media_params)).perform
+ end.not_to raise_error
+
+ expect(sms_channel.inbox.messages.first.content).to eq('test message')
+ expect(sms_channel.inbox.messages.first.attachments).to be_empty
+ end
end
end
end
diff --git a/spec/services/telegram/incoming_message_service_spec.rb b/spec/services/telegram/incoming_message_service_spec.rb
index b81b18756..e740d9e4e 100644
--- a/spec/services/telegram/incoming_message_service_spec.rb
+++ b/spec/services/telegram/incoming_message_service_spec.rb
@@ -2,31 +2,33 @@ require 'rails_helper'
describe Telegram::IncomingMessageService do
before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('chatwoot-assets.local').and_return(['93.184.216.34'])
stub_request(:any, /api.telegram.org/).to_return(headers: { content_type: 'application/json' }, body: {}.to_json, status: 200)
stub_request(:get, 'https://chatwoot-assets.local/sample.png').to_return(
status: 200,
body: File.read('spec/assets/sample.png'),
- headers: {}
+ headers: { 'Content-Type' => 'image/png' }
)
stub_request(:get, 'https://chatwoot-assets.local/sample.mov').to_return(
status: 200,
body: File.read('spec/assets/sample.mov'),
- headers: {}
+ headers: { 'Content-Type' => 'video/quicktime' }
)
stub_request(:get, 'https://chatwoot-assets.local/sample.mp3').to_return(
status: 200,
body: File.read('spec/assets/sample.mp3'),
- headers: {}
+ headers: { 'Content-Type' => 'audio/mpeg' }
)
stub_request(:get, 'https://chatwoot-assets.local/sample.ogg').to_return(
status: 200,
body: File.read('spec/assets/sample.ogg'),
- headers: {}
+ headers: { 'Content-Type' => 'audio/ogg' }
)
stub_request(:get, 'https://chatwoot-assets.local/sample.pdf').to_return(
status: 200,
body: File.read('spec/assets/sample.pdf'),
- headers: {}
+ headers: { 'Content-Type' => 'application/pdf' }
)
end
@@ -323,6 +325,29 @@ describe Telegram::IncomingMessageService do
end
end
+ context 'when the download path is an SSRF target' do
+ it 'skips the attachment' do
+ allow(telegram_channel.inbox.channel).to receive(:get_telegram_file_path).and_return('http://127.0.0.1/blocked.pdf')
+ params = {
+ 'update_id' => 2_342_342_343_242,
+ 'message' => {
+ 'document' => {
+ 'file_id' => 'AwADBAADbXXXXXXXXXXXGBdhD2l6_XX',
+ 'file_name' => 'blocked.pdf',
+ 'mime_type' => 'application/pdf',
+ 'file_size' => 536_392
+ }
+ }.merge(message_params)
+ }.with_indifferent_access
+
+ expect do
+ described_class.new(inbox: telegram_channel.inbox, params: params).perform
+ end.not_to raise_error
+
+ expect(telegram_channel.inbox.messages.first.attachments).to be_empty
+ end
+ end
+
context 'when valid location message params' do
it 'creates appropriate conversations, message and contacts' do
params = {
diff --git a/spec/services/tiktok/message_service_spec.rb b/spec/services/tiktok/message_service_spec.rb
index ee7b113ac..036575735 100644
--- a/spec/services/tiktok/message_service_spec.rb
+++ b/spec/services/tiktok/message_service_spec.rb
@@ -6,6 +6,7 @@ RSpec.describe Tiktok::MessageService do
let(:inbox) { channel.inbox }
let(:contact) { create(:contact, account: account) }
let(:contact_inbox) { create(:contact_inbox, inbox: inbox, contact: contact, source_id: 'tt-conv-1') }
+ let(:image_download_url) { 'https://chatwoot-assets.local/tiktok.png' }
let(:text_content) do
{
type: 'text',
@@ -20,6 +21,12 @@ RSpec.describe Tiktok::MessageService do
}.deep_symbolize_keys
end
+ before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('chatwoot-assets.local').and_return(['93.184.216.34'])
+ allow(channel).to receive(:validated_access_token).and_return('valid-access-token')
+ end
+
describe '#perform' do
subject(:perform_text_message) do
service = described_class.new(channel: channel, content: current_content)
@@ -115,15 +122,13 @@ RSpec.describe Tiktok::MessageService do
to_user: { id: 'biz-123' }
}.deep_symbolize_keys
- tempfile = Tempfile.new(['tiktok', '.png'])
- tempfile.write('fake-image')
- tempfile.rewind
- tempfile.define_singleton_method(:original_filename) { 'tiktok.png' }
- tempfile.define_singleton_method(:content_type) { 'image/png' }
+ stub_request(:get, image_download_url)
+ .with(headers: { 'X-User' => 'valid-access-token' })
+ .to_return(status: 200, body: File.read('spec/assets/sample.png'), headers: { 'Content-Type' => 'image/png' })
+ allow_any_instance_of(Tiktok::Client).to receive(:file_download_url).and_return(image_download_url)
service = described_class.new(channel: channel, content: content)
allow(service).to receive(:create_contact_inbox).and_return(contact_inbox)
- allow(service).to receive(:fetch_attachment).and_return(tempfile)
service.perform
@@ -131,8 +136,32 @@ RSpec.describe Tiktok::MessageService do
expect(message.attachments.count).to eq(1)
expect(message.attachments.last.file_type).to eq('image')
expect(message.attachments.last.file).to be_attached
- ensure
- tempfile.close!
+ expect(WebMock).to(have_requested(:get, image_download_url)
+ .with { |request| request.headers['X-User'] == 'valid-access-token' })
+ end
+
+ it 'skips blocked image attachment URLs' do
+ content = {
+ type: 'image',
+ message_id: 'tt-msg-5',
+ timestamp: 1_700_000_000_000,
+ conversation_id: 'tt-conv-1',
+ image: { media_id: 'media-1' },
+ from: 'Alice',
+ from_user: { id: 'user-1' },
+ to: 'Biz',
+ to_user: { id: 'biz-123' }
+ }.deep_symbolize_keys
+
+ allow_any_instance_of(Tiktok::Client).to receive(:file_download_url).and_return('http://127.0.0.1/blocked.png')
+
+ service = described_class.new(channel: channel, content: content)
+ allow(service).to receive(:create_contact_inbox).and_return(contact_inbox)
+
+ expect { service.perform }.not_to raise_error
+
+ message = Message.last
+ expect(message.attachments).to be_empty
end
context 'when lock_to_single_conversation is enabled' do
diff --git a/spec/services/twilio/incoming_message_service_spec.rb b/spec/services/twilio/incoming_message_service_spec.rb
index 190a6c45a..4f2ecb4f9 100644
--- a/spec/services/twilio/incoming_message_service_spec.rb
+++ b/spec/services/twilio/incoming_message_service_spec.rb
@@ -10,6 +10,11 @@ describe Twilio::IncomingMessageService do
let(:contact_inbox) { create(:contact_inbox, source_id: '+12345', contact: contact, inbox: twilio_channel.inbox) }
let!(:conversation) { create(:conversation, contact: contact, inbox: twilio_channel.inbox, contact_inbox: contact_inbox) }
+ before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('chatwoot-assets.local').and_return(['93.184.216.34'])
+ end
+
describe '#perform' do
it 'creates a new message in existing conversation' do
params = {
@@ -195,14 +200,23 @@ describe Twilio::IncomingMessageService do
expect(conversation.reload.messages.last.attachments.count).to eq(1)
expect(conversation.reload.messages.last.attachments.first.file_type).to eq('image')
end
+
+ it 'downloads the attachment using basic auth' do
+ described_class.new(params: params_with_attachment).perform
+
+ expect(WebMock).to(have_requested(:get, 'https://chatwoot-assets.local/sample.png')
+ .with { |request| request.headers['Authorization']&.start_with?('Basic ') })
+ end
end
context 'when there is an error downloading the attachment' do
before do
stub_request(:get, 'https://chatwoot-assets.local/sample.png')
- .to_raise(Down::Error.new('Download error'))
+ .with { |request| request.headers['Authorization']&.start_with?('Basic ') }
+ .to_raise(SocketError.new('Download error'))
stub_request(:get, 'https://chatwoot-assets.local/sample.png')
+ .with { |request| request.headers['Authorization'].blank? }
.to_return(status: 200, body: 'image data', headers: { 'Content-Type' => 'image/png' })
end
@@ -227,6 +241,8 @@ describe Twilio::IncomingMessageService do
expect(conversation.reload.messages.last.content).to eq('testing3')
expect(conversation.reload.messages.last.attachments.count).to eq(1)
expect(conversation.reload.messages.last.attachments.first.file_type).to eq('image')
+ expect(WebMock).to(have_requested(:get, 'https://chatwoot-assets.local/sample.png')
+ .with { |request| request.headers['Authorization'].blank? })
end
end
@@ -283,6 +299,31 @@ describe Twilio::IncomingMessageService do
end
end
+ context 'when an attachment URL is an SSRF target' do
+ let(:params_with_blocked_attachment) do
+ {
+ SmsSid: 'SMxx',
+ From: '+12345',
+ AccountSid: 'ACxxx',
+ MessagingServiceSid: twilio_channel.messaging_service_sid,
+ Body: 'blocked media',
+ NumMedia: '1',
+ MediaContentType0: 'image/jpeg',
+ MediaUrl0: 'http://127.0.0.1/blocked.png'
+ }
+ end
+
+ it 'creates the message and skips the attachment' do
+ expect do
+ described_class.new(params: params_with_blocked_attachment).perform
+ end.not_to raise_error
+
+ message = conversation.reload.messages.last
+ expect(message.content).to eq('blocked media')
+ expect(message.attachments.count).to eq(0)
+ end
+ end
+
context 'when ProfileName is provided for WhatsApp' do
it 'uses ProfileName as contact name' do
params = {
diff --git a/spec/services/whatsapp/incoming_message_service_spec.rb b/spec/services/whatsapp/incoming_message_service_spec.rb
index 2ecf60acb..76c08e984 100644
--- a/spec/services/whatsapp/incoming_message_service_spec.rb
+++ b/spec/services/whatsapp/incoming_message_service_spec.rb
@@ -3,6 +3,8 @@ require 'rails_helper'
describe Whatsapp::IncomingMessageService do
describe '#perform' do
before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('waba.360dialog.io').and_return(['93.184.216.34'])
stub_request(:post, 'https://waba.360dialog.io/v1/configs/webhook')
end
@@ -214,7 +216,7 @@ describe Whatsapp::IncomingMessageService do
stub_request(:get, whatsapp_channel.media_url('b1c68f38-8734-4ad3-b4a1-ef0c10d683')).to_return(
status: 200,
body: File.read('spec/assets/sample.png'),
- headers: {}
+ headers: { 'Content-Type' => 'image/png' }
)
params = {
'contacts' => [{ 'profile' => { 'name' => 'Sojan Jose' }, 'wa_id' => '2423423243' }],
@@ -231,6 +233,26 @@ describe Whatsapp::IncomingMessageService do
expect(whatsapp_channel.inbox.messages.first.content).to eq('Check out my product!')
expect(whatsapp_channel.inbox.messages.first.attachments.present?).to be true
end
+
+ it 'skips blocked attachment URLs' do
+ allow(whatsapp_channel).to receive(:media_url).and_return('http://127.0.0.1/blocked.png')
+ params = {
+ 'contacts' => [{ 'profile' => { 'name' => 'Sojan Jose' }, 'wa_id' => '2423423243' }],
+ 'messages' => [{ 'from' => '2423423243', 'id' => 'SDFADSf23sfasdafasdfa',
+ 'image' => { 'id' => 'b1c68f38-8734-4ad3-b4a1-ef0c10d683',
+ 'mime_type' => 'image/jpeg',
+ 'sha256' => '29ed500fa64eb55fc19dc4124acb300e5dcca0f822a301ae99944db',
+ 'caption' => 'Check out my product!' },
+ 'timestamp' => '1633034394', 'type' => 'image' }]
+ }.with_indifferent_access
+
+ expect do
+ described_class.new(inbox: whatsapp_channel.inbox, params: params).perform
+ end.not_to raise_error
+
+ expect(whatsapp_channel.inbox.messages.first.content).to eq('Check out my product!')
+ expect(whatsapp_channel.inbox.messages.first.attachments).to be_empty
+ end
end
context 'when valid location message params' do
diff --git a/spec/services/whatsapp/incoming_message_whatsapp_cloud_service_spec.rb b/spec/services/whatsapp/incoming_message_whatsapp_cloud_service_spec.rb
index 4b6841811..4f0e17cbd 100644
--- a/spec/services/whatsapp/incoming_message_whatsapp_cloud_service_spec.rb
+++ b/spec/services/whatsapp/incoming_message_whatsapp_cloud_service_spec.rb
@@ -2,6 +2,11 @@ require 'rails_helper'
describe Whatsapp::IncomingMessageWhatsappCloudService do
describe '#perform' do
+ before do
+ allow(Resolv).to receive(:getaddresses).and_call_original
+ allow(Resolv).to receive(:getaddresses).with('chatwoot-assets.local').and_return(['93.184.216.34'])
+ end
+
after do
Redis::Alfred.scan_each(match: 'MESSAGE_SOURCE_KEY::*') { |key| Redis::Alfred.delete(key) }
end
@@ -42,6 +47,28 @@ describe Whatsapp::IncomingMessageWhatsappCloudService do
expect_message_has_attachment
end
+ it 'sends the provider headers when downloading the resolved media URL' do
+ stub_media_url_request
+ stub_sample_png_request
+
+ described_class.new(inbox: whatsapp_channel.inbox, params: params).perform
+
+ expect(WebMock).to(have_requested(:get, 'https://chatwoot-assets.local/sample.png')
+ .with { |request| request.headers['Authorization']&.start_with?('Bearer ') })
+ end
+
+ it 'skips blocked resolved media URLs' do
+ stub_media_url_request(url: 'http://127.0.0.1/blocked.png')
+
+ expect do
+ described_class.new(inbox: whatsapp_channel.inbox, params: params).perform
+ end.not_to raise_error
+
+ expect_conversation_created
+ expect_message_content
+ expect(whatsapp_channel.inbox.messages.first.attachments).to be_empty
+ end
+
it 'increments reauthorization count if fetching attachment fails' do
stub_request(
:get,
@@ -169,7 +196,7 @@ describe Whatsapp::IncomingMessageWhatsappCloudService do
# Métodos auxiliares para reduzir o tamanho do exemplo
- def stub_media_url_request
+ def stub_media_url_request(url: 'https://chatwoot-assets.local/sample.png')
stub_request(
:get,
whatsapp_channel.media_url('b1c68f38-8734-4ad3-b4a1-ef0c10d683')
@@ -177,7 +204,7 @@ describe Whatsapp::IncomingMessageWhatsappCloudService do
status: 200,
body: {
messaging_product: 'whatsapp',
- url: 'https://chatwoot-assets.local/sample.png',
+ url: url,
mime_type: 'image/jpeg',
sha256: 'sha256',
file_size: 'SIZE',
@@ -190,7 +217,8 @@ describe Whatsapp::IncomingMessageWhatsappCloudService do
def stub_sample_png_request
stub_request(:get, 'https://chatwoot-assets.local/sample.png').to_return(
status: 200,
- body: File.read('spec/assets/sample.png')
+ body: File.read('spec/assets/sample.png'),
+ headers: { 'Content-Type' => 'image/png' }
)
end