fix: [CW-6940] Fix SSRF issue for webhook trigger used by macros and automations (#14155)
This routes external downloads used by webhook fetch used by macros and acutomations 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-6940](https://linear.app/chatwoot/issue/CW-6940/ssrf-via-webhooksautomationmacros-non-upload-non-avatar)
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
class AgentBots::WebhookJob < WebhookJob
|
||||
queue_as :high
|
||||
retry_on RestClient::TooManyRequests, RestClient::InternalServerError, wait: 3.seconds, attempts: 3 do |job, error|
|
||||
retry_on Webhooks::Trigger::RetryableError, wait: 3.seconds, attempts: 3 do |job, error|
|
||||
url, payload, webhook_type = job.arguments
|
||||
kwargs = job.arguments.last.is_a?(Hash) ? job.arguments.last : {}
|
||||
Webhooks::Trigger.new(url, payload, webhook_type || :agent_bot_webhook, secret: kwargs[:secret],
|
||||
@@ -9,7 +9,7 @@ class AgentBots::WebhookJob < WebhookJob
|
||||
|
||||
def perform(url, payload, webhook_type = :agent_bot_webhook, secret: nil, delivery_id: nil)
|
||||
super(url, payload, webhook_type, secret: secret, delivery_id: delivery_id)
|
||||
rescue RestClient::TooManyRequests, RestClient::InternalServerError => e
|
||||
rescue Webhooks::Trigger::RetryableError => e
|
||||
Rails.logger.warn("[AgentBots::WebhookJob] attempt #{executions} failed #{e.class.name} payload=#{payload.to_json}")
|
||||
raise
|
||||
end
|
||||
|
||||
+10
-92
@@ -2,6 +2,8 @@ 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 proxy-authorization].freeze
|
||||
DEFAULT_OPEN_TIMEOUT = 2
|
||||
DEFAULT_READ_TIMEOUT = 20
|
||||
DEFAULT_MAX_BYTES_FALLBACK_MB = 40
|
||||
@@ -19,106 +21,22 @@ module SafeFetch
|
||||
class HttpError < Error; end
|
||||
class FileTooLargeError < Error; end
|
||||
class UnsupportedContentTypeError < Error; end
|
||||
class UnsupportedMethodError < Error; end
|
||||
end
|
||||
|
||||
def self.fetch(url,
|
||||
max_bytes: nil,
|
||||
allowed_content_type_prefixes: DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES,
|
||||
allowed_content_types: [])
|
||||
require_relative 'safe_fetch/request_options'
|
||||
require_relative 'safe_fetch/fetcher'
|
||||
|
||||
module SafeFetch
|
||||
def self.fetch(url, **, &)
|
||||
raise ArgumentError, 'block required' unless block_given?
|
||||
|
||||
effective_max_bytes = max_bytes || default_max_bytes
|
||||
filename = filename_for(parse_and_validate_url!(url))
|
||||
tempfile = Tempfile.new('chatwoot-safe-fetch', binmode: true)
|
||||
response = fetch_response(url, tempfile, effective_max_bytes, allowed_content_type_prefixes, allowed_content_types)
|
||||
yield build_result(tempfile, filename, response)
|
||||
Fetcher.new(RequestOptions.new(url: url, **)).fetch(&)
|
||||
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 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, allowed_content_types)
|
||||
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 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
|
||||
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, content_types)
|
||||
mime = normalized_content_type(value)
|
||||
return false if mime.blank?
|
||||
|
||||
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
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
class SafeFetch::Fetcher
|
||||
def initialize(options)
|
||||
@options = options
|
||||
end
|
||||
|
||||
def fetch
|
||||
with_tempfile do |tempfile|
|
||||
response = stream_response(tempfile)
|
||||
raise SafeFetch::HttpError, "#{response.code} #{response.message}" unless response.is_a?(Net::HTTPSuccess)
|
||||
|
||||
tempfile.rewind
|
||||
yield SafeFetch::Result.new(
|
||||
tempfile: tempfile,
|
||||
filename: options.filename,
|
||||
content_type: normalized_content_type(response['content-type'])
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
attr_reader :options
|
||||
|
||||
def with_tempfile
|
||||
tempfile = Tempfile.new('chatwoot-safe-fetch', binmode: true)
|
||||
yield tempfile
|
||||
ensure
|
||||
tempfile&.close!
|
||||
end
|
||||
|
||||
def stream_response(tempfile)
|
||||
response = nil
|
||||
bytes_written = 0
|
||||
|
||||
SsrfFilter.public_send(options.method, options.url, **options.request_options) do |res|
|
||||
response = res
|
||||
next unless res.is_a?(Net::HTTPSuccess)
|
||||
|
||||
validate_content_type!(res['content-type'])
|
||||
bytes_written = write_response_body(res, tempfile, bytes_written)
|
||||
end
|
||||
|
||||
response
|
||||
end
|
||||
|
||||
def validate_content_type!(content_type)
|
||||
return unless options.validate_content_type?
|
||||
return if allowed_content_type?(content_type)
|
||||
|
||||
raise SafeFetch::UnsupportedContentTypeError, "content-type not allowed: #{content_type}"
|
||||
end
|
||||
|
||||
def write_response_body(response, tempfile, bytes_written)
|
||||
response.read_body do |chunk|
|
||||
bytes_written += chunk.bytesize
|
||||
raise SafeFetch::FileTooLargeError, "exceeded #{options.effective_max_bytes} bytes" if bytes_written > options.effective_max_bytes
|
||||
|
||||
tempfile.write(chunk)
|
||||
end
|
||||
|
||||
bytes_written
|
||||
end
|
||||
|
||||
def allowed_content_type?(value)
|
||||
mime = normalized_content_type(value)
|
||||
return false if mime.blank?
|
||||
|
||||
options.allowed_content_type_prefixes.any? { |prefix| mime.start_with?(prefix) } ||
|
||||
options.allowed_content_types.include?(mime)
|
||||
end
|
||||
|
||||
def normalized_content_type(value)
|
||||
value.to_s.split(';').first&.strip&.downcase
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,116 @@
|
||||
class SafeFetch::RequestOptions
|
||||
DEFAULTS = {
|
||||
method: :get,
|
||||
body: nil,
|
||||
max_bytes: nil,
|
||||
open_timeout: SafeFetch::DEFAULT_OPEN_TIMEOUT,
|
||||
read_timeout: SafeFetch::DEFAULT_READ_TIMEOUT,
|
||||
headers: nil,
|
||||
http_basic_authentication: nil,
|
||||
allowed_content_type_prefixes: SafeFetch::DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES,
|
||||
allowed_content_types: SafeFetch::DEFAULT_ALLOWED_CONTENT_TYPES,
|
||||
validate_content_type: true
|
||||
}.freeze
|
||||
|
||||
attr_reader :allowed_content_type_prefixes, :allowed_content_types, :body, :headers,
|
||||
:http_basic_authentication, :method, :open_timeout, :read_timeout, :uri, :url
|
||||
|
||||
def initialize(url:, **options)
|
||||
config = DEFAULTS.merge(options)
|
||||
@url = url
|
||||
@uri = parse_and_validate_url!(url)
|
||||
@method = normalize_method(config[:method])
|
||||
@body = config[:body]
|
||||
@max_bytes = config[:max_bytes]
|
||||
@open_timeout = config[:open_timeout]
|
||||
@read_timeout = config[:read_timeout]
|
||||
@headers = normalize_headers(config[:headers])
|
||||
@http_basic_authentication = config[:http_basic_authentication]
|
||||
@allowed_content_type_prefixes = Array(config[:allowed_content_type_prefixes])
|
||||
@allowed_content_types = Array(config[:allowed_content_types])
|
||||
@validate_content_type = config[:validate_content_type]
|
||||
end
|
||||
|
||||
def effective_max_bytes
|
||||
@effective_max_bytes ||= @max_bytes || default_max_bytes
|
||||
end
|
||||
|
||||
def filename
|
||||
@filename ||= File.basename(uri.path).presence || "download-#{Time.current.to_i}-#{SecureRandom.hex(4)}"
|
||||
end
|
||||
|
||||
def request_options
|
||||
{
|
||||
headers: headers,
|
||||
body: body,
|
||||
request_proc: request_proc,
|
||||
sensitive_headers: sensitive_headers,
|
||||
http_options: { open_timeout: open_timeout, read_timeout: read_timeout }
|
||||
}
|
||||
end
|
||||
|
||||
def validate_content_type?
|
||||
@validate_content_type
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def default_max_bytes
|
||||
limit_mb = GlobalConfigService.load('MAXIMUM_FILE_UPLOAD_SIZE', SafeFetch::DEFAULT_MAX_BYTES_FALLBACK_MB).to_i
|
||||
limit_mb = SafeFetch::DEFAULT_MAX_BYTES_FALLBACK_MB if limit_mb <= 0
|
||||
limit_mb.megabytes
|
||||
end
|
||||
|
||||
def parse_and_validate_url!(value)
|
||||
parsed_uri = URI.parse(value)
|
||||
raise SafeFetch::InvalidUrlError, 'scheme must be http or https' unless parsed_uri.is_a?(URI::HTTP) || parsed_uri.is_a?(URI::HTTPS)
|
||||
raise SafeFetch::InvalidUrlError, 'missing host' if parsed_uri.host.blank?
|
||||
|
||||
parsed_uri
|
||||
end
|
||||
|
||||
def normalize_method(value)
|
||||
http_method = value.to_s.downcase.to_sym
|
||||
return http_method if SsrfFilter::VERB_MAP.key?(http_method)
|
||||
|
||||
raise SafeFetch::UnsupportedMethodError, "unsupported method: #{value}"
|
||||
end
|
||||
|
||||
def normalize_headers(value)
|
||||
value&.to_h
|
||||
end
|
||||
|
||||
def request_proc
|
||||
proc do |request|
|
||||
credentials = http_basic_authentication.presence || basic_authentication_for(request.uri)
|
||||
request.basic_auth(*credentials) if credentials.present?
|
||||
end
|
||||
end
|
||||
|
||||
def sensitive_headers
|
||||
SafeFetch::DEFAULT_SENSITIVE_HEADERS
|
||||
end
|
||||
|
||||
def basic_authentication_for(request_uri)
|
||||
uri_basic_authentication(request_uri) || original_uri_basic_authentication(request_uri)
|
||||
end
|
||||
|
||||
def original_uri_basic_authentication(request_uri)
|
||||
return unless same_origin?(request_uri, uri)
|
||||
|
||||
uri_basic_authentication(uri)
|
||||
end
|
||||
|
||||
def same_origin?(request_uri, other_uri)
|
||||
request_uri.scheme == other_uri.scheme && request_uri.hostname == other_uri.hostname && request_uri.port == other_uri.port
|
||||
end
|
||||
|
||||
def uri_basic_authentication(value)
|
||||
return if value.user.blank?
|
||||
|
||||
[
|
||||
URI.decode_uri_component(value.user),
|
||||
URI.decode_uri_component(value.password.to_s)
|
||||
]
|
||||
end
|
||||
end
|
||||
+30
-10
@@ -1,5 +1,15 @@
|
||||
class Webhooks::Trigger
|
||||
SUPPORTED_ERROR_HANDLE_EVENTS = %w[message_created message_updated].freeze
|
||||
RETRYABLE_AGENT_BOT_STATUSES = [429, 500].freeze
|
||||
|
||||
class RetryableError < StandardError
|
||||
attr_reader :status
|
||||
|
||||
def initialize(status:, message:)
|
||||
@status = status
|
||||
super(message)
|
||||
end
|
||||
end
|
||||
|
||||
def initialize(url, payload, webhook_type, secret: nil, delivery_id: nil)
|
||||
@url = url
|
||||
@@ -15,11 +25,9 @@ class Webhooks::Trigger
|
||||
|
||||
def execute
|
||||
perform_request
|
||||
rescue RestClient::TooManyRequests, RestClient::InternalServerError => e
|
||||
raise if @webhook_type == :agent_bot_webhook
|
||||
|
||||
handle_failure(e)
|
||||
rescue StandardError => e
|
||||
raise RetryableError.new(status: http_status(e), message: e.message) if retryable_agent_bot_error?(e)
|
||||
|
||||
handle_failure(e)
|
||||
end
|
||||
|
||||
@@ -32,17 +40,19 @@ class Webhooks::Trigger
|
||||
|
||||
def perform_request
|
||||
body = @payload.to_json
|
||||
RestClient::Request.execute(
|
||||
SafeFetch.fetch(
|
||||
@url,
|
||||
method: :post,
|
||||
url: @url,
|
||||
payload: body,
|
||||
body: body,
|
||||
headers: request_headers(body),
|
||||
timeout: webhook_timeout
|
||||
)
|
||||
open_timeout: webhook_timeout,
|
||||
read_timeout: webhook_timeout,
|
||||
validate_content_type: false
|
||||
) { |_response| nil }
|
||||
end
|
||||
|
||||
def request_headers(body)
|
||||
headers = { content_type: :json, accept: :json }
|
||||
headers = { 'Content-Type' => 'application/json', 'Accept' => 'application/json' }
|
||||
headers['X-Chatwoot-Delivery'] = @delivery_id if @delivery_id.present?
|
||||
if @secret.present?
|
||||
ts = Time.now.to_i.to_s
|
||||
@@ -111,4 +121,14 @@ class Webhooks::Trigger
|
||||
|
||||
timeout&.positive? ? timeout : 5
|
||||
end
|
||||
|
||||
def retryable_agent_bot_error?(error)
|
||||
@webhook_type == :agent_bot_webhook && RETRYABLE_AGENT_BOT_STATUSES.include?(http_status(error))
|
||||
end
|
||||
|
||||
def http_status(error)
|
||||
return unless error.is_a?(SafeFetch::HttpError)
|
||||
|
||||
error.message.to_s[/\A(\d{3})\b/, 1]&.to_i
|
||||
end
|
||||
end
|
||||
|
||||
@@ -8,7 +8,7 @@ RSpec.describe AgentBots::WebhookJob do
|
||||
let(:url) { 'https://test.com' }
|
||||
let(:payload) { { name: 'test' } }
|
||||
let(:webhook_type) { :agent_bot_webhook }
|
||||
let(:retryable_error) { RestClient::InternalServerError.new(nil, 500) }
|
||||
let(:retryable_error) { Webhooks::Trigger::RetryableError.new(status: 500, message: '500 Internal Server Error') }
|
||||
|
||||
before do
|
||||
ActiveJob::Base.queue_adapter = :test
|
||||
@@ -33,7 +33,7 @@ RSpec.describe AgentBots::WebhookJob do
|
||||
it 'configures retry handlers for 429 and 500 errors' do
|
||||
handlers = described_class.rescue_handlers.map(&:first)
|
||||
|
||||
expect(handlers).to include('RestClient::TooManyRequests', 'RestClient::InternalServerError')
|
||||
expect(handlers).to include('Webhooks::Trigger::RetryableError')
|
||||
end
|
||||
|
||||
it 'retries 3 times and handles failure after retries are exhausted' do
|
||||
@@ -43,7 +43,7 @@ RSpec.describe AgentBots::WebhookJob do
|
||||
allow(Rails.logger).to receive(:warn)
|
||||
|
||||
expect(Webhooks::Trigger).to receive(:execute).exactly(3).times
|
||||
expect(trigger_instance).to receive(:handle_failure).with(instance_of(RestClient::InternalServerError)).once
|
||||
expect(trigger_instance).to receive(:handle_failure).with(instance_of(Webhooks::Trigger::RetryableError)).once
|
||||
expect(Rails.logger).to receive(:warn).with(/AgentBots::WebhookJob/).exactly(3).times
|
||||
|
||||
perform_enqueued_jobs { job }
|
||||
|
||||
@@ -80,6 +80,66 @@ RSpec.describe SafeFetch do
|
||||
expect(result.content_type).to eq('image/png')
|
||||
end
|
||||
end
|
||||
|
||||
it 'preserves embedded credentials after a same-origin redirect removes userinfo' do
|
||||
authenticated_url = 'http://user:pass@example.com/protected.png'
|
||||
initial_url = 'http://example.com/protected.png'
|
||||
redirect_url = 'http://example.com/public.png'
|
||||
redirected_headers = nil
|
||||
|
||||
stub_request(:get, initial_url)
|
||||
.with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNz' })
|
||||
.to_return(
|
||||
status: 302,
|
||||
headers: { 'Location' => '/public.png' }
|
||||
)
|
||||
stub_request(:get, redirect_url)
|
||||
.with do |request|
|
||||
redirected_headers = request.headers.transform_keys(&:downcase)
|
||||
true
|
||||
end
|
||||
.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
|
||||
|
||||
expect(redirected_headers).to include('authorization' => 'Basic dXNlcjpwYXNz')
|
||||
end
|
||||
|
||||
it 'strips embedded credentials on cross-origin redirects' do
|
||||
authenticated_url = 'http://user:pass@example.com/protected.png'
|
||||
initial_url = 'http://example.com/protected.png'
|
||||
redirect_url = 'https://example.com/public.png'
|
||||
redirected_headers = nil
|
||||
|
||||
stub_request(:get, initial_url)
|
||||
.with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNz' })
|
||||
.to_return(
|
||||
status: 302,
|
||||
headers: { 'Location' => redirect_url }
|
||||
)
|
||||
stub_request(:get, redirect_url)
|
||||
.with do |request|
|
||||
redirected_headers = request.headers.transform_keys(&:downcase)
|
||||
true
|
||||
end
|
||||
.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
|
||||
|
||||
expect(redirected_headers).not_to include('authorization')
|
||||
end
|
||||
end
|
||||
|
||||
context 'with URL validation' do
|
||||
@@ -223,6 +283,75 @@ RSpec.describe SafeFetch do
|
||||
end
|
||||
end
|
||||
|
||||
context 'with custom request options' do
|
||||
let(:post_body) { { hello: 'world' }.to_json }
|
||||
let(:headers) do
|
||||
{
|
||||
'Authorization' => 'Bearer test-token',
|
||||
'Content-Type' => 'application/json'
|
||||
}
|
||||
end
|
||||
|
||||
it 'supports POST requests with custom headers when content-type validation is disabled' do
|
||||
stub_request(:post, url)
|
||||
.with(body: post_body, headers: headers)
|
||||
.to_return(status: 200, body: '', headers: {})
|
||||
|
||||
expect do
|
||||
described_class.fetch(
|
||||
url,
|
||||
method: :post,
|
||||
body: post_body,
|
||||
headers: headers,
|
||||
validate_content_type: false
|
||||
) { nil }
|
||||
end.not_to raise_error
|
||||
end
|
||||
|
||||
it 'preserves non-credential headers on cross-origin redirects' do
|
||||
redirect_url = 'https://example.com/image.png'
|
||||
redirected_headers = nil
|
||||
headers = {
|
||||
'Authorization' => 'Bearer test-token',
|
||||
'Cookie' => 'session=test',
|
||||
'Content-Type' => 'application/json',
|
||||
'X-Chatwoot-Delivery' => 'test-uuid',
|
||||
'X-Chatwoot-Signature' => 'sha256=test-signature'
|
||||
}
|
||||
|
||||
stub_request(:post, url).to_return(
|
||||
status: 307,
|
||||
headers: { 'Location' => redirect_url }
|
||||
)
|
||||
stub_request(:post, redirect_url)
|
||||
.with do |request|
|
||||
redirected_headers = request.headers.transform_keys(&:downcase)
|
||||
true
|
||||
end
|
||||
.to_return(status: 200, body: '', headers: {})
|
||||
|
||||
described_class.fetch(
|
||||
url,
|
||||
method: :post,
|
||||
body: post_body,
|
||||
headers: headers,
|
||||
validate_content_type: false
|
||||
) { nil }
|
||||
|
||||
expect(redirected_headers).to include(
|
||||
'content-type' => 'application/json',
|
||||
'x-chatwoot-delivery' => 'test-uuid',
|
||||
'x-chatwoot-signature' => 'sha256=test-signature'
|
||||
)
|
||||
expect(redirected_headers).not_to include('authorization', 'cookie')
|
||||
end
|
||||
|
||||
it 'raises UnsupportedMethodError for unsupported HTTP methods' do
|
||||
expect { described_class.fetch(url, method: :options) { nil } }
|
||||
.to raise_error(described_class::UnsupportedMethodError)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with body size cap' do
|
||||
it 'honours a custom max_bytes argument' do
|
||||
stub_request(:get, url).to_return(
|
||||
|
||||
@@ -11,10 +11,13 @@ describe Webhooks::Trigger do
|
||||
let!(:message) { create(:message, account: account, inbox: inbox, conversation: conversation) }
|
||||
|
||||
let(:webhook_type) { :api_inbox_webhook }
|
||||
let!(:url) { 'https://test.com' }
|
||||
let(:url) { 'https://test.com' }
|
||||
let(:payload) { { hello: :hello } }
|
||||
let(:fetch_result) { instance_double(SafeFetch::Result) }
|
||||
let(:agent_bot_error_content) { I18n.t('conversations.activity.agent_bot.error_moved_to_open') }
|
||||
let(:default_timeout) { 5 }
|
||||
let(:webhook_timeout) { default_timeout }
|
||||
let(:base_headers) { { 'Content-Type' => 'application/json', 'Accept' => 'application/json' } }
|
||||
|
||||
before do
|
||||
ActiveJob::Base.queue_adapter = :test
|
||||
@@ -30,30 +33,23 @@ describe Webhooks::Trigger do
|
||||
|
||||
describe '#execute' do
|
||||
it 'triggers webhook' do
|
||||
payload = { hello: :hello }
|
||||
expect(SafeFetch).to receive(:fetch).with(
|
||||
url,
|
||||
method: :post,
|
||||
body: payload.to_json,
|
||||
headers: base_headers,
|
||||
open_timeout: webhook_timeout,
|
||||
read_timeout: webhook_timeout,
|
||||
validate_content_type: false
|
||||
).and_yield(fetch_result)
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).once
|
||||
trigger.execute(url, payload, webhook_type)
|
||||
end
|
||||
|
||||
it 'updates message status if webhook fails for message-created event' do
|
||||
payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error'))
|
||||
|
||||
expect { trigger.execute(url, payload, webhook_type) }.to change { message.reload.status }.from('sent').to('failed')
|
||||
end
|
||||
@@ -61,17 +57,18 @@ describe Webhooks::Trigger do
|
||||
it 'updates message status if webhook fails for message-updated event' do
|
||||
payload = { event: 'message_updated', conversation: { id: conversation.id }, id: message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error'))
|
||||
|
||||
expect { trigger.execute(url, payload, webhook_type) }.to change { message.reload.status }.from('sent').to('failed')
|
||||
end
|
||||
|
||||
it 'treats blocked private webhook URLs as failures' do
|
||||
payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id }
|
||||
|
||||
expect { trigger.execute('http://127.0.0.1/webhook', payload, webhook_type) }
|
||||
.to change { message.reload.status }.from('sent').to('failed')
|
||||
end
|
||||
|
||||
context 'when webhook type is agent bot' do
|
||||
let(:webhook_type) { :agent_bot_webhook }
|
||||
let!(:pending_conversation) { create(:conversation, inbox: inbox, status: :pending, account: account) }
|
||||
@@ -80,16 +77,13 @@ describe Webhooks::Trigger do
|
||||
it 'raises 500 errors for retry and does not reopen conversation immediately' do
|
||||
payload = { event: 'message_created', id: pending_message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::InternalServerError.new(nil, 500)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error'))
|
||||
|
||||
expect { trigger.execute(url, payload, webhook_type) }.to raise_error(RestClient::InternalServerError)
|
||||
expect { trigger.execute(url, payload, webhook_type) }
|
||||
.to(raise_error do |error|
|
||||
expect(error.class.name).to eq('Webhooks::Trigger::RetryableError')
|
||||
expect(error.status).to eq(500)
|
||||
end)
|
||||
expect(pending_conversation.reload.status).to eq('pending')
|
||||
expect(Conversations::ActivityMessageJob).not_to have_been_enqueued
|
||||
end
|
||||
@@ -97,16 +91,13 @@ describe Webhooks::Trigger do
|
||||
it 'raises 429 errors for retry and does not reopen conversation immediately' do
|
||||
payload = { event: 'message_created', id: pending_message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::TooManyRequests.new(nil, 429)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('429 Too Many Requests'))
|
||||
|
||||
expect { trigger.execute(url, payload, webhook_type) }.to raise_error(RestClient::TooManyRequests)
|
||||
expect { trigger.execute(url, payload, webhook_type) }
|
||||
.to(raise_error do |error|
|
||||
expect(error.class.name).to eq('Webhooks::Trigger::RetryableError')
|
||||
expect(error.status).to eq(429)
|
||||
end)
|
||||
expect(pending_conversation.reload.status).to eq('pending')
|
||||
expect(Conversations::ActivityMessageJob).not_to have_been_enqueued
|
||||
end
|
||||
@@ -114,14 +105,7 @@ describe Webhooks::Trigger do
|
||||
it 'reopens conversation and enqueues activity message if pending' do
|
||||
payload = { event: 'message_created', id: pending_message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('404 Not Found'))
|
||||
|
||||
expect do
|
||||
perform_enqueued_jobs do
|
||||
@@ -139,14 +123,7 @@ describe Webhooks::Trigger do
|
||||
it 'does not change message status or enqueue activity when conversation is not pending' do
|
||||
payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('404 Not Found'))
|
||||
|
||||
expect do
|
||||
trigger.execute(url, payload, webhook_type)
|
||||
@@ -160,14 +137,7 @@ describe Webhooks::Trigger do
|
||||
account.update(keep_pending_on_bot_failure: true)
|
||||
payload = { event: 'message_created', id: pending_message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('404 Not Found'))
|
||||
|
||||
trigger.execute(url, payload, webhook_type)
|
||||
|
||||
@@ -179,14 +149,8 @@ describe Webhooks::Trigger do
|
||||
account.update(keep_pending_on_bot_failure: false)
|
||||
payload = { event: 'message_created', id: pending_message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('404 Not Found'))
|
||||
|
||||
expect do
|
||||
perform_enqueued_jobs do
|
||||
trigger.execute(url, payload, webhook_type)
|
||||
@@ -204,14 +168,7 @@ describe Webhooks::Trigger do
|
||||
it 'handles 500 without raising for non-agent webhooks' do
|
||||
payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::InternalServerError.new(nil, 500)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error'))
|
||||
|
||||
expect { trigger.execute(url, payload, webhook_type) }.not_to raise_error
|
||||
expect(message.reload.status).to eq('failed')
|
||||
@@ -224,20 +181,30 @@ describe Webhooks::Trigger do
|
||||
|
||||
context 'without secret or delivery_id' do
|
||||
it 'sends only content-type and accept headers' do
|
||||
expect(RestClient::Request).to receive(:execute).with(
|
||||
hash_including(headers: { content_type: :json, accept: :json })
|
||||
)
|
||||
expect(SafeFetch).to receive(:fetch).with(
|
||||
url,
|
||||
method: :post,
|
||||
body: body,
|
||||
headers: base_headers,
|
||||
open_timeout: webhook_timeout,
|
||||
read_timeout: webhook_timeout,
|
||||
validate_content_type: false
|
||||
).and_yield(fetch_result)
|
||||
|
||||
trigger.execute(url, payload, webhook_type)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with delivery_id' do
|
||||
it 'adds X-Chatwoot-Delivery header' do
|
||||
expect(RestClient::Request).to receive(:execute) do |args|
|
||||
expect(args[:headers]['X-Chatwoot-Delivery']).to eq('test-uuid')
|
||||
expect(args[:headers]).not_to have_key('X-Chatwoot-Signature')
|
||||
expect(args[:headers]).not_to have_key('X-Chatwoot-Timestamp')
|
||||
expect(SafeFetch).to receive(:fetch) do |received_url, **options, &block|
|
||||
expect(received_url).to eq(url)
|
||||
expect(options[:headers]['X-Chatwoot-Delivery']).to eq('test-uuid')
|
||||
expect(options[:headers]).not_to have_key('X-Chatwoot-Signature')
|
||||
expect(options[:headers]).not_to have_key('X-Chatwoot-Timestamp')
|
||||
block.call(fetch_result)
|
||||
end
|
||||
|
||||
trigger.execute(url, payload, webhook_type, delivery_id: 'test-uuid')
|
||||
end
|
||||
end
|
||||
@@ -246,38 +213,45 @@ describe Webhooks::Trigger do
|
||||
let(:secret) { 'test-secret' }
|
||||
|
||||
it 'adds X-Chatwoot-Timestamp header' do
|
||||
expect(RestClient::Request).to receive(:execute) do |args|
|
||||
expect(args[:headers]['X-Chatwoot-Timestamp']).to match(/\A\d+\z/)
|
||||
expect(SafeFetch).to receive(:fetch) do |_received_url, **options, &block|
|
||||
expect(options[:headers]['X-Chatwoot-Timestamp']).to match(/\A\d+\z/)
|
||||
block.call(fetch_result)
|
||||
end
|
||||
|
||||
trigger.execute(url, payload, webhook_type, secret: secret)
|
||||
end
|
||||
|
||||
it 'adds X-Chatwoot-Signature header with correct HMAC' do
|
||||
expect(RestClient::Request).to receive(:execute) do |args|
|
||||
ts = args[:headers]['X-Chatwoot-Timestamp']
|
||||
expect(SafeFetch).to receive(:fetch) do |_received_url, **options, &block|
|
||||
ts = options[:headers]['X-Chatwoot-Timestamp']
|
||||
expected_sig = "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', secret, "#{ts}.#{body}")}"
|
||||
expect(args[:headers]['X-Chatwoot-Signature']).to eq(expected_sig)
|
||||
expect(options[:headers]['X-Chatwoot-Signature']).to eq(expected_sig)
|
||||
block.call(fetch_result)
|
||||
end
|
||||
|
||||
trigger.execute(url, payload, webhook_type, secret: secret)
|
||||
end
|
||||
|
||||
it 'signs timestamp.body not just body' do
|
||||
expect(RestClient::Request).to receive(:execute) do |args|
|
||||
args[:headers]['X-Chatwoot-Timestamp']
|
||||
expect(SafeFetch).to receive(:fetch) do |_received_url, **options, &block|
|
||||
wrong_sig = "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', secret, body)}"
|
||||
expect(args[:headers]['X-Chatwoot-Signature']).not_to eq(wrong_sig)
|
||||
expect(options[:headers]['X-Chatwoot-Signature']).not_to eq(wrong_sig)
|
||||
block.call(fetch_result)
|
||||
end
|
||||
|
||||
trigger.execute(url, payload, webhook_type, secret: secret)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with both secret and delivery_id' do
|
||||
it 'includes all three security headers' do
|
||||
expect(RestClient::Request).to receive(:execute) do |args|
|
||||
expect(args[:headers]['X-Chatwoot-Delivery']).to eq('abc-123')
|
||||
expect(args[:headers]['X-Chatwoot-Timestamp']).to be_present
|
||||
expect(args[:headers]['X-Chatwoot-Signature']).to start_with('sha256=')
|
||||
expect(SafeFetch).to receive(:fetch) do |_received_url, **options, &block|
|
||||
expect(options[:headers]['X-Chatwoot-Delivery']).to eq('abc-123')
|
||||
expect(options[:headers]['X-Chatwoot-Timestamp']).to be_present
|
||||
expect(options[:headers]['X-Chatwoot-Signature']).to start_with('sha256=')
|
||||
block.call(fetch_result)
|
||||
end
|
||||
|
||||
trigger.execute(url, payload, webhook_type, secret: 'mysecret', delivery_id: 'abc-123')
|
||||
end
|
||||
end
|
||||
@@ -286,14 +260,7 @@ describe Webhooks::Trigger do
|
||||
it 'does not update message status if webhook fails for other events' do
|
||||
payload = { event: 'conversation_created', conversation: { id: conversation.id }, id: message.id }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: webhook_timeout
|
||||
).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once
|
||||
expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error'))
|
||||
|
||||
expect { trigger.execute(url, payload, webhook_type) }.not_to(change { message.reload.status })
|
||||
end
|
||||
@@ -302,16 +269,15 @@ describe Webhooks::Trigger do
|
||||
let(:webhook_timeout) { nil }
|
||||
|
||||
it 'falls back to default timeout' do
|
||||
payload = { hello: :hello }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: default_timeout
|
||||
).once
|
||||
expect(SafeFetch).to receive(:fetch).with(
|
||||
url,
|
||||
method: :post,
|
||||
body: payload.to_json,
|
||||
headers: base_headers,
|
||||
open_timeout: default_timeout,
|
||||
read_timeout: default_timeout,
|
||||
validate_content_type: false
|
||||
).and_yield(fetch_result)
|
||||
|
||||
trigger.execute(url, payload, webhook_type)
|
||||
end
|
||||
@@ -321,16 +287,15 @@ describe Webhooks::Trigger do
|
||||
let(:webhook_timeout) { -1 }
|
||||
|
||||
it 'falls back to default timeout' do
|
||||
payload = { hello: :hello }
|
||||
|
||||
expect(RestClient::Request).to receive(:execute)
|
||||
.with(
|
||||
method: :post,
|
||||
url: url,
|
||||
payload: payload.to_json,
|
||||
headers: { content_type: :json, accept: :json },
|
||||
timeout: default_timeout
|
||||
).once
|
||||
expect(SafeFetch).to receive(:fetch).with(
|
||||
url,
|
||||
method: :post,
|
||||
body: payload.to_json,
|
||||
headers: base_headers,
|
||||
open_timeout: default_timeout,
|
||||
read_timeout: default_timeout,
|
||||
validate_content_type: false
|
||||
).and_yield(fetch_result)
|
||||
|
||||
trigger.execute(url, payload, webhook_type)
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user