-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Attempt to improve CSP #17854
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
base: develop
Are you sure you want to change the base?
Attempt to improve CSP #17854
Conversation
We've definitely tried this before - generally speaking, it breaks everything, since we do have inline javascript on many pages. |
(Also please make sure you're branching from and targeting the |
7322fbd
to
4263e04
Compare
@snipe rebased, please also tell me if i should create a more general issue for this :) |
Wrt inline javascript: I passed through most things that i could find, and i haven't seen any errors reported in the console after a bit of use. This is why i'd like to install reporting into this, though, since then (when we use this at work), we can collect those errors, and i can add it to this PR, or create more PRs which fix those issues i've noticed. Furthermore, when we feel comfortable, we can enable the strict CSP option, oblige with our security requirements, and then we can continue submitting fixes, and eventually (hopefully, maybe), the strict CSP option can be enabled by default. :) That's my hope with this PR |
At work, we're using https://internet.nl to verify that our domains are up-to-date with the latest security standards. One thing that sticks out about snipe-it is various 'insecure' CSP options, such as
script-src 'unsafe-inline'
, and the likes.Some of this is easily fixable, but there's other items which require further attention, and may break the application if it's not properly vetted.
As such, I've added the new tentative CSP options into a section which sets it to the
Content-Security-Policy-Report-Only
header, which only reports, but does not block, based on its findings.This would allow the new security options to be trialed, and any violations are then sent to the URL defined by env var
CSP_REPORT_TO
. If not set, it'll only report in the browser console.This PR adds two environment variables:
ENABLE_STRICT_CSP
, defaultfalse
, uses the new "strict"(er) CSP policy instead of the (old) "lax" one; this can break the application, so its experimental.CSP_REPORT_TO
, defaultnull
, defines the endpoint to report violations of the new "strict" CSP policy to, basically meant for data collection of the new policy, until its vetted enough.