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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

PoC: Support removal of unsafe-inline from CSP #87295

Closed
wants to merge 7 commits into from

Conversation

ashharrison90
Copy link
Contributor

@ashharrison90 ashharrison90 commented May 3, 2024

What is this feature?

  • exploration around what changes are needed so people can remove unsafe-inline from their CSP
  • very roughly, the types of changes are:
    • passing the nonce to <DragDropContext>
    • ensuring the nonce is passed to emotion. there are several parts to this:
      • creating a custom emotion instance
      • adding a webpack alias to remap @emotion/css imports to this custom instance
      • exposing that custom instance to plugins
      • wrapping the app in a custom emotion cache (this handles emotion global styles)
    • ensuring style-loader picks up __webpack_nonce__ correctly in dev mode
      • in prod, this is already included in index.ts and picked up correctly
      • in dev, there is a bug that means we need to use DefinePlugin to set it correctly
    • rewriting .setAttribute('style', ...) to use .style.setProperty() or style.cssText
  • there will also be a separate change needed in the webpack config externalised by create-plugin
    • it will also need to add:
        new DefinePlugin({
          __webpack_nonce__: 'window.nonce',
        }),
    • this is so style-loader/css-loader will pick up the nonce correctly and attach it to any style tags it creates due to css imports in the plugin itself
    • this will get picked up by plugins when they update their version of create-plugin and rebuild
  • there is 1 final problem that renders most of this moot: monaco
    • monaco does lots of html/style manipulation directly
    • existing issue here
    • does not seem to be any way to pass a nonce to monaco or even a workaround
    • this means json editor/some query code editors are kind of broken 馃槶

i'm a little unsure whether we should deliver this. on the one hand, it moves us very close to the final goal. but without monaco, it's kind of pointless because we can't tell people to remove unsafe-inline since code query editors are broken without it and they are qUiTe iMpOrTaNt 馃槄

Why do we need this feature?

  • so people can remove unsafe-inline from their CSP

Who is this feature for?

  • anyone that wants to remove unsafe-inline from their CSP

Which issue(s) does this PR fix?:

For #51047

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@joshhunt
Copy link
Contributor

joshhunt commented May 8, 2024

Good stuff researching this 馃憤

The monaco situation is unfortunate. I think for the time being, we shouldn't merge this because of monaco preventing us from turning this on by default. The branch/PR can remain as a reference if we need it later.

@ashharrison90
Copy link
Contributor Author

gonna close this and leave it as reference if we come back to this 馃憤

@grafana-delivery-bot grafana-delivery-bot bot removed this from the 11.1.x milestone May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 馃殌 Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants