Replies: 2 comments 4 replies
-
|
An example would be the following, #3131. I need to clean it up to reintroduce inline styles for people who don't want the progressive enhancement, but this would be a CSP safe implementation of the |
Beta Was this translation helpful? Give feedback.
-
|
Just to clarify, the issue here is that your CSP prevents you from using a library that uses inline styles in any capacity. Is that correct? Granted this is not my specialty, but AFAIK nothing we are doing is actually opening you up to XSS attacks. |
Beta Was this translation helpful? Give feedback.
-
|
Hi @chaance, thanks for getting back to me! You are correct about our CSP preventing us from using libraries that employ inline styles. When a site has a Content Security Policy, it applies to all aspects of the site, including the component library, injected scripts (such as Google Tag Manager), and any other dependencies or integrations on the page. I completely agree that Radix does not introduce XSS vectors. However, due to the "all for one, one for all" nature of CSP, sites often need to relax their policies to utilize features that could potentially be exploited for XSS elsewhere. Teams then face a decision of compromising security to incorporate tools like Radix into their projects. The specific CSP policy that Radix violates is style-src unsafe-inline. This seems to stem from aiming to enhance developer experience: "As a developer using Radix, I simply import the primitives, and much of the core styling is applied inline." While this consolidation is convenient, it breaches CSP guidelines. There have been some attempts to improve Radix's CSP compliance by allowing nonces in certain components, but this requires all components to render server-side. My proposal aims to balance these concerns. Radix can maintain its styling capabilities without necessitating a relaxation of CSP policies for consumers. The caveat is that consumers would need to ensure newly exposed CSS files are included in their build process to be integrated into the HTML. This brings me to the proof of concept for the I hope this clarifies the issue and outlines a potential way forward that satisfies both strict CSP adherents and those operating with more relaxed or no CSP policies. Excited to talk more on how I can help improve Radix in this regard! |
Beta Was this translation helpful? Give feedback.
-
|
Hi @chaance, I know you mentioned you have a large backlog you're working through, but I wanted to circle back here and see if this is something your team would accept as a change? I saw you liked the message, so wanted to talk next steps. I have no problem taking on the work to implement this and work progressively through each of Radix's components. Not sure what your backlog looks like but I think this would be an epic of its own that I could help implement. One callout would be that this would require a mentality shift for contributors to not use inline styles in their own contributions. Radix should totally support consumer's passing in their own inline styles as props, but the package should not add its own inline styles. That PR I had has examples on how to dynamically change styles via the CSS Object Model. All of this depends on packages exporting css files for consuming apps to include. We could try using javascript to manipulate the CSS Object Model in all cases, but I think people will quickly find pitfalls in this approach since you don't have access to pseudo-classes or other browser specific fields. Cheers! Let me know what you think, and I'm happy to help take some burden off of your shoulders! |
Beta Was this translation helpful? Give feedback.
-
|
I'm not quite sold on the If this is time-sensitive on your end I'd suggest forking or using patch-package to make whatever changes you need. I understand this is not something most folks want to hear, but it's open source for a reason! We're playing catch-up on competing issues/features but this is near the top of my todo list. |
Beta Was this translation helpful? Give feedback.
-
|
No rush on our end, we currently use inline-styles, but it's something I'm passionate about and want to help UI libraries I use improve! -- One alternative is to implement something like emotion's cache where SSR apps can pass in a nonce that can be pulled out of a context for all components to use for their inline styles. In this case, there's no change on how a component is consumed, a consuming application would need to put this provider at the root so all component would have access to this context. This would only solve the issue for SSR applications though, CSR applications cannot use nonces since there's no server to set the nonce-- unless you're doing something crazy with your hosting provider, but that's beyond me. I think this solution hurts companies that may reach for a microfrontend in their overall application. I liked the exported CSS file method because it solved the case for both SSR and CSR apps. The outcome would look really similar to the current inline styles in the Select component, but the nonce would be pulled out of context and not as a component's prop. This solved your concerns around the need for a new prop, a breaking change, and additional work for consumers. I do think this places a burden on the package maintainers. You're essentially taking that CSS file example I gave and turning it into a string to be used inline. You don't get a lot of intellisense when using Curious to see what you think about something like this. |
Beta Was this translation helpful? Give feedback.
-
Hi all,
I believe Radix could benefit from becoming more security-conscious about its styling and avoiding the use of inline styles entirely. While there have been efforts to support nonces in style tags, if your application is rendered on the client side, or a certain component needs to be, you're out of luck unless you're willing to weaken your CSP. For those who don't know, inline styles can be a potential vector for cross-site scripting (XSS) attacks. Currently, the majority of components within Radix's primitives use inline styles. I'm not saying Radix is creating vulnerabilities, but it shouldn't be the reason people feel they need to lower their security standards, potentially introducing vulnerabilities elsewhere.
I’m proposing a switch to using CSS classes and exporting these classes as part of each package. Consumers who want to remain CSP-conscious would need to import the CSS file for each component that requires one. Most modern build systems, like Webpack and Vite, do a great job of combining CSS files, so I'm not concerned about ending up with 50 different stylesheets.
The majority of these inline styles don’t even depend on the component’s state and could be directly converted into traditional CSS classes. For more complex styles, two or more classes may need to be created, and the component’s state can be used to determine which class to apply. One example of this would be the pointer-event in NavigationMenu. The guideline would be to create two classes, or use a CSS variable to control these values.
In cases where a component’s state directly controls the value of a CSS property, the guideline would be to use CSS variables. I’ve been experimenting with setProperty, but I think manipulating an element’s CSS Object Model (CSSOM) could work as well.
Some open questions I have relate to creating a single source of truth between the existing inline styles and the proposed CSS classes. I’m trying to avoid a major version bump to implement a change like this, opting instead for a progressive enhancement. This has led me to consider how to consolidate the CSS classes with the inline styles so they can coexist until the next major version, where users would need to migrate anyway. If anyone has thoughts on this area, feel free to chime in!
The goal of this post is to gauge interest and see if this is something worth pursuing. I know one of Radix's pillars is ease of use, and this would increase the complexity of using a component, but I think complexity with reason is better than just doing what's easy. Thanks for taking a read, open to suggestions and ideas!
Beta Was this translation helpful? Give feedback.
All reactions