fix(security): harden macro webhook delivery

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')

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

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

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

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) }

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

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 }
 

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

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

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