novuhq / novu

Open-Source Notification Platform. Embeddable Notification Center, E-mail, Push and Slack Integrations.
https://novu.co
Other
34.47k stars 3.51k forks source link

πŸ› Bug Report: LogDecorator mistakenly redacted function parameters when LoggingLevel = debug #5655

Open tam-endue opened 3 months ago

tam-endue commented 3 months ago

πŸ“œ Description

When LOGGING_LEVEL set to debug, the @LogDecorator set methods parameters that are sensitive fields like email to REDACTED, which lead to issue with the api /widgets/session/initialize, which ever body param sent, it will always return "Email must be an email" error message.

πŸ‘Ÿ Reproduction steps

  1. Set LOGGING_LEVEL to debug
  2. Initialize session using /widgets/session/initialize

πŸ‘ Expected behavior

Initialize session successfully and return token

πŸ‘Ž Actual Behavior with Screenshots

InitializeSessionCommand {
  subscriberId: 'endue199381',
  applicationIdentifier: 'xxxxx',
  email: '[REDACTED]',
  firstName: '[REDACTED]',
  lastName: '[REDACTED]',
  phone: '[REDACTED]',
  hmacHash: 'xyz'
}

Novu version

0.24.2

npm version

No response

node version

No response

πŸ“ƒ Provide any additional context for the Bug.

No response

πŸ‘€ Have you spent some time to check if this bug has been raised before?

🏒 Have you read the Contributing Guidelines?

Are you willing to submit PR?

None

linear[bot] commented 3 months ago

NV-3838 πŸ› Bug Report:

tam-endue commented 3 months ago

@scopsy look like you knows the LogDecorator class well, maybe you want to take a look

rifont commented 3 months ago

Thanks for reporting this @tam-endue .

We've encountered this issue too (Linear ref: NV-3274) . We traced the root cause back to fast-redact as a 4th layer transitive dependency of nestjs-pino, which mutates the objects passed for redaction in place.

Unfortunately the fix hasn't yet arrived in our direct dependency. Our only option as of now is to specify an overrides in the monorepo root package.json to manually specify the fix version.

Would you be open to submitting a PR for this fix?