express-winston icon indicating copy to clipboard operation
express-winston copied to clipboard

bodyBlacklist doesn't work for responses

Open hpurmann opened this issue 4 years ago • 4 comments

When using this library, I noticed that the bodyBlacklist feature doesn't work as I thought it should.

bodyBlacklist

I expected the following code to include the request/response bodies in the logs but strip top-level key/value pairs where the key is "secret".

expressWinston.responseWhitelist.push('body')
expressWinston.requestWhitelist.push('body')

expressWinston.bodyBlacklist.push('secret')

However, when performing a request such as

curl localhost:3000/test -H "content-type:application/json" -d '{"secret": "request", "other": 1}'

I can see that "secret" is stripped from the request logs, but not from the response logs.

responseWhitelist

I also tried to only allow certain keys by using the responseWhitelist like that:

expressWinston.responseWhitelist.push('body.foo')

but even though my route is returning a key/value pair with the key "foo", no response body gets logged.

I prepared a reproduction example for you to try it out quickly.

hpurmann avatar Nov 11 '20 11:11 hpurmann

+1 it would be very nice to have this feature.

mi-mazouz avatar May 07 '21 10:05 mi-mazouz

This can be achieved by using a responseFilter, e.g.

import * as expressWinston from 'express-winston';

function bodySanitizer(
  body: Record<string, unknown> | undefined,
  bodyBlacklist: string[] | undefined,
): Record<string, unknown> | undefined {
  if (body && bodyBlacklist) {
    for (const key of bodyBlacklist) {
      if (body[key]) {
        body[key] = 'REDACTED';
      }
    }
  }
  return body;
}

const bodyBlacklist = ['secret'];

expressWinston.logger({
  bodyBlacklist,
  responseFilter: (res: expressWinston.FilterResponse, propName: string) => {
    if (propName === 'body') {
      res['body'] = bodySanitizer({ ...res['body'] }, bodyBlacklist);
    }
    return (res as any)[propName];
  },
});

nicolaspearson avatar Jul 01 '21 05:07 nicolaspearson

+1 to this. The bodyBlacklist feature doesn't indicate that its only for requests so you would assume both req/res are covered. Having to use one list for req and a more in-depth approach for res sucks for consistency 😢

cbeardsmore avatar Nov 30 '21 18:11 cbeardsmore

+1

prakashchokalingam avatar Aug 13 '22 06:08 prakashchokalingam