aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorZhang Kang <piecehealth@sina.com>2018-09-07 09:46:54 +0800
committerZhang Kang <piecehealth@sina.com>2018-09-07 09:52:13 +0800
commit180dcd1bfa801ad132343c64245db606bd85ed96 (patch)
treee96e757058b3c088342e8c0681b87b9cde4ac1c3 /activerecord
parent736edb982856f0de04d4566f657c0c84f145e7ef (diff)
downloadrails-180dcd1bfa801ad132343c64245db606bd85ed96.tar.gz
rails-180dcd1bfa801ad132343c64245db606bd85ed96.tar.bz2
rails-180dcd1bfa801ad132343c64245db606bd85ed96.zip
Configuration item `config.filter_parameters` could also filter out sensitive value of database column when call `#inspect`
* Why Some sensitive data will be exposed in log accidentally by calling `#inspect`, e.g. ```ruby @account = Account.find params[:id] payload = { account: @account } logger.info "payload will be #{ payload }" ``` All the information of `@account` will be exposed in log. * Solution Add a class attribute filter_attributes to specify which values of columns shouldn't be exposed. This attribute equals to `Rails.application.config.filter_parameters` by default. ```ruby Rails.application.config.filter_parameters += [:credit_card_number] Account.last.insepct # => #<Account id: 123, credit_card_number: [FILTERED] ...> ```
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md9
-rw-r--r--activerecord/lib/active_record/core.rb20
-rw-r--r--activerecord/lib/active_record/railtie.rb6
-rw-r--r--activerecord/test/cases/filter_attributes_test.rb80
4 files changed, 113 insertions, 2 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index a0ab0d30bc..09af076aa8 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,12 @@
+* Configuration item `config.filter_parameters` could also filter out sensitive value of database column when call `#inspect`.
+
+ ```
+ Rails.application.config.filter_parameters += [:credit_card_number]
+ Account.last.insepct # => #<Account id: 123, credit_card_number: [FILTERED] ...>
+ ```
+
+ *Zhang Kang*
+
* Deprecate `column_name_length`, `table_name_length`, `columns_per_table`,
`indexes_per_table`, `columns_per_multicolumn_index`, `sql_query_length`,
and `joins_per_query` methods in `DatabaseLimits`.
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index 9ec6ba14fd..82cf7563a2 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -8,6 +8,8 @@ module ActiveRecord
module Core
extend ActiveSupport::Concern
+ FILTERED = "[FILTERED]" # :nodoc:
+
included do
##
# :singleton-method:
@@ -123,6 +125,10 @@ module ActiveRecord
class_attribute :default_connection_handler, instance_writer: false
+ ##
+ # Specifies columns which don't want to be exposed while calling #inspect
+ class_attribute :filter_attributes, instance_writer: false, default: []
+
def self.connection_handler
ActiveRecord::RuntimeRegistry.connection_handler || default_connection_handler
end
@@ -487,12 +493,17 @@ module ActiveRecord
# Returns the contents of the record as a nicely formatted string.
def inspect
+ filter_attributes = self.filter_attributes.map(&:to_s).to_set
# We check defined?(@attributes) not to issue warnings if the object is
# allocated but not initialized.
inspection = if defined?(@attributes) && @attributes
self.class.attribute_names.collect do |name|
if has_attribute?(name)
- "#{name}: #{attribute_for_inspect(name)}"
+ if filter_attributes.include?(name) && !read_attribute(name).nil?
+ "#{name}: #{ActiveRecord::Core::FILTERED}"
+ else
+ "#{name}: #{attribute_for_inspect(name)}"
+ end
end
end.compact.join(", ")
else
@@ -506,6 +517,7 @@ module ActiveRecord
# when pp is required.
def pretty_print(pp)
return super if custom_inspect_method_defined?
+ filter_attributes = self.filter_attributes.map(&:to_s).to_set
pp.object_address_group(self) do
if defined?(@attributes) && @attributes
column_names = self.class.column_names.select { |name| has_attribute?(name) || new_record? }
@@ -516,7 +528,11 @@ module ActiveRecord
pp.text column_name
pp.text ":"
pp.breakable
- pp.pp column_value
+ if filter_attributes.include?(column_name) && !column_value.nil?
+ pp.text ActiveRecord::Core::FILTERED
+ else
+ pp.pp column_value
+ end
end
end
else
diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb
index 7ece083fd4..47351588d3 100644
--- a/activerecord/lib/active_record/railtie.rb
+++ b/activerecord/lib/active_record/railtie.rb
@@ -235,5 +235,11 @@ MSG
end
end
end
+
+ initializer "active_record.set_filter_attributes" do
+ ActiveSupport.on_load(:active_record) do
+ self.filter_attributes += Rails.application.config.filter_parameters
+ end
+ end
end
end
diff --git a/activerecord/test/cases/filter_attributes_test.rb b/activerecord/test/cases/filter_attributes_test.rb
new file mode 100644
index 0000000000..f88cecfe2b
--- /dev/null
+++ b/activerecord/test/cases/filter_attributes_test.rb
@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+require "cases/helper"
+require "models/admin"
+require "models/admin/user"
+require "models/admin/account"
+require "pp"
+
+class FilterAttributesTest < ActiveRecord::TestCase
+ fixtures :"admin/users", :"admin/accounts"
+
+ setup do
+ ActiveRecord::Base.filter_attributes = [:name]
+ end
+
+ teardown do
+ ActiveRecord::Base.filter_attributes = []
+ end
+
+ test "filter_attributes" do
+ Admin::User.all.each do |user|
+ assert_includes user.inspect, "name: [FILTERED]"
+ assert_equal 1, user.inspect.scan("[FILTERED]").length
+ end
+
+ Admin::Account.all.each do |account|
+ assert_includes account.inspect, "name: [FILTERED]"
+ assert_equal 1, account.inspect.scan("[FILTERED]").length
+ end
+ end
+
+ test "filter_attributes could be overwritten by models" do
+ Admin::Account.all.each do |account|
+ assert_includes account.inspect, "name: [FILTERED]"
+ assert_equal 1, account.inspect.scan("[FILTERED]").length
+ end
+
+ Admin::Account.filter_attributes = []
+
+ # Above changes should not impact other models
+ Admin::User.all.each do |user|
+ assert_includes user.inspect, "name: [FILTERED]"
+ assert_equal 1, user.inspect.scan("[FILTERED]").length
+ end
+
+ Admin::Account.all.each do |account|
+ assert_not_includes account.inspect, "name: [FILTERED]"
+ assert_equal 0, account.inspect.scan("[FILTERED]").length
+ end
+
+ Admin::Account.filter_attributes = [:name]
+ end
+
+ test "filter_attributes should not filter nil value" do
+ account = Admin::Account.new
+
+ assert_includes account.inspect, "name: nil"
+ assert_not_includes account.inspect, "name: [FILTERED]"
+ assert_equal 0, account.inspect.scan("[FILTERED]").length
+ end
+
+ test "filter_attributes on pretty_print" do
+ user = admin_users(:david)
+ actual = "".dup
+ PP.pp(user, StringIO.new(actual))
+
+ assert_includes actual, "name: [FILTERED]"
+ assert_equal 1, actual.scan("[FILTERED]").length
+ end
+
+ test "filter_attributes on pretty_print should not filter nil value" do
+ user = Admin::User.new
+ actual = "".dup
+ PP.pp(user, StringIO.new(actual))
+
+ assert_includes actual, "name: nil"
+ assert_not_includes actual, "name: [FILTERED]"
+ assert_equal 0, actual.scan("[FILTERED]").length
+ end
+end