Don’t log recipients when sending mail

Since production applications typically run with log level info and
email adresses should be considered as sensitive data we want to prevent 
them from ending up in the logs. In development mode (with log level
debug) they are still logged as part of the Mail::Message object.

5 files changed, 26 insertions, 10 deletions

diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb
index 5610212fad..033a7f5b9e 100644
--- a/actionmailer/lib/action_mailer/base.rb
+++ b/actionmailer/lib/action_mailer/base.rb
@@ -593,6 +593,7 @@ module ActionMailer
     private
 
       def set_payload_for_mail(payload, mail)
+        payload[:mail]               = mail.encoded
         payload[:mailer]             = name
         payload[:message_id]         = mail.message_id
         payload[:subject]            = mail.subject
@@ -601,7 +602,6 @@ module ActionMailer
         payload[:bcc]                = mail.bcc if mail.bcc.present?
         payload[:cc]                 = mail.cc  if mail.cc.present?
         payload[:date]               = mail.date
-        payload[:mail]               = mail.encoded
         payload[:perform_deliveries] = mail.perform_deliveries
       end
 
diff --git a/actionmailer/lib/action_mailer/log_subscriber.rb b/actionmailer/lib/action_mailer/log_subscriber.rb
index 25c99342c2..26910f20f0 100644
--- a/actionmailer/lib/action_mailer/log_subscriber.rb
+++ b/actionmailer/lib/action_mailer/log_subscriber.rb
@@ -10,11 +10,10 @@ module ActionMailer
     def deliver(event)
       info do
         perform_deliveries = event.payload[:perform_deliveries]
-        recipients = Array(event.payload[:to]).join(", ")
         if perform_deliveries
-          "Sent mail to #{recipients} (#{event.duration.round(1)}ms)"
+          "Delivered mail #{event.payload[:message_id]} (#{event.duration.round(1)}ms)"
         else
-          "Skipped sending mail to #{recipients} as `perform_deliveries` is false"
+          "Skipped delivery of mail #{event.payload[:message_id]} as `perform_deliveries` is false"
         end
       end
 
diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb
index d0c4f189fd..15613d4dce 100644
--- a/actionmailer/test/base_test.rb
+++ b/actionmailer/test/base_test.rb
@@ -913,6 +913,23 @@ class BaseTest < ActiveSupport::TestCase
     ActiveSupport::Notifications.unsubscribe "process.action_mailer"
   end
 
+  test "notification for deliver" do
+    begin
+      events = []
+      ActiveSupport::Notifications.subscribe("deliver.action_mailer") do |*args|
+        events << ActiveSupport::Notifications::Event.new(*args)
+      end
+
+      BaseMailer.welcome(body: "Hello there").deliver_now
+
+      assert_equal 1, events.length
+      assert_equal "deliver.action_mailer", events[0].name
+      assert_not_nil events[0].payload[:message_id]
+    ensure
+      ActiveSupport::Notifications.unsubscribe "deliver.action_mailer"
+    end
+  end
+
   private
 
     # Execute the block setting the given values and restoring old values after
diff --git a/actionmailer/test/log_subscriber_test.rb b/actionmailer/test/log_subscriber_test.rb
index fb569ce45f..f09f1997c0 100644
--- a/actionmailer/test/log_subscriber_test.rb
+++ b/actionmailer/test/log_subscriber_test.rb
@@ -24,11 +24,11 @@ class AMLogSubscriberTest < ActionMailer::TestCase
   end
 
   def test_deliver_is_notified
-    BaseMailer.welcome.deliver_now
+    BaseMailer.welcome(message_id: "123@abc").deliver_now
     wait
 
     assert_equal(1, @logger.logged(:info).size)
-    assert_match(/Sent mail to system@test\.lindsaar\.net/, @logger.logged(:info).first)
+    assert_match(/Delivered mail 123@abc/, @logger.logged(:info).first)
 
     assert_equal(2, @logger.logged(:debug).size)
     assert_match(/BaseMailer#welcome: processed outbound mail in [\d.]+ms/, @logger.logged(:debug).first)
@@ -38,11 +38,11 @@ class AMLogSubscriberTest < ActionMailer::TestCase
   end
 
   def test_deliver_message_when_perform_deliveries_is_false
-    BaseMailer.welcome_without_deliveries.deliver_now
+    BaseMailer.welcome_without_deliveries(message_id: "123@abc").deliver_now
     wait
 
     assert_equal(1, @logger.logged(:info).size)
-    assert_match("Skipped sending mail to system@test.lindsaar.net as `perform_deliveries` is false", @logger.logged(:info).first)
+    assert_match("Skipped delivery of mail 123@abc as `perform_deliveries` is false", @logger.logged(:info).first)
 
     assert_equal(2, @logger.logged(:debug).size)
     assert_match(/BaseMailer#welcome_without_deliveries: processed outbound mail in [\d.]+ms/, @logger.logged(:debug).first)
diff --git a/actionmailer/test/mailers/base_mailer.rb b/actionmailer/test/mailers/base_mailer.rb
index c1bb48cc96..dbe1c4f0e6 100644
--- a/actionmailer/test/mailers/base_mailer.rb
+++ b/actionmailer/test/mailers/base_mailer.rb
@@ -21,8 +21,8 @@ class BaseMailer < ActionMailer::Base
     mail(template_name: "welcome", template_path: path)
   end
 
-  def welcome_without_deliveries
-    mail(template_name: "welcome")
+  def welcome_without_deliveries(hash = {})
+    mail({ template_name: "welcome" }.merge!(hash))
     mail.perform_deliveries = false
   end