From d272a64ff7e66d4b04592d3ea635a3ebc04ecf6e Mon Sep 17 00:00:00 2001
From: Sojan Jose <sojan@pepalo.com>
Date: Wed, 11 Feb 2026 11:02:38 -0800
Subject: [PATCH] fix(mailbox): handle malformed sender address headers
 (#13486)

## How to reproduce
When an inbound email has malformed sender headers (for example `From:
McDonald <info@example.com` without a closing `>`), 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 <accounts@chatwoot.com>')).address
+    configured_sender = Mail::Address.new(ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot <accounts@chatwoot.com>')).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 <info@example.com'
+          to 'Inbox <inbox@example.com>'
+          subject :header
+          body 'Hi'
+        end
+      end
+
+      let(:mail_with_malformed_reply_to) do
+        Mail.new do
+          from 'Sender <sender@example.com>'
+          to 'Inbox <inbox@example.com>'
+          subject :header
+          body 'Hi'
+          header['Reply-To'] = 'Reply User <reply@example.com'
+        end
+      end
+
+      let(:mail_with_original_sender_header) do
+        Mail.new do
+          from 'Sender <sender@example.com>'
+          to 'Inbox <inbox@example.com>'
+          subject :header
+          body 'Hi'
+          header['Reply-To'] = 'Reply User <reply@example.com'
+          header['X-Original-Sender'] = 'Forwarded Sender <forwarded.sender@example.com>'
+        end
+      end
+
+      let(:mail_with_invalid_original_sender_header) do
+        Mail.new do
+          from 'Sender <sender@example.com>'
+          to 'Inbox <inbox@example.com>'
+          subject :header
+          body 'Hi'
+          header['Reply-To'] = 'Reply User <reply@example.com'
+          header['X-Original-Sender'] = 'not an email address'
+        end
+      end
+
+      it 'returns nil sender values when from header is malformed' do
+        presenter = described_class.new(mail_with_malformed_from)
+
+        expect(presenter.original_sender).to be_nil
+        expect(presenter.sender_name).to be_nil
+        expect(presenter.notification_email_from_chatwoot?).to be(false)
+      end
+
+      it 'falls back to from header when reply_to is malformed' do
+        presenter = described_class.new(mail_with_malformed_reply_to)
+        expect(presenter.original_sender).to eq('sender@example.com')
+      end
+
+      it 'uses parsed X-Original-Sender value when available' do
+        presenter = described_class.new(mail_with_original_sender_header)
+        expect(presenter.original_sender).to eq('forwarded.sender@example.com')
+      end
+
+      it 'falls back to from when X-Original-Sender is invalid' do
+        presenter = described_class.new(mail_with_invalid_original_sender_header)
+        expect(presenter.original_sender).to eq('sender@example.com')
+      end
+
+      it 'matches notification sender emails case-insensitively' do
+        mail_with_uppercase_sender = Mail.new do
+          from 'Chatwoot <ACCOUNTS@CHATWOOT.COM>'
+          to 'Inbox <inbox@example.com>'
+          subject :header
+          body 'Hi'
+        end
+
+        with_modified_env MAILER_SENDER_EMAIL: 'Chatwoot <accounts@chatwoot.com>' do
+          presenter = described_class.new(mail_with_uppercase_sender)
+          expect(presenter.notification_email_from_chatwoot?).to be(true)
+        end
+      end
+    end
   end
 end