akadmin/chatwoot
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix: [CW-6931] Harden external downloads against SSRF [avatar from url job] (#14153)

Browse Source 
This routes external downloads used by avatar sync through SafeFetch. It closes the SSRF exposure from raw Down.download paths, preserves provider-specific auth and header flows, and adds regression coverage
for blocked internal URLs plus authenticated downloads.
Fixes # (issue): [CW-6931](https://linear.app/chatwoot/issue/CW-6931/avatarwidget-url-ssrf-downdownload-unprotected-unauth)
This commit is contained in:
Sony Mathew
2026-04-24 18:59:45 +05:30
committed by GitHub
parent c5fb8d73cc
commit 661608c0b1
6 changed files with 274 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/jobs/avatar/avatar_from_url_job.rb
+42 -17
View File
@@ -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)
app/models/concerns/avatarable.rb
+3 -2
View File
@@ -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
lib/safe_fetch.rb
+41 -15
View File
@@ -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
spec/jobs/avatar/avatar_from_url_job_spec.rb
+126 -28
View File
@@ -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('<invalid>content</invalid>')
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: '<invalid>content</invalid>',
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('<invalid>content</invalid>')
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
spec/lib/safe_fetch_spec.rb
+55 -3
View File
@@ -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
spec/models/contact_spec.rb
+7
View File
@@ -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