From 180dcd1bfa801ad132343c64245db606bd85ed96 Mon Sep 17 00:00:00 2001 From: Zhang Kang Date: Fri, 7 Sep 2018 09:46:54 +0800 Subject: 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 # => # ``` --- activerecord/CHANGELOG.md | 9 +++ activerecord/lib/active_record/core.rb | 20 +++++- activerecord/lib/active_record/railtie.rb | 6 ++ activerecord/test/cases/filter_attributes_test.rb | 80 +++++++++++++++++++++++ 4 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 activerecord/test/cases/filter_attributes_test.rb (limited to 'activerecord') 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 # => # + ``` + + *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 -- cgit v1.2.3