Skip to content

Ensure filtered params aren't revealed in sql #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions lib/rails_semantic_logger/active_record/log_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,25 @@ def sql(event)

# When multiple values are received for a single bound field, it is converted into an array
def add_bind_value(binds, key, value)
key = key.downcase.to_sym unless key.nil?
value = (Array(binds[key]) << value) if binds.key?(key)
key = key.downcase.to_sym unless key.nil?

if rails_filter_params_include?(key)
value = "[FILTERED]"
elsif binds.key?(key)
value = (Array(binds[key]) << value)
end

binds[key] = value
end

def rails_filter_params_include?(key)
filter_parameters = Rails.configuration.filter_parameters

return filter_parameters.first.match? key if filter_parameters.first.is_a? Regexp

filter_parameters.include? key
end

def logger
self.class.logger
end
Expand Down
56 changes: 56 additions & 0 deletions test/active_record_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,62 @@ class ActiveRecordTest < Minitest::Test
assert_instance_of Integer, messages[0].payload[:allocations] if Rails.version.to_i >= 6
end

it "filtered bind value" do
filter_params_setting true, %i[name] do
expected_sql =
if Rails.version.to_f >= 5.2
"SELECT #{extra_space}\"samples\".* FROM \"samples\" WHERE \"samples\".\"name\" = ? ORDER BY \"samples\".\"id\" ASC LIMIT ?"
else
"SELECT \"samples\".* FROM \"samples\" WHERE \"samples\".\"name\" = ? ORDER BY \"samples\".\"id\" ASC LIMIT ?"
end

messages = semantic_logger_events do
Sample.where(name: "Jack").first
end
assert_equal 1, messages.count, messages

assert_semantic_logger_event(
messages[0],
level: :debug,
name: "ActiveRecord",
message: "Sample Load",
payload_includes: {
sql: expected_sql,
binds: {name: "[FILTERED]", limit: 1}
}
)
assert_instance_of Integer, messages[0].payload[:allocations] if Rails.version.to_i >= 6
end
end

it "filtered bind value when filter_parameters set as regex" do
filter_params_regex_setting true, %i[name] do
expected_sql =
if Rails.version.to_f >= 5.2
"SELECT #{extra_space}\"samples\".* FROM \"samples\" WHERE \"samples\".\"name\" = ? ORDER BY \"samples\".\"id\" ASC LIMIT ?"
else
"SELECT \"samples\".* FROM \"samples\" WHERE \"samples\".\"name\" = ? ORDER BY \"samples\".\"id\" ASC LIMIT ?"
end

messages = semantic_logger_events do
Sample.where(name: "Jack").first
end
assert_equal 1, messages.count, messages

assert_semantic_logger_event(
messages[0],
level: :debug,
name: "ActiveRecord",
message: "Sample Load",
payload_includes: {
sql: expected_sql,
binds: {name: "[FILTERED]", limit: 1}
}
)
assert_instance_of Integer, messages[0].payload[:allocations] if Rails.version.to_i >= 6
end
end

it "multiple bind values" do
skip "Not applicable to older rails" if Rails.version.to_f <= 5.1

Expand Down
Binary file modified test/dummy/db/test.sqlite3
Binary file not shown.
24 changes: 24 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,27 @@
Minitest::Test.include SemanticLogger::Test::Minitest

ActionMailer::Base.delivery_method = :test

def filter_params_setting(value, user_defined_params, &block)
original_value = Rails.configuration.filter_parameters
Rails.configuration.filter_parameters = user_defined_params
block.call
ensure
Rails.configuration.filter_parameters = original_value
end

def filter_params_regex_setting(value, user_defined_params, &block)
original_value = Rails.configuration.filter_parameters

Rails.configuration.filter_parameters += user_defined_params

filter_params_regex = Rails.configuration.filter_parameters.map do |key|
"(?i:#{key})"
end.join("|")

Rails.configuration.filter_parameters = [/(?-mix:#{filter_params_regex})/]

block.call
ensure
Rails.configuration.filter_parameters = original_value
end