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

Adds security note to param logging #293

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

danielpowell4
Copy link
Contributor

@danielpowell4 danielpowell4 commented Jun 3, 2019

What does this PR do?

Updates the readme to include a note about filtering passwords and other sensitive information.

Commit message:

Adds security note to param logging

Good for this to be top of mind so no one traverses as such:

config.lograge.custom_payload do |controller|
  exceptions = %w(controller action format authenticity_token)
  {
    params: controller.request.params.except(*exceptions), # request.params is DANGEROUS
  }
end

request.filtered_parameters is at least a bit safer as noted in #28

Good for this to be top of mind so no one traverses as such:

```
config.lograge.custom_payload do |controller|
  exceptions = %w(controller action format authenticity_token)
  {
    params: controller.request.params.except(*exceptions), # request.params is DANGEROUS
  }
end
```

request.filtered_parameters is at least a bit safer as noted
in roidrage#28
@joshkinabrew
Copy link

For future travelers... here's an example of how this would work (running Rails 6.1):

  config.lograge.custom_payload do |controller|
    param_filter = ActiveSupport::ParameterFilter.new(Rails.application.config.filter_parameters)
    { params: param_filter.filter(controller.request.params) }
  end

This way you can still keep the settings in Rails.application.config.filter_paramters in config/initializers/filter_parameter_logging.rb

mentioning #28 so it shows in the comments at the bottom (where I came from)

@iloveitaly
Copy link
Collaborator

@joshkinabrew could you create a separate PR to include that example in the readme? I think that would be a helpful example to have!

@iloveitaly iloveitaly merged commit 8ced4f6 into roidrage:master Jan 20, 2022
@danielpowell4 danielpowell4 deleted the patch-1 branch April 4, 2022 15:54
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

Successfully merging this pull request may close these issues.

3 participants