Merge pull request #703 from bugsnag/add-redacted-keys

Add support for "redacted keys"

CHANGELOG.md

@@ -13,11 +13,14 @@ Changelog
   | [#700](https://github.com/bugsnag/bugsnag-ruby/pull/700)
 * Add `Configuration#endpoints` for reading the notify and sessions endpoints and `Configuration#endpoints=` for setting them
   | [#701](https://github.com/bugsnag/bugsnag-ruby/pull/701)
+* Add `Configuration#redacted_keys`. This is like `meta_data_filters` but matches strings with case-insensitive equality, rather than matching based on inclusion
+  | [#703](https://github.com/bugsnag/bugsnag-ruby/pull/703)
 
 ### Deprecated
 
 * In the next major release, `params` will only contain query string parameters. Currently it also contains the request body for form data requests, but this is deprecated in favour of the new `body` property
 * The `Configuration#set_endpoints` method is now deprecated in favour of `Configuration#endpoints=`
+* The `Configuration#meta_data_filters` option is now deprecated in favour of `Configuration#redacted_keys`
 
 ## v6.23.0 (21 September 2021)
 

features/fixtures/docker-compose.yml

@@ -35,6 +35,7 @@ services:
       - BUGSNAG_PROXY_PASSWORD
       - BUGSNAG_PROXY_PORT
       - BUGSNAG_PROXY_USER
+      - BUGSNAG_REDACTED_KEYS
       - BUGSNAG_RELEASE_STAGE
       - BUGSNAG_SEND_CODE
       - BUGSNAG_SEND_ENVIRONMENT

features/fixtures/plain/app/app.rb

@@ -22,6 +22,7 @@ def configure_using_environment
     conf.proxy_password = ENV["BUGSNAG_PROXY_PASSWORD"] if ENV.include? "BUGSNAG_PROXY_PASSWORD"
     conf.proxy_port = ENV["BUGSNAG_PROXY_PORT"] if ENV.include? "BUGSNAG_PROXY_PORT"
     conf.proxy_user = ENV["BUGSNAG_PROXY_USER"] if ENV.include? "BUGSNAG_PROXY_USER"
+    conf.redacted_keys << ENV["BUGSNAG_REDACTED_KEYS"] if ENV.include? "BUGSNAG_REDACTED_KEYS"
     conf.send_environment = ENV["BUGSNAG_SEND_ENVIRONMENT"] != "false"
     conf.send_code = ENV["BUGSNAG_SEND_CODE"] != "false"
     conf.timeout = ENV["BUGSNAG_TIMEOUT"] if ENV.include? "BUGSNAG_TIMEOUT"

features/plain_features/filters.feature

@@ -12,3 +12,10 @@ Scenario: Additional filters can be added to the filter list
   And I wait to receive a request
   Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier"
   And the event "metaData.filter.filter_me" equals "[FILTERED]"
+
+Scenario: Redacted keys can also be used to filter sensitive data
+  Given I set environment variable "BUGSNAG_REDACTED_KEYS" to "filter_me"
+  When I run the service "plain-ruby" with the command "bundle exec ruby filters/additional_filters.rb"
+  And I wait to receive a request
+  Then the request is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier"
+  And the event "metaData.filter.filter_me" equals "[FILTERED]"

lib/bugsnag/cleaner.rb

@@ -25,7 +25,7 @@ def clean_object(object)
     # @param url [String]
     # @return [String]
     def clean_url(url)
-      return url if @configuration.meta_data_filters.empty?
+      return url if @configuration.meta_data_filters.empty? && @configuration.redacted_keys.empty?
 
       uri = URI(url)
       return url unless uri.query
@@ -43,6 +43,33 @@ def clean_url(url)
       uri.to_s
     end
 
+    ##
+    # @param key [String, #to_s]
+    # @return [Boolean]
+    def filters_match?(key)
+      str = key.to_s
+
+      matched = @configuration.meta_data_filters.any? do |filter|
+        case filter
+        when Regexp
+          str.match(filter)
+        else
+          str.include?(filter.to_s)
+        end
+      end
+
+      return true if matched
+
+      @configuration.redacted_keys.any? do |redaction_pattern|
+        case redaction_pattern
+        when Regexp
+          str.match(redaction_pattern)
+        when String
+          str.downcase == redaction_pattern.downcase
+        end
+      end
+    end
+
     private
 
     ##
@@ -54,9 +81,11 @@ def clean_url(url)
     #
     # @return [Boolean]
     def deep_filters?
-      @configuration.meta_data_filters.any? do |filter|
+      is_deep_filter = proc do |filter|
         filter.is_a?(Regexp) && filter.to_s.include?("\\.".freeze)
       end
+
+      @configuration.meta_data_filters.any?(&is_deep_filter) || @configuration.redacted_keys.any?(&is_deep_filter)
     end
 
     def clean_string(str)
@@ -137,22 +166,6 @@ def traverse_object(obj, seen, scope)
       value
     end
 
-    ##
-    # @param key [String, #to_s]
-    # @return [Boolean]
-    def filters_match?(key)
-      str = key.to_s
-
-      @configuration.meta_data_filters.any? do |filter|
-        case filter
-        when Regexp
-          str.match(filter)
-        else
-          str.include?(filter.to_s)
-        end
-      end
-    end
-
     ##
     # If someone has a Rails filter like /^stuff\.secret/, it won't match
     # "request.params.stuff.secret", so we try it both with and without the

lib/bugsnag/configuration.rb

@@ -56,9 +56,20 @@ class Configuration
 
     # A list of keys that should be filtered out from the report and breadcrumb
     # metadata before sending them to Bugsnag
+    # @deprecated Use {#redacted_keys} instead
     # @return [Set<String, Regexp>]
     attr_accessor :meta_data_filters
 
+    # A set of keys that should be redacted from the report and breadcrumb
+    # metadata before sending them to Bugsnag
+    #
+    # When adding strings, keys that are equal to the string (ignoring case)
+    # will be redacted. When adding regular expressions, any keys which match
+    # the regular expression will be redacted
+    #
+    # @return [Set<String, Regexp>]
+    attr_accessor :redacted_keys
+
     # The logger to use for Bugsnag log messages
     # @return [Logger]
     attr_accessor :logger
@@ -201,6 +212,7 @@ def initialize
       self.send_environment = false
       self.send_code = true
       self.meta_data_filters = Set.new(DEFAULT_META_DATA_FILTERS)
+      @redacted_keys = Set.new
       self.scopes_to_filter = DEFAULT_SCOPES_TO_FILTER
       self.hostname = default_hostname
       self.runtime_versions = {}

lib/bugsnag/middleware/rack_request.rb

@@ -138,7 +138,7 @@ def parsed_request_body(request, env)
     end
 
     def add_cookies(report, request)
-      return unless record_cookies?(report.configuration)
+      return unless record_cookies?
 
       cookies = request.cookies rescue nil
 
@@ -147,17 +147,10 @@ def add_cookies(report, request)
       report.add_metadata(:request, :cookies, cookies)
     end
 
-    def record_cookies?(configuration)
+    def record_cookies?
       # only record cookies in the request if none of the filters match "Cookie"
       # the "Cookie" header will be filtered as normal
-      configuration.meta_data_filters.none? do |filter|
-        case filter
-        when Regexp
-          COOKIE_HEADER.match(filter)
-        else
-          COOKIE_HEADER.include?(filter.to_s)
-        end
-      end
+      !Bugsnag.cleaner.filters_match?(COOKIE_HEADER)
     end
   end
 end

spec/cleaner_spec.rb

@@ -276,6 +276,200 @@ def id
       expect(cleaner.clean_object(params)).to eq({ request: { params: { foo: { bar: 'baz' } } } })
     end
 
+    describe "redacted keys" do
+      it "redacts exact string match" do
+        object = { events: { metaData: { foo: 'bar', bar: 'foo' } } }
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set['foo']
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+        expect(cleaner.clean_object(object)).to eq({
+          events: {
+            metaData: {
+              foo: '[FILTERED]',
+              bar: 'foo'
+            }
+          }
+        })
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set['bar']
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+        expect(cleaner.clean_object(object)).to eq({
+          events: {
+            metaData: {
+              foo: 'bar',
+              bar: '[FILTERED]'
+            }
+          }
+        })
+      end
+
+      it "redacts case insensitive string match" do
+        object = { events: { metaData: { foo: 'bar', bar: 'foo' } } }
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set['FOO']
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+        cleaner = Bugsnag::Cleaner.new(configuration)
+        expect(cleaner.clean_object(object)).to eq({
+          events: {
+            metaData: {
+              foo: '[FILTERED]',
+              bar: 'foo'
+            }
+          }
+        })
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set['bAr']
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+        cleaner = Bugsnag::Cleaner.new(configuration)
+        expect(cleaner.clean_object(object)).to eq({
+          events: {
+            metaData: {
+              foo: 'bar',
+              bar: '[FILTERED]'
+            }
+          }
+        })
+      end
+
+      it "redacts by regular expression" do
+        object = { events: { metaData: { foo: 'bar', bar: 'foo' } } }
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/fb?/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+        expect(cleaner.clean_object(object)).to eq({
+          events: {
+            metaData: {
+              foo: '[FILTERED]',
+              bar: 'foo'
+            }
+          }
+        })
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/fb+/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+        expect(cleaner.clean_object(object)).to eq({
+          events: {
+            metaData: {
+              foo: 'bar',
+              bar: 'foo'
+            }
+          }
+        })
+      end
+
+      it "redacts deeply nested keys" do
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/^foo\.bar/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        params = { events: { metaData: { foo: { bar: 'baz', baz: 'bar' } } } }
+        expect(cleaner.clean_object(params)).to eq({
+          events: {
+            metaData: {
+              foo: {
+                bar: '[FILTERED]',
+                baz: 'bar'
+              }
+            }
+          }
+        })
+      end
+
+      it "redacts deeply nested request parameters" do
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/^foo\.bar/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        params = { events: { metaData: { request: { params: { foo: { bar: 'baz', baz: 'bar' } } } } } }
+        expect(cleaner.clean_object(params)).to eq({
+          events: {
+            metaData: {
+              request: {
+                params: {
+                  foo: {
+                    bar: '[FILTERED]',
+                    baz: 'bar'
+                  }
+                }
+              }
+            }
+          }
+        })
+      end
+
+      it "doesn't filter by string match when the scope is not in 'scopes_to_filter'" do
+        object = { foo: 'bar', bar: 'foo' }
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set['foo']
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(object)).to eq({ foo: 'bar', bar: 'foo' })
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set['bar']
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(object)).to eq({ foo: 'bar', bar: 'foo' })
+      end
+
+      it "doesn't filter by regular expression when the scope is not in 'scopes_to_filter'" do
+        object = { foo: 'bar', bar: 'foo' }
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/fb?/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(object)).to eq({ foo: 'bar', bar: 'foo' })
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/fb+/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(object)).to eq({ foo: 'bar', bar: 'foo' })
+      end
+
+      it "doesn't filter deeply nested keys when the scope is not in 'scopes_to_filter'" do
+        params = { foo: { bar: 'baz', baz: 'bar' } }
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/^foo\.bar/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(params)).to eq({ foo: { bar: 'baz', baz: 'bar' } })
+      end
+
+      it "doesn't filter deeply nested request parameters when the scope is not in 'scopes_to_filter'" do
+        params = { request: { params: { foo: { bar: 'baz', baz: 'bar' } } } }
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/^foo\.bar/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(params)).to eq({ request: { params: { foo: { bar: 'baz', baz: 'bar' } } } })
+      end
+    end
+
     it "filters objects which can't be stringified" do
       class StringRaiser
         def to_s