diff --git a/app/models/macro.rb b/app/models/macro.rb index fc25beeac..552258ef7 100644 --- a/app/models/macro.rb +++ b/app/models/macro.rb @@ -29,6 +29,7 @@ class Macro < ApplicationRecord enum visibility: { personal: 0, global: 1 } validate :json_actions_format + validate :validate_webhook_action_urls ACTIONS_ATTRS = %w[send_message add_label assign_team assign_agent mute_conversation change_status remove_label remove_assigned_agent remove_assigned_team resolve_conversation snooze_conversation change_priority send_email_transcript @@ -73,6 +74,20 @@ class Macro < ApplicationRecord errors.add(:actions, "Macro execution actions #{actions.join(',')} not supported.") if actions.any? end + + def validate_webhook_action_urls + return if actions.blank? + + actions.each do |action| + action = action.respond_to?(:with_indifferent_access) ? action.with_indifferent_access : {} + next unless action[:action_name] == 'send_webhook_event' + + webhook_url = Array(action[:action_params]).first + SafeOutboundUrl.validate!(webhook_url) + rescue SafeOutboundUrl::Error + errors.add(:actions, 'Webhook URL for send_webhook_event must be a public http(s) URL.') + end + end end Macro.include_mod_with('Audit::Macro') diff --git a/app/services/macros/execution_service.rb b/app/services/macros/execution_service.rb index df82d4c6f..fbb060caf 100644 --- a/app/services/macros/execution_service.rb +++ b/app/services/macros/execution_service.rb @@ -64,7 +64,10 @@ class Macros::ExecutionService < ActionService end def send_webhook_event(webhook_url) + url = Array(webhook_url).first + SafeOutboundUrl.validate!(url) + payload = @conversation.webhook_data.merge(event: 'macro.executed') - WebhookJob.perform_later(webhook_url.first, payload) + WebhookJob.perform_later(url, payload, :macro_webhook) end end diff --git a/lib/safe_outbound_url.rb b/lib/safe_outbound_url.rb new file mode 100644 index 000000000..230f0d724 --- /dev/null +++ b/lib/safe_outbound_url.rb @@ -0,0 +1,43 @@ +require 'ipaddr' +require 'resolv' +require 'ssrf_filter' +require 'uri' + +module SafeOutboundUrl + class Error < StandardError; end + class InvalidUrlError < Error; end + class UnsafeUrlError < Error; end + + def self.validate!(url, resolver: SsrfFilter::DEFAULT_RESOLVER) + uri = parse_http_url!(url) + ip_addresses = resolve_addresses(uri.hostname, resolver) + + raise UnsafeUrlError, "Could not resolve hostname '#{uri.hostname}'" if ip_addresses.empty? + raise UnsafeUrlError, "Hostname '#{uri.hostname}' has no public ip addresses" if ip_addresses.all? { |ip| unsafe_ip_address?(ip) } + + uri + rescue URI::InvalidURIError => e + raise InvalidUrlError, e.message + rescue IPAddr::InvalidAddressError, Resolv::ResolvError => e + raise UnsafeUrlError, e.message + end + + def self.parse_http_url!(url) + uri = URI.parse(url.to_s) + 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.hostname.blank? + + uri + end + private_class_method :parse_http_url! + + def self.resolve_addresses(hostname, resolver) + Array(resolver.call(hostname)).map { |ip| ip.is_a?(IPAddr) ? ip : IPAddr.new(ip) } + end + private_class_method :resolve_addresses + + def self.unsafe_ip_address?(ip_address) + SsrfFilter.send(:unsafe_ip_address?, ip_address) + end + private_class_method :unsafe_ip_address? +end diff --git a/lib/webhooks/trigger.rb b/lib/webhooks/trigger.rb index 7cb15c836..2a977281e 100644 --- a/lib/webhooks/trigger.rb +++ b/lib/webhooks/trigger.rb @@ -1,3 +1,5 @@ +require 'ssrf_filter' + class Webhooks::Trigger SUPPORTED_ERROR_HANDLE_EVENTS = %w[message_created message_updated].freeze @@ -32,6 +34,8 @@ class Webhooks::Trigger def perform_request body = @payload.to_json + return perform_macro_request(body) if @webhook_type == :macro_webhook + RestClient::Request.execute( method: :post, url: @url, @@ -41,6 +45,19 @@ class Webhooks::Trigger ) end + def perform_macro_request(body) + headers = safe_request_headers(body) + response = SsrfFilter.post( + @url, + body: body, + headers: headers, + sensitive_headers: headers.keys, + http_options: { open_timeout: webhook_timeout, read_timeout: webhook_timeout } + ) + + raise StandardError, "Webhook request failed with status #{response.code}" unless response.is_a?(Net::HTTPSuccess) + end + def request_headers(body) headers = { content_type: :json, accept: :json } headers['X-Chatwoot-Delivery'] = @delivery_id if @delivery_id.present? @@ -52,6 +69,17 @@ class Webhooks::Trigger headers end + def safe_request_headers(body) + 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 + headers['X-Chatwoot-Timestamp'] = ts + headers['X-Chatwoot-Signature'] = "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', @secret, "#{ts}.#{body}")}" + end + headers + end + def handle_error(error) return unless SUPPORTED_ERROR_HANDLE_EVENTS.include?(@payload[:event]) return unless message diff --git a/spec/jobs/webhooks/instagram_events_job_spec.rb b/spec/jobs/webhooks/instagram_events_job_spec.rb index 4128385cf..ae3f46dd6 100644 --- a/spec/jobs/webhooks/instagram_events_job_spec.rb +++ b/spec/jobs/webhooks/instagram_events_job_spec.rb @@ -4,13 +4,15 @@ describe Webhooks::InstagramEventsJob do subject(:instagram_webhook) { described_class } before do + image_body = File.binread(Rails.root.join('spec/assets/sample.png')) + stub_request(:post, /graph\.facebook\.com/) stub_request(:get, 'https://www.example.com/test.jpeg') - .to_return(status: 200, body: '', headers: {}) + .to_return(status: 200, body: image_body, headers: { 'Content-Type' => 'image/jpeg' }) stub_request(:get, 'https://lookaside.fbsbx.com/ig_messaging_cdn/?asset_id=17949487764033669&signature=test') - .to_return(status: 200, body: '', headers: {}) + .to_return(status: 200, body: image_body, headers: { 'Content-Type' => 'image/png' }) stub_request(:get, 'https://lookaside.fbsbx.com/ig_messaging_cdn/?asset_id=18091626484740369&signature=test') - .to_return(status: 200, body: '', headers: {}) + .to_return(status: 200, body: image_body, headers: { 'Content-Type' => 'image/png' }) end let!(:account) { create(:account) } diff --git a/spec/lib/safe_outbound_url_spec.rb b/spec/lib/safe_outbound_url_spec.rb new file mode 100644 index 000000000..dd139e86a --- /dev/null +++ b/spec/lib/safe_outbound_url_spec.rb @@ -0,0 +1,27 @@ +require 'rails_helper' + +RSpec.describe SafeOutboundUrl do + describe '.validate!' do + let(:public_resolver) { ->(_hostname) { [IPAddr.new('93.184.216.34')] } } + let(:private_resolver) { ->(_hostname) { [IPAddr.new('127.0.0.1')] } } + + it 'accepts public http urls' do + uri = described_class.validate!('https://example.com/webhook', resolver: public_resolver) + + expect(uri).to be_a(URI::HTTPS) + expect(uri.host).to eq('example.com') + end + + it 'rejects non-http schemes' do + expect do + described_class.validate!('javascript:alert(1)', resolver: public_resolver) + end.to raise_error(described_class::InvalidUrlError, 'scheme must be http or https') + end + + it 'rejects hosts that resolve only to private addresses' do + expect do + described_class.validate!('http://internal.example.test/webhook', resolver: private_resolver) + end.to raise_error(described_class::UnsafeUrlError, "Hostname 'internal.example.test' has no public ip addresses") + end + end +end diff --git a/spec/lib/webhooks/trigger_spec.rb b/spec/lib/webhooks/trigger_spec.rb index 90d1ce7f8..e01f57283 100644 --- a/spec/lib/webhooks/trigger_spec.rb +++ b/spec/lib/webhooks/trigger_spec.rb @@ -43,6 +43,25 @@ describe Webhooks::Trigger do trigger.execute(url, payload, webhook_type) end + context 'when webhook type is macro' do + let(:webhook_type) { :macro_webhook } + + it 'uses ssrf-protected delivery for the request' do + payload = { hello: :hello } + response = Net::HTTPOK.new('1.1', '200', 'OK') + + expect(SsrfFilter).to receive(:post).with( + url, + body: payload.to_json, + headers: { 'Content-Type' => 'application/json', 'Accept' => 'application/json' }, + sensitive_headers: %w[Content-Type Accept], + http_options: { open_timeout: webhook_timeout, read_timeout: webhook_timeout } + ).and_return(response) + + trigger.execute(url, payload, webhook_type) + end + end + it 'updates message status if webhook fails for message-created event' do payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id } diff --git a/spec/models/macro_spec.rb b/spec/models/macro_spec.rb index 1b7161476..dfdd2656f 100644 --- a/spec/models/macro_spec.rb +++ b/spec/models/macro_spec.rb @@ -19,6 +19,21 @@ RSpec.describe Macro do expect(macro).not_to be_valid expect(macro.errors.full_messages).to eq(['Actions Macro execution actions update_last_seen not supported.']) end + + it 'rejects unsafe webhook action urls' do + allow(SafeOutboundUrl).to receive(:validate!).and_raise(SafeOutboundUrl::UnsafeUrlError, 'unsafe') + + macro = FactoryBot.build( + :macro, + account: account, + created_by: admin, + updated_by: admin, + actions: [{ action_name: 'send_webhook_event', action_params: ['http://169.254.169.254/latest/meta-data'] }] + ) + + expect(macro).not_to be_valid + expect(macro.errors.full_messages).to include('Actions Webhook URL for send_webhook_event must be a public http(s) URL.') + end end describe '#set_visibility' do diff --git a/spec/services/macros/execution_service_spec.rb b/spec/services/macros/execution_service_spec.rb index d44f7793a..46a956c88 100644 --- a/spec/services/macros/execution_service_spec.rb +++ b/spec/services/macros/execution_service_spec.rb @@ -171,8 +171,24 @@ RSpec.describe Macros::ExecutionService, type: :service do describe '#send_webhook_event' do it 'sends a webhook event' do - expect(WebhookJob).to receive(:perform_later) + allow(SafeOutboundUrl).to receive(:validate!).and_return(URI.parse('https://example.com/webhook')) + + expect(WebhookJob).to receive(:perform_later).with( + 'https://example.com/webhook', + hash_including(event: 'macro.executed'), + :macro_webhook + ) + service.send(:send_webhook_event, ['https://example.com/webhook']) end + + it 'raises when the webhook url is unsafe' do + allow(SafeOutboundUrl).to receive(:validate!).and_raise(SafeOutboundUrl::UnsafeUrlError, 'unsafe') + + expect(WebhookJob).not_to receive(:perform_later) + expect do + service.send(:send_webhook_event, ['http://169.254.169.254/latest/meta-data']) + end.to raise_error(SafeOutboundUrl::UnsafeUrlError, 'unsafe') + end end end diff --git a/spec/services/tiktok/message_service_spec.rb b/spec/services/tiktok/message_service_spec.rb index 036575735..e075f4bee 100644 --- a/spec/services/tiktok/message_service_spec.rb +++ b/spec/services/tiktok/message_service_spec.rb @@ -125,10 +125,12 @@ RSpec.describe Tiktok::MessageService do stub_request(:get, image_download_url) .with(headers: { 'X-User' => 'valid-access-token' }) .to_return(status: 200, body: File.read('spec/assets/sample.png'), headers: { 'Content-Type' => 'image/png' }) - allow_any_instance_of(Tiktok::Client).to receive(:file_download_url).and_return(image_download_url) service = described_class.new(channel: channel, content: content) allow(service).to receive(:create_contact_inbox).and_return(contact_inbox) + allow(service).to receive(:tiktok_client) + .with(channel) + .and_return(instance_double(Tiktok::Client, file_download_url: image_download_url)) service.perform @@ -153,10 +155,11 @@ RSpec.describe Tiktok::MessageService do to_user: { id: 'biz-123' } }.deep_symbolize_keys - allow_any_instance_of(Tiktok::Client).to receive(:file_download_url).and_return('http://127.0.0.1/blocked.png') - service = described_class.new(channel: channel, content: content) allow(service).to receive(:create_contact_inbox).and_return(contact_inbox) + allow(service).to receive(:tiktok_client) + .with(channel) + .and_return(instance_double(Tiktok::Client, file_download_url: 'http://127.0.0.1/blocked.png')) expect { service.perform }.not_to raise_error