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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inline styles #64
Comments
👍 There are far too many JS libraries which do this and negatively impact overall security |
Is it possible to replace this with a Sass/scss workflow? One base theme that can be customized by variables? To make a custom theme with red tokens, for example: @import 'variables'; // contains all base variables, including `$token`
$token: red; // overrides token to our custom color
@import 'base'; // uses the variables |
Not every user uses SASS. CodeFlask must be as much technology "agnostic" as possible. So that's out of question. About the CSP concern, I honestly cannot see where could lie the issue with inline CSS, since:
And of course, the main goal by injecting CSS directly via JavaScript is to make it even easier to get CodeFlask up and running. Just import it, run on some element, and you have an editor with basic highlight support. I'm honestly interested in the threats such approach could lead to. If that's a big concern, what could be done is adding an option to prevent any CSS injection, so its up to users if they want to add all the CSS by themselves or let CodeFlask handle it. |
What I meant is that you use Sass to generate the static CSS. I'm sure @lol768 could explain more in depth about why inline CSS is unsafe (the CSP keyword is literally Having to make an exception and allow all unsafe inline styles to use CodeFlask means that I won't be using it. There's no reason to inject CSS when the |
Hey @kazzkiq, I personally don't care too much how the CSS is generated, I'm guessing @jkcclemens saw the interpolation and figured SASS was a good fit at built time. This is an implementation detail though, so do it however you like. All I ask is that you please allow site owners to include the styles themselves in an external file instead of injecting it like you are now which isn't a great practice from a security perspective.
I'm not sure I understand your comments concerning modern browsers. The inline CSS (which is effectively what your style injection is) is an issue in all browsers which support CSP.
If it's static then there should be no problem with it being hosted in a separate file and having the user include it with a Just to clarify, neither of us are suggesting there is a security issue in your library. Rather, the way in which you've written the library makes it incompatible with useful security controls (in the form of Content-Security-Policy).
I can totally see why you do it. It's marginally easier for the end user, but I mean it's only an extra As for why allowing https://github.com/ChALkeR/notes/blob/master/Improper-markup-sanitization.md is an interesting write-up. The Vanilla issue in particular is a good example of why disallowing Take a look at this one, too: http://scarybeastsecurity.blogspot.com/2009/12/generic-cross-browser-cross-domain.html I wouldn't object against an option as long as: you documented how to use it and prominently explained that if people do decide to use the style injection functionality they'll have to compromise on their CSP. I've personally wasted a lot of time dealing with libraries that it ultimately turns out aren't compatible with a secure CSP - a little documentation would've gone a long way. And finally, thanks for actually discussing and reading my own (and @jkcclemens' reasoning). A lot of library maintainers aren't willing to look into this at all. Cheers, |
@lol768 Thanks for your time for a more detailed explanation. I do agree with most of your points, and those articles clarify a lot, too. I still think the best approach here would be to give user the choice of trading ease of use for CSP comply. In this case, I believe the change could go as follows:
This way I think we can satisfy both worlds. Those who want to use CodeFlask "just for that simple weekend project" can stick with the way it works out-of-the-box now. And those who are building serious stuff and care about security can also easily adapt the library to their concerns. |
Sounds good to me 😄 |
Great. I will implement it as soon as possible. |
Thanks for not removing inline, it's good for #68. Note for everyone: you don't have to use |
Yeah, and it's undesirable. Should only be used for transitioning legacy code, will leave the page broken in CSPv1 browsers and it's painful to maintain. Better to ask the library maintainers to do things properly. Nonces wouldn't be suitable here in any case given the current implementation since the JS doesn't introduce any nonce attributes and presumably wouldn't know what the nonce was either.# Hash might work but each script update would break the styles. |
Did this already get implemented? I would love to make use of it, just because injecting CSS with JS is messy business |
workaround for kazzkiq#64
Firstly, looks like you released 1.0! Congrats! :D
Unfortunately, it seems that you have introduced inline styles with this update. As someone enforcing a strict Content Security Policy, the reason I was using CodeFlask is precisely because it didn't do this. Please consider changing this new update to remove the inline styles and be more CSP-friendly!
The text was updated successfully, but these errors were encountered: