Skip to content

Commit

Permalink
filter_chain: separate keys filters and execute them last
Browse files Browse the repository at this point in the history
Fixes #158 (blacklist_keys does not remove sensitive information from
post bodies)

This ensures that blacklist/whitelist filters always execute last.
Without this change blacklist/whitelist filtering is broken in
`airbrake` v5.7.0.

Since `8b8af2c77ea42b70b949e3b85df6cce3c977ee40` in `airbrake/airbrake`
Rack filters are being added after blacklist/whitelist filters, which
means Rack requests are never filtered.

In general, this change makes sure we blacklist/whitelist after all
permutations on a notice are done.
  • Loading branch information
kyrylo committed Feb 3, 2017
1 parent 05cd438 commit 43f780b
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ Airbrake Ruby Changelog

### master

* **IMPORTANT:** fixed bug when `blacklist_keys/whitelist_keys` does not filter
keys at all ([#159](https://github.com/airbrake/airbrake/pull/159))

### [v1.7.0][v1.7.0] (January 20, 2017)

* **IMPORTANT:** support for Ruby 1.9.2, 1.9.3 & JRuby (1.9-mode) is dropped
Expand Down
2 changes: 1 addition & 1 deletion lib/airbrake-ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
require 'airbrake-ruby/nested_exception'
require 'airbrake-ruby/notice'
require 'airbrake-ruby/backtrace'
require 'airbrake-ruby/filter_chain'
require 'airbrake-ruby/payload_truncator'
require 'airbrake-ruby/filters'
require 'airbrake-ruby/filters/keys_filter'
require 'airbrake-ruby/filters/keys_whitelist'
require 'airbrake-ruby/filters/keys_blacklist'
require 'airbrake-ruby/filter_chain'
require 'airbrake-ruby/notifier'

##
Expand Down
13 changes: 12 additions & 1 deletion lib/airbrake-ruby/filter_chain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,15 @@ class FilterChain
end
end

##
# Filters to be executed last. By this time all permutations on a notice
# should be done, so the final step is to blacklist/whitelist keys.
# @return [Array<Class>]
KEYS_FILTERS = [
Airbrake::Filters::KeysBlacklist,
Airbrake::Filters::KeysWhitelist
].freeze

##
# Skip over SystemExit exceptions, because they're just noise.
# @return [Proc]
Expand All @@ -38,6 +47,7 @@ class FilterChain
# @param [Airbrake::Config] config
def initialize(config)
@filters = []
@keys_filters = []

[SYSTEM_EXIT_FILTER, GEM_ROOT_FILTER].each do |filter|
add_filter(filter)
Expand All @@ -51,6 +61,7 @@ def initialize(config)
# Adds a filter to the filter chain.
# @param [#call] filter The filter object (proc, class, module, etc)
def add_filter(filter)
return @keys_filters << filter if KEYS_FILTERS.include?(filter.class)
@filters << filter
end

Expand All @@ -61,7 +72,7 @@ def add_filter(filter)
# @param [Airbrake::Notice] notice The notice to be filtered
# @return [void]
def refine(notice)
@filters.each do |filter|
(@filters + @keys_filters).each do |filter|
break if notice.ignored?
filter.call(notice)
end
Expand Down
15 changes: 15 additions & 0 deletions spec/filter_chain_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,21 @@

expect(nums).to eq([0, 1, 2])
end

it "executes keys filters last" do
notice[:params] = { bingo: 'bango' }
blacklist = Airbrake::Filters::KeysBlacklist.new(config.logger, :bingo)
@chain.add_filter(blacklist)

@chain.add_filter(
proc do |notice|
expect(notice[:params][:bingo]).to eq('bango')
end
)

@chain.refine(notice)
expect(notice[:params][:bingo]).to eq('[Filtered]')
end
end

describe "default backtrace filters" do
Expand Down

0 comments on commit 43f780b

Please sign in to comment.