LeadMagnet/leadchat
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix: harden fetching on upload endpoint (#14012)

Browse Source
This commit is contained in:
Shivam Mishra
2026-04-08 10:47:54 +05:30
committed by GitHub
parent 4f94ad4a75
commit 871f2f4d56
7 changed files with 494 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
Gemfile
View File

@@ -40,6 +40,8 @@ gem 'json_refs'
gem 'rack-attack', '>= 6.7.0'
# a utility tool for streaming, flexible and safe downloading of remote files
gem 'down'
# SSRF-safe URL fetching
gem 'ssrf_filter', '~> 1.5'
# authentication type to fetch and send mail over oauth2.0
gem 'gmail_xoauth'
# Lock net-smtp to 0.3.4 to avoid issues with gmail_xoauth2

2
Gemfile.lock
View File

@@ -942,6 +942,7 @@ GEM
activesupport (>= 5.2)
sprockets (>= 3.0.0)
squasher (0.7.2)
ssrf_filter (1.5.0)
stackprof (0.2.25)
statsd-ruby (1.5.0)
stripe (18.0.1)
@@ -1158,6 +1159,7 @@ DEPENDENCIES
spring
spring-watcher-listen
squasher
ssrf_filter (~> 1.5)
stackprof
stripe (~> 18.0)
telephone_number

42
app/controllers/api/v1/accounts/upload_controller.rb
View File

@@ -5,7 +5,7 @@ class Api::V1::Accounts::UploadController < Api::V1::Accounts::BaseController
elsif params[:external_url].present?
create_from_url
else
render_error('No file or URL provided', :unprocessable_entity)
render_error(I18n.t('errors.upload.missing_input'), :unprocessable_entity)
end
render_success(result) if result.is_a?(ActiveStorage::Blob)
@@ -19,35 +19,21 @@ class Api::V1::Accounts::UploadController < Api::V1::Accounts::BaseController
end
def create_from_url
uri = parse_uri(params[:external_url])
return if performed?
fetch_and_process_file_from_uri(uri)
end
def parse_uri(url)
uri = URI.parse(url)
validate_uri(uri)
uri
rescue URI::InvalidURIError, SocketError
render_error('Invalid URL provided', :unprocessable_entity)
nil
end
def validate_uri(uri)
raise URI::InvalidURIError unless uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS)
end
def fetch_and_process_file_from_uri(uri)
uri.open do |file|
create_and_save_blob(file, File.basename(uri.path), file.content_type)
SafeFetch.fetch(params[:external_url].to_s) do |result|
create_and_save_blob(result.tempfile, result.filename, result.content_type)
end
rescue OpenURI::HTTPError => e
render_error("Failed to fetch file from URL: #{e.message}", :unprocessable_entity)
rescue SocketError
render_error('Invalid URL provided', :unprocessable_entity)
rescue SafeFetch::HttpError => e
render_error(I18n.t('errors.upload.fetch_failed_with_message', message: e.message), :unprocessable_entity)
rescue SafeFetch::FetchError
render_error(I18n.t('errors.upload.fetch_failed'), :unprocessable_entity)
rescue SafeFetch::FileTooLargeError
render_error(I18n.t('errors.upload.file_too_large'), :unprocessable_entity)
rescue SafeFetch::UnsupportedContentTypeError
render_error(I18n.t('errors.upload.unsupported_content_type'), :unprocessable_entity)
rescue SafeFetch::Error
render_error(I18n.t('errors.upload.invalid_url'), :unprocessable_entity)
rescue StandardError
render_error('An unexpected error occurred', :internal_server_error)
render_error(I18n.t('errors.upload.unexpected'), :internal_server_error)
end
def create_and_save_blob(io, filename, content_type)

8
config/locales/en.yml
View File

@@ -66,6 +66,14 @@ en:
not_found: Assignment policy not found
attachments:
invalid: Invalid attachment
upload:
missing_input: 'No file or URL provided'
invalid_url: 'Invalid URL provided'
fetch_failed: 'Failed to fetch file from URL'
fetch_failed_with_message: 'Failed to fetch file from URL: %{message}'
file_too_large: 'File exceeds the maximum allowed size'
unsupported_content_type: 'File type not supported (only images and videos are allowed)'
unexpected: 'An unexpected error occurred'
saml:
feature_not_enabled: SAML feature not enabled for this account
sso_not_enabled: SAML SSO is not enabled for this installation

98
lib/safe_fetch.rb Normal file
View File

@@ -0,0 +1,98 @@
require 'ssrf_filter'
module SafeFetch
DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES = %w[image/ video/].freeze
DEFAULT_OPEN_TIMEOUT = 2
DEFAULT_READ_TIMEOUT = 20
DEFAULT_MAX_BYTES_FALLBACK_MB = 40
Result = Data.define(:tempfile, :filename, :content_type)
class Error < StandardError; end
class InvalidUrlError < Error; end
class UnsafeUrlError < Error; end
class FetchError < Error; end
class HttpError < Error; end
class FileTooLargeError < Error; end
class UnsupportedContentTypeError < Error; end
def self.fetch(url,
max_bytes: nil,
allowed_content_type_prefixes: DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES)
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)
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'])
rescue SsrfFilter::InvalidUriScheme, URI::InvalidURIError => e
raise InvalidUrlError, e.message
rescue SsrfFilter::Error, Resolv::ResolvError => e
raise UnsafeUrlError, e.message
rescue Net::OpenTimeout, Net::ReadTimeout, SocketError, OpenSSL::SSL::SSLError => e
raise FetchError, e.message
ensure
tempfile&.close!
end
class << self
private
def stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes)
response = nil
bytes_written = 0
SsrfFilter.get(
url,
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)
raise UnsupportedContentTypeError, "content-type not allowed: #{res['content-type']}"
end
res.read_body do |chunk|
bytes_written += chunk.bytesize
raise FileTooLargeError, "exceeded #{max_bytes} bytes" if bytes_written > max_bytes
tempfile.write(chunk)
end
end
response
end
def filename_for(uri)
File.basename(uri.path).presence || "download-#{Time.current.to_i}-#{SecureRandom.hex(4)}"
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 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)
raise InvalidUrlError, 'missing host' if uri.host.blank?
uri
end
def allowed_content_type?(value, prefixes)
mime = value.to_s.split(';').first&.strip&.downcase
return false if mime.blank?
prefixes.any? { |prefix| mime.start_with?(prefix) }
end
end
end

113
spec/controllers/api/v1/upload_controller_spec.rb
View File

@@ -39,6 +39,11 @@ RSpec.describe 'Api::V1::Accounts::UploadController', type: :request do
let(:valid_external_url) { 'http://example.com/image.jpg' }
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('error.example.com').and_return(['93.184.216.34'])
allow(Resolv).to receive(:getaddresses).with('nonexistent.example.com').and_return(['93.184.216.34'])
stub_request(:get, valid_external_url)
.to_return(status: 200, body: File.new(Rails.root.join('spec/assets/avatar.png')), headers: { 'Content-Type' => 'image/png' })
end
@@ -82,7 +87,7 @@ RSpec.describe 'Api::V1::Accounts::UploadController', type: :request do
params: { external_url: 'http://nonexistent.example.com' }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid URL provided')
expect(response.parsed_body['error']).to eq('Failed to fetch file from URL')
end
it 'handles HTTP errors' do
@@ -96,6 +101,112 @@ RSpec.describe 'Api::V1::Accounts::UploadController', type: :request do
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to start_with('Failed to fetch file from URL')
end
it 'rejects oversized responses with a file-size message' do
stub_request(:get, valid_external_url)
.to_return(status: 200,
body: 'x' * (41 * 1024 * 1024),
headers: { 'Content-Type' => 'image/png' })
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: valid_external_url }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('File exceeds the maximum allowed size')
end
it 'rejects unsupported content types with a file-type message' do
stub_request(:get, valid_external_url)
.to_return(status: 200,
body: '<html></html>',
headers: { 'Content-Type' => 'text/html' })
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: valid_external_url }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('File type not supported (only images and videos are allowed)')
end
context 'with SSRF attack vectors' do
it 'blocks requests to private IP ranges (10.x.x.x)' do
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: 'http://10.0.0.1/secret' }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid URL provided')
end
it 'blocks requests to private IP ranges (172.16.x.x)' do
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: 'http://172.16.0.1/secret' }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid URL provided')
end
it 'blocks requests to private IP ranges (192.168.x.x)' do
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: 'http://192.168.1.1/secret' }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid URL provided')
end
it 'blocks requests to loopback addresses' do
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: 'http://127.0.0.1/secret' }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid URL provided')
end
it 'blocks requests to AWS metadata service (169.254.169.254)' do
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: 'http://169.254.169.254/latest/meta-data/iam/security-credentials/' }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid URL provided')
end
it 'blocks requests to localhost' do
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: 'http://localhost/secret' }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid URL provided')
end
it 'blocks requests to .local domains' do
allow(Resolv).to receive(:getaddresses).with('server.local').and_return(['192.168.1.100'])
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: 'http://server.local/secret' }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid URL provided')
end
it 'blocks DNS rebinding attacks (hostname resolving to private IP)' do
allow(Resolv).to receive(:getaddresses).with('evil.attacker.com').and_return(['10.0.0.1'])
post upload_url,
headers: user.create_new_auth_token,
params: { external_url: 'http://evil.attacker.com/secret' }
expect(response).to have_http_status(:unprocessable_entity)
expect(response.parsed_body['error']).to eq('Invalid URL provided')
end
end
end
it 'returns an error when no file or URL is provided' do

258
spec/lib/safe_fetch_spec.rb Normal file
View File

@@ -0,0 +1,258 @@
require 'rails_helper'
# `SafeFetch.fetch` is a custom method that requires a block (it yields a Result);
# it is NOT `Hash#fetch`, so RuboCop's autocorrect to `fetch(url, nil)` would break the API.
# rubocop:disable Style/RedundantFetchBlock
RSpec.describe SafeFetch do
let(:url) { 'http://example.com/image.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
describe '.fetch' do
context 'with a valid public URL serving an image' do
before do
stub_request(:get, url).to_return(
status: 200,
body: File.new(Rails.root.join('spec/assets/avatar.png')),
headers: { 'Content-Type' => 'image/png' }
)
end
it 'yields a Result with tempfile, filename, and content_type' do
described_class.fetch(url) do |result|
expect(result.tempfile).to be_a(Tempfile)
expect(result.filename).to eq('image.png')
expect(result.content_type).to eq('image/png')
expect(result.tempfile.size).to be > 0
end
end
it 'closes the tempfile after the block returns' do
captured = nil
described_class.fetch(url) { |result| captured = result.tempfile }
expect(captured.closed?).to be true
end
it 'closes the tempfile even when the block raises' do
captured = nil
expect do
described_class.fetch(url) do |result|
captured = result.tempfile
raise 'boom'
end
end.to raise_error('boom')
expect(captured.closed?).to be true
end
it 'defaults the filename to a unique "download-<timestamp>-<hex>" when the URL has no path' do
bare_url = 'http://example.com'
stub_request(:get, bare_url).to_return(
status: 200,
body: File.new(Rails.root.join('spec/assets/avatar.png')),
headers: { 'Content-Type' => 'image/png' }
)
described_class.fetch(bare_url) do |result|
expect(result.filename).to match(/\Adownload-\d+-[a-f0-9]{8}\z/)
end
end
it 'requires a block' do
expect { described_class.fetch(url) }.to raise_error(ArgumentError, /block required/)
end
end
context 'with URL validation' do
it 'raises InvalidUrlError for javascript: URLs' do
expect { described_class.fetch('javascript:alert(1)') { nil } }
.to raise_error(SafeFetch::InvalidUrlError)
end
it 'raises InvalidUrlError for mailto: URLs' do
expect { described_class.fetch('mailto:test@example.com') { nil } }
.to raise_error(SafeFetch::InvalidUrlError)
end
it 'raises InvalidUrlError for data: URLs' do
expect { described_class.fetch('data:text/html,<x>') { nil } }
.to raise_error(SafeFetch::InvalidUrlError)
end
it 'raises InvalidUrlError for ftp: URLs' do
expect { described_class.fetch('ftp://example.com/file') { nil } }
.to raise_error(SafeFetch::InvalidUrlError)
end
it 'raises InvalidUrlError for malformed URLs' do
expect { described_class.fetch('not_a_url') { nil } }
.to raise_error(SafeFetch::InvalidUrlError)
end
it 'raises InvalidUrlError when host is missing' do
expect { described_class.fetch('http:///path') { nil } }
.to raise_error(SafeFetch::InvalidUrlError, /missing host/)
end
end
context 'with SSRF protection (integration with ssrf_filter)' do
it 'raises UnsafeUrlError for private IP literals (10.x.x.x)' do
expect { described_class.fetch('http://10.0.0.1/secret') { nil } }
.to raise_error(SafeFetch::UnsafeUrlError)
end
it 'raises UnsafeUrlError for loopback addresses' do
expect { described_class.fetch('http://127.0.0.1/secret') { nil } }
.to raise_error(SafeFetch::UnsafeUrlError)
end
it 'raises UnsafeUrlError for AWS metadata IP (169.254.169.254)' do
expect { described_class.fetch('http://169.254.169.254/latest/meta-data/') { nil } }
.to raise_error(SafeFetch::UnsafeUrlError)
end
it 'raises UnsafeUrlError when hostname resolves to a private IP (DNS rebinding)' do
allow(Resolv).to receive(:getaddresses).with('evil.example.com').and_return(['10.0.0.1'])
expect { described_class.fetch('http://evil.example.com/secret') { nil } }
.to raise_error(SafeFetch::UnsafeUrlError)
end
end
context 'with content-type allowlist' do
it 'rejects text/html responses' do
stub_request(:get, url).to_return(
status: 200,
body: '<html></html>',
headers: { 'Content-Type' => 'text/html' }
)
expect { described_class.fetch(url) { nil } }
.to raise_error(SafeFetch::UnsupportedContentTypeError)
end
it 'rejects application/octet-stream responses' do
stub_request(:get, url).to_return(
status: 200,
body: 'x',
headers: { 'Content-Type' => 'application/octet-stream' }
)
expect { described_class.fetch(url) { nil } }
.to raise_error(SafeFetch::UnsupportedContentTypeError)
end
it 'allows video/mp4 responses' do
stub_request(:get, url).to_return(
status: 200,
body: File.new(Rails.root.join('spec/assets/avatar.png')),
headers: { 'Content-Type' => 'video/mp4' }
)
expect { described_class.fetch(url) { nil } }.not_to raise_error
end
it 'strips charset/boundary parameters before comparing' do
stub_request(:get, url).to_return(
status: 200,
body: 'x',
headers: { 'Content-Type' => 'image/png; charset=binary' }
)
expect { described_class.fetch(url) { nil } }.not_to raise_error
end
it 'rejects when the content-type header is missing' do
stub_request(:get, url).to_return(status: 200, body: 'x', headers: {})
expect { described_class.fetch(url) { nil } }
.to raise_error(SafeFetch::UnsupportedContentTypeError)
end
end
context 'with body size cap' do
it 'honours a custom max_bytes argument' do
stub_request(:get, url).to_return(
status: 200,
body: 'xxxxx',
headers: { 'Content-Type' => 'image/png' }
)
expect { described_class.fetch(url, max_bytes: 2) { nil } }
.to raise_error(SafeFetch::FileTooLargeError)
end
it 'reads the default cap from GlobalConfigService MAXIMUM_FILE_UPLOAD_SIZE (matching Attachment#validate_file_size)' do
allow(GlobalConfigService).to receive(:load).and_call_original
allow(GlobalConfigService).to receive(:load).with('MAXIMUM_FILE_UPLOAD_SIZE', 40).and_return('1')
oversize = 'x' * (1.megabyte + 1)
stub_request(:get, url).to_return(
status: 200,
body: oversize,
headers: { 'Content-Type' => 'image/png' }
)
expect { described_class.fetch(url) { nil } }
.to raise_error(SafeFetch::FileTooLargeError)
end
it 'falls back to 40 MB when GlobalConfigService returns a non-positive value' do
allow(GlobalConfigService).to receive(:load).and_call_original
allow(GlobalConfigService).to receive(:load).with('MAXIMUM_FILE_UPLOAD_SIZE', 40).and_return('-10')
# 1 MB body should pass under the 40 MB fallback
stub_request(:get, url).to_return(
status: 200,
body: 'x' * 1.megabyte,
headers: { 'Content-Type' => 'image/png' }
)
expect { described_class.fetch(url) { nil } }.not_to raise_error
end
it 'allows uploads between the old hardcoded 10 MB and the configured limit (regression check)' do
# Default config is 40 MB; a 15 MB upload must succeed.
# This is the exact regression scenario: with the old hardcoded 10 MB cap,
# this would have failed even though direct file uploads of the same size succeed.
allow(GlobalConfigService).to receive(:load).and_call_original
allow(GlobalConfigService).to receive(:load).with('MAXIMUM_FILE_UPLOAD_SIZE', 40).and_return('40')
stub_request(:get, url).to_return(
status: 200,
body: 'x' * (15 * 1024 * 1024),
headers: { 'Content-Type' => 'image/png' }
)
expect { described_class.fetch(url) { nil } }.not_to raise_error
end
end
context 'with network failures' do
it 'maps Net::ReadTimeout to FetchError' do
stub_request(:get, url).to_raise(Net::ReadTimeout)
expect { described_class.fetch(url) { nil } }
.to raise_error(SafeFetch::FetchError)
end
it 'maps SocketError to FetchError' do
stub_request(:get, url).to_raise(SocketError.new('connection refused'))
expect { described_class.fetch(url) { nil } }
.to raise_error(SafeFetch::FetchError)
end
end
context 'with non-2xx upstream responses' do
it 'raises HttpError with the status code in the message' do
stub_request(:get, url).to_return(status: 404, body: '', headers: {})
expect { described_class.fetch(url) { nil } }
.to raise_error(SafeFetch::HttpError, /404/)
end
end
end
end
# rubocop:enable Style/RedundantFetchBlock