bithavoc / express-winston

express.js middleware for winstonjs
https://www.npmjs.com/package/express-winston
MIT License
795 stars 187 forks source link

bodyBlacklist doesn't work for responses #253

Open hpurmann opened 3 years ago

hpurmann commented 3 years ago

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.

mi-mazouz commented 3 years ago

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

nicolaspearson commented 3 years ago

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];
  },
});
cbeardsmore commented 2 years ago

+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 😢

prakashchokalingam commented 1 year ago

+1