w3c / trusted-types

A browser API to prevent DOM-Based Cross Site Scripting in modern web applications.
https://w3c.github.io/trusted-types/dist/spec/
Other
586 stars 68 forks source link

Update IDL for script enforcement #484

Closed lukewarlow closed 2 months ago

lukewarlow commented 3 months ago

~HTMLScriptElement/textContent uses a union type rather than [StringContext] because it uses a nullable type.~

All of the properties are using union types as that seems to be the direction the spec is going to go in. But I can revert that commit and go back to using the extended attribute.

This PR is part of 1 of addressing #437 it specifically doesn't address https://github.com/w3c/trusted-types/issues/483 or https://github.com/w3c/trusted-types/issues/252


Preview | Diff

lukewarlow commented 3 months ago

cc @mbrodesser-Igalia might be of interest to you as you start implementing more in Gecko

lukewarlow commented 2 months ago

@annevk hopefully this is now in a better shape. Shadows the properties onto HTMLScriptElement rather than messing with Node/textContent and Element/innerText. Uses the IDL attribute where possible so algorithms are mostly just setting the internal slot and calling the parent steps

HTMLScriptElement/textContent uses a union type instead of the IDL extended attribute to avoid issues with nullable IDL type handling.

lukewarlow commented 2 months ago

Updated this to link to the correct steps in the DOM and html specs so it should read better.

lukewarlow commented 2 months ago

@koto and @annevk does this seem ready to merge into the spec now?

lukewarlow commented 2 months ago

@annevk

Currently: We validate during prepare the text so we do need to store the text. The "invalidation" is currently only something we propose for the parsing behaviour.

It's possible that if all the ways to change the children of a script element end up firing children changed steps, and we get the neccessary changes there (still need to open a DOM PR) that we can do away with the script text entirely and just use invalidation and a trusted boolean "slot".

I'll raise a separate issue to discuss that as that's tangental to this PR itself.

lukewarlow commented 2 months ago

Raised #507 to discuss the "slot" vs invalidation model. Aside from that do the changes to the IDL here make sense?

annevk commented 2 months ago

I think so.

lukewarlow commented 2 months ago

I'm going to go ahead and get this merged, we'll get another chance when I upstream it to HTML to double check it all makes sense within the wider context of the full script enforcement.