-
Notifications
You must be signed in to change notification settings - Fork 369
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
Feature: CSP should allow simultaneous Content-Security-Policy-Report-Only
and Content-Security-Policy-Report-Only
#351
Comments
This is an interesting idea and one I'll keep in mind. It's possible that we could add this without a breaking change, which I'll have to think about. Here's one way you could do this with Helmet today: app.use(
helmet.contentSecurityPolicy({
reportOnly: false,
directives: {
/* ... */
},
})
);
app.use(
helmet.contentSecurityPolicy({
reportOnly: true,
directives: {
/* ... */
},
})
); Would something like this work for you? |
That is interesting, was not aware that works. That is good info. From an API standpoint I think the initially proposed solution is the most elegant one. Of course carefully contemplating this before doing anything is good. |
After some consideration, I don't think I'm going to add this to Helmet for three reasons:
We'd have to document the new feature anyway. Why not just document the workaround instead? I've added a note to the documentation (see 7848f5a) to make this more obvious for people in the future. I'm going to close this issue because I think it's resolved, but let me know if you disagree. |
@EvanHahn documentation it makes perfect sense, and I do think the workaround is good as-is. Thanks for a great project. |
Hi @EvanHahn I couldn't find such interface implemented in helmetJS when I wanted using it. So I was thinking if it's wise extending Thanks. |
@iamochuko You should be able to use the app.use(
helmet.contentSecurityPolicy({
directives: {
// ...
"report-to": ["groupname"],
},
})
); Please open a new issue if you run into problems. |
@EvanHahn I will give it a go once I return to my station. Thanks |
Currently an option exists to allow only reporting of violations:
But according to https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP#testing_your_policy it should be allowed to include both a
Content-Security-Policy
andContent-Security-Policy-Report-Only
header.Usage scenario
Let's say you have a current CSP policy, but want to evaluate a new, future policy. This means you want to continue using the one you already enforce, but at the same time evaluate the new one. Read more at https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy-Report-Only
I think this in turn should remove
options.reportOnly
in a new major release. Users would instead get full functionality via:Unfortunately this means the top-level properties needs to be moved into a
policy
andreportPolicy
property. This is to allow options such asoptions.useDefaults
to be set per header.The text was updated successfully, but these errors were encountered: