gutenberg icon indicating copy to clipboard operation
gutenberg copied to clipboard

Published 20 hours ago • verified publisher icon openzim

Remove inline JavaScript to comply with some CSPs

Open Jaifroid opened this issue 3 years ago • 7 comments

This is the same issue in Gutenberg that has been reported for PhET here: https://github.com/openzim/phet/issues/134 . To summarize, Content Security Policies of browser extensions forbid running inline JavaScript. Our ZIMs should avoid having JS inline, and should put required blocks in separate js files. For example, in the landing page of Gutenberg ZIMs, we have:

<body onload="init(); showBooks(); " class="pure-skin-gutenberg home">

This could easily be replaced with a much safer bit of code in the JS file (tools.js) where init() and showBooks() are defined. All that would be needed is something like:

document.querySelector('body').addEventListener('load', function() {
    init();
    showBooks();
});

For more info, see https://github.com/kiwix/kiwix-js/issues/789.

The effect of the current inline JS can be seen in our Chromium extension. Compare the left screenshot from the PWA version with the screenshot from the Chromium extension (both running in Service Worker mode, and both capable of running JS). The books are not shown in the extension because init() and showBooks() are blocked by the CSP.

image

Jaifroid avatar Jan 04 '22 14:01 Jaifroid

The base.html template fills the onload attribute this way: onload="init(); {% if show_books %} showBooks(); {% else %} populateFilters(); {% endif %} {%if bookshelf_home %} showBookshelfSearchResults(''); {%endif%} {%if bookshelf is defined%} showBookshelf({{ bookshelf }} ); {%endif%} so the show_books, bookshelf_home and bookshelf attributes would need to be exposed to the javascript somehow - perhaps using data- attributes?

eshellman avatar Jan 04 '22 15:01 eshellman

Would you be able to submit a PR ?

What about:

https://github.com/openzim/gutenberg/blob/cb6a16c1e717d381a26c8593738ada45c98ca0fa/gutenbergtozim/templates/cover_article.html#L20

Also, it looks like there's an unused <script> in bookshelf.html.

rgaudin avatar Jan 04 '22 15:01 rgaudin

The base.html template fills the onload attribute this way: onload="init(); {% if show_books %} showBooks(); {% else %} populateFilters(); {% endif %} {%if bookshelf_home %} showBookshelfSearchResults(''); {%endif%} {%if bookshelf is defined%} showBookshelf({{ bookshelf }} ); {%endif%}

@eshellman Thanks for the quick reply. Are these templates that are filled at build time? Or are they processed client-side?

Jaifroid avatar Jan 04 '22 15:01 Jaifroid

@rgaudin That looks a bit like ReactJS to me, but it might be some other templating system... We realize it may not be possible to eliminate all inline scripting for ZIMs that rely heavily on custom UIs, especially those made with React. But if we can ensure a base level of accessibility for clients with restrictive CSPs, it would be really helpful.

Jaifroid avatar Jan 04 '22 15:01 Jaifroid

I believe it's Jinja templates, rendered at build time. Fixing this onload seems doable. What about that link I mentioned above?

rgaudin avatar Jan 04 '22 15:01 rgaudin

What about that link I mentioned above?

This looks like the inline link that users can click (from any book cover) to see more books by the same author. It's also blocked by the Chrome Extension CSP, so again that could be a link added from within one of the attached JS files. Code needed would be to give that anchor an ID (let's say authorName), and then:

document.getElementById('authorName').addEventListener('click', function(e) {
    let author = e.target.innerHTML;
    if (author) goToAuthor(author);
});

Code untested of course!

Jaifroid avatar Jan 04 '22 16:01 Jaifroid

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

stale[bot] avatar Mar 30 '22 05:03 stale[bot]

With openedx an kolibri, this is the last of this kind in all our scrapers. Would be really great to have it fixes and allow KiwixJS to fully enjoy Gutenberg ZIM files.

kelson42 avatar Jan 27 '23 05:01 kelson42

This is a priority bug in case there is a doubt somewhere. Have pinned it.

kelson42 avatar Feb 26 '23 09:02 kelson42

Just to say that. having experimentally (and successfully) ported the Kiwix JS chromium extension to the next-generation manifest v3 format,, the CSP block on unsafe-inline scripts (and unsafe-eval) continues to be highly relevant. This issue isn't going away, and I suspect it will become more generalized as security-conscious CSPs become more widespread.

Jaifroid avatar Apr 16 '23 21:04 Jaifroid