Sitelet https://github.com/angular/angular/pull/39211
Skip to content

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

Closed
bjarkler wants to merge 2 commits into
angular:masterfrom
bjarkler:trusted-types-constants
Closed

bjarkler wants to merge 2 commits into
angular:masterfrom
bjarkler:trusted-types-constants

Conversation

@bjarkler

Copy link
Copy Markdown
Contributor

Angular treats constant values of attributes and properties in templates
as secure. This means that these values are not sanitized, and are
instead passed directly to the corresponding setAttribute or setProperty
function. In cases where the given attribute or property is
security-sensitive, this causes a Trusted Types violation.

To address this, functions for promoting constant strings to each of the
three Trusted Types are introduced to Angular's private codegen API. The
compiler is updated to wrap constant strings with calls to these
functions as appropriate when constructing the consts array. This is
only done for security-sensitive attributes and properties, as
classified by Angular's dom_security_schema.

This is based on #39207. See the individual commits for more details.

PR Checklist

Please check if your PR fulfills the following requirements:

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.

@google-cla google-cla Bot added the cla: yes label Oct 10, 2020
@bjarkler
bjarkler force-pushed the trusted-types-constants branch 2 times, most recently from 08a7a1f to 6e2e964 Compare October 11, 2020 14:00
@bjarkler
bjarkler force-pushed the trusted-types-constants branch from 6e2e964 to 898d78c Compare October 11, 2020 18:31
@bjarkler bjarkler mentioned this pull request Oct 11, 2020
10 of 23 tasks
@bjarkler
bjarkler force-pushed the trusted-types-constants branch from 898d78c to 6a0273b Compare October 13, 2020 15:49
@bjarkler
bjarkler force-pushed the trusted-types-constants branch from 6a0273b to 4f7a86d Compare October 13, 2020 18:41
@bjarkler
bjarkler force-pushed the trusted-types-constants branch from 4f7a86d to 64aba01 Compare October 13, 2020 21:15
@bjarkler
bjarkler force-pushed the trusted-types-constants branch from 64aba01 to 2a7204c Compare October 13, 2020 21:31
@atscott atscott added the area: compiler Issues related to `ngc`, Angular's template compiler label Oct 13, 2020
@ngbot ngbot Bot added this to the needsTriage milestone Oct 13, 2020
@bjarkler
bjarkler force-pushed the trusted-types-constants branch from 2a7204c to afff6f0 Compare October 14, 2020 19:24

@IgorMinar IgorMinar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change lgtm with just one nit about the naming of the private types.

the CI is happy as well. I kicked of g3 presubmit just so that we have a g3 signal as well, but I'm expecting that g3 will be happy as well.

Comment thread packages/types.d.ts Outdated
Comment thread packages/core/src/util/security/trusted_type_defs.ts Outdated
@pullapprove
pullapprove Bot requested review from IgorMinar October 14, 2020 21:37
@bjarkler
bjarkler force-pushed the trusted-types-constants branch from afff6f0 to 3d6ae16 Compare October 14, 2020 22:31
The @types/trusted-types type definitions are currently imported in
types.d.ts, which causes them to eventually be imported in core.d.ts.
This forces anyone compiling against @angular/core to provide the
@types/trusted-types package in their compilation unit, which we don't
want.

To address this, get rid of the @types/trusted-types and instead import
a minimal version of the Trusted Types type definitions directly into
Angular's codebase.

Update the existing references to Trusted Types to point to the new
definitions.
Angular treats constant values of attributes and properties in templates
as secure. This means that these values are not sanitized, and are
instead passed directly to the corresponding setAttribute or setProperty
function. In cases where the given attribute or property is
security-sensitive, this causes a Trusted Types violation.

To address this, functions for promoting constant strings to each of the
three Trusted Types are introduced to Angular's private codegen API. The
compiler is updated to wrap constant strings with calls to these
functions as appropriate when constructing the `consts` array. This is
only done for security-sensitive attributes and properties, as
classified by Angular's dom_security_schema.
@bjarkler
bjarkler force-pushed the trusted-types-constants branch from 3d6ae16 to b26a984 Compare October 14, 2020 22:40

@IgorMinar IgorMinar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming that CI passes. 🤞

Reviewed-for: global-approvers

@IgorMinar IgorMinar added target: minor This PR is targeted for the next minor release action: merge The PR is ready for merge by the caretaker labels Oct 14, 2020
@atscott atscott closed this in 9bfb508 Oct 15, 2020
atscott pushed a commit that referenced this pull request Oct 15, 2020
Angular treats constant values of attributes and properties in templates
as secure. This means that these values are not sanitized, and are
instead passed directly to the corresponding setAttribute or setProperty
function. In cases where the given attribute or property is
security-sensitive, this causes a Trusted Types violation.

To address this, functions for promoting constant strings to each of the
three Trusted Types are introduced to Angular's private codegen API. The
compiler is updated to wrap constant strings with calls to these
functions as appropriate when constructing the `consts` array. This is
only done for security-sensitive attributes and properties, as
classified by Angular's dom_security_schema.

PR Close #39211
bjarkler added a commit to bjarkler/angular that referenced this pull request Oct 15, 2020
When an attribute has a constant value that is marked for translation
(i18n-attrName), a special variable is created in the `consts` array of
the compiled template. Since this is not a constant, it is not promoted
to a Trusted Type by angular#39211, and can thus cause Trusted Types
violations. The same applies to constant attributes parsed out of
(potentially translated) i18n ICU messages.

Use the newly introduced `SanitizerFn` to promote these constants
directly to appropriate Trusted Types. Tree shakability is not a concern
here since Trusted Types are already required for i18n to work (angular#39208).
bjarkler added a commit to bjarkler/angular that referenced this pull request Oct 16, 2020
When an attribute has a constant value that is marked for translation
(i18n-attrName), a special variable is created in the `consts` array of
the compiled template. Since this is not a constant, it is not promoted
to a Trusted Type by angular#39211, and can thus cause Trusted Types
violations. The same applies to constant attributes parsed out of
(potentially translated) i18n ICU messages.

Use the newly introduced `SanitizerFn` to promote these constants
directly to appropriate Trusted Types. Tree shakability is not a concern
here since Trusted Types are already required for i18n to work (angular#39208).
bjarkler added a commit to bjarkler/angular that referenced this pull request Oct 29, 2020
Angular-internal type definitions for Trusted Types were added in angular#39211.
When compiled using the Closure compiler with certain optimization
flags, identifiers from these type definitions (such as createPolicy)
are currently uglified and renamed to shorter strings. This causes
Angular applications compiled in this way to fail to create a Trusted
Types policy, and fall bock to using strings.

To fix this, mark the internal Trusted Types definitions as declarations
using the "declare" keyword. Also convert types to interfaces, for
the reasons explained in https://ncjamieson.com/prefer-interfaces/
josephperrott pushed a commit that referenced this pull request Oct 29, 2020
Angular-internal type definitions for Trusted Types were added in #39211.
When compiled using the Closure compiler with certain optimization
flags, identifiers from these type definitions (such as createPolicy)
are currently uglified and renamed to shorter strings. This causes
Angular applications compiled in this way to fail to create a Trusted
Types policy, and fall bock to using strings.

To fix this, mark the internal Trusted Types definitions as declarations
using the "declare" keyword. Also convert types to interfaces, for
the reasons explained in https://ncjamieson.com/prefer-interfaces/

PR Close #39471
@angular-automatic-lock-bot

Copy link
Copy Markdown

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.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Nov 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler cla: yes target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants