Mechanism to redact sensitive information in the logs

[t7tran]

Initially reported on Discord that printing access_token to the logs in a security issue. In an environment where logs are aggregated centrally, i.e. Google GCP or AWS, accidentally setting log level to info would cause a situation if a static token is used.

directus_1  | 10:49:20 ✨ request completed GET 200 /items/xyz?access_token=xxx 32ms
directus_1  | 10:49:20 ✨ request completed GET 200 /items/xyz?access_token=xxx 37ms

@rijkvanzanten suggested that logger package (pino) has a built-in redaction option: https://github.com/pinojs/pino/blob/master/docs/redaction.md. However, it's unsure if that applies to query parameters within the URL as well 🤔

Further read into pino docs and log structure point to the minimum pino redact options of {"redact": ["req.headers.authorization"]}. However, it doesn't support redacting part of the value (would be with a great cost if it did).
Interestingly, pino page mentions GDPR COMPLIANT which makes this more likely to be an issue without some measures in place.

If we can make the redact keys configurable, one could use {"redact": ["req.headers.authorization", "req.url"]} to be completely on the safe side. But the log would become less useful without request path.

Another option is to block the access_token query param combining with the aforementioned minimum redact options.

@rijkvanzanten replied: That's the main thing we'll have to look into. Redacting the authorization header is a good thing of course, but we'll have to make sure that the query param is also handled, otherwise it's a half-measure 🤔

The token may (need to) dynamically be added to the URLs (something similar to #5920). So whatever the solution is, it should support this use case.

[rijkvanzanten]

We should try adding req.query.access_token to the redact options of Pino. Pino logs as JSON by default, so the request might already be deconstructed before it's prettified by pino-colada in this instance

[t7tran]

I can't confirm if it is redacted before passing to the prettifier plugin (most likely). But there is no such key for req.query.access_token when I set LOG_STYLE to raw and made some requests with token in the query.

[t7tran]

pino-http has autoLogging.getPath option which can be used to alter the path extraction logic. I'm not sure how relevant this is to the pino library used in Directus.

Another option is to use a custom serializer for serialising the request.

[rijkvanzanten]

@t7tran I thought this was going to be an easy one to quickly throw in haha.. I was wrong. Ended up going this route:

Another option is to use a custom serializer for serialising the request.

#6347 🙂

[t7tran]

Well, throwing random ideas is always the easy task. Much appreciated the team's prompt action.

[br41nslug]

This was implemented