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,15 @@ 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 list of keys that should be redacted from the report and breadcrumb
+    # metadata before sending them to Bugsnag
+    # @return [Set<String, Regexp>]
+    attr_accessor :redacted_keys
+
     # The logger to use for Bugsnag log messages
     # @return [Logger]
     attr_accessor :logger
@@ -201,6 +207,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,134 @@ 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' } } }
+
+        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]' } } })
+
+        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' } } })
+      end
+
+      it "redacts case insensitive string match" do
+        object = { events: { metaData: { foo: 'bar' } } }
+
+        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]' } } })
+
+        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' } } })
+      end
+
+      it "redacts by regular expression" do
+        object = { events: { metaData: { foo: 'bar' } } }
+
+        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]' } } })
+
+        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' } } })
+      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' } } } }
+        expect(cleaner.clean_object(params)).to eq({ events: { metaData: { foo: { bar: '[FILTERED]' } } } })
+      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' } } } } } }
+        expect(cleaner.clean_object(params)).to eq({ events: { metaData: { request: { params: { foo: { bar: '[FILTERED]' } } } } } })
+      end
+
+      it "doesn't filter by string inclusion when the scope is not in 'scopes_to_filter'" do
+        object = { foo: 'bar' }
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set['f']
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(object)).to eq({ foo: 'bar' })
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set['b']
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(object)).to eq({ foo: 'bar' })
+      end
+
+      it "doesn't filter by regular expression when the scope is not in 'scopes_to_filter'" do
+        object = { foo: 'bar' }
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/fb?/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(object)).to eq({ foo: 'bar' })
+
+        configuration = Bugsnag::Configuration.new
+        configuration.redacted_keys = Set[/fb+/]
+
+        cleaner = Bugsnag::Cleaner.new(configuration)
+
+        expect(cleaner.clean_object(object)).to eq({ foo: 'bar' })
+      end
+
+      it "doesn't filter deeply nested keys when the scope is not in 'scopes_to_filter'" do
+        params = { foo: { bar: 'baz' } }
+
+        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' } })
+      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' } } } }
+
+        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' } } } })
+      end
+    end
+
     it "filters objects which can't be stringified" do
       class StringRaiser
         def to_s

spec/integrations/rack_spec.rb

@@ -152,6 +152,63 @@ class Request
       expect(report.metadata[:session]).to eq({ session: true })
     end
 
+    it "correctly redacts from url and referer any value indicated by redacted_keys" do
+      rack_env = {
+        env: true,
+        HTTP_REFERER: "https://bugsnag.com/about?email=hello@world.com&another_param=thing",
+        "rack.session" => { session: true }
+      }
+
+      rack_request = double
+      allow(rack_request).to receive_messages(
+        params: { param: 'test', param2: 'test2' },
+        ip: "rack_ip",
+        request_method: "TEST",
+        path: "/TEST_PATH",
+        scheme: "http",
+        host: "test_host",
+        port: 80,
+        referer: "https://bugsnag.com/about?email=hello@world.com&another_param=thing",
+        fullpath: "/TEST_PATH?email=hello@world.com&another_param=thing",
+        form_data?: true,
+        POST: { param: 'test' },
+        cookies: { session_id: 12345 }
+      )
+
+      expect(::Rack::Request).to receive(:new).with(rack_env).and_return(rack_request)
+
+      Bugsnag.configure do |config|
+        config.send_environment = true
+        config.meta_data_filters = []
+        config.redacted_keys = Set["email", "cookie"]
+        config.request_data[:rack_env] = rack_env
+      end
+
+      report = Bugsnag::Report.new(RuntimeError.new('abc'), Bugsnag.configuration)
+
+      callback = double
+      expect(callback).to receive(:call).with(report)
+
+      middleware = Bugsnag::Middleware::RackRequest.new(callback)
+      middleware.call(report)
+
+      expect(report.request).to eq({
+        url: "http://test_host/TEST_PATH?email=[FILTERED]&another_param=thing",
+        httpMethod: "TEST",
+        params: { param: 'test', param2: 'test2' },
+        referer: "https://bugsnag.com/about?email=[FILTERED]&another_param=thing",
+        clientIp: "rack_ip",
+        headers: {
+          "Referer" => "https://bugsnag.com/about?email=[FILTERED]&another_param=thing"
+        },
+        body: { param: 'test' }
+      })
+
+      expect(report.metadata[:request]).to be(report.request)
+      expect(report.metadata[:environment]).to eq(rack_env)
+      expect(report.metadata[:session]).to eq({ session: true })
+    end
+
     it "correctly extracts data from rack middleware" do
       rack_env = {
         env: true,