marko and CSP

[maberer]

I am creating this issue to raise awareness for the use of CSP (content security policy) on a page that is built with marko.

There are primarily two concerns I currently see:

1. If marko widgets should be rendered server side (in response to an ajax call) the documentation states the use of eval() to bind the widgets on the client side.
   https://github.com/raptorjs/marko-widgets#rendering-widgets-on-the-server
   When CSP is used, eval() is one of the first things that are prohibited; now if
   eval() is not an option, marko fails to bind the behaviour to the DOM. If one would be able to call the bind code manually (in response to the ajax call), eval() would not be necessary. It is also discussed here: Provide public method to initialize widgets with given IDs #26
2. I am not completely sure about this one but I fear, that marko adds quite a few custom script tags (inlined) into the html on the first page load (or later). One of the strategies when using CSP is to refactor all inline script tags into script tags with a trusted resource… now, as there is usually only one client side bundle, why not calling all required code within this package… So if marko utilizes inlined script blocks (or adds them dynamically) please be aware, that it hurts CSP.

Libraries like youtube SPF seem to add inline script tags as well… I am not sure how/if they handle CSP… but I think that SPF really shines when used together with marko and I feel that both should play nice with the security settings of a page.

[patrick-steele-idem]

We can support CSP (specifically, disabling eval and inline scripts) but there will be a few tradeoffs. Originally, Marko Widgets allowed a more strict CSP but we backtracked a bit because of some performance concerns. I think we should revisit that decision and see what we can do to enable a more strict CSP for those apps that can take advantage of it.

I think Issue #26 should definitely be resolved, but here are a few other issues that would need to be resolved in order to enable a more strict CSP:

• Client-side reordering of async fragments relies on the reordering code being in an inline script to avoid having to download external JS before async fragments can be moved into the correct place on the DOM.
• When immediate is set to true (e.g., <init-widgets immediate> we generate inline script code to immediately initialize widgets for each async fragment instead of waiting until the DOM is ready (which doesn't happen until all of the async fragments are completed...which is way too late).
• We are using eval to parse serialized widget config data instead of JSON.parse. We decided not to use pure JSON because output HTML will be slightly larger because JSON uses double quotes for strings and HTML uses double quotes. This means that the double quotes need to be escaped to a longer character sequence. For example: {"hello":"world"} would need to be encoded as the following:

data-w-config="{"hello":"world"}"

Using JSON.parse also requires the JSON polyfill.

---

I feel like disallowing inline scripts will negatively impact progressive HTML rendering (with immediate initialization of widgets). Using JSON.parse instead of eval should be a minimal impact with a few slight changes, but it should probably be opt-in since it requires the JSON polyfill and will make the HTML slightly large (unless we replace double quotes with another safe character after JSON.stringify).

I'll be glad to work with you on this if you have some time. For the immediate timeframe I am focusing on making widgets stateful to bring in some of the great ideas from React.

Let us know if you have any other thoughts/questions/concerns.

Thanks for bringing up this important issue.

[maberer]

Ok, the statements make total sense - thank you.

For inline scripts, they seem to add a lot of the power that make marko awesome... therefore, avoiding inline scripts seems hard... as they are placed on the first, initial render, they might even work with a script nonce, to verify their origin... - then, policy can be made more strict... Anyway as they provide real value for marko, we might have to accept "unsafe-inline" for now.

eval() seems to be another beast.... and I was not aware of its use on widget config data, aside from the binding initialization that needs to run when templates are rendered server side... I am happy if #26 will be resolved, though.

Basically, I think it is a good thing to avoid the use of eval() when possible; maybe even completely in the future - as always, making it optional is great.... not everybody likes to use polyfills for this...

So I am leaving this up for further discussion/brainstorming - thanks for participation.