Skip to content

Mail::SMTP monkey patch overrides defaults

Summary

In 2023-08-23: accidentally sent out emails to som... (gitlab-com/gl-infra/production#16223 - closed) we sent out emails using a custom email configuration (Configurable e-mail address for service desk (#329990 - closed)).

The custom email feature for service desk overrides the delivery_method for a given single Mail::Message.

90beaced introduced a monkey patch for Mail::SMTP in config/initializers/mail_starttls_patch.rb where we override the constructor and call .merge! on the settings, which could lead to permanently changed defaults.

During the incident, we changed the delivery_method for a single email and the defaults. So all new emails generated and delivered by that sidekiq worker node received the altered defaults.

Steps to reproduce

Run this in the rails console:

# What are the defaults?
Mail.delivery_method
# Create a new email and override the delivery method here
m = Mail.new
# This should only be valid for this message
m.delivery_method Mail::SMTP, {user_name: 'some_username'}
# But defaults are now based in the input above
Mail.delivery_method

What is the current bug behavior?

Overriding the delivery_method using the Mail::SMTP class and custom settings also overrides the defaults of Mail::SMTP. This leads to other emails also using the custom settings instead of the original defaults.

What is the expected correct behavior?

Overriding the delivery_method using the Mail::SMTP class on a Mail::Message object should only override the settings for the current object and not touch the defaults of Mail::SMTP.

Possible fixes

To fix this, we should make a copy of the defaults before we further process them. That leads to a fresh set of defaults every time we initialize a Mail::SMTP instance.