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

Content Security Policy errors (injected inline style) #3

Open
davidjr82 opened this issue Jul 3, 2023 · 10 comments
Open

Content Security Policy errors (injected inline style) #3

davidjr82 opened this issue Jul 3, 2023 · 10 comments

Comments

@davidjr82
Copy link

When using a strict CSP, I have 2 warnings because these lines are injecting styles:

let toolbar = document.createElement('div')

let style = document.createElement('style')

Is it possible to modify it so can enable strict CSP?

Thanks.

@itsgoingd
Copy link
Member

Hey, I've just published clockwork-browser 1.1.2, with added support for CSP.

To use clockwork-browser with CSP, you will need to setup your CSP implementation to use nonce.

Then you can either add a csp-nonce meta tag to your page, e.g.:

<meta name="csp-nonce" content="{{ csp_nonce() }}">

Or you can import the clockwork-browser package locally and initialize the Toolbar constructor with the nonce directly:

import Toolbar from  'clockwork-browser/toolbar/toolbar'

let toolbar = new Toolbar({ cspNonce: 'foo' })

toolbar.show()

@davidjr82
Copy link
Author

Hi, thanks for looking into it, I appreciate it.

However, I still have the CSP issues here:

imagen

I guess adding the style tag with the nonce value from a js script is not working (what would make sense from a CSP perspective, to be honest).

Could it be possible to create a css file to be imported apart from the js files? So we can load it as a regular resource with the nonce directly in the style tag.

@itsgoingd
Copy link
Member

You could just copy the styles, but that unfortunately does not help, as you would run into the same issue when the library inserts the toolbar html itself into the document.

I was able to successfully test the toolbar with CSP by installing spatie/laravel-csp to an empty Laravel project and adding the following two lines to the template:

<meta name="csp-nonce" content="{{ csp_nonce() }}">
...
<script src="https://cdn.jsdelivr.net/npm/clockwork-browser@latest/dist/toolbar.js" nonce="{{ csp_nonce() }}"></script>

@davidjr82
Copy link
Author

I still have the same issue. Steps to reproduce it:

  • create a new laravel repo
  • add spatie/laravel-csp package and enable it in web group routes in App\Http\Kernel.php
  • add itsgoingd/clockwork package
  • add the meta and script to the head in welcome.blade.php as you explained in your post
  • php artisan serve
  • throws the same CSP error in console

Here is the example repo (you can check the commits doing the steps I mentioned).
https://github.com/davidjr82/clockwork-browser-csp-test

Just clone it and run php artisan serve. Then open http://127.0.0.1:8000 and you will see the welcome page (without styles because we didn't run vite yet), and the toolbar at the bottom of the page. In console you will see the CSP errors blocking inline styles (style-src).

Context: I am running windows with WSL2, and PHP 8.1.2.

@itsgoingd
Copy link
Member

I've pulled your example and it works for me with no changes, the toolbar renders just fine. Can you try clearing the cache to make sure you have the latest toolbar js file?

Screenshot 2023-07-11 at 23 20 31

@davidjr82
Copy link
Author

Yes, I have the latest version. Have you tried to open the console to see there are no console.errors regarding CSP?

@itsgoingd
Copy link
Member

There are some irrelevant CSP errors from the Laravel welcome page itself, though as you can see on the screenshot, the toolbar renders and works fine.

@davidjr82
Copy link
Author

davidjr82 commented Jul 12, 2023

Please feel free to close the issue if you consider it irrelevant. However, I would suggest to keep it open:

If the toolbar works fine without those styles, why to inject them?

If they are needed, why not to serve them in a separate css file that would not raise CSP errors?

Anyhow, as I said, please feel free to close this and tag it as won't fix if it's ok for you.

Thanks.

@itsgoingd
Copy link
Member

Hey, it would be possible to split the base styles into a separate stylesheet, but we also generate some inline styles. The nonce approach is the only one I know of, which works for our use-case

@davidjr82
Copy link
Author

Hi, if inline styles are needed because they are calculated in runtime, then it is better to not split the base styles in a separate stylesheet (because it would not solve the problem but it would increase the complexity of start working with it).

I think that with a comment in the readmes about that the project needs inline styling for CSP, and the advise to enable it only in local development, then it would be ok.

Thanks for looking into it!

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

No branches or pull requests

2 participants