Skip to content

Conversation

fallaciousreasoning
Copy link
Contributor

@fallaciousreasoning fallaciousreasoning commented Jul 1, 2025

Before submitting the PR, please make sure you do the following

Resolves #14438
Resolves #10826

This PR makes it possible to use Svelte on pages which require TrustedTypes support via their CSP by wrapping assignments to innerHTML in a TrustedTypePolicy called svelte-trusted-html if the TrustedTypes API exists.

Servers can allowlist the policy by setting require-trusted-types-for 'script'; trusted-types svelte-trusted-html in their Content-Security-Policy header.

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

Note: I haven't run the tests since I don't have pnpm setup properly.

I have tested that:

  1. A project with a CSP fails with Tip of Tree Svelte
  2. That project works when installing this revision of Svelte
  3. The project (with this revision) works in Browsers with no TrustedTypes support (i.e. Firefox, Safari)
  • Run the tests with pnpm test and lint the project with pnpm lint

My test project is here: https://github.com/fallaciousreasoning/svelte-tt-test/blob/master/src/routes/%2Bpage.server.js

The only changes to the default project is adding the CSP in src/routes/page.server.js

Copy link

changeset-bot bot commented Jul 1, 2025

🦋 Changeset detected

Latest commit: 0be07ca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@svelte-docs-bot
Copy link

Copy link
Contributor

github-actions bot commented Jul 1, 2025

Playground

pnpm add https://pkg.pr.new/svelte@16271

@7nik
Copy link
Contributor

7nik commented Jul 1, 2025

Btw, I think configuring CSP should be added to the docs. Or adding trusted-types svelte-trusted-html integrated to SvelteKit.

@Rich-Harris
Copy link
Member

Thank you, but I don't think this is the right fix — it's saying that all HTML is trusted regardless of its provenance, which is the opposite of the intent of TrustedTypes.

What if {@html ...} accepted either a string or an instance of TrustedHTML? That way people can define their own policies with their own sanitization logic. It becomes slightly more cumbersome to use component libraries that inject HTML — those libraries would need to either also accept TrustedHTML or a function that created TrustedHTML given some input — but that's a feature rather than a bug.

@Rich-Harris
Copy link
Member

(admittedly not sure how to support SVG and MathML in this case; might not be possible. Perhaps that's just the cost of CSP support though, to be borne by apps that need CSP support)

@dummdidumm
Copy link
Member

I agree that this shouldn't affect {@html ...}, but it's also needed for all of our internal template generation which is known to be safe. So if we can adjust it so that {@html ...} does not use the svelte-html trusted types then this should be a good addition.

@7nik
Copy link
Contributor

7nik commented Jul 1, 2025

@html and bind:innerHTML can use a separate policy name. Or implement #15528, so users can sanitize and trustify html in it.

Another CSP thing is that require-trusted-types-for 'script' breaks dynamic creation of scripts:

{#if condition}
  <div>
    <script>console.log("inline")</script>
    <script src="/static/my-script.js"></script>
  </div>
{/if}

@fallaciousreasoning
Copy link
Contributor Author

fallaciousreasoning commented Jul 1, 2025

Thank you, but I don't think this is the right fix — it's saying that all HTML is trusted regardless of its provenance, which is the opposite of the intent of TrustedTypes.

I think its okay to trust the output of Svelte's internal template generation - its what the other Frameworks do (i.e. Polymer/Lit). I'll try and work out how to carve out {@html ...} 😄

@html and bind:innerHTML can use a separate policy name. Or implement #15528, so users can sanitize and trustify html in it.

@7nik - I think I'll try and get it working with a separate policy for now Actually, thinking about this, its untrusted so it should go through the default policy - the ideal would be if users could do:

{@html customPolicy.createHTML('<span>Hello</span>')}

but I'm not going to dig into that here.

Another CSP thing is that require-trusted-types-for 'script' breaks dynamic creation of scripts:

I think you should be able to allow dynamic script creation via some of the other policies (i.e. unsafe-inline or whitelisting the script URL). Either way, I'd prefer it to be tackled separately.

@fallaciousreasoning
Copy link
Contributor Author

fallaciousreasoning commented Jul 1, 2025

Okay, @html doesn't go through policy now. I tested and bind:innerHTML already wasn't going through the policy so that should be fine.

FWIW, I think this a pretty useful improvement as is because it means that apps which don't use bind:innerHTML and @html will be able to use Svelte with require-trusted-types-for 'script' now.

I think the ideal solution for @html and bind:innerHTML would be to allow authors to pass in TrustedHTML rather than creating a custom policy for them (which would throw an error if we try to register it and it isn't supported).

@7nik
Copy link
Contributor

7nik commented Jul 2, 2025

What about using trustedTypes?.isHTML(html) to check if the passed value is TrustedHTML?

Offtop: it's funny that window.trustedTypes is protected from writing, but trustedTypes.isHTML = () => true and TrustedTypePolicyFactory.prototype.isHTML = () => true; are still allowed 😄

@fallaciousreasoning
Copy link
Contributor Author

What about using trustedTypes?.isHTML(html) to check if the passed value is TrustedHTML?

You mean to be able to set {@html ...} to TrustedHTML? Yeah, that's probably a good approach - tbh, I didn't really dig into why it wasn't working, and maybe we won't even need to check 😆

Just wondering if the Svelte team would consider merging without it? This PR fixes Svelte for our use case (so we don't need to create a default policy).

Offtop: it's funny that window.trustedTypes is protected from writing, but trustedTypes.isHTML = () => true and TrustedTypePolicyFactory.prototype.isHTML = () => true; are still allowed

Huh, that is kind of weird - I guess it'll still fail when you try and assign a sink to the non-trusted HTML though.

@7nik
Copy link
Contributor

7nik commented Jul 3, 2025

Look at

var html = value + '';
if (svg) html = `<svg>${html}</svg>`;
else if (mathml) html = `<math>${html}</math>`;

In the case of SVG, to create elements with the correct namespace, code is wrapped in <svg> so it loses its trustiness and the trustiness should be checked before it somehow. But SVG has <foreignObject> that allows to include arbitrary HTML.

@fallaciousreasoning
Copy link
Contributor Author

Okay, I think something like

var html = trustedTypes.isTrusted(value) ? value : (value + '') should work for the non-SVG / non-MATH elements - before going and changing that, just want to confirm with @dummdidumm that this is something Svelte would consider merging.

@7nik
Copy link
Contributor

7nik commented Jul 4, 2025

I think this should allow creating fragments with the desired namespace without losing trustiness:

export function create_fragment_from_html(html, untrusted = false, namespace = null) {
	var template = document.createElement('template');
        if (namespace) {
                var container = namespace == 'svg'
                	? document.createElementNS(NAMESPACE_SVG, 'svg')
                	: document.createElementNS(NAMESPACE_MATHML, 'mathml');
                container.innerHTML = untrusted ? html : create_trusted_html(html.replaceAll('<!>', '<!---->'));
                template.append(...container.childNodes);
        } else {
		template.innerHTML = untrusted ? html : create_trusted_html(html.replaceAll('<!>', '<!---->'));
	}
	return template.content;
}

plus a bit simplifies html() function

@corrideat
Copy link

corrideat commented Sep 22, 2025

I believe @7nik's snippet could be simplified further to something like the following (using the same logic everywhere, which avoids repeating the replace bit for <!>).

I've removed the redundant template element, since the call to .append negates the benefit of having a fragment created by just calling .content. I'm also not sure how reliant the code truly is on having a document fragment (from a quick search, it'd seem it's not), in which case the call to .append could likely be avoided too by simply returning the container.

export function create_fragment_from_html(html, untrusted = false, namespace = NAMESPACE_HTML) {
        var fragment = document.createDocumentFragment();
        var containerEl;
        switch (namespace) {
            case NAMESPACE_SVG:
              containerEl = 'svg';
              break;
            case NAMESPACE_MATHML:
              containerEl = 'mathml';
              break;
            default:
              containerEl = 'div';
              break;
        }
        var container = document.createElementNS(namespace, containerEl);
        html = html.replaceAll('<!>', '<!---->'); // XHTML compliance
        container.innerHTML = untrusted ? html : create_trusted_html(html);
        fragment.append(...container.childNodes);
        return fragment;
}

However, unless I'm missing something, the special NS handling (which would indeed simplify html()) doesn't seem to have an effect, since <svg><path/><circle/></svg> seems to parse fine as HTML, see https://codepen.io/corrideat/pen/YPwXpLe (I didn't try in XML mode, it could be that using createElementNS is indeed better in this case, as <svg>${html}</svg> might fail due to missing xmlns).

As for this PR specifically, the changes seem to work, although I'd suggest that the auto-added policy should be optional and configurable (e.g., it'd be nice to be able to provide a custom policy, for example when creating a component).

@fallaciousreasoning
Copy link
Contributor Author

pinging @dummdidumm @Rich-Harris - is this something that Svelte would consider landing? It lets us set a stricter CSP, so its definitely useful as-is.

although I'd suggest that the auto-added policy should be optional and configurable

yeah agreed, this would be ideal - I think its useful without it though, and easy enough to add later if people need it.

FWIW, this is pretty similar to how it was implemented in Polymer
Polymer/polymer@10220c9
so there is precedent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants