iarna / iarna-toml

Better TOML parsing and stringifying all in that familiar JSON interface.
ISC License
316 stars 43 forks source link

Package doesn't work with Content Security Policy unsafe-eval #45

Open taras opened 2 years ago

taras commented 2 years ago

NetlifyCMS uses this package as a dependency. This package includes an eval function that is not permitted with CSP 'unsafe-eval' which prevents NetlifyCMS from working in secure environments.

https://github.com/netlify/netlify-cms/issues/4367#issuecomment-699884163

Please, let me know if you're open to removing the eval - I can submit a PR.

glauberramos commented 2 years ago

Hi, any update on this? Can we merge the PR?

taras commented 2 years ago

I believe so. I don't remember. It's been so long :)

iarna commented 2 years ago

(I've been away from Github for a while for medical reasons, I'm not really back per se, but today's a good day.)

So the actual eval is safe, but you have rules that improperly flag it as unsafe? Am I understanding that correctly? I can't say I'm keen on "delete it and move on" as a solution...

Edit: Ah, I see, so this is in fact for bundled versions running inside browsers... Let's see if we can find a better solution than just deleting a feature that's quite useful server-side.

WebReflection commented 1 year ago

@iarna

So the actual eval is safe

accordingly to CSP eval is never safe and this is blocking our project to adopt this otherwise wonderful module ... no eval, Function, or any variant of these is possible under CSP rules that don't allow eval, and coming from a CDN this is also a foreign module so even more relevant as considered "not safe".

CSP doesn't care if you just eval("1 + 2"), it cares that no code evaluation ever happens at runtime.

Please consider using typeof require and then require?.(...) within that try/catch as eval is not strictly needed in the first place, "env detection" would do a way better job without bothering the safety of the module, thank you!

WebReflection commented 1 year ago

@iarna I can file a clean PR if needed, just let me know, thanks!

edit although I really don't understand this comment:

/* eval require not available in transpiled bundle */

I am not sure what you mean but transpilers make that eval happens: see https://cdn.jsdelivr.net/npm/@iarna/toml/parse-string/+esm

WebReflection commented 1 year ago

@iarna I don't see much activity in this repo ... are you OK for a fork? Happy to push back in here changes I might make to this, but I ma focusing mostly on web compat so any require will be removed from the codebase if any eval is needed, yet the rest would remain, as require('util').inspect imho shouldn't be ever needed in the first place, when we have better tools and ways these days to debug issues ... would that work?

silverwind commented 3 months ago

+1 on removal of inspect dependency and especially eval which also causes warnings during my build process in vite:

node_modules/@iarna/toml/lib/toml-parser.js (178:22): Use of eval in "node_modules/@iarna/toml/lib/toml-parser.js" is strongly discouraged as it poses security risks and may cause issues with minification.

Edit: I found https://github.com/squirrelchat/smol-toml which looks to be a good alternative to this module.