From 802630378e5ed7ffae500034bc926b8318524e43 Mon Sep 17 00:00:00 2001
From: Ari Pollak <ajp@aripollak.com>
Date: Fri, 28 Nov 2014 14:19:16 -0500
Subject: [PATCH] Hide potentially sensitive ActiveJob params from logs

* Show GlobalID instead of full object .inspect output
---
 activejob/lib/active_job/logging.rb  | 17 ++++++++++++++++-
 activejob/test/cases/logging_test.rb | 28 ++++++++++++++++++----------
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb
index 21d2fda3ff..74c02e149a 100644
--- a/activejob/lib/active_job/logging.rb
+++ b/activejob/lib/active_job/logging.rb
@@ -6,6 +6,8 @@ module ActiveJob
   module Logging #:nodoc:
     extend ActiveSupport::Concern
 
+    FILTERED = '[FILTERED]'.freeze
+
     included do
       cattr_accessor(:logger) { ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT)) }
 
@@ -84,8 +86,21 @@ module ActiveJob
           event.payload[:adapter].name.demodulize.remove('Adapter') + "(#{event.payload[:job].queue_name})"
         end
 
+        def global_id_or_filtered(argument)
+          if argument.is_a?(GlobalID::Identification)
+            argument.to_global_id.to_s
+          else
+            FILTERED
+          end
+        end
+
         def args_info(job)
-          job.arguments.any? ? " with arguments: #{job.arguments.map(&:inspect).join(", ")}" : ""
+          if job.arguments.any?
+            ' with arguments: ' +
+              job.arguments.map { |arg| global_id_or_filtered(arg) }.join(', ')
+          else
+            ""
+          end
         end
 
         def scheduled_at(event)
diff --git a/activejob/test/cases/logging_test.rb b/activejob/test/cases/logging_test.rb
index 3d4e561117..e308232073 100644
--- a/activejob/test/cases/logging_test.rb
+++ b/activejob/test/cases/logging_test.rb
@@ -42,37 +42,45 @@ class AdapterTest < ActiveSupport::TestCase
 
 
   def test_uses_active_job_as_tag
-    HelloJob.perform_later "Cristian"
+    HelloJob.perform_later nil
     assert_match(/\[ActiveJob\]/, @logger.messages)
   end
 
   def test_uses_job_name_as_tag
-    LoggingJob.perform_later "Dummy"
+    LoggingJob.perform_later nil
     assert_match(/\[LoggingJob\]/, @logger.messages)
   end
 
   def test_uses_job_id_as_tag
-    LoggingJob.perform_later "Dummy"
+    LoggingJob.perform_later nil
     assert_match(/\[LOGGING-JOB-ID\]/, @logger.messages)
   end
 
   def test_logs_correct_queue_name
     original_queue_name = LoggingJob.queue_name
     LoggingJob.queue_as :php_jobs
-    LoggingJob.perform_later("Dummy")
+    LoggingJob.perform_later nil
     assert_match(/to .*?\(php_jobs\).*/, @logger.messages)
   ensure
     LoggingJob.queue_name = original_queue_name
   end
 
+  def test_globalid_parameter_logging
+    person = Person.new(123)
+    LoggingJob.perform_later person
+    assert_match(%r{Enqueued.*gid://aj/Person/123}, @logger.messages)
+    assert_match(%r{Dummy, here is it: #<Person:.*>}, @logger.messages)
+    assert_match(%r{Performing.*gid://aj/Person/123}, @logger.messages)
+  end
+
   def test_enqueue_job_logging
     HelloJob.perform_later "Cristian"
-    assert_match(/Enqueued HelloJob \(Job ID: .*?\) to .*?:.*Cristian/, @logger.messages)
+    assert_match(/Enqueued HelloJob \(Job ID: .*?\) to .*?:.*\[FILTERED\]/, @logger.messages)
   end
 
   def test_perform_job_logging
     LoggingJob.perform_later "Dummy"
-    assert_match(/Performing LoggingJob from .*? with arguments:.*Dummy/, @logger.messages)
+    assert_match(/Performing LoggingJob from .*? with arguments:.*\[FILTERED\]/, @logger.messages)
     assert_match(/Dummy, here is it: Dummy/, @logger.messages)
     assert_match(/Performed LoggingJob from .*? in .*ms/, @logger.messages)
   end
@@ -82,8 +90,8 @@ class AdapterTest < ActiveSupport::TestCase
     assert_match(/\[LoggingJob\] \[.*?\]/, @logger.messages)
     assert_match(/\[ActiveJob\] Enqueued NestedJob \(Job ID: .*\) to/, @logger.messages)
     assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Performing NestedJob from/, @logger.messages)
-    assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Enqueued LoggingJob \(Job ID: .*?\) to .* with arguments: "NestedJob"/, @logger.messages)
-    assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Performing LoggingJob from .* with arguments: "NestedJob"/, @logger.messages)
+    assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Enqueued LoggingJob \(Job ID: .*?\) to .* with arguments: \[FILTERED\]/, @logger.messages)
+    assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Performing LoggingJob from .* with arguments: \[FILTERED\]/, @logger.messages)
     assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Dummy, here is it: NestedJob/, @logger.messages)
     assert_match(/\[ActiveJob\].*\[LoggingJob\] \[LOGGING-JOB-ID\] Performed LoggingJob from .* in/, @logger.messages)
     assert_match(/\[ActiveJob\] \[NestedJob\] \[NESTED-JOB-ID\] Performed NestedJob from .* in/, @logger.messages)
@@ -91,14 +99,14 @@ class AdapterTest < ActiveSupport::TestCase
 
   def test_enqueue_at_job_logging
     HelloJob.set(wait_until: 24.hours.from_now).perform_later "Cristian"
-    assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*Cristian/, @logger.messages)
+    assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*\[FILTERED\]/, @logger.messages)
   rescue NotImplementedError
     skip
   end
 
   def test_enqueue_in_job_logging
     HelloJob.set(wait: 2.seconds).perform_later "Cristian"
-    assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*Cristian/, @logger.messages)
+    assert_match(/Enqueued HelloJob \(Job ID: .*\) to .*? at.*\[FILTERED\]/, @logger.messages)
   rescue NotImplementedError
     skip
   end