From d272a64ff7e66d4b04592d3ea635a3ebc04ecf6e Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 11 Feb 2026 11:02:38 -0800 Subject: [PATCH 1/3] fix(mailbox): handle malformed sender address headers (#13486) ## How to reproduce When an inbound email has malformed sender headers (for example `From: McDonald `), mailbox processing can raise `Mail::Field::IncompleteParseError` while resolving sender data in `MailPresenter`. ## What changed This PR hardens sender parsing in `MailPresenter` with a small, readable implementation: - Added/used a safe parser (`parse_mail_address`) that rescues `Mail::Field::ParseError` and `Mail::Field::IncompleteParseError`. - `sender_name` now uses the same safe parser path. - `original_sender` now resolves candidates in order via a compact `filter_map` flow: - `Reply-To` - `X-Original-Sender` - `From` - All three candidates are parsed as email addresses before use (including `X-Original-Sender`), and invalid values are ignored. - `notification_email_from_chatwoot?` now compares sender addresses case-insensitively (`casecmp?`) to avoid case-only mismatches. ## Test coverage Added focused presenter specs for: - malformed `From` header returns nil sender values and does not classify as notification sender - malformed `Reply-To` falls back to valid `From` - valid `X-Original-Sender` is used when present - invalid `X-Original-Sender` falls back to valid `From` - mixed-case sender address still matches configured `MAILER_SENDER_EMAIL` ## How this was tested Ran: - `bundle exec rspec spec/presenters/mail_presenter_spec.rb` - `bundle exec rubocop app/presenters/mail_presenter.rb spec/presenters/mail_presenter_spec.rb` Sentry issue: [CHATWOOT-B9Y](https://chatwoot-p3.sentry.io/issues/7005483640/) --- app/presenters/mail_presenter.rb | 23 +++++--- spec/presenters/mail_presenter_spec.rb | 80 ++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/app/presenters/mail_presenter.rb b/app/presenters/mail_presenter.rb index 08e370cd8..62cb0ed9a 100644 --- a/app/presenters/mail_presenter.rb +++ b/app/presenters/mail_presenter.rb @@ -130,11 +130,15 @@ class MailPresenter < SimpleDelegator end def sender_name - Mail::Address.new((@mail[:reply_to] || @mail[:from]).value).name + parse_mail_address((@mail[:reply_to] || @mail[:from]).value)&.name end def original_sender - from_email_address(@mail[:reply_to].try(:value)) || @mail['X-Original-Sender'].try(:value) || from_email_address(from.first) + [ + @mail[:reply_to]&.value, + @mail['X-Original-Sender']&.value, + @mail[:from]&.value + ].filter_map { |email| parse_mail_address(email)&.address }.first end def headers_data @@ -147,10 +151,6 @@ class MailPresenter < SimpleDelegator headers.presence end - def from_email_address(email) - Mail::Address.new(email).address - end - def email_forwarded_for @mail['X-Forwarded-For'].try(:value) end @@ -175,11 +175,20 @@ class MailPresenter < SimpleDelegator def notification_email_from_chatwoot? # notification emails are send via mailer sender email address. so it should match - original_sender == Mail::Address.new(ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot ')).address + configured_sender = Mail::Address.new(ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot ')).address + original_sender.to_s.casecmp?(configured_sender) end private + def parse_mail_address(email) + return if email.blank? + + Mail::Address.new(email) + rescue Mail::Field::ParseError, Mail::Field::IncompleteParseError + nil + end + def auto_submitted? @mail['Auto-Submitted'].present? && @mail['Auto-Submitted'].value != 'no' end diff --git a/spec/presenters/mail_presenter_spec.rb b/spec/presenters/mail_presenter_spec.rb index bbe51c52d..f5ed27537 100644 --- a/spec/presenters/mail_presenter_spec.rb +++ b/spec/presenters/mail_presenter_spec.rb @@ -178,5 +178,85 @@ RSpec.describe MailPresenter do expect(decorated_mail.serialized_data[:auto_reply]).to be_falsey end end + + describe 'malformed sender headers' do + let(:mail_with_malformed_from) do + Mail.new do + header['From'] = 'Kevin McDonald ' + subject :header + body 'Hi' + end + end + + let(:mail_with_malformed_reply_to) do + Mail.new do + from 'Sender ' + to 'Inbox ' + subject :header + body 'Hi' + header['Reply-To'] = 'Reply User ' + to 'Inbox ' + subject :header + body 'Hi' + header['Reply-To'] = 'Reply User ' + to 'Inbox ' + subject :header + body 'Hi' + header['Reply-To'] = 'Reply User ' + to 'Inbox ' + subject :header + body 'Hi' + end + + with_modified_env MAILER_SENDER_EMAIL: 'Chatwoot ' do + presenter = described_class.new(mail_with_uppercase_sender) + expect(presenter.notification_email_from_chatwoot?).to be(true) + end + end + end end end From c7193c791770ff8afedb573ff0476e1c87f25ebc Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 11 Feb 2026 17:05:44 -0800 Subject: [PATCH 2/3] fix(slack): handle archived channel errors in SendOnSlackJob (#13520) When a Slack-integrated channel is archived, posting from Chatwoot raises `Slack::Web::Api::Errors::IsArchived` in `SendOnSlackJob`, which retries and can end up in dead jobs. This can be reproduced by archiving the connected Slack channel for a valid hook and creating outgoing messages. This change adds `IsArchived` to the existing handled Slack API rescue path in `Integrations::Slack::SendOnSlackService#send_message`, so archived-channel failures now follow the same flow as related Slack failures (`prompt_reauthorization!` + `disable`) instead of bubbling and retrying repeatedly. I tested this by running `bundle exec rubocop lib/integrations/slack/send_on_slack_service.rb` (with `rbenv` initialized), and it passes with no offenses. Sentry issue: https://chatwoot-p3.sentry.io/issues/7150427066/ --- lib/integrations/slack/send_on_slack_service.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/integrations/slack/send_on_slack_service.rb b/lib/integrations/slack/send_on_slack_service.rb index 4e959c469..de6d20ed0 100644 --- a/lib/integrations/slack/send_on_slack_service.rb +++ b/lib/integrations/slack/send_on_slack_service.rb @@ -102,7 +102,8 @@ class Integrations::Slack::SendOnSlackService < Base::SendOnChannelService def send_message post_message if message_content.present? upload_files if message.attachments.any? - rescue Slack::Web::Api::Errors::AccountInactive, Slack::Web::Api::Errors::MissingScope, Slack::Web::Api::Errors::InvalidAuth, + rescue Slack::Web::Api::Errors::IsArchived, Slack::Web::Api::Errors::AccountInactive, Slack::Web::Api::Errors::MissingScope, + Slack::Web::Api::Errors::InvalidAuth, Slack::Web::Api::Errors::ChannelNotFound, Slack::Web::Api::Errors::NotInChannel => e Rails.logger.error e hook.prompt_reauthorization! From 2c2f0547f7232fcc5c4e470b72560e79535aaa47 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 12 Feb 2026 10:07:56 +0530 Subject: [PATCH 3/3] fix: Captain not responding to campaign conversations (#13489) Co-authored-by: Aakash Bakhle <48802744+aakashb95@users.noreply.github.com> --- .../hook_execution_service.rb | 5 +- .../conversation/response_builder_job.rb | 4 ++ .../hook_execution_service.rb | 4 ++ enterprise/lib/captain/tools/handoff_tool.rb | 4 ++ .../hook_execution_service_spec.rb | 71 +++++++++++++++++++ .../hook_execution_service_spec.rb | 34 +++++++++ 6 files changed, 121 insertions(+), 1 deletion(-) diff --git a/app/services/message_templates/hook_execution_service.rb b/app/services/message_templates/hook_execution_service.rb index 6205c8f3c..93f0447f3 100644 --- a/app/services/message_templates/hook_execution_service.rb +++ b/app/services/message_templates/hook_execution_service.rb @@ -2,7 +2,6 @@ class MessageTemplates::HookExecutionService pattr_initialize [:message!] def perform - return if conversation.campaign.present? return if conversation.last_incoming_message.blank? return if message.auto_reply_email? @@ -21,6 +20,7 @@ class MessageTemplates::HookExecutionService end def should_send_out_of_office_message? + return false if conversation.campaign.present? # should not send if its a tweet message return false if conversation.tweet? # should not send for outbound messages @@ -37,6 +37,7 @@ class MessageTemplates::HookExecutionService end def should_send_greeting? + return false if conversation.campaign.present? # should not send if its a tweet message return false if conversation.tweet? @@ -49,6 +50,8 @@ class MessageTemplates::HookExecutionService # TODO: we should be able to reduce this logic once we have a toggle for email collect messages def should_send_email_collect? + return false if conversation.campaign.present? + !contact_has_email? && inbox.web_widget? && !email_collect_was_sent? end diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index 0f5e82e7f..297e78181 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -93,6 +93,10 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob end def send_out_of_office_message_if_applicable + # Campaign conversations should never receive OOO templates — the campaign itself + # serves as the initial outreach, and OOO would be confusing in that context. + return if @conversation.campaign.present? + ::MessageTemplates::Template::OutOfOffice.perform_if_applicable(@conversation) end diff --git a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb index 5c9a8a184..56dbc7245 100644 --- a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb +++ b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb @@ -68,6 +68,10 @@ module Enterprise::MessageTemplates::HookExecutionService end def send_out_of_office_message_after_handoff + # Campaign conversations should never receive OOO templates — the campaign itself + # serves as the initial outreach, and OOO would be confusing in that context. + return if conversation.campaign.present? + ::MessageTemplates::Template::OutOfOffice.perform_if_applicable(conversation) end diff --git a/enterprise/lib/captain/tools/handoff_tool.rb b/enterprise/lib/captain/tools/handoff_tool.rb index 797f248ff..d126840be 100644 --- a/enterprise/lib/captain/tools/handoff_tool.rb +++ b/enterprise/lib/captain/tools/handoff_tool.rb @@ -42,6 +42,10 @@ class Captain::Tools::HandoffTool < Captain::Tools::BasePublicTool end def send_out_of_office_message_if_applicable(conversation) + # Campaign conversations should never receive OOO templates — the campaign itself + # serves as the initial outreach, and OOO would be confusing in that context. + return if conversation.campaign.present? + ::MessageTemplates::Template::OutOfOffice.perform_if_applicable(conversation) end diff --git a/spec/enterprise/services/enterprise/message_templates/hook_execution_service_spec.rb b/spec/enterprise/services/enterprise/message_templates/hook_execution_service_spec.rb index 9074baa57..151e9101a 100644 --- a/spec/enterprise/services/enterprise/message_templates/hook_execution_service_spec.rb +++ b/spec/enterprise/services/enterprise/message_templates/hook_execution_service_spec.rb @@ -238,6 +238,77 @@ RSpec.describe MessageTemplates::HookExecutionService do end end + context 'when conversation has a campaign' do + let(:campaign) { create(:campaign, account: account) } + let(:campaign_conversation) { create(:conversation, inbox: inbox, account: account, contact: contact, status: :pending, campaign: campaign) } + + it 'schedules captain response job for incoming messages on pending campaign conversations' do + expect(Captain::Conversation::ResponseBuilderJob).to receive(:perform_later).with(campaign_conversation, assistant) + + create(:message, conversation: campaign_conversation, message_type: :incoming) + end + + it 'does not send greeting template on campaign conversations' do + inbox.update!(greeting_enabled: true, greeting_message: 'Hello! How can we help you?', enable_email_collect: false) + + greeting_service = instance_double(MessageTemplates::Template::Greeting) + allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service) + allow(greeting_service).to receive(:perform).and_return(true) + + create(:message, conversation: campaign_conversation, message_type: :incoming) + + expect(MessageTemplates::Template::Greeting).not_to have_received(:new) + end + + it 'does not send out of office template on campaign conversations' do + inbox.update!(working_hours_enabled: true, out_of_office_message: 'We are currently closed') + inbox.working_hours.find_by(day_of_week: Time.current.in_time_zone(inbox.timezone).wday).update!( + closed_all_day: true, + open_all_day: false + ) + + out_of_office_service = instance_double(MessageTemplates::Template::OutOfOffice) + allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service) + allow(out_of_office_service).to receive(:perform).and_return(true) + + create(:message, conversation: campaign_conversation, message_type: :incoming) + + expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new) + end + + it 'does not send email collect template on campaign conversations' do + contact.update!(email: nil) + inbox.update!(enable_email_collect: true) + + email_collect_service = instance_double(MessageTemplates::Template::EmailCollect) + allow(MessageTemplates::Template::EmailCollect).to receive(:new).and_return(email_collect_service) + allow(email_collect_service).to receive(:perform).and_return(true) + + create(:message, conversation: campaign_conversation, message_type: :incoming) + + expect(MessageTemplates::Template::EmailCollect).not_to have_received(:new) + end + + it 'does not send out of office template after handoff on campaign conversations when quota is exceeded' do + account.update!( + limits: { 'captain_responses' => 100 }, + custom_attributes: account.custom_attributes.merge('captain_responses_usage' => 100) + ) + inbox.update!( + working_hours_enabled: true, + out_of_office_message: 'We are currently closed' + ) + inbox.working_hours.find_by(day_of_week: Time.current.in_time_zone(inbox.timezone).wday).update!( + closed_all_day: true, + open_all_day: false + ) + + expect do + create(:message, conversation: campaign_conversation, message_type: :incoming) + end.not_to(change { campaign_conversation.messages.template.count }) + end + end + context 'when Captain quota is exceeded and handoff happens' do before do account.update!( diff --git a/spec/services/message_templates/hook_execution_service_spec.rb b/spec/services/message_templates/hook_execution_service_spec.rb index 13a820f17..fb9437111 100644 --- a/spec/services/message_templates/hook_execution_service_spec.rb +++ b/spec/services/message_templates/hook_execution_service_spec.rb @@ -111,6 +111,40 @@ describe MessageTemplates::HookExecutionService do end end + context 'when conversation has a campaign' do + let(:campaign) { create(:campaign) } + + it 'does not call ::MessageTemplates::Template::Greeting on campaign conversations' do + contact = create(:contact, email: nil) + conversation = create(:conversation, contact: contact, campaign: campaign) + conversation.inbox.update(greeting_enabled: true, greeting_message: 'Hi, this is a greeting message', enable_email_collect: false) + + greeting_service = double + allow(MessageTemplates::Template::Greeting).to receive(:new).and_return(greeting_service) + allow(greeting_service).to receive(:perform).and_return(true) + + create(:message, conversation: conversation) + + expect(MessageTemplates::Template::Greeting).not_to have_received(:new) + end + + it 'does not call ::MessageTemplates::Template::OutOfOffice on campaign conversations' do + contact = create(:contact) + conversation = create(:conversation, contact: contact, campaign: campaign) + + conversation.inbox.update(working_hours_enabled: true, out_of_office_message: 'We are out of office') + conversation.inbox.working_hours.today.update!(closed_all_day: true) + + out_of_office_service = double + allow(MessageTemplates::Template::OutOfOffice).to receive(:new).and_return(out_of_office_service) + allow(out_of_office_service).to receive(:perform).and_return(true) + + create(:message, conversation: conversation) + + expect(MessageTemplates::Template::OutOfOffice).not_to have_received(:new) + end + end + context 'when message is an auto reply email' do it 'does not call any template hooks' do contact = create(:contact)