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

Add CSP header to webapp #11921

Merged
merged 1 commit into from
Apr 18, 2022
Merged

Add CSP header to webapp #11921

merged 1 commit into from
Apr 18, 2022

Conversation

timroes
Copy link
Collaborator

@timroes timroes commented Apr 12, 2022

What

This adds CSP headers for the webapp in development mode and into the built docker image.

The used CSP headers are at the moment: script-src: * 'unsafe-inline';. This will prevent unsafe-eval execution, which will prevent executing any form of eval, new Function, etc. where code is executed from a string. This will increase security slightly.

We unfortunately can't go much beyond this, since several third-party libraries we're using are injecting script directly into HTML (and thus require unsafe-inline). Given that we have no pre-processing web server, we can't leverage dynamically generated nounces, to make those inline scripts potentially safer.

Another problem is, that we're using Google Tag Manager and Segment, which might pull in scripts dynamically configured in those systems. To keep those scripts working (and allow our Go-To-Market team to further use and add more scripts), we can't limit the source to more than *, since we'd otherwise need a new release for every new script/tool embedded via Google Tag Manager, which would defy the whole purpose of using tools like Google Tag Manager. Since all those libraries can or already do pull in styles we can't narrow that down any further.

@timroes timroes requested a review from a team as a code owner April 12, 2022 11:53
@github-actions github-actions bot added area/frontend area/platform issues related to the platform labels Apr 12, 2022
@timroes timroes requested a review from davinchia April 12, 2022 11:53
Copy link
Contributor

@krishnaglick krishnaglick left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Seems like we won't be able to go further, ever, as long as we have gtm and segment.

Copy link
Contributor

@davinchia davinchia left a comment

Choose a reason for hiding this comment

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

Discussed offline: we want to do this for OSS users. This will also provide short term cover on Cloud.

Long term we want to turn this on on the CDN headers when we finally move this to a CDN.

@timroes timroes merged commit 1bc78af into master Apr 18, 2022
@timroes timroes deleted the tim/csp-header branch April 18, 2022 15:26
@timroes timroes self-assigned this Apr 20, 2022
suhomud pushed a commit that referenced this pull request May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants