From aa1e03b8cded58fd6e660f169277cf66ca4b7eb8 Mon Sep 17 00:00:00 2001 From: Chris Gannon Date: Thu, 7 Nov 2024 10:11:56 +0000 Subject: [PATCH] ensure filtered params aren't revealed in sql --- .../active_record/log_subscriber.rb | 9 ++++-- test/active_record_test.rb | 28 ++++++++++++++++++ test/dummy/db/test.sqlite3 | Bin 28672 -> 28672 bytes test/test_helper.rb | 7 +++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/lib/rails_semantic_logger/active_record/log_subscriber.rb b/lib/rails_semantic_logger/active_record/log_subscriber.rb index 357303b..46632d8 100644 --- a/lib/rails_semantic_logger/active_record/log_subscriber.rb +++ b/lib/rails_semantic_logger/active_record/log_subscriber.rb @@ -54,8 +54,13 @@ 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.configuration.filter_parameters.include? key + value = "[FILTERED]" + elsif binds.key?(key) + value = (Array(binds[key]) << value) + end + binds[key] = value end diff --git a/test/active_record_test.rb b/test/active_record_test.rb index 0fc3866..7714a18 100644 --- a/test/active_record_test.rb +++ b/test/active_record_test.rb @@ -87,6 +87,34 @@ 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 "multiple bind values" do skip "Not applicable to older rails" if Rails.version.to_f <= 5.1 diff --git a/test/dummy/db/test.sqlite3 b/test/dummy/db/test.sqlite3 index a720ed84c4595109d6ce2ec5c691af3adfe28eea..cebf7e6c7c8ca20e98108c7843cdedb0f3d57bad 100644 GIT binary patch delta 48 zcmZp8z}WDBQ6@OhC$l6~AuYcsH?c&)m_dMniHX5ML4kpRfqkNkGb=lTUR}h-lsWkT DSLO`J delta 48 zcmZp8z}WDBQ6@OhC$l6~AuYcsH?c&)m_dMnk&(ecL4kpRfo-CUGb