fix: Issue with processing variables in outgoing email content (#12799)

Co-authored-by: Shivam Mishra <scm.mymail@gmail.com>
Co-authored-by: Vinay Keerthi <11478411+stonecharioteer@users.noreply.github.com>
Co-authored-by: Sojan Jose <sojan@pepalo.com>

app/builders/messages/message_builder.rb

@@ -1,5 +1,8 @@
 class Messages::MessageBuilder
   include ::FileTypeHelper
+  include ::EmailHelper
+  include ::DataHelper
+
   attr_reader :message
 
   def initialize(user, conversation, params)
@@ -38,30 +41,12 @@ class Messages::MessageBuilder
     params = convert_to_hash(@params)
     content_attributes = params.fetch(:content_attributes, {})
 
-    return parse_json(content_attributes) if content_attributes.is_a?(String)
+    return safe_parse_json(content_attributes) if content_attributes.is_a?(String)
     return content_attributes if content_attributes.is_a?(Hash)
 
     {}
   end
 
-  # Converts the given object to a hash.
-  # If it's an instance of ActionController::Parameters, converts it to an unsafe hash.
-  # Otherwise, returns the object as-is.
-  def convert_to_hash(obj)
-    return obj.to_unsafe_h if obj.instance_of?(ActionController::Parameters)
-
-    obj
-  end
-
-  # Attempts to parse a string as JSON.
-  # If successful, returns the parsed hash with symbolized names.
-  # If unsuccessful, returns nil.
-  def parse_json(content)
-    JSON.parse(content, symbolize_names: true)
-  rescue JSON::ParserError
-    {}
-  end
-
   def process_attachments
     return if @attachments.blank?
 
@@ -110,12 +95,6 @@ class Messages::MessageBuilder
     email_string.gsub(/\s+/, '').split(',')
   end
 
-  def validate_email_addresses(all_emails)
-    all_emails&.each do |email|
-      raise StandardError, 'Invalid email address' unless email.match?(URI::MailTo::EMAIL_REGEXP)
-    end
-  end
-
   def message_type
     if @conversation.inbox.channel_type != 'Channel::Api' && @message_type == 'incoming'
       raise StandardError, 'Incoming messages are only allowed in Api inboxes'
@@ -178,14 +157,17 @@ class Messages::MessageBuilder
     email_attributes = ensure_indifferent_access(@message.content_attributes[:email] || {})
     normalized_content = normalize_email_body(@message.content)
 
+    # Process liquid templates in normalized content with code block protection
+    processed_content = process_liquid_in_email_body(normalized_content)
+
     # Use custom HTML content if provided, otherwise generate from message content
     email_attributes[:html_content] = if custom_email_content_provided?
                                         build_custom_html_content
                                       else
-                                        build_html_content(normalized_content)
+                                        build_html_content(processed_content)
                                       end
 
-    email_attributes[:text_content] = build_text_content(normalized_content)
+    email_attributes[:text_content] = build_text_content(processed_content)
     email_attributes
   end
 
@@ -204,22 +186,6 @@ class Messages::MessageBuilder
     text_content
   end
 
-  def ensure_indifferent_access(hash)
-    return {} if hash.blank?
-
-    hash.respond_to?(:with_indifferent_access) ? hash.with_indifferent_access : hash
-  end
-
-  def normalize_email_body(content)
-    content.to_s.gsub("\r\n", "\n")
-  end
-
-  def render_email_html(content)
-    return '' if content.blank?
-
-    ChatwootMarkdownRenderer.new(content).render_message.to_s
-  end
-
   def custom_email_content_provided?
     @params[:email_html_content].present?
   end
@@ -232,4 +198,27 @@ class Messages::MessageBuilder
 
     html_content
   end
+
+  # Liquid processing methods for email content
+  def process_liquid_in_email_body(content)
+    return content if content.blank?
+    return content unless should_process_liquid?
+
+    # Protect code blocks from liquid processing
+    modified_content = modified_liquid_content(content)
+    template = Liquid::Template.parse(modified_content)
+    template.render(drops_with_sender)
+  rescue Liquid::Error
+    content
+  end
+
+  def should_process_liquid?
+    @message_type == 'outgoing' || @message_type == 'template'
+  end
+
+  def drops_with_sender
+    message_drops(@conversation).merge({
+                                         'agent' => UserDrop.new(sender)
+                                       })
+  end
 end

app/helpers/data_helper.rb

@@ -0,0 +1,24 @@
+# Provides utility methods for data transformation, hash manipulation, and JSON parsing.
+# This module contains helper methods for converting between different data types,
+# normalizing hashes, and safely handling JSON operations.
+module DataHelper
+  # Ensures a hash supports indifferent access (string or symbol keys).
+  # Returns an empty hash if the input is blank.
+  def ensure_indifferent_access(hash)
+    return {} if hash.blank?
+
+    hash.respond_to?(:with_indifferent_access) ? hash.with_indifferent_access : hash
+  end
+
+  def convert_to_hash(obj)
+    return obj.to_unsafe_h if obj.instance_of?(ActionController::Parameters)
+
+    obj
+  end
+
+  def safe_parse_json(content)
+    JSON.parse(content, symbolize_names: true)
+  rescue JSON::ParserError
+    {}
+  end
+end

app/helpers/email_helper.rb

@@ -4,6 +4,19 @@ module EmailHelper
     domain.split('.').first
   end
 
+  def render_email_html(content)
+    return '' if content.blank?
+
+    ChatwootMarkdownRenderer.new(content).render_message.to_s
+  end
+
+  # Raise a standard error if any email address is invalid
+  def validate_email_addresses(emails_to_test)
+    emails_to_test&.each do |email|
+      raise StandardError, 'Invalid email address' unless email.match?(URI::MailTo::EMAIL_REGEXP)
+    end
+  end
+
   # ref: https://www.rfc-editor.org/rfc/rfc5233.html
   # This is not a  mandatory requirement for email addresses, but it is a common practice.
   # john+test@xyc.com is the same as john@xyc.com
@@ -21,6 +34,10 @@ module EmailHelper
     end
   end
 
+  def normalize_email_body(content)
+    content.to_s.gsub("\r\n", "\n")
+  end
+
   def modified_liquid_content(email)
     # This regex is used to match the code blocks in the content
     # We don't want to process liquid in code blocks
@@ -29,7 +46,10 @@ module EmailHelper
 
   def message_drops(conversation)
     {
-      'contact' => ContactDrop.new(conversation.contact)
+      'contact' => ContactDrop.new(conversation.contact),
+      'conversation' => ConversationDrop.new(conversation),
+      'inbox' => InboxDrop.new(conversation.inbox),
+      'account' => AccountDrop.new(conversation.account)
     }
   end
 end

spec/builders/messages/message_builder_spec.rb

@@ -223,6 +223,69 @@ describe Messages::MessageBuilder do
           expect(message.content_attributes.dig('email', 'text_content', 'full')).to eq 'Regular **markdown** content'
         end
       end
+
+      context 'when liquid templates are present in email content' do
+        let(:contact) { create(:contact, name: 'John', email: 'john@example.com') }
+        let(:conversation) { create(:conversation, inbox: channel_email.inbox, account: account, contact: contact) }
+
+        it 'processes liquid variables in email content' do
+          params = ActionController::Parameters.new({
+                                                      content: 'Hello {{contact.name}}, your email is {{contact.email}}'
+                                                    })
+
+          message = described_class.new(user, conversation, params).perform
+
+          expect(message.content_attributes.dig('email', 'html_content', 'full')).to include('Hello John')
+          expect(message.content_attributes.dig('email', 'html_content', 'full')).to include('john@example.com')
+          expect(message.content_attributes.dig('email', 'text_content', 'full')).to eq 'Hello John, your email is john@example.com'
+        end
+
+        it 'does not process liquid in code blocks' do
+          params = ActionController::Parameters.new({
+                                                      content: 'Hello {{contact.name}}, use this code: `{{contact.email}}`'
+                                                    })
+
+          message = described_class.new(user, conversation, params).perform
+
+          expect(message.content_attributes.dig('email', 'text_content', 'full')).to eq 'Hello John, use this code: `{{contact.email}}`'
+        end
+
+        it 'handles broken liquid syntax gracefully' do
+          params = ActionController::Parameters.new({
+                                                      content: 'Hello {{contact.name}  {{invalid}}'
+                                                    })
+
+          message = described_class.new(user, conversation, params).perform
+
+          expect(message.content_attributes.dig('email', 'text_content', 'full')).to eq 'Hello {{contact.name}  {{invalid}}'
+        end
+
+        it 'does not process liquid for incoming messages' do
+          params = ActionController::Parameters.new({
+                                                      content: 'Hello {{contact.name}}',
+                                                      message_type: 'incoming'
+                                                    })
+
+          api_channel = create(:channel_api, account: account)
+          api_conversation = create(:conversation, inbox: api_channel.inbox, account: account, contact: contact)
+
+          message = described_class.new(user, api_conversation, params).perform
+
+          expect(message.content).to eq 'Hello {{contact.name}}'
+        end
+
+        it 'does not process liquid for private messages' do
+          params = ActionController::Parameters.new({
+                                                      content: 'Hello {{contact.name}}',
+                                                      private: true
+                                                    })
+
+          message = described_class.new(user, conversation, params).perform
+
+          expect(message.content_attributes.dig('email', 'html_content')).to be_nil
+          expect(message.content_attributes.dig('email', 'text_content')).to be_nil
+        end
+      end
     end
   end
 end