Skip to content
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

DOM lib: Add support for Trusted Types API #30024

Open
5 tasks done
koto opened this issue Feb 21, 2019 · 12 comments
Open
5 tasks done

DOM lib: Add support for Trusted Types API #30024

koto opened this issue Feb 21, 2019 · 12 comments
Labels
Revisit An issue worth coming back to Suggestion An idea for TypeScript

Comments

@koto
Copy link

koto commented Feb 21, 2019

Search Terms

Trusted Types, DOM

Suggestion

Trusted Types (spec, introductory article) is a new experimental DOM API implemented within the WICG , with a working Chrome implementation.

The API creates a few new objects available on the global object in the browser, like most other web APIs (impl in TS and in Closure compiler).

Under certain conditions, controlled by a HTTP header (analogous to Content-Security-Policy behavior), the API can enable the enforcement - then it changes the signature of several DOM API functions and property setters, such that they accept specific object types, and reject strings. Colloquially, DOM API becomes strongly typed.

For example, with Trusted Types Element.innerHTML property setter accepts a TrustedHTML object.

Trusted Type objects stringify to their inner value. This API shape is a deliberate choice that enables existing web applications and libraries to gradually migrate from strings to Trusted Types without breaking functionality. In our example, it makes it possible to write the following:

const policy = TrustedTypes.createPolicy('foo', { 
  createHTML: (s) => { /* some validation*/; return s} 
});

const trustedHTML = policy.createHTML('bar');
anElement.innerHTML = trustedHTML

anElement.innerHTML === 'bar'

The above code works regardless if the Trusted Types enforcement is enabled or not.

Reading from the DOM is unaffected, so Element.innerHTML getter returns a string. That's for practical reasons -- web applications read from DOM more often than they write to it, and only writing exposes the application to DOM XSS risks. Typing only the setters allows us to secure web applications with minimal code changes.

It's difficult to map that API using TS types (due to #2521). The only way is to change the DOM sink functions to have the any type, which is obviously suboptimal.

Use Cases

Writing a TS application that uses the Trusted Types API.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Revisit An issue worth coming back to labels Feb 27, 2019
@engelsdamien
Copy link
Contributor

For an additional data point, the way the closure compiler supports those sink assignments seems to be through the @implicitCast annotation.

@koto
Copy link
Author

koto commented Mar 25, 2020

Trusted Types are now available in Chrome canary.

@wibimaster
Copy link

Need it, it's on Chrome official release :x

@shicks
Copy link
Contributor

shicks commented Jan 29, 2021

FWIW, it looks like there's finally some traction on #2521 so this might become feasible at some point.

@shhnjk
Copy link

shhnjk commented Apr 3, 2021

@RyanCavanaugh, any update on this?

@koto
Copy link
Author

koto commented Apr 8, 2021

The setters/getters typing needed to unblock this will, to my understanding, ship in 4.3. Should we create a PR to change the lib.dom.d.ts type annotation for the sinks to align with https://w3c.github.io/webappsec-trusted-types/dist/spec/ ? Essentially, in a couple of places in DOM the setter type would be TrustedHTML | string (or similar unions for other types), instead of string.

@aryzing
Copy link

aryzing commented Nov 28, 2021

Seems to we well supported now

Would be great to have TS support for this.

@shicks
Copy link
Contributor

shicks commented Nov 29, 2021

The necessary TS language support landed in 4.3: https://devblogs.microsoft.com/typescript/announcing-typescript-4-3/#separate-write-types

I had assumed somebody was updating the standard library declarations to accommodate this, but maybe that never happened? In any case, as far as I'm aware that's all that's left to do, and it should be pretty straightforward.

@shhnjk
Copy link

shhnjk commented Jan 11, 2022

Should we create a PR to change the lib.dom.d.ts type annotation for the sinks to align with https://w3c.github.io/webappsec-trusted-types/dist/spec/ ? Essentially, in a couple of places in DOM the setter type would be TrustedHTML | string (or similar unions for other types), instead of string.

Yes, if you have bandwidth @koto! :)

@tosmolka
Copy link
Member

This is starting to block our adoption of Trusted Types so we took a look as well. Is microsoft/TypeScript-DOM-lib-generator#1246 roughly what you guys had in mind as next step? Can you pls take a look? Thanks.

@shicks
Copy link
Contributor

shicks commented Jan 14, 2022

microsoft/TypeScript-DOM-lib-generator#1246 is backward-incompatible and will break a bunch of existing code. The fix is to only widen the type on writing, while leaving the read type the same:

interface Element {
  get outerHTML(): string;
  set outerHTML(html: string|TrustedHTML);
}

@tosmolka
Copy link
Member

@shicks , the PR with the fix has been updated, can you please check the fix again for backward-compatibility? Thanks.

aarongable pushed a commit to chromium/chromium that referenced this issue Apr 12, 2022
Specifically, this adds @types/trusted-types as dependency.

NOTE: This type definition might be included in future versions of TS:
microsoft/TypeScript#30024

Also updates npm_include and npm_exclude to have the .d.ts in those
packages to be actually included in the packed node_modules.tar.gz.

This increases the uncompressed node_modules size by about 1.2k.

Trusted Types is used by //ui/webui/resources/js/static_types.js which
in turn is used by cr.ui.Tree and Files app.

Bug: b:228404803
Change-Id: Icb50826e520665f74eb21c88e976b3df5f0d00d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3577873
Auto-Submit: Luciano Pacheco <[email protected]>
Reviewed-by: Demetrios Papadopoulos <[email protected]>
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Cr-Commit-Position: refs/heads/main@{#991648}
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 10, 2022
In this part updating all TypeScript test files to use emptyHTML
when clearing the DOM between tests. Unfortunately have to use an `as
unknown as string` type cast to work around TypeScript's bug at [1].

This is in preparation of enabling TrustedTypes for Polymer-based
WebUIs.

[1] microsoft/TypeScript#30024

Bug: 1098690
Change-Id: I38beb8746086818bd1969ece74c4690fadd7d753
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3936598
Reviewed-by: John Lee <[email protected]>
Auto-Submit: Demetrios Papadopoulos <[email protected]>
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1057027}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Specifically, this adds @types/trusted-types as dependency.

NOTE: This type definition might be included in future versions of TS:
microsoft/TypeScript#30024

Also updates npm_include and npm_exclude to have the .d.ts in those
packages to be actually included in the packed node_modules.tar.gz.

This increases the uncompressed node_modules size by about 1.2k.

Trusted Types is used by //ui/webui/resources/js/static_types.js which
in turn is used by cr.ui.Tree and Files app.

Bug: b:228404803
Change-Id: Icb50826e520665f74eb21c88e976b3df5f0d00d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3577873
Auto-Submit: Luciano Pacheco <[email protected]>
Reviewed-by: Demetrios Papadopoulos <[email protected]>
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Cr-Commit-Position: refs/heads/main@{#991648}
NOKEYCHECK=True
GitOrigin-RevId: 5ffe647ebf66a8d49a2a03c490a281665060df4d
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 21, 2022
Adding a local modification in third_party/node/node_modules/typescript
so that Element#innerHTML accepts TrustedHTML as a valid input to work
around [1]. This allows removing 'as unknown as string' type cast every
time 'foo.innerHTML = ...' is used.

As part of this change, need to add 'trusted-types' to the default
tsconfig_base.json |types|, and consequently also add it in any
tsconfig file that is overriding the default |types|.

[1] microsoft/TypeScript#30024

Bug: 1098690
Change-Id: I05f0d737495aac12260779c24ea0af1221a383e2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3970137
Reviewed-by: Rebekah Potter <[email protected]>
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1062256}
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 22, 2022
This type cast is no longer necessary after adding a workaround for [1]
in third_party/node/node_modules/typescript at crrev.com/c/3970137.

[1] microsoft/TypeScript#30024

Bug: 1098690
Change-Id: I7cf2ba009057d3b2f1d06393e0aeeb9c26a3f6a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3970098
Reviewed-by: John Lee <[email protected]>
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1062431}
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 22, 2022
This type cast is no longer necessary after adding a workaround for [1]
in third_party/node/node_modules/typescript at crrev.com/c/3970137.

[1] microsoft/TypeScript#30024

Bug: 1098690
Change-Id: I990822c645cba1416ba9096a65eaa527350bb30e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3972236
Reviewed-by: Rebekah Potter <[email protected]>
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1062442}
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 26, 2022
Adding a local modification in third_party/node/node_modules/typescript
so that HTMLScriptElement#src accepts TrustedScriptURL as a valid input
to work around [1]. This allows removing 'as unknown as string' type cast
every time the following pattern is used.

fooScript.src = getTrustedScriptURL`./lazy_load.js`

[1] microsoft/TypeScript#30024

Bug: 1098690
Change-Id: I9e679ebd812f3b497b91cb701628fcbf885b1662
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3978936
Reviewed-by: Rebekah Potter <[email protected]>
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063619}
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 26, 2022
This is done directly in ts_library.py to avoid exposing a workaround
for [1] in all cases where the default 'types' property is overridden.

[1] microsoft/TypeScript#30024

Bug: 1098690
Change-Id: I1150923d4433ffd834a6548147a38c70e95673c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3972221
Commit-Queue: Demetrios Papadopoulos <[email protected]>
Reviewed-by: Rebekah Potter <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1063894}
aarongable pushed a commit to chromium/chromium that referenced this issue Feb 28, 2023
This CL does a couple of things:

1. Moves the worker script into its own ts_library because it needs
   a different tsconfig.json than the rest of the code. Ideally we would
   be able to use the same target for both, but declarations in the
   lib.webworker.d.ts conflict with lib.dom.d.ts. See
   microsoft/TypeScript#20595

2. The tsconfig.json needs 'webworker' for web worker APIs.

3. In the worker script, add an empty export so we can re-declare
   `self` and re-declare `self` as SharedWorkerGlobalScope so we can
   use shared worker APIs. See
   microsoft/TypeScript#14877

4. Fixes shared worker creation to work with trusted types. We loose
   some type safety when we create the worker because lib.dom.d.ts
   doesn't support Trusted Types yet. See
   microsoft/TypeScript#30024

5. Random fixes for the linter and typescript compilation.

Change-Id: I1d21e67f58b5f3d6b879c21af9e18c18aa6025fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4289336
Reviewed-by: Zain Afzal <[email protected]>
Commit-Queue: Giovanni Ortuno Urquidi <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1110774}
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 18, 2024
As part of updating had to:
 - Remove @types/dom-view-transitions NPM dependency, since these
   types are now included in TypeScript's lib.dom.d.ts.
 - Remove custom ViewTransition definitions in
   ash/webui/recorder_app_ui/.
 - Fix newly discovered Iterator errors due to stricter checks, see [1].
   Note that the ash/webui/media_app_ui/test/driver.ts patch was
   provided by jopalmer@.
 - Update third_party/node/patches/typescript.patch as needed to keep
   working around the lack of TrustedTypes API support at [2].

[1] https://devblogs.microsoft.com/typescript/announcing-typescript-5-6/#strict-builtin-iterator-checks-(and---strictbuiltiniteratorreturn)
[2] microsoft/TypeScript#30024

Fixed: 368519840
Change-Id: I038b6cbaecb014f7ac4799d3eaea73a66488efe0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5560823
Reviewed-by: Rebekah Potter <[email protected]>
Commit-Queue: Rebekah Potter <[email protected]>
Auto-Submit: Demetrios Papadopoulos <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1370737}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Revisit An issue worth coming back to Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

8 participants