-
Notifications
You must be signed in to change notification settings - Fork 73
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
Create [TrustedTypes].fromLiteral method #347
Comments
In my uninformed opinion, this could easily be a footgun used by server-side templating languages to bypass the protection offered by Trusted Types. I could easily see something like this happening:
|
Trusted Types are explicitly not covering server-side injections. Without |
@koto, what's your thought on this? |
I have some thoughts.
So I'm split. I think we should, however, explore how feasible the spec+impl could be. @domenic, are you aware of any Web API that is a template tag function? Is there a reason not to have one? WebIDL wise, I think just defining as interface TrustedHTML {
TrustedHTML fromLiteral(object literal, any... args);
} would work? The algorithm would have to lookup the argument in the It's a similar issue to WICG/sanitizer-api#102, but we don't have the side-effects concerns here. |
Strategically, I think this kind of thing is better than waiting on Array.isTemplateObject. No existing web API defines a template tag function. Normally they would define it using the IDL Peeking into ES slots is discouraged in general---Web IDL is supposed to provide a uniform interface for interfacing with JS objects. But doing so makes sense in some cases, and IMO this is one of them. If it comes up more than a few times then we should define a wrapper algorithm in the IDL spec (e.g., the "underlying buffer" wrapper algorithm abstracts away the process of looking at the [[ViewedArrayBuffer]] internal slot.) One thing I didn't see mentioned is how you plan to do the interpolation? E.g. if someone does const foo = `<script>console.log(1)</script>`
const s = TrustedHTML.fromLiteral`<s>${foo}</s>`; what is the result? Since you control the tag you could make the policy be that any interpolations involve HTML escaping, i.e. the result would be a trusted HTML string |
Thanks @domenic, this seems workable :)
At least here, the intention is to throw on any interpolation attempt (i.e. we assert that the template array is of length 1). Template literals are just a way to enforce constant literals that we know are safe in a TT model (i.e. they are not user-injected). I would love to eventually allow to e.g. contextually escape HTML, but I'm not sure TT (or any web API to be honest) would be the right layer to implement it. lit-html already does it, so we know it's feasible in a lib, and it's probably better suited there. |
Interesting. I would have thought that just escaping all of |
Nope :) |
If this requires policy, then I'm not sure why we don't require policy for setHTML.
IF we can have this primitive without a policy, there are few advantages over library.
|
To clarify, I meant guards, and not TT policy. For example, a keyword in
I see. It could be an opt-out keyword though. |
Yup, opt-out sounds great! |
Assuming this will be an opt-out keyword, how should we go about designing what opt-out looks like? My suggestion would be to create a built-in policy (just like Example:
|
I'm confused - I thought the whole advantage of A single, global policy - like a default policy - looks attractive, but is problematic - the model breaks if you integrate two codebases that use it in a single document. Policy in a If rules for conversions are needed, I'd rather we keep using the regular policy API , as that one allows devs to have local rules (e.g. per module). We could, for example, change the first argument of const foo = trustedTypes.createPolicy('literalsOnly', {
createScriptURL(template, ...args) {
// in the future - Array.isTemplateObject
if (!Array.isArray(template) || !Array.isArray(template.raw) || template.length !== 1) {
throw 'bad';
}
// your rules here.
return template[0];
}
});
foo.createScriptURL`https://foo.bar`
|
I do want In that case, we can simply let |
I would rather avoid returning different return types from a function, API like that is hard to define types for, and requires either explicit type checking at the callsite, or leads to functional bugs. For example, devs may expect getting a string and call I'm afraid that, to avoid such errors, at most we should be returning I changed my mind - if That way a "best effort" return value would be: foo.innerHTML = (TrustedHTML?.fromLiteral || (s => s[0]))`<p>this will be a TrustedHTML, or a string</p>`
// You can also store the tag function separately, and e.g. add some more checks for strings.
const bestLiteral = TrustedHTML?.fromLiteral || (s => {
if (s.length > 1) {
console.log('Please don't interpolate here'); // You can also throw.
}
return s[0]
});
bestLiteral`<p>ha!</p>` |
We had a bit of discussions internally. It generally looks great! That said, an issue came up with In particular, concatenating TrustedHTMLs is not safe anymore, while it's reasonable to assume someone would (and thus, would shoot themselves in the foot). For example, we generally allow concatenating of our safe HTMLs. There is a simple and relatively unobtrusive change we could add to address that. In TrustedHTML.fromLiteral`<div foo>`.toString() // <div foo=""></div> It's easy to polyfill with a DOMParser. It also opens up a potential internal performance optimization in browsers, that could cache the document fragment it used for parsing, and reuse that when working with a sink. For b/w compatibility, this would not apply to existing policy-based TrustedHTMLs. There the user has full control over the types. We might need another function for creating a full document HTML if that's needed (if someone does |
If you go that direction, use template instead of div as the wrapper; it's canonical for these sorts of context-free scenarios. |
Mostly sounds good, however, I don't think concatenating I've verified this in Chrome Canary.
Is concatenating |
The latter. To some extent, for example you can still shoot yourself in a foot by creating a |
Got it. That sounds good to me :) |
Any plan to merge the PR and start an implementation? |
We still have some thoughts on the TrustedHTML variant, but I think the other two types are uncontroversial, the delay is mostly because I want to write the polyfill too. |
Requirement of Trusted Types policy for assigning static HTML, Script, and ScriptURL is a pain point of TT conversion.
Creating some element using DOM APIs are hard, and creating policy just for assigning a static ScriptURL increases work and decreases readability without (almost) any benefit.
In order to make migration easier for website to increase adaptation of Trusted Types, safe code should require much less work to migrate.
While Array.isTemplateObject aims to provide this primitive, it's unsure if that's landing anywhere, and that won't eliminate the requirement of TT Policy.
Instead, can we create a
fromLiteral
method under each Trusted Types, so that staticness can be verified in the Web Platform (using same primitive asArray.isTemplateObject
)?Example would be:
The text was updated successfully, but these errors were encountered: