diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 20cd574a58a..3fa11ca23ba 100644 --- a/lib/site_setting_extension.rb +++ b/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 diff --git a/spec/components/site_setting_extension_spec.rb b/spec/components/site_setting_extension_spec.rb index 77e9b735d6a..8dba4429fa1 100644 --- a/spec/components/site_setting_extension_spec.rb +++ b/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