feat: add per-webhook secret with backfill migration (#13573)

spec/jobs/agent_bots/webhook_job_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe AgentBots::WebhookJob do
   end
 
   it 'executes perform' do
-    expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type)
+    expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type, secret: nil, delivery_id: nil)
     perform_enqueued_jobs { job }
   end
 end

spec/jobs/webhook_job_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe WebhookJob do
   end
 
   it 'executes perform with default webhook type' do
-    expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type)
+    expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type, secret: nil, delivery_id: nil)
     perform_enqueued_jobs { job }
   end
 
@@ -24,7 +24,7 @@ RSpec.describe WebhookJob do
     let(:webhook_type) { :api_inbox_webhook }
 
     it 'executes perform with inbox webhook type' do
-      expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type)
+      expect(Webhooks::Trigger).to receive(:execute).with(url, payload, webhook_type, secret: nil, delivery_id: nil)
       perform_enqueued_jobs { job }
     end
   end

spec/lib/webhooks/trigger_spec.rb

@@ -168,6 +168,71 @@ describe Webhooks::Trigger do
     end
   end
 
+  describe 'request headers' do
+    let(:payload) { { event: 'message_created' } }
+    let(:body) { payload.to_json }
+
+    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 })
+        )
+        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')
+        end
+        trigger.execute(url, payload, webhook_type, delivery_id: 'test-uuid')
+      end
+    end
+
+    context 'with secret' 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/)
+        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']
+          expected_sig = "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', secret, "#{ts}.#{body}")}"
+          expect(args[:headers]['X-Chatwoot-Signature']).to eq(expected_sig)
+        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']
+          wrong_sig = "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', secret, body)}"
+          expect(args[:headers]['X-Chatwoot-Signature']).not_to eq(wrong_sig)
+        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=')
+        end
+        trigger.execute(url, payload, webhook_type, secret: 'mysecret', delivery_id: 'abc-123')
+      end
+    end
+  end
+
   it 'does not update message status if webhook fails for other events' do
     payload = { event: 'conversation_created', conversation: { id: conversation.id }, id: message.id }
 

spec/listeners/webhook_listener_spec.rb

@@ -28,7 +28,10 @@ describe WebhookListener do
     context 'when webhook is configured and event is subscribed' do
       it 'triggers the webhook event' do
         webhook = create(:webhook, inbox: inbox, account: account)
-        expect(WebhookJob).to receive(:perform_later).with(webhook.url, message.webhook_data.merge(event: 'message_created')).once
+        expect(WebhookJob).to receive(:perform_later).with(
+          webhook.url, message.webhook_data.merge(event: 'message_created'), :account_webhook,
+          secret: webhook.secret, delivery_id: instance_of(String)
+        ).once
         listener.message_created(message_created_event)
       end
     end
@@ -54,8 +57,10 @@ describe WebhookListener do
           conversation: api_conversation
         )
         api_event = Events::Base.new(event_name, Time.zone.now, message: api_message)
-        expect(WebhookJob).to receive(:perform_later).with(channel_api.webhook_url, api_message.webhook_data.merge(event: 'message_created'),
-                                                           :api_inbox_webhook).once
+        expect(WebhookJob).to receive(:perform_later).with(
+          channel_api.webhook_url, api_message.webhook_data.merge(event: 'message_created'),
+          :api_inbox_webhook, delivery_id: instance_of(String)
+        ).once
         listener.message_created(api_event)
       end
 
@@ -90,7 +95,10 @@ describe WebhookListener do
     context 'when webhook is configured' do
       it 'triggers webhook' do
         webhook = create(:webhook, inbox: inbox, account: account)
-        expect(WebhookJob).to receive(:perform_later).with(webhook.url, conversation.webhook_data.merge(event: 'conversation_created')).once
+        expect(WebhookJob).to receive(:perform_later).with(
+          webhook.url, conversation.webhook_data.merge(event: 'conversation_created'), :account_webhook,
+          secret: webhook.secret, delivery_id: instance_of(String)
+        ).once
         listener.conversation_created(conversation_created_event)
       end
     end
@@ -101,9 +109,11 @@ describe WebhookListener do
         api_inbox = channel_api.inbox
         api_conversation = create(:conversation, account: account, inbox: api_inbox, assignee: user)
         api_event = Events::Base.new(event_name, Time.zone.now, conversation: api_conversation)
-        expect(WebhookJob).to receive(:perform_later).with(channel_api.webhook_url,
-                                                           api_conversation.webhook_data.merge(event: 'conversation_created'),
-                                                           :api_inbox_webhook).once
+        expect(WebhookJob).to receive(:perform_later).with(
+          channel_api.webhook_url,
+          api_conversation.webhook_data.merge(event: 'conversation_created'),
+          :api_inbox_webhook, delivery_id: instance_of(String)
+        ).once
         listener.conversation_created(api_event)
       end
 
@@ -156,7 +166,9 @@ describe WebhookListener do
                 }
               }
             ]
-          )
+          ),
+          :account_webhook,
+          secret: webhook.secret, delivery_id: instance_of(String)
         ).once
 
         listener.conversation_updated(conversation_updated_event)
@@ -177,7 +189,10 @@ describe WebhookListener do
     context 'when webhook is configured' do
       it 'triggers webhook' do
         webhook = create(:webhook, account: account)
-        expect(WebhookJob).to receive(:perform_later).with(webhook.url, contact.webhook_data.merge(event: 'contact_created')).once
+        expect(WebhookJob).to receive(:perform_later).with(
+          webhook.url, contact.webhook_data.merge(event: 'contact_created'), :account_webhook,
+          secret: webhook.secret, delivery_id: instance_of(String)
+        ).once
         listener.contact_created(contact_event)
       end
     end
@@ -213,7 +228,9 @@ describe WebhookListener do
           contact.webhook_data.merge(
             event: 'contact_updated',
             changed_attributes: [{ 'name' => { :current_value => 'Jane Doe', :previous_value => 'Jane' } }]
-          )
+          ),
+          :account_webhook,
+          secret: webhook.secret, delivery_id: instance_of(String)
         ).once
         listener.contact_updated(contact_updated_event)
       end
@@ -235,7 +252,10 @@ describe WebhookListener do
       it 'triggers webhook' do
         inbox_data = Inbox::EventDataPresenter.new(inbox).push_data
         webhook = create(:webhook, account: account, subscriptions: ['inbox_created'])
-        expect(WebhookJob).to receive(:perform_later).with(webhook.url, inbox_data.merge(event: 'inbox_created')).once
+        expect(WebhookJob).to receive(:perform_later).with(
+          webhook.url, inbox_data.merge(event: 'inbox_created'), :account_webhook,
+          secret: webhook.secret, delivery_id: instance_of(String)
+        ).once
         listener.inbox_created(inbox_created_event)
       end
     end
@@ -272,7 +292,9 @@ describe WebhookListener do
 
         expect(WebhookJob).to receive(:perform_later).with(
           webhook.url,
-          inbox_data.merge(event: 'inbox_updated', changed_attributes: changed_attributes_data)
+          inbox_data.merge(event: 'inbox_updated', changed_attributes: changed_attributes_data),
+          :account_webhook,
+          secret: webhook.secret, delivery_id: instance_of(String)
         ).once
 
         listener.inbox_updated(inbox_updated_event)
@@ -302,7 +324,10 @@ describe WebhookListener do
           is_private: false
         }
 
-        expect(WebhookJob).to receive(:perform_later).with(webhook.url, payload).once
+        expect(WebhookJob).to receive(:perform_later).with(
+          webhook.url, payload, :account_webhook,
+          secret: webhook.secret, delivery_id: instance_of(String)
+        ).once
         listener.conversation_typing_on(typing_event)
       end
     end
@@ -321,7 +346,10 @@ describe WebhookListener do
           is_private: false
         }
 
-        expect(WebhookJob).to receive(:perform_later).with(channel_api.webhook_url, payload, :api_inbox_webhook).once
+        expect(WebhookJob).to receive(:perform_later).with(
+          channel_api.webhook_url, payload, :api_inbox_webhook,
+          delivery_id: instance_of(String)
+        ).once
         listener.conversation_typing_on(api_event)
       end
     end
@@ -349,7 +377,10 @@ describe WebhookListener do
           is_private: false
         }
 
-        expect(WebhookJob).to receive(:perform_later).with(webhook.url, payload).once
+        expect(WebhookJob).to receive(:perform_later).with(
+          webhook.url, payload, :account_webhook,
+          secret: webhook.secret, delivery_id: instance_of(String)
+        ).once
         listener.conversation_typing_off(typing_event)
       end
     end

spec/models/webhook_spec.rb

@@ -8,4 +8,20 @@ RSpec.describe Webhook do
   describe 'associations' do
     it { is_expected.to belong_to(:account) }
   end
+
+  describe 'secret token' do
+    let!(:account) { create(:account) }
+
+    it 'auto-generates a secret on create' do
+      webhook = create(:webhook, account: account)
+      expect(webhook.secret).to be_present
+    end
+
+    it 'does not regenerate the secret on update' do
+      webhook = create(:webhook, account: account)
+      original_secret = webhook.secret
+      webhook.update!(url: "#{webhook.url}?updated=1")
+      expect(webhook.reload.secret).to eq(original_secret)
+    end
+  end
 end