diff --git a/sentry-rails/lib/sentry/rails/breadcrumb/active_support_logger.rb b/sentry-rails/lib/sentry/rails/breadcrumb/active_support_logger.rb index a301cb960..92a86e70a 100644 --- a/sentry-rails/lib/sentry/rails/breadcrumb/active_support_logger.rb +++ b/sentry-rails/lib/sentry/rails/breadcrumb/active_support_logger.rb @@ -1,16 +1,22 @@ # frozen_string_literal: true +require "sentry/utils/http_tracing" + module Sentry module Rails module Breadcrumb module ActiveSupportLogger class << self + include Sentry::Utils::HttpTracing def add(name, started, _finished, _unique_id, data) + return unless Sentry.initialized? + # skip Rails' internal events return if name.start_with?("!") if data.is_a?(Hash) data = data.slice(*@allowed_keys[name]) + filter_data_collection!(data) end crumb = Sentry::Breadcrumb.new( @@ -35,6 +41,30 @@ def inject(allowed_keys) def detach ::ActiveSupport::Notifications.unsubscribe(@subscriber) end + + private + + def filter_data_collection!(data) + filter_params!(data) + filter_path!(data) + end + + def filter_params!(data) + return unless data.key?(:params) && data[:params].is_a?(Hash) + + data[:params] = Sentry.configuration.data_collection.url_query_params.filter(data[:params]) + data.delete(:params) if data[:params].empty? + end + + def filter_path!(data) + return unless data.key?(:path) && data[:path].is_a?(String) + + path, query = data[:path].split("?", 2) + return unless query + + filtered_query = filter_query_params(query) + data[:path] = filtered_query ? "#{path}?#{filtered_query}" : path + end end end end diff --git a/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_logger_spec.rb b/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_logger_spec.rb index 2fb493793..c7ad9eddc 100644 --- a/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_logger_spec.rb +++ b/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_logger_spec.rb @@ -43,7 +43,6 @@ { "controller" => "HelloController", "action" => "exception", - "params" => { "controller" => "hello", "action" => "exception" }, "format" => "html", "method" => "GET", "path" => "/exception" } @@ -83,7 +82,6 @@ expect(breadcrumb["data"]).to include( { "action" => "exception", - "params" => { "controller" => "hello", "action" => "exception" }, "format" => "html", "method" => "GET", "path" => "/exception" } @@ -112,7 +110,6 @@ { "controller" => "HelloController", "action" => "exception", - "params" => { "controller" => "hello", "action" => "exception" }, "format" => "html", "method" => "GET", "path" => "/exception" } @@ -120,6 +117,46 @@ end end + context "with data collection" do + before do + make_basic_app do |sentry_config| + sentry_config.breadcrumbs_logger = [:active_support_logger] + sentry_config.data_collection.url_query_params.mode = :deny_list + end + end + + it "filters request params and query params in the path" do + get "/exception?foo=bar&password=42" + + breadcrumb = event.dig("breadcrumbs", "values").detect { |b| b["category"] == "process_action.action_controller" } + + expect(breadcrumb["data"]).to include( + "params" => { + "controller" => "hello", + "action" => "exception", + "foo" => "bar", + "password" => "[Filtered]" + }, + "path" => "/exception?foo=bar&password=[Filtered]" + ) + end + + context "when URL query params collection is disabled" do + before do + Sentry.configuration.data_collection.url_query_params.mode = :off + end + + it "does not include request params or query params in the path" do + get "/exception?foo=bar&password=42" + + breadcrumb = event.dig("breadcrumbs", "values").detect { |b| b["category"] == "process_action.action_controller" } + + expect(breadcrumb["data"]).not_to have_key("params") + expect(breadcrumb["data"]["path"]).to eq("/exception") + end + end + end + context "with tracing" do before do make_basic_app do |sentry_config| @@ -140,7 +177,6 @@ { "controller" => "PostsController", "action" => "show", - "params" => { "controller" => "posts", "action" => "show", "id" => p.id.to_s }, "format" => "html", "method" => "GET", "path" => "/posts/#{p.id}" }