Option to disable 'request-id' header

[philippviereck]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Add a option to disable requestIdHeader overwriting genReqId

Motivation

As far as I understand, fastify will by default use the value of the requestIdHeader,if such a header is present in the request, over the genReqId value.

Now, one does not necessarily know that a server is a fastify server, and one can change (obfuscate) the header key, but essentially a malicious actor could pass the same request-id for all requests and therefore reduce the quality of the logs. If one is trying to investigate an incident multiple requests now share the same id.

This is of low impact I'd assume, still I think it would be nice to disable this behaviour in a proper way instead of relying on obfuscation.

Example

const fastify = require('fastify')({
  requestIdHeader: false // (disabled instead of default value)
})

[Eomm]

It is already doable (not straight forward tbh)

const fastify = require('fastify')({
  logger: true,
  requestIdHeader: 'jdnsndfjegrnjsdnij',
  genReqId() {
    return undefined
  },
})
fastify.get('/', async (request, reply) => {
  return { hello: 'world' }
})

fastify.inject('/')

What I understood is that you don't want to log the reqId field

[philippviereck]

@Eomm Thank you for the quick reply.
What I meant was to disable requestIdHeader overwriting genReqId(), so untrusted requests can not overwrite my reqId.

This is the internal code:
const id = req.headers[requestIdHeader] || genReqId(req)

The only way is to (like you did) change the requestIdHeader to a bogus value. Which is honestly secure enough. Just not very clean in my opinion.

[Eomm]

Oh, I got it! The issue here is to choose the priority of the genId option over the header 👍🏼

Yeah, I agree!

Would you like to submit a pr?

[mcollina]

I think a better implementation is to move that || switch inside genReqId itself: https://github.com/fastify/fastify/blob/main/lib/reqIdGenFactory.js.

Would you like to send a PR?

[philippviereck]

@mcollina Sorry, I don't think I understand how/what you mean.

Simply req.headers[requestIdHeader] || genReqId(req) inside genReqId(factory) would obviously get overwritten by any custom genRegId(setup), therefore removing the requestIdHeader functionality. So, I don't think you mean it this way.

[mcollina]

Just move the check inside genReqId(req).

[philippviereck]

Okey, I have a PR, it's my first one ever. It does not use the reqIdGenFactory because I could not get it to work using the factory just yet. Sorry. Have a look at it maybe, it's to hackish...

Maybe in 2 weeks I have time to dig deeper...