underground-works / clockwork-browser

Clockwork - php dev tools in your browser - client-side metrics and toolbar
MIT License
4 stars 0 forks source link

Content Security Policy errors (injected inline style) #3

Open davidjr82 opened 1 year ago

davidjr82 commented 1 year ago

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

https://github.com/underground-works/clockwork-browser/blob/76f84c985fc69963a91926dbc5e56b88e744116b/toolbar/toolbar.js#L157

https://github.com/underground-works/clockwork-browser/blob/76f84c985fc69963a91926dbc5e56b88e744116b/toolbar/toolbar.js#L230

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

Thanks.

itsgoingd commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

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 commented 1 year ago

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 commented 1 year ago

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

itsgoingd commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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!