From d1c482cb644fb015306af8b56c64c8593e773550 Mon Sep 17 00:00:00 2001 From: Vishnu Narayanan Date: Mon, 8 Jun 2026 17:12:25 +0530 Subject: [PATCH] fix(email): strip null bytes from inbound mailbox messages (#14546) Inbound emails containing null bytes could fail while Chatwoot persisted the incoming message, causing IMAP sync to drop and repeatedly retry the same malformed email. This strips null bytes at the inbound mailbox message persistence boundary so the rest of the email can be saved normally. Fixes https://linear.app/chatwoot/issue/CW-7165/argumenterror-string-contains-null-byte-argumenterror Sentry: https://chatwoot-p3.sentry.io/issues/7405946944/ ## Test - [x] added specs - [x] manually tested the logic on prod for the inbox(email) which was throwing the error ## What changed - Sanitizes null bytes only while building attributes for inbound mailbox message persistence. - Uses the sanitized source ID for both duplicate lookup and message creation. - Adds regression coverage for the mailbox message creation path. --------- Co-authored-by: Sony Mathew <2040199+sony-mathew@users.noreply.github.com> Co-authored-by: Sony Mathew --- app/mailboxes/imap/imap_mailbox.rb | 19 +++++++---- app/mailboxes/mailbox_helper.rb | 30 ++++++---------- app/mailboxes/mailbox_sanitizer.rb | 34 +++++++++++++++++++ .../base_strategy.rb | 2 ++ .../in_reply_to_strategy.rb | 2 +- .../new_conversation_strategy.rb | 8 ++--- .../references_strategy.rb | 2 +- spec/mailboxes/imap/imap_mailbox_spec.rb | 27 +++++++++++++++ spec/mailboxes/mailbox_helper_spec.rb | 25 ++++++++++++++ spec/mailboxes/reply_mailbox_spec.rb | 34 +++++++++++++++++++ 10 files changed, 151 insertions(+), 32 deletions(-) create mode 100644 app/mailboxes/mailbox_sanitizer.rb diff --git a/app/mailboxes/imap/imap_mailbox.rb b/app/mailboxes/imap/imap_mailbox.rb index 27bc88e06..25b8f4d85 100644 --- a/app/mailboxes/imap/imap_mailbox.rb +++ b/app/mailboxes/imap/imap_mailbox.rb @@ -65,11 +65,10 @@ class Imap::ImapMailbox end def in_reply_to - @processed_mail.in_reply_to + sanitize_mailbox_value(@processed_mail.in_reply_to) end def find_conversation_by_references - references = Array.wrap(@inbound_mail.references) references.each do |message_id| match = FALLBACK_CONVERSATION_PATTERN.match(message_id) @@ -80,8 +79,6 @@ class Imap::ImapMailbox def find_message_by_references message_to_return = nil - references = Array.wrap(@inbound_mail.references) - references.each do |message_id| message = @inbox.messages.find_by(source_id: message_id) message_to_return = message if message.present? @@ -100,7 +97,7 @@ class Imap::ImapMailbox source: 'email', in_reply_to: in_reply_to, auto_reply: @processed_mail.auto_reply?, - mail_subject: @processed_mail.subject, + mail_subject: sanitize_mailbox_value(@processed_mail.subject), initiated_at: { timestamp: Time.now.utc } @@ -110,7 +107,7 @@ class Imap::ImapMailbox end def find_or_create_contact - @contact = @inbox.contacts.from_email(@processed_mail.original_sender) + @contact = @inbox.contacts.from_email(original_sender_email) if @contact.present? @contact_inbox = ContactInbox.find_by(inbox: @inbox, contact: @contact) else @@ -119,6 +116,14 @@ class Imap::ImapMailbox end def identify_contact_name - processed_mail.sender_name || processed_mail.from.first.split('@').first + sanitize_mailbox_value(processed_mail.sender_name || processed_mail.from.first.split('@').first) + end + + def original_sender_email + sanitize_mailbox_value(@processed_mail.original_sender) + end + + def references + sanitize_mailbox_value(Array.wrap(@inbound_mail.references)) end end diff --git a/app/mailboxes/mailbox_helper.rb b/app/mailboxes/mailbox_helper.rb index edabfd4ce..fd3e0fb16 100644 --- a/app/mailboxes/mailbox_helper.rb +++ b/app/mailboxes/mailbox_helper.rb @@ -1,27 +1,16 @@ module MailboxHelper include MailboxInlineAttachmentHelper + include MailboxSanitizer include ::FileTypeHelper private def create_message Rails.logger.info "[MailboxHelper] Creating message #{processed_mail.message_id}" - return if @conversation.messages.find_by(source_id: processed_mail.message_id).present? + source_id = sanitize_mailbox_value(processed_mail.message_id) + return if @conversation.messages.find_by(source_id: source_id).present? - @message = @conversation.messages.create!( - account_id: @conversation.account_id, - sender: @conversation.contact, - content: mail_content&.truncate(150_000), - inbox_id: @conversation.inbox_id, - message_type: 'incoming', - content_type: 'incoming_email', - source_id: processed_mail.message_id, - content_attributes: { - email: processed_mail.serialized_data, - cc_email: processed_mail.cc, - bcc_email: processed_mail.bcc - } - ) + @message = @conversation.messages.create!(sanitized_message_attributes(source_id)) end def add_attachments_to_message @@ -101,13 +90,16 @@ module MailboxHelper end def create_contact + sender_email = sanitize_mailbox_value(processed_mail.original_sender) + message_id = sanitize_mailbox_value(processed_mail.message_id) + @contact_inbox = ::ContactInboxWithContactBuilder.new( - source_id: processed_mail.original_sender, + source_id: sender_email, inbox: @inbox, contact_attributes: { - name: identify_contact_name, - email: processed_mail.original_sender, - additional_attributes: { source_id: "email:#{processed_mail.message_id}" } + name: sanitize_mailbox_value(identify_contact_name), + email: sender_email, + additional_attributes: { source_id: "email:#{message_id}" } } ).perform diff --git a/app/mailboxes/mailbox_sanitizer.rb b/app/mailboxes/mailbox_sanitizer.rb new file mode 100644 index 000000000..b78bb1258 --- /dev/null +++ b/app/mailboxes/mailbox_sanitizer.rb @@ -0,0 +1,34 @@ +module MailboxSanitizer + NULL_BYTE = "\u0000".freeze + + private + + def sanitized_message_attributes(source_id) + { + account_id: @conversation.account_id, + sender: @conversation.contact, + content: sanitize_mailbox_value(mail_content)&.truncate(150_000), + inbox_id: @conversation.inbox_id, + message_type: 'incoming', + content_type: 'incoming_email', + source_id: source_id, + content_attributes: sanitized_content_attributes + } + end + + def sanitized_content_attributes + sanitize_mailbox_value( + email: processed_mail.serialized_data, + cc_email: processed_mail.cc, + bcc_email: processed_mail.bcc + ) + end + + def sanitize_mailbox_value(value) + return value.delete(NULL_BYTE) if value.is_a?(String) + return value.map { |item| sanitize_mailbox_value(item) } if value.is_a?(Array) + return value.transform_values { |item| sanitize_mailbox_value(item) } if value.is_a?(Hash) + + value + end +end diff --git a/app/services/mailbox/conversation_finder_strategies/base_strategy.rb b/app/services/mailbox/conversation_finder_strategies/base_strategy.rb index c1e738805..fd61fd9e0 100644 --- a/app/services/mailbox/conversation_finder_strategies/base_strategy.rb +++ b/app/services/mailbox/conversation_finder_strategies/base_strategy.rb @@ -1,4 +1,6 @@ class Mailbox::ConversationFinderStrategies::BaseStrategy + include MailboxSanitizer + attr_reader :mail def initialize(mail) diff --git a/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb b/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb index b14f851f5..e3d8270f1 100644 --- a/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb +++ b/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb @@ -14,7 +14,7 @@ class Mailbox::ConversationFinderStrategies::InReplyToStrategy < Mailbox::Conver def find return nil if mail.in_reply_to.blank? - in_reply_to_addresses = Array.wrap(mail.in_reply_to) + in_reply_to_addresses = sanitize_mailbox_value(Array.wrap(mail.in_reply_to)) in_reply_to_addresses.each do |in_reply_to| # Try extracting UUID from patterns diff --git a/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb b/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb index a21fcebe5..f9f5c3cf8 100644 --- a/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb +++ b/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb @@ -45,11 +45,11 @@ class Mailbox::ConversationFinderStrategies::NewConversationStrategy < Mailbox:: end def original_sender_email - @processed_mail.original_sender&.downcase + sanitize_mailbox_value(@processed_mail.original_sender)&.downcase end def identify_contact_name - @processed_mail.sender_name || @processed_mail.from.first.split('@').first + sanitize_mailbox_value(@processed_mail.sender_name || @processed_mail.from.first.split('@').first) end def build_conversation @@ -63,7 +63,7 @@ class Mailbox::ConversationFinderStrategies::NewConversationStrategy < Mailbox:: in_reply_to: in_reply_to, source: 'email', auto_reply: @processed_mail.auto_reply?, - mail_subject: @processed_mail.subject, + mail_subject: sanitize_mailbox_value(@processed_mail.subject), initiated_at: { timestamp: Time.now.utc } @@ -72,7 +72,7 @@ class Mailbox::ConversationFinderStrategies::NewConversationStrategy < Mailbox:: end def in_reply_to - mail['In-Reply-To'].try(:value) + sanitize_mailbox_value(mail['In-Reply-To'].try(:value)) end def find_conversation_by_in_reply_to diff --git a/app/services/mailbox/conversation_finder_strategies/references_strategy.rb b/app/services/mailbox/conversation_finder_strategies/references_strategy.rb index 86a0aa3c5..4420c5c7d 100644 --- a/app/services/mailbox/conversation_finder_strategies/references_strategy.rb +++ b/app/services/mailbox/conversation_finder_strategies/references_strategy.rb @@ -21,7 +21,7 @@ class Mailbox::ConversationFinderStrategies::ReferencesStrategy < Mailbox::Conve return nil if mail.references.blank? return nil unless @channel # No valid channel found - references = Array.wrap(mail.references) + references = sanitize_mailbox_value(Array.wrap(mail.references)) references.each do |reference| conversation = find_conversation_from_reference(reference) diff --git a/spec/mailboxes/imap/imap_mailbox_spec.rb b/spec/mailboxes/imap/imap_mailbox_spec.rb index 309a38a65..9a6797aaf 100644 --- a/spec/mailboxes/imap/imap_mailbox_spec.rb +++ b/spec/mailboxes/imap/imap_mailbox_spec.rb @@ -99,6 +99,33 @@ RSpec.describe Imap::ImapMailbox do end end + context 'when a new email contains null bytes' do + let(:inbound_mail) do + Mail.new.tap do |mail| + mail.from = 'email@gmail.com' + mail.to = 'imap@gmail.com' + mail.subject = "Hello\u0000" + mail.message_id = "message\u0000@example.com" + mail['In-Reply-To'] = "source\u0000@example.com" + mail.references = ["reference\u0000@example.com"] + mail.content_type = 'text/plain' + mail.body = "Body\u0000 text" + end + end + + it 'creates sanitized conversation and message records' do + expect { class_instance.process(inbound_mail, channel) }.to change(Conversation, :count).by(1) + + message = conversation.messages.last + + expect(conversation.additional_attributes['in_reply_to']).to eq('source@example.com') + expect(conversation.additional_attributes['mail_subject']).to eq('Hello') + expect(message.source_id).to eq('message@example.com') + expect(message.content).to eq('Body text') + expect(message.content_attributes.to_json).not_to include('\u0000') + end + end + context 'when a new email with invalid from' do let(:inbound_mail) { create_inbound_email_from_mail(from: 'invalidemail', to: 'imap@gmail.com', subject: 'Hello!') } diff --git a/spec/mailboxes/mailbox_helper_spec.rb b/spec/mailboxes/mailbox_helper_spec.rb index 613040cb3..4b02205ec 100644 --- a/spec/mailboxes/mailbox_helper_spec.rb +++ b/spec/mailboxes/mailbox_helper_spec.rb @@ -47,6 +47,31 @@ RSpec.describe MailboxHelper do helper_instance.send(:create_message) end end + + context 'when message data contains null bytes' do + let(:mail) do + mail = Mail.new + mail.from = 'Sender ' + mail.to = 'Inbox ' + mail.subject = "Hello\u0000" + mail.message_id = "message\u0000@example.com" + mail.content_type = 'text/plain' + mail.body = "Body\u0000 text" + mail + end + + it 'creates the message with sanitized values' do + helper_instance = mailbox_helper_obj.new(conversation, processed_mail) + + expect { helper_instance.send(:create_message) }.to change(conversation.messages, :count).by(1) + + message = conversation.messages.last + expect(message.source_id).to eq('message@example.com') + expect(message.content).to eq('Body text') + expect(message.content_attributes.dig('email', 'message_id')).to eq('message@example.com') + expect(message.content_attributes.to_json).not_to include('\u0000') + end + end end describe '#embed_plain_text_email_with_inline_image' do diff --git a/spec/mailboxes/reply_mailbox_spec.rb b/spec/mailboxes/reply_mailbox_spec.rb index d062c7d73..20ce60dad 100644 --- a/spec/mailboxes/reply_mailbox_spec.rb +++ b/spec/mailboxes/reply_mailbox_spec.rb @@ -67,6 +67,40 @@ RSpec.describe ReplyMailbox do end end + context 'when new conversation email contains null bytes' do + let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) } + let(:null_byte_mail) { create_inbound_email_from_mail(from: 'sender@example.com', to: email_channel.email, subject: 'Hello') } + let(:mail_with_null_bytes) do + Mail.new.tap do |mail| + mail.from = 'sender@example.com' + mail.to = email_channel.email + mail.subject = "Hello\u0000" + mail.message_id = "message\u0000@example.com" + mail['In-Reply-To'] = "source\u0000@example.com" + mail.references = ["reference\u0000@example.com"] + mail.content_type = 'text/plain' + mail.body = "Body\u0000 text" + end + end + + before do + allow(null_byte_mail).to receive(:mail).and_return(mail_with_null_bytes) + end + + it 'creates sanitized conversation and message records' do + expect { described_class.receive null_byte_mail }.to change(Conversation, :count).by(1) + + conversation = Conversation.last + message = conversation.messages.last + + expect(conversation.additional_attributes['in_reply_to']).to eq('source@example.com') + expect(conversation.additional_attributes['mail_subject']).to eq('Hello') + expect(message.source_id).to eq('message@example.com') + expect(message.content).to eq('Body text') + expect(message.content_attributes.to_json).not_to include('\u0000') + end + end + context 'with inline attachments' do let(:mail_with_inline_images) { create_inbound_email_from_fixture('mail_with_inline_images.eml') } let(:described_subject) { described_class.receive mail_with_inline_images }