Skip to content

Commit

Permalink
Fix send_default_pii handling in rails controller spans
Browse files Browse the repository at this point in the history
The change in #1793 accidentally exposed params always in the controller action span.
This change now respects both `Rails.config.filter_parameters` and `Sentry.coniguration.send_default_pii`
together to decide what to send.
  • Loading branch information
sl0thentr0py committed Oct 22, 2024
1 parent 957c8d6 commit 7c1c4f8
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 2 deletions.
6 changes: 4 additions & 2 deletions sentry-rails/lib/sentry/rails/controller_transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ def sentry_around_action
child_span.set_http_status(response.status)
child_span.set_data(:format, request.format)
child_span.set_data(:method, request.method)
child_span.set_data(:path, request.path)
child_span.set_data(:params, request.params)

pii = Sentry.configuration.send_default_pii
child_span.set_data(:path, pii ? request.fullpath : request.filtered_path)
child_span.set_data(:params, pii ? request.params : request.filtered_parameters)
end

result
Expand Down
1 change: 1 addition & 0 deletions sentry-rails/spec/dummy/test_rails_app/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def self.name
app.config.active_job.queue_adapter = :test
app.config.cache_store = :memory_store
app.config.action_controller.perform_caching = true
app.config.filter_parameters += [:password, :secret]

# Eager load namespaces can be accumulated after repeated initializations and make initialization
# slower after each run
Expand Down
46 changes: 46 additions & 0 deletions sentry-rails/spec/sentry/rails/tracing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,52 @@
end
end

describe "filtering pii data" do
context "with send_default_pii = false" do
before do
make_basic_app do |config|
config.traces_sample_rate = 1.0
config.send_default_pii = false
end
end

it "does not record sensitive fields" do
get "/posts?foo=bar&password=42&secret=baz"
transaction = transport.events.last.to_hash

params = transaction[:spans][0][:data][:params]
expect(params["foo"]).to eq("bar")
expect(params["password"]).to eq("[FILTERED]")
expect(params["secret"]).to eq("[FILTERED]")

path = transaction[:spans][0][:data][:path]
expect(path).to eq("/posts?foo=bar&password=[FILTERED]&secret=[FILTERED]")
end
end

context "with send_default_pii = true" do
before do
make_basic_app do |config|
config.traces_sample_rate = 1.0
config.send_default_pii = true
end
end

it "does not record sensitive fields" do
get "/posts?foo=bar&password=42&secret=baz"
transaction = transport.events.last.to_hash

params = transaction[:spans][0][:data][:params]
expect(params["foo"]).to eq("bar")
expect(params["password"]).to eq("42")
expect(params["secret"]).to eq("baz")

path = transaction[:spans][0][:data][:path]
expect(path).to eq("/posts?foo=bar&password=42&secret=baz")
end
end
end

context "with instrumenter :otel" do
before do
make_basic_app do |config|
Expand Down

0 comments on commit 7c1c4f8

Please sign in to comment.