Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

blacklist_keys not being filtered out in version 4.2.3 #468

Closed
nbdavies opened this issue Apr 9, 2019 · 3 comments
Closed

blacklist_keys not being filtered out in version 4.2.3 #468

nbdavies opened this issue Apr 9, 2019 · 3 comments

Comments

@nbdavies
Copy link

nbdavies commented Apr 9, 2019

  • airbrake-ruby version 4.2.3
  • airbrake version 9.0.2 (also saw this issue with 9.0.0, 8.3.2)
  • Ruby version: 2.5.5
  • Framework name & version: Rails 5.1.6.2

Airbrake config

Airbrake.configure do |c|
  c.root_directory = Rails.root
  c.logger = Rails.logger
  c.environment = Rails.env
  c.performance_stats = true
  c.blacklist_keys = Rails.application.config.filter_parameters
end

Description

The blacklist_keys are no longer being filtered out of the parameters. I can test this way from the Rails console:

Airbrake.notify("Error Message", { password: "My voice is my passport. Verify me." })

On airbrake-ruby version 4.2.2, the parameters show up in the Airbrake UI as:

{
  "password": "[Filtered]"
}

On version 4.2.3 of the gem, that filtering doesn't happen:

{
  "password": "My voice is my passport. Verify me."
}

I was able to get blacklist filtering to work again by adding this to the initializer, right after the Airbrake.configure block:

Airbrake.instance_variable_get(:@notice_notifier).send(:add_default_filters)
@nbdavies
Copy link
Author

nbdavies commented Apr 9, 2019

It might be related to this change: https://github.com/airbrake/airbrake-ruby/pull/464/files?file-filters%5B%5D=.rb#diff-7826f2319fdc504a7b95dc815db44b04R465

My suspicion is: calling Airbrake.configure at the bottom of the file would initialize the notifiers, without the app's specific blacklist_keys configuration. Then when the app's airbrake initializer also calls Airbrake.configure, some of the app's airbrake settings take effect (project_id, project_key, etc.), but the notifiers wouldn't get reset because they already exist (but lack some filters). Could that be what's going on?

@scream3
Copy link

scream3 commented Apr 9, 2019

I can confirm the issue.
Now Airbrake::NoticeNotifier#add_default_filters is called before initializers/airbrake.rb is loaded.

kyrylo added a commit that referenced this issue Apr 10, 2019
Fixes #468 (blacklist_keys not being filtered out in version 4.2.3)

The problem was that at the time when we initialize NoticeNotifier (on library
load) the Config has no blacklist/whitelist keys specified. Therefore, when we
`configure` Airbrake, newly set values have no effect.

With this change we move conditional filter initialization out of
NoticeNotifier. The risk is that calling `configure` twice will append the same
conditional filters twice. Therefore, `reset` must be called prior to second
`configure` (but it's probably a good idea not to call it twice and it should be
very uncommon).
kyrylo added a commit that referenced this issue Apr 10, 2019
Fixes #468 (blacklist_keys not being filtered out in version 4.2.3)

The problem was that at the time when we initialize NoticeNotifier (on library
load) the Config has no blacklist/whitelist keys specified. Therefore, when we
`configure` Airbrake, newly set values have no effect.

With this change we move conditional filter initialization out of
NoticeNotifier. The risk is that calling `configure` twice will append the same
conditional filters twice. Therefore, `reset` must be called prior to second
`configure` (but it's probably a good idea not to call it twice and it should be
very uncommon).
@kyrylo
Copy link
Contributor

kyrylo commented Apr 10, 2019

Should be fixed in v4.2.4.

kyrylo added a commit to kyrylo/ruby-advisory-db that referenced this issue Sep 5, 2019
kyrylo added a commit to kyrylo/ruby-advisory-db that referenced this issue Sep 10, 2019
reedloden pushed a commit to rubysec/ruby-advisory-db that referenced this issue Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants