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

Too easy to override params_filters defaults #264

Closed
bhelx opened this issue Dec 15, 2015 · 4 comments
Closed

Too easy to override params_filters defaults #264

bhelx opened this issue Dec 15, 2015 · 4 comments

Comments

@bhelx
Copy link

bhelx commented Dec 15, 2015

In the config object you allow setting some extra params filters via this API:

config.params_filters += [
   'extraparam1',
   'extraparam2'
]

You also have some defaults that contain security related items:

https://github.com/bugsnag/bugsnag-ruby/blob/master/lib/bugsnag/configuration.rb#L41

This is good, but it seems very easy to mistakenly overwrite them. The API gives you direct access to the params_filters Set. So a one character mistake means you will be leaking sensitive information:

config.params_filters = [
   'extraparam1',
   'extraparam2'
]

Perhaps the API here can be slightly altered to disallow overriding security related defaults. I don't have any specific suggestions right now but wanted to open a discussion.

@kattrali
Copy link
Contributor

Thanks for starting the discussion, @bhelx. There is certainly room for accidentally removing the defaults here. I'll need to give some thought to a potential fix. We could always append the default filters, for example.

@snmaynard
Copy link
Contributor

It should still definitely be possible in my opinion - as sometimes people want to send fields containing those strings through to bugsnag - so a potential solution is to make it more explicit. Or we could potentially have a config setting to enable/disable default params filters.

@kattrali
Copy link
Contributor

kattrali commented Jan 5, 2016

We could also make a (breaking) change and support separate methods on config, like config.override_params_filters(array) and config.append_params_filters(array). It would be clearer at the expense of being more verbose.

@kattrali
Copy link
Contributor

kattrali commented Feb 1, 2016

We are going to hold on this for now, though its worth noting for the next major release.. In the meantime, I'm updating the documentation to make this possibility clearer.

@kattrali kattrali closed this as completed Feb 1, 2016
kattrali added a commit that referenced this issue Feb 1, 2016
Indicate the possibility of overwriting the params_filter defaults

Related to #264
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