From 66850838abf9e584ae89db4cc920869db3ff74f4 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sat, 27 Feb 2021 16:51:20 +0800 Subject: [PATCH 1/2] Treat query string as pii too Query string could contain data like password reset token or payment token, which can be used to identify users as well. So it should also be excluded when `send_default_pii` is disabled. --- sentry-ruby/lib/sentry/interfaces/request.rb | 2 +- sentry-ruby/spec/sentry/event_spec.rb | 1 - .../interfaces/request_interface_spec.rb | 46 +++++++++++++------ 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/sentry-ruby/lib/sentry/interfaces/request.rb b/sentry-ruby/lib/sentry/interfaces/request.rb index d3f94baa3..feabc1f34 100644 --- a/sentry-ruby/lib/sentry/interfaces/request.rb +++ b/sentry-ruby/lib/sentry/interfaces/request.rb @@ -40,11 +40,11 @@ def initialize(request:) if Sentry.configuration.send_default_pii self.data = read_data_from(request) self.cookies = request.cookies + self.query_string = request.query_string end self.url = request.scheme && request.url.split('?').first self.method = request.request_method - self.query_string = request.query_string self.headers = filter_and_format_headers(env) self.env = filter_and_format_env(env) diff --git a/sentry-ruby/spec/sentry/event_spec.rb b/sentry-ruby/spec/sentry/event_spec.rb index db2bd8b1a..91736d250 100644 --- a/sentry-ruby/spec/sentry/event_spec.rb +++ b/sentry-ruby/spec/sentry/event_spec.rb @@ -70,7 +70,6 @@ env: { 'SERVER_NAME' => 'localhost', 'SERVER_PORT' => '80' }, headers: { 'Host' => 'localhost', 'X-Request-Id' => 'abcd-1234-abcd-1234' }, method: 'POST', - query_string: 'biz=baz', url: 'http://localhost/lol', ) expect(event.to_hash[:tags][:request_id]).to eq("abcd-1234-abcd-1234") diff --git a/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb b/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb index cce836ebb..082920ebc 100644 --- a/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb +++ b/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb @@ -137,27 +137,33 @@ def to_s end end - context "with form data" do - it "doesn't store request body by default" do - new_env = env.merge( - "REQUEST_METHOD" => "POST", - ::Rack::RACK_INPUT => StringIO.new("data=ignore me") - ) + it "doesn't store request body by default" do + new_env = env.merge( + "REQUEST_METHOD" => "POST", + ::Rack::RACK_INPUT => StringIO.new("data=ignore me") + ) - interface = described_class.build(env: new_env) + interface = described_class.build(env: new_env) - expect(interface.data).to eq(nil) - end + expect(interface.data).to eq(nil) end - context "with request body" do - it "doesn't store request body by default" do - new_env = env.merge(::Rack::RACK_INPUT => StringIO.new("ignore me")) + it "doesn't store request body by default" do + new_env = env.merge(::Rack::RACK_INPUT => StringIO.new("ignore me")) - interface = described_class.build(env: new_env) + interface = described_class.build(env: new_env) - expect(interface.data).to eq(nil) - end + expect(interface.data).to eq(nil) + end + + it "doesn't store query_string by default" do + new_env = env.merge( + "QUERY_STRING" => "token=xxxx" + ) + + interface = described_class.build(env: new_env) + + expect(interface.query_string).to eq(nil) end context "with config.send_default_pii = true" do @@ -186,6 +192,16 @@ def to_s expect(interface.data).to eq({ "data" => "catch me" }) end + it "stores query string" do + new_env = env.merge( + "QUERY_STRING" => "token=xxxx" + ) + + interface = described_class.build(env: new_env) + + expect(interface.query_string).to eq("token=xxxx") + end + it "stores request body" do new_env = env.merge(::Rack::RACK_INPUT => StringIO.new("catch me")) From c6c29cd55e50d8a484185e092eda35e6b4ba7fa8 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sat, 27 Feb 2021 17:04:56 +0800 Subject: [PATCH 2/2] Update changelog --- sentry-ruby/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry-ruby/CHANGELOG.md b/sentry-ruby/CHANGELOG.md index 6c8a75c3d..e8e286ba3 100644 --- a/sentry-ruby/CHANGELOG.md +++ b/sentry-ruby/CHANGELOG.md @@ -3,6 +3,8 @@ ## Unreleased - Refactor interface construction [#1296](https://github.com/getsentry/sentry-ruby/pull/1296) +- Treat query string as pii too [#1302](https://github.com/getsentry/sentry-ruby/pull/1302) + - Fixes [#1301](https://github.com/getsentry/sentry-ruby/issues/1301) ## 4.2.2