feat(core): create internal Trusted Types module #39207

bjarkler · 2020-10-09T23:11:03Z

Add a module that provides a Trusted Types policy for use internally by
Angular. The policy is created lazily and stored in a module-local
variable. For now the module does not allow configuring custom policies
or policy names, and instead creates its own policy with 'angular' as a
fixed policy name. This is to more easily support tree-shakability.

Helper functions for unsafely converting strings to each of the three
Trusted Types are also introduced, with names that make it clear that
their use requires a security review. When Trusted Types are not
available, these helper functions fall back to returning strings.

The TypeScript type definitions for Trusted Types is also added as
a dependency. See the individual commits for more details.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

Does this PR introduce a breaking change?

Yes
No

Other information

This is part of an ongoing effort to add support for Trusted Types to Angular.

gkalpak · 2020-10-10T13:40:15Z

packages/core/package.json

@@ -5,6 +5,7 @@
  "author": "angular",
  "license": "MIT",
  "dependencies": {
+    "@types/trusted-types": "^1.0.6",


Is this really necessary? Seems redundant 😕

Good question, and I would definitely appreciate insights others might have on this. As far as my limited knowledge on npm package dependencies, this seemed to be necessary. Before adding this, the issue was that @types/trusted-types would not be available as a dependency in an Angular application (even after doing an ng update, it wouldn't appear in yarn.lock), since none of Angular's packages had a dependency on it. When I then ran ng build the application would not compile due to missing type definitions for Trusted Types. So without this (and unless there's a different way), existing Angular applications will break unless they explicitly add a dependency on @types/trusted-types themselves.

Why would the app fail? Can you give an example of how to reproduce this?
An app would consume the published Angular packages, which are compiled to JS. At that point, any types would have been stripped of (they are a compile-time only thing).

I share @gkalpak's concerns.

I discussed this with @bjarkler and we decided not to expose any of the trusted types as part of Angular's public api for now.

Once microsoft/TypeScript#30024 is fixed (introduces TT into lib.dom.d.ts) then we can expose TT through our core apis, but until then we should avoid it.

One alternative for still shipping TrustedSanitizer would be to ship it as a new package that has a peer dependency on @types/trusted-types. But this is something we can consider once Angular is compatible with TT, which should be the top priority.

The issue seems to be that #39219 and potentially other changes make Trusted Types a part of Angular's public API, which is why dependent applications will also need the type definitions to compile. We're looking into ways to circumvent that, at least until microsoft/TypeScript#30024 is fixed.

The problem is that later PRs introduce Trusted Types into Angular's public api surface, which then causes the types to be present in our d.ts files. In order for tsc to typecheck our d.ts files when compiling the angular app (without skipLibCheck), tsc will need the definitions.

As described in the other comment, we decided to change our rollout strategy and not expose TT in Angular's public api surface for now.

ahh.. @bjarkler also explained it :) sorry about the noise.

packages/core/src/util/security/trusted_types.ts

To facilitate the upcoming Trusted Types support being added to Angular, add the TypeScript type definitions for the Trusted Types browser API as a dependency in the root package.json and types.d.ts since they're needed for compiling the Angular packages.

Add a module that provides a Trusted Types policy for use internally by Angular. The policy is created lazily and stored in a module-local variable. For now the module does not allow configuring custom policies or policy names, and instead creates its own policy with 'angular' as a fixed policy name. This is to more easily support tree-shakability. Helper functions for unsafely converting strings to each of the three Trusted Types are also introduced, with documentation that make it clear that their use requires a security review. When Trusted Types are not available, these helper functions fall back to returning strings.

IgorMinar

discussed offline, we'll remove the dep on @types/trusted-types. See other comments for details.

IgorMinar · 2020-10-13T17:01:16Z

packages/core/src/util/security/trusted_types.ts

+ * Returns the Trusted Types policy, or null if Trusted Types are not
+ * enabled/supported. The first call to this function will create the policy.
+ */
+function getPolicy(): TrustedTypePolicy|null {


I see that you changed all of these from methods to functions. that's great for minification with Terser! Thank you!

Thanks to @engelsdamien for pointing that out :)

IgorMinar

LGTM now!

Reviewed-for: global-approvers

IgorMinar · 2020-10-13T18:04:46Z

I'm marking this as "target: minor" since we are not making any breaking change here or introducing a risky change.

Add a module that provides a Trusted Types policy for use internally by Angular. The policy is created lazily and stored in a module-local variable. For now the module does not allow configuring custom policies or policy names, and instead creates its own policy with 'angular' as a fixed policy name. This is to more easily support tree-shakability. Helper functions for unsafely converting strings to each of the three Trusted Types are also introduced, with documentation that make it clear that their use requires a security review. When Trusted Types are not available, these helper functions fall back to returning strings. PR Close #39207

angular-automatic-lock-bot · 2020-11-13T16:25:09Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

google-cla bot added the cla: yes label Oct 9, 2020

pullapprove bot requested review from alxhub and josephperrott October 9, 2020 23:11

bjarkler force-pushed the trusted-types-module branch from f79b382 to e912eaf Compare October 9, 2020 23:21

This was referenced Oct 9, 2020

fix(core): pass a Trusted Types policy to inert DOM builder #39208

Closed

fix(core): make development mode Trusted Types compatible #39209

Closed

fix(compiler): promote constants in templates to Trusted Types #39211

Closed

gkalpak reviewed Oct 10, 2020

View reviewed changes

bjarkler mentioned this pull request Oct 10, 2020

feat(core): support passing Trusted Types around sanitization pipeline #39217

Closed

14 tasks

bjarkler force-pushed the trusted-types-module branch 2 times, most recently from a666a48 to 35631b1 Compare October 11, 2020 14:00

bjarkler mentioned this pull request Oct 11, 2020

fix(core): promote i18n constants to Trusted Types #39221

Closed

14 tasks

bjarkler force-pushed the trusted-types-module branch from 35631b1 to 455362c Compare October 11, 2020 18:31

bjarkler mentioned this pull request Oct 11, 2020

[tracking] Support Trusted Types in Angular #39222

Closed

23 tasks

IgorMinar self-requested a review October 12, 2020 17:34

bjarkler force-pushed the trusted-types-module branch from 455362c to 80dee55 Compare October 13, 2020 15:02

bjarkler force-pushed the trusted-types-module branch from 80dee55 to c22fec5 Compare October 13, 2020 15:12

IgorMinar reviewed Oct 13, 2020

View reviewed changes

IgorMinar removed request for alxhub and josephperrott October 13, 2020 17:19

pullapprove bot requested a review from alxhub October 13, 2020 17:19

IgorMinar added target: major action: merge labels Oct 13, 2020

IgorMinar reviewed Oct 13, 2020

View reviewed changes

IgorMinar approved these changes Oct 13, 2020

View reviewed changes

IgorMinar removed the request for review from alxhub October 13, 2020 17:29

IgorMinar added target: minor and removed target: major labels Oct 13, 2020

atscott closed this in c4266fb Oct 13, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): create internal Trusted Types module #39207

feat(core): create internal Trusted Types module #39207

bjarkler commented Oct 9, 2020

gkalpak Oct 10, 2020

bjarkler Oct 10, 2020

gkalpak Oct 10, 2020

IgorMinar Oct 12, 2020

bjarkler Oct 12, 2020

IgorMinar Oct 13, 2020

IgorMinar Oct 13, 2020

IgorMinar left a comment

IgorMinar Oct 13, 2020

bjarkler Oct 13, 2020

IgorMinar left a comment

IgorMinar commented Oct 13, 2020

angular-automatic-lock-bot bot commented Nov 13, 2020

feat(core): create internal Trusted Types module #39207

feat(core): create internal Trusted Types module #39207

Conversation

bjarkler commented Oct 9, 2020

PR Checklist

PR Type

Does this PR introduce a breaking change?

Other information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IgorMinar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IgorMinar left a comment

Choose a reason for hiding this comment

IgorMinar commented Oct 13, 2020

angular-automatic-lock-bot bot commented Nov 13, 2020