diff --git a/app/jobs/avatar/avatar_from_url_job.rb b/app/jobs/avatar/avatar_from_url_job.rb
index 929e76597..49bf25803 100644
--- a/app/jobs/avatar/avatar_from_url_job.rb
+++ b/app/jobs/avatar/avatar_from_url_job.rb
@@ -9,27 +9,17 @@ class Avatar::AvatarFromUrlJob < ApplicationJob
include UrlHelper
queue_as :purgable
- MAX_DOWNLOAD_SIZE = 15 * 1024 * 1024
+ ALLOWED_CONTENT_TYPES = Avatarable::ALLOWED_AVATAR_CONTENT_TYPES
+ MAX_DOWNLOAD_SIZE = 15.megabytes
RATE_LIMIT_WINDOW = 1.minute
def perform(avatarable, avatar_url)
- return unless avatarable.respond_to?(:avatar)
- return unless url_valid?(avatar_url)
+ return unless syncable_avatar?(avatarable, avatar_url)
- 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)
-
- 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
+ fetch_and_attach_avatar(avatarable, avatar_url)
+ rescue SafeFetch::HttpError => e
+ log_http_error(avatar_url, e)
+ rescue SafeFetch::Error => e
Rails.logger.error "AvatarFromUrlJob error for #{avatar_url}: #{e.class} - #{e.message}"
ensure
update_avatar_sync_attributes(avatarable, avatar_url)
@@ -37,6 +27,41 @@ class Avatar::AvatarFromUrlJob < ApplicationJob
private
+ def syncable_avatar?(avatarable, avatar_url)
+ avatarable.respond_to?(:avatar) &&
+ url_valid?(avatar_url) &&
+ should_sync_avatar?(avatarable, avatar_url)
+ end
+
+ def fetch_and_attach_avatar(avatarable, avatar_url)
+ SafeFetch.fetch(
+ avatar_url,
+ max_bytes: MAX_DOWNLOAD_SIZE,
+ allowed_content_type_prefixes: [],
+ allowed_content_types: ALLOWED_CONTENT_TYPES
+ ) do |avatar_file|
+ attach_avatar(avatarable, avatar_file)
+ end
+ end
+
+ def attach_avatar(avatarable, avatar_file)
+ raise SafeFetch::FetchError, 'Invalid file' unless valid_file?(avatar_file)
+
+ avatarable.avatar.attach(
+ io: avatar_file.tempfile,
+ filename: avatar_file.original_filename,
+ content_type: avatar_file.content_type
+ )
+ end
+
+ def log_http_error(avatar_url, error)
+ if error.message.start_with?('404')
+ Rails.logger.info "AvatarFromUrlJob: avatar not found at #{avatar_url}"
+ else
+ Rails.logger.error "AvatarFromUrlJob error for #{avatar_url}: #{error.class} - #{error.message}"
+ end
+ end
+
def should_sync_avatar?(avatarable, avatar_url)
# Only Contacts are rate-limited and hash-gated.
return true unless avatarable.is_a?(Contact)
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb
index 94ca55037..3057f8d44 100644
--- a/app/models/concerns/avatarable.rb
+++ b/app/models/concerns/avatarable.rb
@@ -4,6 +4,8 @@ module Avatarable
extend ActiveSupport::Concern
include Rails.application.routes.url_helpers
+ ALLOWED_AVATAR_CONTENT_TYPES = %w[image/jpeg image/png image/gif image/webp].freeze
+
included do
has_one_attached :avatar
validate :acceptable_avatar, if: -> { avatar.changed? }
@@ -30,7 +32,6 @@ module Avatarable
errors.add(:avatar, 'is too big') if avatar.byte_size > 15.megabytes
- acceptable_types = ['image/jpeg', 'image/png', 'image/gif'].freeze
- errors.add(:avatar, 'filetype not supported') unless acceptable_types.include?(avatar.content_type)
+ errors.add(:avatar, 'filetype not supported') unless ALLOWED_AVATAR_CONTENT_TYPES.include?(avatar.content_type)
end
end
diff --git a/lib/safe_fetch.rb b/lib/safe_fetch.rb
index e6635c9c3..2264b2850 100644
--- a/lib/safe_fetch.rb
+++ b/lib/safe_fetch.rb
@@ -6,7 +6,11 @@ module SafeFetch
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
+ end
class Error < StandardError; end
class InvalidUrlError < Error; end
@@ -18,19 +22,15 @@ module SafeFetch
def self.fetch(url,
max_bytes: nil,
- allowed_content_type_prefixes: DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES)
+ allowed_content_type_prefixes: DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES,
+ allowed_content_types: [])
raise ArgumentError, 'block required' unless block_given?
effective_max_bytes = max_bytes || default_max_bytes
- uri = parse_and_validate_url!(url)
- filename = filename_for(uri)
+ filename = filename_for(parse_and_validate_url!(url))
tempfile = Tempfile.new('chatwoot-safe-fetch', binmode: true)
-
- response = stream_to_tempfile(url, tempfile, effective_max_bytes, allowed_content_type_prefixes)
- 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'])
+ response = fetch_response(url, tempfile, effective_max_bytes, allowed_content_type_prefixes, allowed_content_types)
+ yield build_result(tempfile, filename, response)
rescue SsrfFilter::InvalidUriScheme, URI::InvalidURIError => e
raise InvalidUrlError, e.message
rescue SsrfFilter::Error, Resolv::ResolvError => e
@@ -44,18 +44,23 @@ module SafeFetch
class << self
private
- def stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes)
+ def fetch_response(url, tempfile, max_bytes, allowed_content_type_prefixes, allowed_content_types)
+ stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes, allowed_content_types)
+ end
+
+ def stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes, allowed_content_types)
response = nil
bytes_written = 0
SsrfFilter.get(
url,
+ request_proc: ->(request) { apply_url_basic_auth(request) },
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)
+ unless allowed_content_type?(res['content-type'], allowed_content_type_prefixes, allowed_content_types)
raise UnsupportedContentTypeError, "content-type not allowed: #{res['content-type']}"
end
@@ -74,6 +79,14 @@ module SafeFetch
File.basename(uri.path).presence || "download-#{Time.current.to_i}-#{SecureRandom.hex(4)}"
end
+ def build_result(tempfile, filename, response)
+ raise HttpError, "#{response.code} #{response.message}" unless response.is_a?(Net::HTTPSuccess)
+
+ tempfile.rewind
+ content_type = normalized_content_type(response['content-type'])
+ Result.new(tempfile: tempfile, filename: filename, content_type: content_type)
+ 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
@@ -88,11 +101,24 @@ module SafeFetch
uri
end
- def allowed_content_type?(value, prefixes)
- mime = value.to_s.split(';').first&.strip&.downcase
+ def allowed_content_type?(value, prefixes, content_types)
+ mime = normalized_content_type(value)
return false if mime.blank?
- prefixes.any? { |prefix| mime.start_with?(prefix) }
+ prefixes.any? { |prefix| mime.start_with?(prefix) } || content_types.include?(mime)
+ end
+
+ def normalized_content_type(value)
+ value.to_s.split(';').first&.strip&.downcase
+ end
+
+ def apply_url_basic_auth(request)
+ uri = request.uri
+ return if uri.user.blank?
+
+ username = URI.decode_uri_component(uri.user)
+ password = URI.decode_uri_component(uri.password.to_s)
+ request.basic_auth(username, password)
end
end
end
diff --git a/spec/jobs/avatar/avatar_from_url_job_spec.rb b/spec/jobs/avatar/avatar_from_url_job_spec.rb
index 8db3769ad..e85c46390 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
@@ -22,10 +32,71 @@ RSpec.describe Avatar::AvatarFromUrlJob do
expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present
end
+ it 'attaches webp avatars and updates sync attributes' do
+ webp_url = 'https://example.com/avatar.webp'
+
+ stub_request(:get, webp_url)
+ .to_return(
+ status: 200,
+ body: File.read(Rails.root.join('spec/assets/avatar.png')),
+ headers: { 'Content-Type' => 'image/webp' }
+ )
+
+ described_class.perform_now(avatarable, webp_url)
+ avatarable.reload
+
+ expect(avatarable.avatar).to be_attached
+ expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(webp_url))
+ expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present
+ end
+
+ it 'attaches avatars with parameterized content type headers' do
+ parameterized_url = 'https://example.com/avatar-parameterized.png'
+
+ stub_request(:get, parameterized_url)
+ .to_return(
+ status: 200,
+ body: File.read(Rails.root.join('spec/assets/avatar.png')),
+ headers: { 'Content-Type' => 'IMAGE/PNG; charset=binary' }
+ )
+
+ described_class.perform_now(avatarable, parameterized_url)
+ avatarable.reload
+
+ expect(avatarable.avatar).to be_attached
+ expect(avatarable.avatar.blob.content_type).to eq('image/png')
+ expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(parameterized_url))
+ end
+
+ it 'attaches avatars from URLs with embedded basic auth credentials' do
+ authenticated_url = 'https://user:pass@example.com/avatar-authenticated.png'
+
+ stub_request(:get, 'https://example.com/avatar-authenticated.png')
+ .with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNz' })
+ .to_return(
+ status: 200,
+ body: File.read(Rails.root.join('spec/assets/avatar.png')),
+ headers: { 'Content-Type' => 'image/png' }
+ )
+
+ described_class.perform_now(avatarable, authenticated_url)
+ avatarable.reload
+
+ expect(avatarable.avatar).to be_attached
+ expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(authenticated_url))
+ end
+
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 +104,29 @@ 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 +135,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 +148,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 +168,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,22 +183,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: Avatar::AvatarFromUrlJob::ALLOWED_CONTENT_TYPES
+ ).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
diff --git a/spec/lib/safe_fetch_spec.rb b/spec/lib/safe_fetch_spec.rb
index d8d37115a..e2c513587 100644
--- a/spec/lib/safe_fetch_spec.rb
+++ b/spec/lib/safe_fetch_spec.rb
@@ -65,6 +65,23 @@ RSpec.describe SafeFetch do
end
end
+ context 'with embedded basic auth credentials' do
+ it 'passes decoded credentials to the request' do
+ authenticated_url = 'http://user+avatar%40example.com:p%40ss+word%3A1@example.com/image.png'
+ stub_request(:get, url)
+ .with(headers: { 'Authorization' => 'Basic dXNlcithdmF0YXJAZXhhbXBsZS5jb206cEBzcyt3b3JkOjE=' })
+ .to_return(
+ status: 200,
+ body: File.new(Rails.root.join('spec/assets/avatar.png')),
+ headers: { 'Content-Type' => 'image/png' }
+ )
+
+ described_class.fetch(authenticated_url) do |result|
+ expect(result.content_type).to eq('image/png')
+ end
+ end
+ end
+
context 'with URL validation' do
it 'raises InvalidUrlError for javascript: URLs' do
expect { described_class.fetch('javascript:alert(1)') { nil } }
@@ -153,14 +170,49 @@ RSpec.describe SafeFetch do
expect { described_class.fetch(url) { nil } }.not_to raise_error
end
- it 'strips charset/boundary parameters before comparing' do
+ it 'normalizes parameters and casing before yielding content_type' do
stub_request(:get, url).to_return(
status: 200,
body: 'x',
- headers: { 'Content-Type' => 'image/png; charset=binary' }
+ headers: { 'Content-Type' => 'IMAGE/PNG; charset=binary' }
)
- expect { described_class.fetch(url) { nil } }.not_to raise_error
+ described_class.fetch(url) do |result|
+ expect(result.content_type).to eq('image/png')
+ end
+ 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 'rejects exact content-type mismatches when prefixes are empty' do
+ stub_request(:get, url).to_return(
+ status: 200,
+ body: 'x',
+ headers: { 'Content-Type' => 'image/webp' }
+ )
+
+ expect do
+ described_class.fetch(
+ url,
+ allowed_content_type_prefixes: [],
+ allowed_content_types: ['image/png']
+ ) { nil }
+ end.to raise_error(described_class::UnsupportedContentTypeError)
end
it 'rejects when the content-type header is missing' do
diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb
index f21a81978..159454eec 100644
--- a/spec/models/contact_spec.rb
+++ b/spec/models/contact_spec.rb
@@ -16,6 +16,13 @@ RSpec.describe Contact do
describe 'concerns' do
it_behaves_like 'avatarable'
+
+ it 'accepts webp avatars' do
+ contact = build(:contact, account: create(:account))
+ contact.avatar.attach(get_blob_for(Rails.root.join('spec/assets/avatar.png'), 'image/webp'))
+
+ expect(contact).to be_valid
+ end
end
context 'when prepare contact attributes before validation' do