💡 RFC: Deprecate and remove support for html inside of expressions

[natemoo-re]

Background & Motivation

Background & Motivation

One big TODO on #1222 is:

To finalize: build output security, XSS, etc. (owner: not yet claimed)

Currently, Astro does not escape any content inside of expressions, meaning the following example will print out the HTML inside of content.

---
const content = "<h1>Hello world!</h1>";
---
<div>{content}</div>

This can lead to accidental XSS vulnerabilities! It is unexpected, since it goes against the behavior of most other frameworks (React, Preact, Vue, Svelte, Solid).

It has also lead users down bad paths—I have seen community projects where HTML is dynamically concatenated into a giant string then stuffed into the Astro markup. This makes it impossible for Astro to optimize this markup because it never runs through our compiler.

Proposed Solution

Possible solutions

Deprecate this "feature" and remove it. Begin escaping all HTML content inside of expressions.

Alternatives considered

⛔️ Sanitize HTML content rather than escape it

Rejected because while possible, "sanitization" is a loaded concept that each user could have wildly different expectations for. Inevitably, users will likely want some control over how sanitization happens.

This is also not an approach that any other framework has used, likely for good reason.

Risks, downsides, and/or tradeoffs

• The biggest risk is to not do this. At a certain point, we need to rip the band-aid off.
• This change will likely break a lot of existing code. This is mostly a risk if we don't introduce an alternative like 💡 RFC: Ability to set an element's innerHTML and textContent #1827.

Open Questions

• As outlined in 💡 RFC: Ability to set an element's innerHTML and textContent #1827, we need an alternative happy path before we can remove support for this.
• How do we sanitize some expressions but not others? For (contrived) example, an expression might contain {variable + <Component />}, where variable needs escaping but <Component /> explicitly can't be escaped. Our compiler might need to get a little smarter!

Detailed Design

TBD. This will be a big compiler change.

Help make it happen!

• I am willing to submit a PR to implement this change.
• I am willing to submit a PR to implement this change, but would need some guidance.
• I am not willing to submit a PR to implement this change.

[jonathantneal]

Thanks for taking on XSS. It’s definitely not great that Astro can’t escape any content inside expressions.

Okay, sooooo ... on the one hand, I applaud and am in favor of adding XSS protection. Buuuuut ... on the other hand, this specific change would mean losing the most intuitive way to add a rich blog post from a CMS to Astro. What to do. What to do.

It’s difficult for me to separate this proposal from #1827. I don’t know if you agree, but I think it’s not reasonable to accept this RFC — Deprecate and remove support for html inside of expressions — without also accepting a 'replacement' like set:innerHTML in #1827. My issue with both proposals combined is that they might not do enough. If I use set:innerHTML to import a blog post, isn’t that just as dangerous? Only less intuitive, too, maybe?

Would you consider, as an alternative, providing escaping and sanitization APIs over removing {html}?

WordPress gives developers escaping tools they need to do the right thing. They also have sanitization.

But I don’t want to ignore your other comments.

... "sanitization" is a loaded concept.
... This is also not an approach that any other framework has used, likely for good reason.

I agree! That is, I agree if we’re talking about client frameworks. IMHO, early 'Jamstack' kinda sold the world on client JS rendering a whole website, and so forcing the browser to, say, process 144kB of minified script was probably a kind of 'buzzkill' that inspired dozens of subpar alternatives. Would you agree, or could you tell me more about your experiences?

EDIT: Tho 18kB is a lot to pop onto a client-side library, shout out to dompurify.

I think backend tech has taken sanitization more seriously. Beyond the WordPress functions mentioned earlier, many backend languages have long provided sanitization tools and escaping tools.

Google and Mozilla have also written a Sanitizer API for the frontend, but it’s brand, brand new.

Again, I am totally in favor of XSS protection, and I agree with the sentiment about sanitization.

So, how would you feel about built-in escaping functions, or supporting the Sanitizer API in the same way we support fetch?

[FredKSchott]

Hey everyone! Our current RFC process is beginning to break down at this size, with over 50 open RFCs currently in the "discussing" stage. A growing community is a great problem to have, but our ability to give you RFC feedback has suffered as a result. In an effort to improve our RFC process, we are making some changes to better organize things.

From now on, all RFCs will live in a standalone repo: https://github.com/withastro/rfcs

This allows us to do three things: 1) Use threaded discussions for high-level ideas and improvements, without necessarily requiring an implementation for every idea. 2) Improve the quality of our RFC template and the speed/quality of all feedback. 3) Support inline comments and explicit approvals on RFCs, via a new Pull Request review process.

We hope that this new process leads to better RFC weekly calls and faster feedback on your RFCs from maintainers. More detail can be found in the new RFC repo README.

---

We can't automatically convert this issue to an RFC in the new repo because new RFC template is more detailed that this one. But, you can still continue this discussion in the new repo by creating a new Discussion in the RFC repo and copy-and-pasting this post (and any relevant follow-up comments) into it. Discussions are available for high-level ideas and suggestions without the requirement of a full implementation proposal.

Then, when you are ready to propose (or re-propose) an implementation for feedback and approval, you can create a new RFC using the new RFC template. More detail about how to do this can be found in the new RFC repo README.

Thanks for your patience as we attempt to improve things for both authors and reviewers. If you have any questions, don't hesitate to reach out on Discord. https://astro.build/chat