feat: sanitize inbox name (#11597)

Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
This commit is contained in:
Shivam Mishra
2025-06-09 14:46:12 +05:30
committed by GitHub
parent 9b43a0f72b
commit 25f947223d
7 changed files with 180 additions and 48 deletions

View File

@@ -71,6 +71,7 @@ class OauthCallbackController < ApplicationController
def create_channel_with_inbox def create_channel_with_inbox
ActiveRecord::Base.transaction do ActiveRecord::Base.transaction do
channel_email = Channel::Email.create!(email: users_data['email'], account: account) channel_email = Channel::Email.create!(email: users_data['email'], account: account)
account.inboxes.create!( account.inboxes.create!(
account: account, account: account,
channel: channel_email, channel: channel_email,

View File

@@ -4,7 +4,8 @@ class AgentNotifications::ConversationNotificationsMailer < ApplicationMailer
@agent = agent @agent = agent
@conversation = conversation @conversation = conversation
subject = "#{@agent.available_name}, A new conversation [ID - #{@conversation.display_id}] has been created in #{@conversation.inbox&.name}." inbox_name = @conversation.inbox&.sanitized_name
subject = "#{@agent.available_name}, A new conversation [ID - #{@conversation.display_id}] has been created in #{inbox_name}."
@action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id) @action_url = app_account_conversation_url(account_id: @conversation.account_id, id: @conversation.display_id)
send_mail_with_liquid(to: @agent.email, subject: subject) and return send_mail_with_liquid(to: @agent.email, subject: subject) and return
end end

View File

@@ -104,7 +104,7 @@ class ConversationReplyMailer < ApplicationMailer
end end
def business_name def business_name
@inbox.business_name || @inbox.name @inbox.business_name || @inbox.sanitized_name
end end
def from_email def from_email

View File

@@ -47,8 +47,6 @@ class Inbox < ApplicationRecord
# Not allowing characters: # Not allowing characters:
validates :name, presence: true validates :name, presence: true
validates :name, if: :check_channel_type?, format: { with: %r{^^\b[^/\\<>@]*\b$}, multiline: true,
message: I18n.t('errors.inboxes.validations.name') }
validates :account_id, presence: true validates :account_id, presence: true
validates :timezone, inclusion: { in: TZInfo::Timezone.all_identifiers } validates :timezone, inclusion: { in: TZInfo::Timezone.all_identifiers }
validates :out_of_office_message, length: { maximum: Limits::OUT_OF_OFFICE_MESSAGE_MAX_LENGTH } validates :out_of_office_message, length: { maximum: Limits::OUT_OF_OFFICE_MESSAGE_MAX_LENGTH }
@@ -99,6 +97,16 @@ class Inbox < ApplicationRecord
update_account_cache update_account_cache
end end
# Sanitizes inbox name for balanced email provider compatibility
# ALLOWS: /'._- and Unicode letters/numbers/emojis
# REMOVES: Forbidden chars (\<>@") + spam-trigger symbols (!#$%&*+=?^`{|}~)
def sanitized_name
return default_name_for_blank_name if name.blank?
sanitized = apply_sanitization_rules(name)
sanitized.blank? && email? ? display_name_from_email : sanitized
end
def sms? def sms?
channel_type == 'Channel::Sms' channel_type == 'Channel::Sms'
end end
@@ -178,6 +186,22 @@ class Inbox < ApplicationRecord
private private
def default_name_for_blank_name
email? ? display_name_from_email : ''
end
def apply_sanitization_rules(name)
name.gsub(/[\\<>@"!#$%&*+=?^`{|}~]/, '') # Remove forbidden chars
.gsub(/[\x00-\x1F\x7F]/, ' ') # Replace control chars with spaces
.gsub(/\A[[:punct:]]+|[[:punct:]]+\z/, '') # Remove leading/trailing punctuation
.gsub(/\s+/, ' ') # Normalize spaces
.strip
end
def display_name_from_email
channel.email.split('@').first.parameterize.titleize
end
def dispatch_create_event def dispatch_create_event
return if ENV['ENABLE_INBOX_EVENTS'].blank? return if ENV['ENABLE_INBOX_EVENTS'].blank?

View File

@@ -18,7 +18,7 @@ RSpec.describe AgentNotifications::ConversationNotificationsMailer do
it 'renders the subject' do it 'renders the subject' do
expect(mail.subject).to eq("#{agent.available_name}, A new conversation [ID - #{conversation expect(mail.subject).to eq("#{agent.available_name}, A new conversation [ID - #{conversation
.display_id}] has been created in #{conversation.inbox&.name}.") .display_id}] has been created in #{conversation.inbox&.sanitized_name}.")
end end
it 'renders the receiver email' do it 'renders the receiver email' do

View File

@@ -87,7 +87,7 @@ RSpec.describe ConversationReplyMailer do
let(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now } let(:mail) { described_class.reply_with_summary(message.conversation, message.id).deliver_now }
it 'has correct name' do it 'has correct name' do
expect(mail[:from].display_names).to eq(["#{message.sender.available_name} from Inbox"]) expect(mail[:from].display_names).to eq(["#{message.sender.available_name} from #{message.conversation.inbox.sanitized_name}"])
end end
end end
@@ -224,11 +224,11 @@ RSpec.describe ConversationReplyMailer do
end end
context 'when smtp enabled for email channel' do context 'when smtp enabled for email channel' do
let(:smtp_email_channel) do let(:smtp_channel) do
create(:channel_email, smtp_enabled: true, smtp_address: 'smtp.gmail.com', smtp_port: 587, smtp_login: 'smtp@gmail.com', create(:channel_email, smtp_enabled: true, smtp_address: 'smtp.gmail.com', smtp_port: 587, smtp_login: 'smtp@gmail.com',
smtp_password: 'password', smtp_domain: 'smtp.gmail.com', account: account) smtp_password: 'password', smtp_domain: 'smtp.gmail.com', account: account)
end end
let(:conversation) { create(:conversation, assignee: agent, inbox: smtp_email_channel.inbox, account: account).reload } let(:conversation) { create(:conversation, assignee: agent, inbox: smtp_channel.inbox, account: account).reload }
let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') } let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') }
it 'use smtp mail server' do it 'use smtp mail server' do
@@ -240,19 +240,19 @@ RSpec.describe ConversationReplyMailer do
it 'renders sender name in the from address' do it 'renders sender name in the from address' do
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq "#{message.sender.available_name} from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" expect(mail['from'].value).to eq "#{message.sender.available_name} from #{smtp_channel.inbox.sanitized_name} <#{smtp_channel.email}>"
end end
it 'renders sender name even when assignee is not present' do it 'renders sender name even when assignee is not present' do
conversation.update(assignee_id: nil) conversation.update(assignee_id: nil)
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq "#{message.sender.available_name} from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" expect(mail['from'].value).to eq "#{message.sender.available_name} from #{smtp_channel.inbox.sanitized_name} <#{smtp_channel.email}>"
end end
it 'renders assignee name in the from address when sender_name not available' do it 'renders assignee name in the from address when sender_name not available' do
message.update(sender_id: nil) message.update(sender_id: nil)
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq "#{conversation.assignee.available_name} from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" expect(mail['from'].value).to eq "#{conversation.assignee.available_name} from #{smtp_channel.inbox.sanitized_name} <#{smtp_channel.email}>"
end end
it 'renders inbox name as sender and assignee or business_name not present' do it 'renders inbox name as sender and assignee or business_name not present' do
@@ -260,7 +260,7 @@ RSpec.describe ConversationReplyMailer do
conversation.update(assignee_id: nil) conversation.update(assignee_id: nil)
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq "Notifications from #{smtp_email_channel.inbox.name} <#{smtp_email_channel.email}>" expect(mail['from'].value).to eq "Notifications from #{smtp_channel.inbox.sanitized_name} <#{smtp_channel.email}>"
end end
context 'when friendly name enabled' do context 'when friendly name enabled' do
@@ -276,7 +276,7 @@ RSpec.describe ConversationReplyMailer do
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq "Notifications from #{conversation.inbox.name} <#{smtp_email_channel.email}>" expect(mail['from'].value).to eq "Notifications from #{conversation.inbox.sanitized_name} <#{smtp_channel.email}>"
end end
it 'renders sender name as sender and assignee nil and business_name present' do it 'renders sender name as sender and assignee nil and business_name present' do
@@ -286,7 +286,7 @@ RSpec.describe ConversationReplyMailer do
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq( expect(mail['from'].value).to eq(
"Notifications from #{conversation.inbox.business_name} <#{smtp_email_channel.email}>" "Notifications from #{conversation.inbox.business_name} <#{smtp_channel.email}>"
) )
end end
@@ -295,7 +295,7 @@ RSpec.describe ConversationReplyMailer do
conversation.update(assignee_id: agent.id) conversation.update(assignee_id: agent.id)
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq "#{agent.available_name} from #{conversation.inbox.business_name} <#{smtp_email_channel.email}>" expect(mail['from'].value).to eq "#{agent.available_name} from #{conversation.inbox.business_name} <#{smtp_channel.email}>"
end end
it 'renders sender name as sender and assignee and business_name present' do it 'renders sender name as sender and assignee and business_name present' do
@@ -304,7 +304,7 @@ RSpec.describe ConversationReplyMailer do
conversation.update(assignee_id: agent.id) conversation.update(assignee_id: agent.id)
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq "#{agent_2.available_name} from #{conversation.inbox.business_name} <#{smtp_email_channel.email}>" expect(mail['from'].value).to eq "#{agent_2.available_name} from #{conversation.inbox.business_name} <#{smtp_channel.email}>"
end end
end end
@@ -321,7 +321,7 @@ RSpec.describe ConversationReplyMailer do
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq "#{conversation.inbox.name} <#{smtp_email_channel.email}>" expect(mail['from'].value).to eq "#{conversation.inbox.sanitized_name} <#{smtp_channel.email}>"
end end
it 'renders sender name as business_name present' do it 'renders sender name as business_name present' do
@@ -330,17 +330,17 @@ RSpec.describe ConversationReplyMailer do
mail = described_class.email_reply(message) mail = described_class.email_reply(message)
expect(mail['from'].value).to eq "#{conversation.inbox.business_name} <#{smtp_email_channel.email}>" expect(mail['from'].value).to eq "#{conversation.inbox.business_name} <#{smtp_channel.email}>"
end end
end end
end end
context 'when smtp enabled for microsoft email channel' do context 'when smtp enabled for microsoft email channel' do
let(:ms_smtp_email_channel) do let(:ms_smtp_channel) do
create(:channel_email, imap_login: 'smtp@outlook.com', create(:channel_email, imap_login: 'smtp@outlook.com',
imap_enabled: true, account: account, provider: 'microsoft', provider_config: { access_token: 'access_token' }) imap_enabled: true, account: account, provider: 'microsoft', provider_config: { access_token: 'access_token' })
end end
let(:conversation) { create(:conversation, assignee: agent, inbox: ms_smtp_email_channel.inbox, account: account).reload } let(:conversation) { create(:conversation, assignee: agent, inbox: ms_smtp_channel.inbox, account: account).reload }
let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') } let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') }
it 'use smtp mail server' do it 'use smtp mail server' do
@@ -352,11 +352,11 @@ RSpec.describe ConversationReplyMailer do
end end
context 'when smtp enabled for google email channel' do context 'when smtp enabled for google email channel' do
let(:ms_smtp_email_channel) do let(:ms_smtp_channel) do
create(:channel_email, imap_login: 'smtp@gmail.com', create(:channel_email, imap_login: 'smtp@gmail.com',
imap_enabled: true, account: account, provider: 'google', provider_config: { access_token: 'access_token' }) imap_enabled: true, account: account, provider: 'google', provider_config: { access_token: 'access_token' })
end end
let(:conversation) { create(:conversation, assignee: agent, inbox: ms_smtp_email_channel.inbox, account: account).reload } let(:conversation) { create(:conversation, assignee: agent, inbox: ms_smtp_channel.inbox, account: account).reload }
let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') } let(:message) { create(:message, conversation: conversation, account: account, message_type: 'outgoing', content: 'Outgoing Message 2') }
it 'use smtp mail server' do it 'use smtp mail server' do
@@ -430,7 +430,7 @@ RSpec.describe ConversationReplyMailer do
it 'sets reply to email to be based on the domain' do it 'sets reply to email to be based on the domain' do
reply_to_email = "reply+#{message.conversation.uuid}@#{conversation.account.domain}" reply_to_email = "reply+#{message.conversation.uuid}@#{conversation.account.domain}"
reply_to = "#{message.sender.available_name} from #{conversation.inbox.name} <#{reply_to_email}>" reply_to = "#{message.sender.available_name} from #{conversation.inbox.sanitized_name} <#{reply_to_email}>"
expect(mail['REPLY-TO'].value).to eq(reply_to) expect(mail['REPLY-TO'].value).to eq(reply_to)
expect(mail.reply_to).to eq([reply_to_email]) expect(mail.reply_to).to eq([reply_to_email])
end end

View File

@@ -164,31 +164,7 @@ RSpec.describe Inbox do
let(:inbox) { FactoryBot.create(:inbox) } let(:inbox) { FactoryBot.create(:inbox) }
context 'when validating inbox name' do context 'when validating inbox name' do
it 'does not allow any special character at the end' do it 'does not allow empty string' do
inbox.name = 'this is my inbox name-'
expect(inbox).not_to be_valid
expect(inbox.errors.full_messages).to eq(
['Name should not start or end with symbols, and it should not have < > / \\ @ characters.']
)
end
it 'does not allow any special character at the start' do
inbox.name = '-this is my inbox name'
expect(inbox).not_to be_valid
expect(inbox.errors.full_messages).to eq(
['Name should not start or end with symbols, and it should not have < > / \\ @ characters.']
)
end
it 'does not allow chacters like /\@<> in the entire string' do
inbox.name = 'inbox@name'
expect(inbox).not_to be_valid
expect(inbox.errors.full_messages).to eq(
['Name should not start or end with symbols, and it should not have < > / \\ @ characters.']
)
end
it 'does not empty string' do
inbox.name = '' inbox.name = ''
expect(inbox).not_to be_valid expect(inbox).not_to be_valid
expect(inbox.errors.full_messages[0]).to eq( expect(inbox.errors.full_messages[0]).to eq(
@@ -282,4 +258,134 @@ RSpec.describe Inbox do
inbox.touch # rubocop:disable Rails/SkipsModelValidations inbox.touch # rubocop:disable Rails/SkipsModelValidations
end end
end end
describe '#sanitized_name' do
context 'when inbox name contains forbidden characters' do
it 'removes forbidden and spam-trigger characters' do
inbox = FactoryBot.build(:inbox, name: 'Test/Name\\With<Bad>@Characters"And\'Quotes!#$%')
expect(inbox.sanitized_name).to eq('Test/NameWithBadCharactersAnd\'Quotes')
end
end
context 'when inbox name has leading/trailing non-word characters' do
it 'removes leading and trailing symbols' do
inbox = FactoryBot.build(:inbox, name: '!!!Test Name***')
expect(inbox.sanitized_name).to eq('Test Name')
end
it 'handles mixed leading/trailing characters' do
inbox = FactoryBot.build(:inbox, name: '###@@@Test Inbox Name$$$%%')
expect(inbox.sanitized_name).to eq('Test Inbox Name')
end
end
context 'when inbox name has multiple spaces' do
it 'normalizes multiple spaces to single space' do
inbox = FactoryBot.build(:inbox, name: 'Test Multiple Spaces')
expect(inbox.sanitized_name).to eq('Test Multiple Spaces')
end
it 'handles tabs and other whitespace' do
inbox = FactoryBot.build(:inbox, name: "Test\t\nMultiple\r\nSpaces")
expect(inbox.sanitized_name).to eq('Test Multiple Spaces')
end
end
context 'when inbox name has leading/trailing whitespace' do
it 'strips whitespace' do
inbox = FactoryBot.build(:inbox, name: ' Test Name ')
expect(inbox.sanitized_name).to eq('Test Name')
end
end
context 'when inbox name becomes empty after sanitization' do
context 'with email channel' do
it 'falls back to email local part' do
email_channel = FactoryBot.build(:channel_email, email: 'support@example.com')
inbox = FactoryBot.build(:inbox, name: '\\<>@"', channel: email_channel)
expect(inbox.sanitized_name).to eq('Support')
end
it 'handles email with complex local part' do
email_channel = FactoryBot.build(:channel_email, email: 'help-desk_team@example.com')
inbox = FactoryBot.build(:inbox, name: '!!!@@@', channel: email_channel)
expect(inbox.sanitized_name).to eq('Help Desk Team')
end
end
context 'with non-email channel' do
it 'returns empty string when name becomes blank' do
web_widget_channel = FactoryBot.build(:channel_widget)
inbox = FactoryBot.build(:inbox, name: '\\<>@"', channel: web_widget_channel)
expect(inbox.sanitized_name).to eq('')
end
end
end
context 'when inbox name is blank initially' do
context 'with email channel' do
it 'uses email local part as fallback' do
email_channel = FactoryBot.build(:channel_email, email: 'customer-care@example.com')
inbox = FactoryBot.build(:inbox, name: '', channel: email_channel)
expect(inbox.sanitized_name).to eq('Customer Care')
end
end
context 'with non-email channel' do
it 'returns empty string' do
api_channel = FactoryBot.build(:channel_api)
inbox = FactoryBot.build(:inbox, name: '', channel: api_channel)
expect(inbox.sanitized_name).to eq('')
end
end
end
context 'when inbox name contains valid characters' do
it 'preserves valid characters like hyphens, underscores, and dots' do
inbox = FactoryBot.build(:inbox, name: 'Test-Name_With.Valid-Characters')
expect(inbox.sanitized_name).to eq('Test-Name_With.Valid-Characters')
end
it 'preserves alphanumeric characters and spaces' do
inbox = FactoryBot.build(:inbox, name: 'Customer Support 123')
expect(inbox.sanitized_name).to eq('Customer Support 123')
end
it 'preserves balanced safe characters but removes spam-trigger symbols' do
inbox = FactoryBot.build(:inbox, name: "Test!#$%&'*+/=?^_`{|}~-Name")
expect(inbox.sanitized_name).to eq("Test'/_-Name")
end
it 'keeps commonly used safe characters' do
inbox = FactoryBot.build(:inbox, name: "Support/Help's Team.Desk_2024-Main")
expect(inbox.sanitized_name).to eq("Support/Help's Team.Desk_2024-Main")
end
end
context 'when inbox name contains problematic characters for email headers' do
it 'preserves Unicode symbols (trademark, etc.)' do
inbox = FactoryBot.build(:inbox, name: 'Test™Name®With©Special™Characters')
expect(inbox.sanitized_name).to eq('Test™Name®With©Special™Characters')
end
end
context 'with edge cases' do
it 'handles nil name gracefully' do
inbox = FactoryBot.build(:inbox)
allow(inbox).to receive(:name).and_return(nil)
expect { inbox.sanitized_name }.not_to raise_error
end
it 'handles very long names' do
long_name = 'A' * 1000
inbox = FactoryBot.build(:inbox, name: long_name)
expect(inbox.sanitized_name).to eq(long_name)
end
it 'handles unicode characters and preserves emojis' do
inbox = FactoryBot.build(:inbox, name: 'Test Name with émojis 🎉')
expect(inbox.sanitized_name).to eq('Test Name with émojis 🎉')
end
end
end
end end