FIX: scrub secret setting values from logs

lib/site_setting_extension.rb

@@ -275,10 +275,18 @@ module SiteSettingExtension
     end
   end
 
+  SECRET_SETTINGS ||= %w{
+    google_oauth2_client_secret twitter_consumer_secret instagram_consumer_secret
+    facebook_app_secret github_client_secret s3_secret_access_key
+  }
+
   def set_and_log(name, value, user = Discourse.system_user)
     prev_value = send(name)
     set(name, value)
-    StaffActionLogger.new(user).log_site_setting_change(name, prev_value, value) if has_setting?(name)
+    if has_setting?(name)
+      value = prev_value = "[FILTERED]" if SECRET_SETTINGS.include?(name)
+      StaffActionLogger.new(user).log_site_setting_change(name, prev_value, value)
+    end
   end
 
   protected

spec/components/site_setting_extension_spec.rb

@@ -393,11 +393,30 @@ describe SiteSettingExtension do
   end
 
   describe ".set_and_log" do
+    before do
+      settings.setting(:s3_secret_access_key, "old_secret_key")
+      settings.setting(:title, "Discourse v1")
+      settings.refresh!
+    end
+
     it "raises an error when set for an invalid setting name" do
       expect {
         settings.set_and_log("provider", "haxxed")
       }.to raise_error(ArgumentError)
     end
+
+    it "scrubs secret setting values from logs" do
+      settings.set_and_log("s3_secret_access_key", "new_secret_key")
+      expect(UserHistory.last.previous_value).to eq("[FILTERED]")
+      expect(UserHistory.last.new_value).to eq("[FILTERED]")
+    end
+
+    it "works" do
+      settings.set_and_log("title", "Discourse v2")
+      expect(settings.title).to eq("Discourse v2")
+      expect(UserHistory.last.previous_value).to eq("Discourse v1")
+      expect(UserHistory.last.new_value).to eq("Discourse v2")
+    end
   end
 
   describe "filter domain name" do