microsoft / monaco-editor

A browser based code editor
https://microsoft.github.io/monaco-editor/
MIT License
40.56k stars 3.61k forks source link

Do not use inline styles! #271

Open Peter-Juhasz opened 8 years ago

Peter-Juhasz commented 8 years ago

monaco-editor npm version: 0.7.0 Browser: Edge, Chrome OS: Windows

We use up to date CSP security rules and deny inline styles to prevent browser-based attacks: default-src: 'none'; style-src: 'self';

The editor uses inline styles instead of DOM manipulation which is not allowed in a context like this. The browsers refuse to apply these styles which makes the editor render unreadable:


Refused to apply inline style because it violates the following Content Security Policy directive: "style-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-DNZrVDWDsOLjYnOQ2E2tq7OIosyNLfBDcLuoNqGotlQ='), or a nonce ('nonce-...') is required to enable inline execution.```
lol768 commented 6 years ago

Any plans for addressing this in the future?

starpit commented 5 years ago

i'm curious, given that this issue is several years old now... does everyone employing monaco-editor resort to unsafe-inline for the style-src aspect of their CSP?

JacobEvelyn commented 5 years ago

@starpit I do unfortunately. Would love a better solution.

KamilaBorowska commented 5 years ago

From what I can tell, this is required due to use of innerHTML which poisons dynamic style attributes (for instance, see https://github.com/microsoft/vscode/blob/9089a79ecf76a25fccc7c232d80cdd374a7cc66e/src/vs/editor/browser/view/viewLayer.ts#L510).

kfox112 commented 4 years ago

Any updates or workarounds (that don't leave the app completely vulnerable) on this?

fxBrBowman commented 4 years ago

It looks like CSP nonce support has been added to the vscode-loader. Could this be something that is could be added as an option to the monaco editor?

https://github.com/microsoft/vscode/pull/58827

rhys-e commented 4 years ago

I've been trying to fix this all day and much to my dismay it's not even possible by hooking createElement("style") to enforce a nonce, since monaco uses lots of innerHTML to set things up.

I also wrote a script to log and grab hashes of style content so that I could instead go the fixed hash route and just supply all the inline style hashes (on the assumption that they're deterministic), but it looks like monaco updates style tag content (e.g. create and add style to the DOM, then update its content, meaning the hash changes) so it's extremely difficult to make that work, as well.

Honestly, if someone wanted to build a CSP resistant lib this would be a pretty good implementation

bthorben commented 3 years ago

Any updates on this? We really like monaco and use it quite extensively. Monaco is unfortunately the only frontend library that can't seem to work nicely with CSP.

bthorben commented 3 years ago

It looks like CSP nonce support has been added to the vscode-loader. Could this be something that is could be added as an option to the monaco editor?

microsoft/vscode#58827

It looks like this would only apply to scripts, which can already be solved nicely by bundling monaco together with the rest of the javascript. There doesn't seem to be a solution for styles (yet).

bthorben commented 3 years ago

Not trying to be pushy, but is there an official response as to this is even planned to happen? I would like to figure out if we have to look for alternatives.

lol768 commented 3 years ago

Given there's been no response in about 5 years, I suspect CSP compat. isn't a priority for the development team at all.

Product idea: database of all "badly-behaving" frontend libraries (like this one) that are incompatible with a proper strict Content-Security-Policy header so we can avoid them.

bthorben commented 3 years ago

@lol768 to be honest, that would be a very helpful database. But what would be even better: Suggest an alternative library!

Do you (or anybody else) know a Monaco alternative that allows using proper CSP?

crenshaw-dev commented 2 years ago

Argo CD, an extremely security-sensitive application, uses monaco-editor. Would love the ability to use monaco-editor without enabling unsafe-inline.

kkxiaoa commented 11 months ago

There are still no plans to support csp nonce?

hediet commented 11 months ago

We do not plan to address this in the next 6 months. However, feel free to explore what needs to be done on our side to resolve this issue. This could make it more likely for us to implement this feature.

lol768 commented 11 months ago

feel free to explore what needs to be done on our side to resolve this issue

It's fairly simple, stop doing shit like this:

https://github.com/microsoft/vscode/blob/3fdb0d4e83a68fa9e2c9f7c39324975ffa11e521/src/vs/editor/contrib/inlineCompletions/browser/ghostTextWidget.ts#L288-L290

Add .view-line to the DOM and then set domElement.style.top and domElement.style.height.

mika1111 commented 3 days ago

Are there any chance that CSP would be supported?

Peter-Juhasz commented 3 days ago

I can see a performance theme emerging among the tickets, which is great to see!

And with that, resolving this ticket would be a win-win, not only for security (which is critical in a development environment), but for performance as well. Although I'm not sure about the exact impact, but parsing CSS code from strings over and over again should be definitely slower than setting CSS properties explicitly as data in the DOM.

By approaching this topic from both ends, it may positively impact the priority and resource allocation of this item.