From fa423d842acf595f8ff995b4e40bb818d37addf9 Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Mon, 19 Sep 2022 11:47:49 +0300 Subject: [PATCH] feature: Add query_string config to filter query string --- README.md | 1 + lib/sentry/sanitizer/cleaner.rb | 22 +- lib/sentry/sanitizer/configuration.rb | 14 +- lib/sentry/sanitizer/version.rb | 2 +- spec/sentry/sanitizer/cleaner_spec.rb | 400 ++++++++++++-------- spec/sentry/sanitizer/configuration_spec.rb | 13 + 6 files changed, 283 insertions(+), 169 deletions(-) diff --git a/README.md b/README.md index 4df5e7b..61a7828 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ Currently this gem provides following features - [x] Sanitizing POST params - [x] Sanitizing HTTP headers - [x] Sanitizing cookies +- [x] Sanitizing query string - [x] Sanitizing extras ([see](https://docs.sentry.io/platforms/ruby/enriching-events/context/#additional-data) `Sentry.set_extras`) ## Installation diff --git a/lib/sentry/sanitizer/cleaner.rb b/lib/sentry/sanitizer/cleaner.rb index 43439d7..f0551e2 100644 --- a/lib/sentry/sanitizer/cleaner.rb +++ b/lib/sentry/sanitizer/cleaner.rb @@ -13,6 +13,7 @@ def initialize(config) @fields = config.fields || [] @http_headers = config.http_headers || DEFAULT_SENSITIVE_HEADERS @do_cookies = config.cookies || false + @do_query_string = config.query_string || false end def call(event) @@ -33,14 +34,17 @@ def sanitize_request(event, type) event.request.data = sanitize_data(event.request.data) event.request.headers = sanitize_headers(event.request.headers) event.request.cookies = sanitize_cookies(event.request.cookies) + event.request.query_string = sanitize_query_string(event.request.query_string) when :stringified_hash event['request']['data'] = sanitize_data(event['request']['data']) event['request']['headers'] = sanitize_headers(event['request']['headers']) event['request']['cookies'] = sanitize_cookies(event['request']['cookies']) + event['request']['query_string'] = sanitize_query_string(event['request']['query_string']) when :symbolized_hash event[:request][:data] = sanitize_data(event[:request][:data]) event[:request][:headers] = sanitize_headers(event[:request][:headers]) event[:request][:cookies] = sanitize_cookies(event[:request][:cookies]) + event[:request][:query_string] = sanitize_query_string(event[:request][:query_string]) end end @@ -53,7 +57,7 @@ def sanitize_data(hash) private - attr_reader :fields, :http_headers, :do_cookies + attr_reader :fields, :http_headers, :do_cookies, :do_query_string # Sanitize specified headers def sanitize_headers(headers) @@ -76,12 +80,26 @@ def sanitize_headers(headers) # Sanitize all cookies def sanitize_cookies(cookies) - return cookies unless cookies.is_a? Hash return cookies unless do_cookies + return cookies unless cookies.is_a? Hash cookies.transform_values { DEFAULT_MASK } end + def sanitize_query_string(query_string) + return query_string unless do_query_string + return query_string unless query_string.is_a? String + + sanitized_array = query_string.split('&').map do |kv_pair| + k, v = kv_pair.split('=') + new_v = sanitize_string(k, v) + + "#{k}=#{new_v}" + end + + sanitized_array.join('&') + end + def sanitize_value(value, key) case value when Hash diff --git a/lib/sentry/sanitizer/configuration.rb b/lib/sentry/sanitizer/configuration.rb index 0cbc976..5a34e7d 100644 --- a/lib/sentry/sanitizer/configuration.rb +++ b/lib/sentry/sanitizer/configuration.rb @@ -24,10 +24,10 @@ class Configuration module Sanitizer class Configuration - attr_accessor :fields, :http_headers, :cookies + attr_accessor :fields, :http_headers, :cookies, :query_string def configured? - [fields, http_headers, cookies].any? { |setting| !setting.nil? } + [fields, http_headers, cookies, query_string].any? { |setting| !setting.nil? } end def fields=(fields) @@ -48,11 +48,19 @@ def http_headers=(headers) def cookies=(cookies) unless [TrueClass, FalseClass].include?(cookies.class) - raise ArgumentError, 'sanitize_cookies must be boolean' + raise ArgumentError, 'cookies must be boolean' end @cookies = cookies end + + def query_string=(query_string) + unless [TrueClass, FalseClass].include?(query_string.class) + raise ArgumentError, 'query_string must be boolean' + end + + @query_string = query_string + end end end end diff --git a/lib/sentry/sanitizer/version.rb b/lib/sentry/sanitizer/version.rb index d689e28..fc0dafe 100644 --- a/lib/sentry/sanitizer/version.rb +++ b/lib/sentry/sanitizer/version.rb @@ -1,5 +1,5 @@ module Sentry module Sanitizer - VERSION = '0.5.1' + VERSION = '0.6.0' end end diff --git a/spec/sentry/sanitizer/cleaner_spec.rb b/spec/sentry/sanitizer/cleaner_spec.rb index 97f478f..3ec1085 100644 --- a/spec/sentry/sanitizer/cleaner_spec.rb +++ b/spec/sentry/sanitizer/cleaner_spec.rb @@ -3,39 +3,6 @@ RSpec.describe Sentry::Sanitizer::Cleaner do subject { described_class.new(configuration.sanitize) } - before do - Sentry.init do |config| - config.sanitize.fields = [:password, 'secret_token'] - config.sanitize.http_headers = ['H-1', 'H-2'] - config.sanitize.cookies = true - config.send_default_pii = true - end - - Sentry.get_current_scope.set_rack_env( - ::Rack::MockRequest.env_for('/', { - method: 'POST', - params: { - 'password' => 'SECRET', - 'secret_token' => 'SECRET', - 'oops' => 'OOPS', - 'hmm' => [ { 'password' => 'SECRET', 'array' => 'too' } ] - }, - 'CONTENT_TYPE' => 'application/json', - 'HTTP_H-1' => 'secret1', - 'HTTP_H-2' => 'secret2', - 'HTTP_H-3' => 'secret3', - 'HTTP_AUTHORIZATION' => 'token', - 'HTTP_X_XSRF_TOKEN' => 'xsrf=token', - ::Rack::RACK_REQUEST_COOKIE_HASH => { - 'cookie1' => 'wooo', - 'cookie2' => 'weee', - 'cookie3' => 'WoWoW' - } - })) - - Sentry.get_current_scope.apply_to_event(event) - end - let(:event) do Sentry::Event.new(configuration: configuration).tap { |e| e.extra = ({ password: 'SECRET', not_password: 'NOT SECRET' }) @@ -46,36 +13,58 @@ Sentry.configuration end - context 'with event as stringified Hash' do - it 'filters everything according to configuration' do + context 'GET request' do + before do + Sentry.init do |config| + config.sanitize.fields = [:password, 'token'] + config.sanitize.http_headers = ['Custom-Header'] + config.sanitize.cookies = false + config.sanitize.query_string = true + config.send_default_pii = true + end + + Sentry.get_current_scope.set_rack_env( + ::Rack::MockRequest.env_for('/', { + method: 'GET', + params: { + 'password' => 'SECRET', + 'token' => 'SECRET', + 'nonsecure' => 'NONESECURE', + 'nested' => [ { 'password' => 'SECRET', 'login' => 'LOGIN' } ] + }, + 'CONTENT_TYPE' => 'application/json', + 'HTTP_CUSTOM-HEADER' => 'secret1', + 'HTTP_CUSTOM-NONSECURE' => 'NONSECURE', + 'HTTP_AUTHORIZATION' => 'token', + 'HTTP_X_XSRF_TOKEN' => 'xsrf=token', + ::Rack::RACK_REQUEST_COOKIE_HASH => { + 'cookie1' => 'wooo', + 'cookie2' => 'weee', + 'cookie3' => 'WoWoW' + } + })) + + Sentry.get_current_scope.apply_to_event(event) + end + + it 'cleans all fields including query string' do event_h = JSON.parse(event.to_hash.to_json) subject.call(event_h) expect(event_h).to match a_hash_including( 'request' => a_hash_including( - 'data' => a_hash_including( - 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'secret_token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'oops' => 'OOPS', - 'hmm' => [ - a_hash_including( - 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'array' => 'too' - ) - ] - ), 'headers' => a_hash_including( - 'H-1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'H-2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'H-3' => 'secret3', + 'Custom-header' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'Custom-nonsecure' => 'NONSECURE', 'Authorization' => 'token', 'X-Xsrf-Token' => 'xsrf=token' ), 'cookies' => a_hash_including( - 'cookie1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'cookie2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'cookie3' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK - ) + 'cookie1' => 'wooo', + 'cookie2' => 'weee', + 'cookie3' => 'WoWoW' + ), + 'query_string' => 'password=[FILTERED]&token=[FILTERED]&nonsecure=NONESECURE&nested[][password]=[FILTERED]&nested[][login]=LOGIN', ), 'extra' => a_hash_including( 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, @@ -83,140 +72,225 @@ ) ) end + + context 'when query_string set to false' do + it "doesn't clean query_string" do + Sentry.get_current_client.configuration.sanitize.query_string = false + subject.call(event) + + expect(event.request.query_string) + .to eq 'password=SECRET&token=SECRET&nonsecure=NONESECURE' \ + '&nested[][password]=SECRET&nested[][login]=LOGIN' + end + end end - context 'with event as symbolized Hash' do - it 'filters everything according to configuration' do - event_h = event.to_hash - subject.call(event_h) + context 'POST request' do + before do + Sentry.init do |config| + config.sanitize.fields = [:password, 'secret_token'] + config.sanitize.http_headers = ['H-1', 'H-2'] + config.sanitize.cookies = true + config.send_default_pii = true + end - expect(event_h).to match a_hash_including( - :request => a_hash_including( - :data => a_hash_including( - 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'secret_token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + Sentry.get_current_scope.set_rack_env( + ::Rack::MockRequest.env_for('/', { + method: 'POST', + params: { + 'password' => 'SECRET', + 'secret_token' => 'SECRET', 'oops' => 'OOPS', - 'hmm' => [ - a_hash_including( - 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'array' => 'too' - ) - ] + 'hmm' => [ { 'password' => 'SECRET', 'array' => 'too' } ] + }, + 'CONTENT_TYPE' => 'application/json', + 'HTTP_H-1' => 'secret1', + 'HTTP_H-2' => 'secret2', + 'HTTP_H-3' => 'secret3', + 'HTTP_AUTHORIZATION' => 'token', + 'HTTP_X_XSRF_TOKEN' => 'xsrf=token', + ::Rack::RACK_REQUEST_COOKIE_HASH => { + 'cookie1' => 'wooo', + 'cookie2' => 'weee', + 'cookie3' => 'WoWoW' + } + })) + Sentry.get_current_scope.apply_to_event(event) + end + + context 'with event as stringified Hash' do + it 'filters everything according to configuration' do + event_h = JSON.parse(event.to_hash.to_json) + subject.call(event_h) + + expect(event_h).to match a_hash_including( + 'request' => a_hash_including( + 'data' => a_hash_including( + 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'secret_token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'oops' => 'OOPS', + 'hmm' => [ + a_hash_including( + 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'array' => 'too' + ) + ] + ), + 'headers' => a_hash_including( + 'H-1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'H-2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'H-3' => 'secret3', + 'Authorization' => 'token', + 'X-Xsrf-Token' => 'xsrf=token' + ), + 'cookies' => a_hash_including( + 'cookie1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'cookie2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'cookie3' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK + ) ), - :headers => a_hash_including( - 'H-1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'H-2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'H-3' => 'secret3', - 'Authorization' => 'token', - 'X-Xsrf-Token' => 'xsrf=token' - ), - :cookies => a_hash_including( - 'cookie1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'cookie2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'cookie3' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK + 'extra' => a_hash_including( + 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'not_password' => 'NOT SECRET' ) - ), - :extra => a_hash_including( - :password => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - :not_password => 'NOT SECRET' ) - ) + end end - end - context 'with raw event' do - it 'filters everything according to configuration' do - subject.call(event) + context 'with event as symbolized Hash' do + it 'filters everything according to configuration' do + event_h = event.to_hash + subject.call(event_h) - expect(event.request.data).to match a_hash_including( - 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'secret_token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'oops' => 'OOPS', - 'hmm' => [ - a_hash_including( - 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'array' => 'too' + expect(event_h).to match a_hash_including( + :request => a_hash_including( + :data => a_hash_including( + 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'secret_token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'oops' => 'OOPS', + 'hmm' => [ + a_hash_including( + 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'array' => 'too' + ) + ] + + ), + :headers => a_hash_including( + 'H-1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'H-2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'H-3' => 'secret3', + 'Authorization' => 'token', + 'X-Xsrf-Token' => 'xsrf=token' + ), + :cookies => a_hash_including( + 'cookie1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'cookie2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'cookie3' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK + ) + ), + :extra => a_hash_including( + :password => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + :not_password => 'NOT SECRET' ) - ] - ) - expect(event.request.headers).to match a_hash_including( - 'H-1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'H-2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'H-3' => 'secret3', - 'Authorization' => 'token', - 'X-Xsrf-Token' => 'xsrf=token' - ) - expect(event.request.cookies).to match a_hash_including( - 'cookie1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'cookie2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'cookie3' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK - ) - expect(event.extra).to match a_hash_including( - :password => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - :not_password => 'NOT SECRET' - ) + ) + end end - end - context 'cleaning all headers' do - it 'filters everything according to configuration' do - Sentry.get_current_client.configuration.sanitize.http_headers = true - subject.call(event) - - expect(event.request.headers).to match a_hash_including( - 'H-1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'H-2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'H-3' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'Authorization' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'X-Xsrf-Token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK - ) + context 'with raw event' do + it 'filters everything according to configuration' do + subject.call(event) + + expect(event.request.data).to match a_hash_including( + 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'secret_token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'oops' => 'OOPS', + 'hmm' => [ + a_hash_including( + 'password' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'array' => 'too' + ) + ] + ) + expect(event.request.headers).to match a_hash_including( + 'H-1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'H-2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'H-3' => 'secret3', + 'Authorization' => 'token', + 'X-Xsrf-Token' => 'xsrf=token' + ) + expect(event.request.cookies).to match a_hash_including( + 'cookie1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'cookie2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'cookie3' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK + ) + expect(event.extra).to match a_hash_including( + :password => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + :not_password => 'NOT SECRET' + ) + end end - end - context 'without configuration' do - before do - Sentry.get_current_client.configuration.instance_eval do - @sanitize = Sentry::Sanitizer::Configuration.new + context 'cleaning all headers' do + it 'filters everything according to configuration' do + Sentry.get_current_client.configuration.sanitize.http_headers = true + subject.call(event) + + expect(event.request.headers).to match a_hash_including( + 'H-1' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'H-2' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'H-3' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'Authorization' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'X-Xsrf-Token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK + ) end end - it 'should not filter anything' do - event_h = event.to_hash - subject.call(event_h) + context 'without configuration' do + before do + Sentry.get_current_client.configuration.instance_eval do + @sanitize = Sentry::Sanitizer::Configuration.new + end + end - expect(event_h).to match a_hash_including( - :request => a_hash_including( - :data => a_hash_including( - 'password' => 'SECRET', - 'secret_token' => 'SECRET', - 'oops' => 'OOPS', - 'hmm' => [ - a_hash_including( - 'password' => 'SECRET', - 'array' => 'too' - ) - ] + it 'should not filter anything' do + event_h = event.to_hash + subject.call(event_h) + expect(event_h).to match a_hash_including( + :request => a_hash_including( + :data => a_hash_including( + 'password' => 'SECRET', + 'secret_token' => 'SECRET', + 'oops' => 'OOPS', + 'hmm' => [ + a_hash_including( + 'password' => 'SECRET', + 'array' => 'too' + ) + ] + + ), + :headers => a_hash_including( + 'H-1' => 'secret1', + 'H-2' => 'secret2', + 'H-3' => 'secret3', + 'Authorization' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, + 'X-Xsrf-Token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK + ), + :cookies => a_hash_including( + 'cookie1' => 'wooo', + 'cookie2' => 'weee', + 'cookie3' => 'WoWoW' + ) ), - :headers => a_hash_including( - 'H-1' => 'secret1', - 'H-2' => 'secret2', - 'H-3' => 'secret3', - 'Authorization' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK, - 'X-Xsrf-Token' => Sentry::Sanitizer::Cleaner::DEFAULT_MASK - ), - :cookies => a_hash_including( - 'cookie1' => 'wooo', - 'cookie2' => 'weee', - 'cookie3' => 'WoWoW' + :extra => a_hash_including( + :password => 'SECRET', + :not_password => 'NOT SECRET' ) - ), - :extra => a_hash_including( - :password => 'SECRET', - :not_password => 'NOT SECRET' ) - ) + end end end end diff --git a/spec/sentry/sanitizer/configuration_spec.rb b/spec/sentry/sanitizer/configuration_spec.rb index 8a74b71..4fee313 100644 --- a/spec/sentry/sanitizer/configuration_spec.rb +++ b/spec/sentry/sanitizer/configuration_spec.rb @@ -52,4 +52,17 @@ expect { subject.cookies = :a }.to raise_error(ArgumentError) end end + + context 'configured query_string' do + it 'is property configured' do + subject.query_string = true + + is_expected.to be_configured + expect(subject.query_string).to eq true + end + + it 'raises error on mistake' do + expect { subject.query_string = :a }.to raise_error(ArgumentError) + end + end end