Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect route-level contentSecurityPolicy: false setting #262

Merged
merged 2 commits into from
Sep 23, 2024

Conversation

AlexandrHoroshih
Copy link
Contributor

@AlexandrHoroshih AlexandrHoroshih commented Sep 23, 2024

Hello and thanks for great project!

I noticed, that currently the contentSecurityPolicy: false is allowed by types (since it is a original's helmet valid configuration), but is not actually respected by @fastify/helmet and always falls back to helmet defaults.
So now it is not possible to remove Content-Security-Policy header for one specific route, even though configuration typings are allowing for that and the rest of the headers supports that - which is pretty confusing 😢

It happens because of how CSP Nonce generation feature is currently implemented and i have added a fix for that + test, so contentSecurityPolicy: false is also respected at individual route level.

Documentation is not changed, since don't think it is needed - current docs are describing the way to pass custom helmet configration for specific routes and contentSecurityPolicy: false is a valid case of helmet configuration.
I had checked that in the checklist anyway, since there is no other option to highlight that documentation doesn't need any further changes in this PR

Checklist

AlexandrHoroshih and others added 2 commits September 23, 2024 15:58
Signed-off-by: Alexander Khoroshikh <32790736+AlexandrHoroshih@users.noreply.github.com>
@AlexandrHoroshih AlexandrHoroshih marked this pull request as ready for review September 23, 2024 09:32
Copy link
Contributor

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Uzlopak
Copy link
Contributor

Uzlopak commented Sep 23, 2024

Thank you for the PR and the explaination. :)

@Uzlopak Uzlopak merged commit 58362be into fastify:master Sep 23, 2024
11 checks passed
@AlexandrHoroshih AlexandrHoroshih deleted the patch-1 branch September 23, 2024 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants