[PATCH] Filter logging of parameter hash

Hi Jeremy,

I know the code may seem trivial, but please add some unit tests for the proposed changes.

In addition, you should only need value.is_a?(Hash) -- All indifferent hashes are hashes, so checking is_a? twice is pointless.

2nd version with unit tests

Add changing the logging message in actionpack

I've added a new patch that includes unit tests and fixes a couple problems with the previous patch.

When I first wrote the patch, checking just is_a?(Hash) wasn't working for HashWithIndifferentAccess. As I was writing the unit tests I found that is_a? wasn't working correctly even for regular hashes, so I changed it to use respond_to?. respond_to? makes more sense anyway, as you could now add that method to any class from which you wanted passwords removed.

While the intentions are good, this is too magical and too business logic-like to fit a core extension to Hash. This needs to be move into Action Controller and a note needs to be added when the password is removed. But even then, it's a bit iffy. Hmm.

Perhaps this would work better as an extention to Logger where you could add a rule on what should be stripped out? So you could do something like:

  Logger.filter(/"password" => (.*),/, '"password" => [REMOVED FOR SECURITY REASONS]')

I think that would be a better way to do it. But I wonder if it can be done in a performant way.

Easy and configurable param logging filter

I've uploaded a patch which adds an easy and fairly configurable paramater logging filter. It takes arguments which match on parameter key substrings, as well as a block for complete control. This route is more reliable than the Logger.filter regular expression and it gives the user more control.

Hopefully this feature is general enough that it can be included into Rails. Filtering sensitive data from logs seems like something that many Rails apps could benefit from.

Fixes problems with replacing strings using a block, doesn't pass hash values to the block.

Fix when no block is provided, add tests

This looks pretty cool.. Is there a reason we wouldn't want to implement this straight on the logger instead of only on the controller? That way SQL statements could be vetted too.

The logger gets passed a string, so you would be forced to rely on regular expressions to filter, which is less flexible and possibly error prone. Because of quote escaping inside parameter keys and values, it don't think it is possible to filter the parameter hash correctly without regular expression support for negative look behind (think quotes inside parameter keys and values). So you'd need to require Oniguruma, which I don't feel is acceptable.

I think that filtering the parameter hash before passing it to the logger is the safest and most flexible way. For the high security environments that this patch is aimed at, SQL logging should be turned off in production anyway.

Closed by [4200].