Skip to content

Commit 9bba2ef

Browse files
authored
Fix send_default_pii handling in rails controller spans (#2443)
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.
1 parent 1c431f0 commit 9bba2ef

File tree

4 files changed

+55
-3
lines changed

4 files changed

+55
-3
lines changed

CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@
33
### Features
44

55
- Add `include_sentry_event` matcher for RSpec [#2424](https://github.com/getsentry/sentry-ruby/pull/2424)
6-
- Add support for Sentry Cache instrumentation, when using Rails.cache ([#2380](https://github.com/getsentry/sentry-ruby/pull/2380)) (MemoryStore and FileStore require Rails 8.0+)
6+
- Add support for Sentry Cache instrumentation, when using Rails.cache [#2380](https://github.com/getsentry/sentry-ruby/pull/2380)
77

8+
Note: MemoryStore and FileStore require Rails 8.0+
89

910
### Bug Fixes
1011

1112
- Fix Vernier profiler not stopping when already stopped [#2429](https://github.com/getsentry/sentry-ruby/pull/2429)
13+
- Fix `send_default_pii` handling in rails controller spans [#2443](https://github.com/getsentry/sentry-ruby/pull/2443)
14+
- Fixes [#2438](https://github.com/getsentry/sentry-ruby/issues/2438)
1215

1316
## 5.21.0
1417

sentry-rails/lib/sentry/rails/controller_transaction.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ def sentry_around_action
2323
child_span.set_http_status(response.status)
2424
child_span.set_data(:format, request.format)
2525
child_span.set_data(:method, request.method)
26-
child_span.set_data(:path, request.path)
27-
child_span.set_data(:params, request.params)
26+
27+
pii = Sentry.configuration.send_default_pii
28+
child_span.set_data(:path, pii ? request.fullpath : request.filtered_path)
29+
child_span.set_data(:params, pii ? request.params : request.filtered_parameters)
2830
end
2931

3032
result

sentry-rails/spec/dummy/test_rails_app/app.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ def self.name
6060
app.config.active_job.queue_adapter = :test
6161
app.config.cache_store = :memory_store
6262
app.config.action_controller.perform_caching = true
63+
app.config.filter_parameters += [:password, :secret]
6364

6465
# Eager load namespaces can be accumulated after repeated initializations and make initialization
6566
# slower after each run

sentry-rails/spec/sentry/rails/tracing_spec.rb

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,52 @@
106106
end
107107
end
108108

109+
describe "filtering pii data" do
110+
context "with send_default_pii = false" do
111+
before do
112+
make_basic_app do |config|
113+
config.traces_sample_rate = 1.0
114+
config.send_default_pii = false
115+
end
116+
end
117+
118+
it "does not record sensitive params" do
119+
get "/posts?foo=bar&password=42&secret=baz"
120+
transaction = transport.events.last.to_hash
121+
122+
params = transaction[:spans][0][:data][:params]
123+
expect(params["foo"]).to eq("bar")
124+
expect(params["password"]).to eq("[FILTERED]")
125+
expect(params["secret"]).to eq("[FILTERED]")
126+
127+
path = transaction[:spans][0][:data][:path]
128+
expect(path).to eq("/posts?foo=bar&password=[FILTERED]&secret=[FILTERED]")
129+
end
130+
end
131+
132+
context "with send_default_pii = true" do
133+
before do
134+
make_basic_app do |config|
135+
config.traces_sample_rate = 1.0
136+
config.send_default_pii = true
137+
end
138+
end
139+
140+
it "records all params" do
141+
get "/posts?foo=bar&password=42&secret=baz"
142+
transaction = transport.events.last.to_hash
143+
144+
params = transaction[:spans][0][:data][:params]
145+
expect(params["foo"]).to eq("bar")
146+
expect(params["password"]).to eq("42")
147+
expect(params["secret"]).to eq("baz")
148+
149+
path = transaction[:spans][0][:data][:path]
150+
expect(path).to eq("/posts?foo=bar&password=42&secret=baz")
151+
end
152+
end
153+
end
154+
109155
context "with instrumenter :otel" do
110156
before do
111157
make_basic_app do |config|

0 commit comments

Comments
 (0)