Merge pull request #2571 from mroderick/fix/invalid-emails

fix: validate email format and add defensive mailer checks

Author: olleolleolle

Gemfile

@@ -18,6 +18,7 @@ gem 'csv' # LOCKED: csv was loaded from the standard library, but is not part of
 gem 'delayed_job'
 gem 'delayed_job_active_record'
 gem 'drb' # LOCKED: Added because of pry-remote
+gem 'email_validator'
 gem 'font_awesome5_rails'
 gem 'bootstrap', '~> 5'
 gem 'friendly_id'

Gemfile.lock

@@ -193,6 +193,8 @@ GEM
       dotenv (= 3.2.0)
       railties (>= 6.1)
     drb (2.2.3)
+    email_validator (2.2.4)
+      activemodel
     erb (6.0.2)
     erubi (1.13.1)
     execjs (2.10.1)
@@ -634,6 +636,7 @@ DEPENDENCIES
   delayed_job_active_record
   dotenv-rails
   drb
+  email_validator
   fabrication
   faker
   faraday

app/helpers/email_header_helper.rb

@@ -2,10 +2,19 @@ module EmailHeaderHelper
   private
 
   def mail_args(member, subject, from_email = 'meetings@codebar.io', cc = '', bcc = '')
+    return nil if invalid_email?(member.email, member.id)
+
     { from: "codebar.io <#{from_email}>",
       to: member.email,
       cc: cc,
       bcc: bcc,
       subject: subject }
   end
+
+  def invalid_email?(email, member_id)
+    return false if EmailValidator.valid?(email, mode: :strict)
+
+    Rails.logger.warn("[EmailHeaderHelper] Invalid email for member_id=#{member_id}: #{email}")
+    true
+  end
 end

app/models/member.rb

@@ -26,6 +26,7 @@ class Member < ApplicationRecord
   validates :auth_services, presence: true
   validates :name, :surname, :email, :about_you, presence: true, if: :can_log_in?
   validates :email, uniqueness: true
+  validates :email, email: { mode: :strict }, if: :can_log_in?
   validates :about_you, length: { maximum: 255 }
 
   DIETARY_RESTRICTIONS = %w[vegan vegetarian pescetarian halal gluten_free dairy_free other].freeze

spec/helpers/email_header_helper_spec.rb

@@ -0,0 +1,60 @@
+RSpec.describe EmailHeaderHelper, type: :helper do
+  before { EmailHeaderHelper.module_eval { public :mail_args } }
+
+  describe '#mail_args' do
+    let(:member) { Struct.new(:id, :email).new(1, 'test@example.com') }
+
+    it 'returns mail arguments for valid email' do
+      result = helper.mail_args(member, 'Test Subject')
+      expect(result[:to]).to eq('test@example.com')
+      expect(result[:subject]).to eq('Test Subject')
+    end
+
+    it 'returns nil for nil email' do
+      member = Struct.new(:id, :email).new(1, nil)
+      result = helper.mail_args(member, 'Test Subject')
+      expect(result).to be_nil
+    end
+
+    it 'returns nil for blank email' do
+      member = Struct.new(:id, :email).new(1, '')
+      result = helper.mail_args(member, 'Test Subject')
+      expect(result).to be_nil
+    end
+
+    it 'returns nil for invalid email format' do
+      member = Struct.new(:id, :email).new(1, 'invalid-email')
+      result = helper.mail_args(member, 'Test Subject')
+      expect(result).to be_nil
+    end
+
+    it 'returns nil for email missing @ symbol' do
+      member = Struct.new(:id, :email).new(1, 'invalidexample.com')
+      result = helper.mail_args(member, 'Test Subject')
+      expect(result).to be_nil
+    end
+
+    it 'returns nil for email missing TLD' do
+      member = Struct.new(:id, :email).new(1, 'invalid@example')
+      result = helper.mail_args(member, 'Test Subject')
+      expect(result).to be_nil
+    end
+
+    it 'returns mail arguments for valid email with plus addressing' do
+      member = Struct.new(:id, :email).new(1, 'user+tag@example.com')
+      result = helper.mail_args(member, 'Test Subject')
+      expect(result[:to]).to eq('user+tag@example.com')
+    end
+
+    it 'includes from email when provided' do
+      result = helper.mail_args(member, 'Test Subject', 'custom@codebar.io')
+      expect(result[:from]).to eq('codebar.io <custom@codebar.io>')
+    end
+
+    it 'includes cc and bcc when provided' do
+      result = helper.mail_args(member, 'Test Subject', 'from@codebar.io', 'cc@codebar.io', 'bcc@codebar.io')
+      expect(result[:cc]).to eq('cc@codebar.io')
+      expect(result[:bcc]).to eq('bcc@codebar.io')
+    end
+  end
+end

spec/models/member_spec.rb

@@ -14,6 +14,37 @@
         it { expect(member).to validate_presence_of(:surname) }
         it { expect(member).to validate_presence_of(:email) }
         it { expect(member).to validate_presence_of(:about_you) }
+
+        it 'accepts valid email format' do
+          member.email = 'valid@example.com'
+          expect(member).to be_valid
+        end
+
+        it 'rejects invalid email format' do
+          member.email = 'invalid-email'
+          expect(member).not_to be_valid
+          expect(member.errors[:email]).to include('is invalid')
+        end
+
+        it 'rejects email missing @ symbol' do
+          member.email = 'invalidexample.com'
+          expect(member).not_to be_valid
+        end
+
+        it 'rejects email missing TLD' do
+          member.email = 'invalid@example'
+          expect(member).not_to be_valid
+        end
+
+        it 'accepts email with valid subdomains' do
+          member.email = 'user@mail.example.com'
+          expect(member).to be_valid
+        end
+
+        it 'accepts email with plus addressing' do
+          member.email = 'user+tag@example.com'
+          expect(member).to be_valid
+        end
       end
     end