Skip to content
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
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require "sentry/rails/log_subscriber"
require "sentry/rails/log_subscribers/parameter_filter"

module Sentry
module Rails
Expand All @@ -21,8 +20,6 @@ module LogSubscribers
# config.rails.structured_logging.subscribers = { action_controller: Sentry::Rails::LogSubscribers::ActionControllerSubscriber }
# end
class ActionControllerSubscriber < Sentry::Rails::LogSubscriber
include ParameterFilter

# Handle process_action.action_controller events
#
# @param event [ActiveSupport::Notifications::Event] The controller action event
Expand Down Expand Up @@ -55,9 +52,9 @@ def process_action(event)
attributes[:db_runtime_ms] = payload[:db_runtime].round(2)
end

if Sentry.configuration.send_default_pii && payload[:params]
filtered_params = filter_sensitive_params(payload[:params])
attributes[:params] = filtered_params unless filtered_params.empty?
if payload[:params]
params = Sentry.configuration.data_collection.url_query_params.filter(payload[:params])
attributes[:params] = params unless params.empty?
end
Comment thread
sl0thentr0py marked this conversation as resolved.

level = level_for_request(payload)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require "sentry/rails/log_subscriber"
require "sentry/rails/log_subscribers/parameter_filter"

module Sentry
module Rails
Expand All @@ -20,8 +19,6 @@
# config.rails.structured_logging.subscribers = { action_mailer: Sentry::Rails::LogSubscribers::ActionMailerSubscriber }
# end
class ActionMailerSubscriber < Sentry::Rails::LogSubscriber
include ParameterFilter

# Handle deliver.action_mailer events
#
# @param event [ActiveSupport::Notifications::Event] The email delivery event
Expand All @@ -41,8 +38,8 @@
attributes[:delivery_method] = payload[:delivery_method] if payload[:delivery_method]
attributes[:date] = payload[:date].to_s if payload[:date]

if Sentry.configuration.send_default_pii
attributes[:message_id] = payload[:message_id] if payload[:message_id]
if Sentry.configuration.data_collection.user_info && payload[:message_id]
attributes[:message_id] = payload[:message_id]
end

message = "Email delivered via #{mailer}"
Expand Down Expand Up @@ -73,9 +70,9 @@
duration_ms: duration
}

if Sentry.configuration.send_default_pii && payload[:params]
filtered_params = filter_sensitive_params(payload[:params])
attributes[:params] = filtered_params unless filtered_params.empty?
if payload[:params]
params = Sentry.configuration.data_collection.url_query_params.filter(payload[:params])
attributes[:params] = params unless params.empty?

Check warning on line 75 in sentry-rails/lib/sentry/rails/log_subscribers/action_mailer_subscriber.rb

View check run for this annotation

@sentry/warden / warden: security-review

[LJX-Z2J] Removing ParameterFilter leaves nested sensitive params unscrubbed in Sentry logs (additional location)

With `ParameterFilter` removed, only top-level param/argument keys are filtered; nested values like `{ user: { password: "..." } }` are sent to Sentry structured logs raw because the new filter never recurses into hash values.
end

message = "#{mailer}##{action}"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require "sentry/rails/log_subscriber"
require "sentry/rails/log_subscribers/parameter_filter"

module Sentry
module Rails
Expand All @@ -20,8 +19,6 @@ module LogSubscribers
# config.rails.structured_logging.subscribers = { active_job: Sentry::Rails::LogSubscribers::ActiveJobSubscriber }
# end
class ActiveJobSubscriber < Sentry::Rails::LogSubscriber
include ParameterFilter

# Handle perform.active_job events
#
# @param event [ActiveSupport::Notifications::Event] The job performance event
Expand All @@ -47,7 +44,7 @@ def perform(event)
attributes[:delay_ms] = ((Time.current - job.scheduled_at) * 1000).round(2)
end

if Sentry.configuration.send_default_pii && job.arguments.present?
if Sentry.configuration.data_collection.queues && job.arguments.present?
filtered_args = filter_sensitive_arguments(job.arguments)
attributes[:arguments] = filtered_args unless filtered_args.empty?
end
Expand Down Expand Up @@ -149,9 +146,11 @@ def filter_sensitive_arguments(arguments)
arguments.map do |arg|
case arg
when Hash
filter_sensitive_params(arg)
# we're using url_query_params here since rails filter_parameters end up there
# and we don't have a dedicated queue params config yet
Sentry.configuration.data_collection.url_query_params.filter(arg, cookie: false)
when String
arg.length > 100 ? "[FILTERED: #{arg.length} chars]" : arg
arg.length > 100 ? "[Filtered: #{arg.length} chars]" : arg
Comment thread
sl0thentr0py marked this conversation as resolved.
else
arg
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require "sentry/rails/log_subscriber"
require "sentry/rails/log_subscribers/parameter_filter"

module Sentry
module Rails
Expand All @@ -21,8 +20,6 @@ module LogSubscribers
# config.rails.structured_logging.subscribers = { active_record: Sentry::Rails::LogSubscribers::ActiveRecordSubscriber }
# end
class ActiveRecordSubscriber < Sentry::Rails::LogSubscriber
include ParameterFilter

EXCLUDED_NAMES = ["SCHEMA", "TRANSACTION"].freeze
EMPTY_ARRAY = [].freeze

Expand All @@ -49,7 +46,7 @@ def sql(event)

binds = event.payload[:binds]

if Sentry.configuration.send_default_pii && (binds && !binds.empty?)
if Sentry.configuration.data_collection.database_query_data && (binds && !binds.empty?)
type_casted_binds = type_casted_binds(event)

type_casted_binds.each_with_index do |value, index|
Expand Down
52 changes: 0 additions & 52 deletions sentry-rails/lib/sentry/rails/log_subscribers/parameter_filter.rb

This file was deleted.

52 changes: 42 additions & 10 deletions sentry-rails/spec/sentry/rails/log_subscriber_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
require "spec_helper"

require "sentry/rails/log_subscriber"
require "sentry/rails/log_subscribers/parameter_filter"

RSpec.describe Sentry::Rails::LogSubscriber, type: :request do
let!(:test_subscriber) { test_subscriber_class.new }
Expand Down Expand Up @@ -215,11 +214,9 @@ def temp_event(event)
end
end

context "parameter filtering integration" do
context "data collection integration" do
let(:test_subscriber_class) do
Class.new(described_class) do
include Sentry::Rails::LogSubscribers::ParameterFilter

attach_to :filtering_test

def filtering_event(event)
Expand All @@ -228,9 +225,9 @@ def filtering_event(event)
component: "filtering_test"
}

if Sentry.configuration.send_default_pii && event.payload[:params]
filtered_params = filter_sensitive_params(event.payload[:params])
attributes[:params] = filtered_params unless filtered_params.empty?
if event.payload[:params]
params = Sentry.configuration.data_collection.url_query_params.filter(event.payload[:params])
attributes[:params] = params unless params.empty?
end

log_structured_event(
Expand All @@ -245,12 +242,47 @@ def filtering_event(event)
make_basic_app do |config, app|
config.enable_logs = true
config.structured_logging.logger_class = Sentry::DebugStructuredLogger
config.send_default_pii = true
config.data_collection.url_query_params.mode = :deny_list
end
end

it_behaves_like "parameter filtering" do
let(:test_instance) { test_subscriber }
it "filters sensitive top-level parameters" do
ActiveSupport::Notifications.instrument(
"filtering_event.filtering_test",
params: {
"name" => "Ada",
"password" => "secret",
"api_token" => "token",
"nested" => { "password" => "nested secret" }
}
) do
sleep(0.01)
end

log_event = Sentry.logger.logged_events.find { |event| event["message"] == "Filtering event occurred" }
params = log_event["attributes"]["params"]

expect(params).to include(
"name" => "Ada",
"password" => "[Filtered]",
"api_token" => "[Filtered]",
"nested" => { "password" => "nested secret" }
)
end

it "does not include parameters when collection is off" do
Sentry.configuration.data_collection.url_query_params.mode = :off

ActiveSupport::Notifications.instrument(
"filtering_event.filtering_test",
params: { "name" => "Ada" }
) do
sleep(0.01)
end

log_event = Sentry.logger.logged_events.find { |event| event["message"] == "Filtering event occurred" }

expect(log_event["attributes"]).not_to have_key("params")
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

RSpec.describe Sentry::Rails::LogSubscribers::ActionControllerSubscriber, type: :request do
context "when logging is enabled" do
let(:send_default_pii) { false }

before do
make_basic_app do |config, app|
config.enable_logs = true
config.send_default_pii = send_default_pii

config.rails.structured_logging.enabled = true
config.rails.structured_logging.subscribers = { action_controller: Sentry::Rails::LogSubscribers::ActionControllerSubscriber }
Expand Down Expand Up @@ -257,13 +260,7 @@
end

context "when send_default_pii is enabled" do
before do
Sentry.configuration.send_default_pii = true
end

after do
Sentry.configuration.send_default_pii = false
end
let(:send_default_pii) { true }

it "includes filtered request parameters" do
get "/world", params: { safe_param: "value", password: "secret" }
Expand All @@ -278,7 +275,7 @@

params = JSON.parse(log_event[:attributes][:params][:value])
expect(params).to include("safe_param" => "value")
expect(params).to include("password" => "[FILTERED]")
expect(params).to include("password" => "[Filtered]")
end

it "filters sensitive parameter names" do
Expand All @@ -299,10 +296,10 @@

params = JSON.parse(log_event[:attributes][:params][:value])
expect(params).to include("normal_param" => "value")
expect(params).to include("password" => "[FILTERED]")
expect(params).to include("api_key" => "[FILTERED]")
expect(params).to include("credit_card" => "[FILTERED]")
expect(params).to include("authorization" => "[FILTERED]")
expect(params).to include("password" => "[Filtered]")
expect(params).to include("api_key" => "[Filtered]")
expect(params).to include("credit_card" => "[Filtered]")
expect(params).to include("authorization" => "[Filtered]")
end

it "handles nested parameters correctly" do
Expand Down Expand Up @@ -399,8 +396,4 @@
expect(sentry_logs.count).to eq(initial_log_count)
end
end

describe "ParameterFilter functionality" do
include_examples "parameter filtering", described_class
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@

RSpec.describe Sentry::Rails::LogSubscribers::ActionMailerSubscriber do
context "when logging is enabled" do
let(:send_default_pii) { false }

before do
make_basic_app do |config|
config.enable_logs = true
config.send_default_pii = send_default_pii

config.rails.structured_logging.enabled = true
config.rails.structured_logging.subscribers = { action_mailer: Sentry::Rails::LogSubscribers::ActionMailerSubscriber }
Expand Down Expand Up @@ -133,13 +136,7 @@
end

context "when send_default_pii is enabled" do
before do
Sentry.configuration.send_default_pii = true
end

after do
Sentry.configuration.send_default_pii = false
end
let(:send_default_pii) { true }

it "includes message_id for deliver events" do
ActiveSupport::Notifications.instrument("deliver.action_mailer",
Expand Down Expand Up @@ -182,8 +179,8 @@
params = JSON.parse(log_event[:attributes][:params][:value])

expect(params).to include("user_id" => 123, "safe_param" => "value")
expect(params["password"]).to eq("[FILTERED]")
expect(params["api_key"]).to eq("[FILTERED]")
expect(params["password"]).to eq("[Filtered]")
expect(params["api_key"]).to eq("[Filtered]")
expect(params).to include("email_address" => "user@example.com", "subject" => "Welcome!")
end
end
Expand Down Expand Up @@ -250,6 +247,4 @@
expect(sentry_logs.count).to eq(initial_log_count)
end
end

include_examples "parameter filtering", described_class
end
Loading
Loading