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

fix(core): do not use Function constructors in development mode to avoid CSP violations #43587

Closed
wants to merge 1 commit into from

Conversation

JoostK
Copy link
Member

@JoostK JoostK commented Sep 25, 2021

This commit removes the dynamic creation of named arrays for internal
runtime storage arrays as they may cause CSP violations in development
mode, when an application's CSP configuration does not include
unsafe-eval.

Named arrays for view data can still be enabled in development mode
using the ngDevMode=namedConstructors query parameter when loading the
application.

The usage of native class syntax for named arrays does not have the
desired effect when the code is downleveled to ES5. Since ES5 targets
are becoming increasingly more rare this is considered less of a problem
than the CSP violation.

Fixes #43494

@JoostK JoostK added area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release cross-cutting: CSP labels Sep 25, 2021
@ngbot ngbot bot modified the milestone: Backlog Sep 25, 2021
@google-cla google-cla bot added the cla: yes label Sep 25, 2021
@JoostK
Copy link
Member Author

JoostK commented Sep 25, 2021

For context: ngDevMode.namedConstructors was introduced when the named constructors were added in #30542 but it was actually never used. It is disabled by default:

export function ngDevModeResetPerfCounters(): NgDevModePerfCounters {
const locationString = typeof location !== 'undefined' ? location.toString() : '';
const newCounters: NgDevModePerfCounters = {
namedConstructors: locationString.indexOf('ngDevMode=namedConstructors') != -1,

@JoostK JoostK force-pushed the core/named-arrays-csp branch 2 times, most recently from 5aa4b96 to 5c25e96 Compare September 25, 2021 13:10
@JoostK JoostK marked this pull request as ready for review September 25, 2021 17:25
@pullapprove pullapprove bot requested a review from atscott September 25, 2021 17:25
@JoostK JoostK added the action: review The PR is still awaiting reviews from at least one requested reviewer label Sep 25, 2021

// If this is being compiled to ES5 then the array subclass has `Array` as constructor
// instead of `SupportsArraySubclassing`.
const targetSupportsArraySubclassing =
Copy link
Member Author

@JoostK JoostK Sep 25, 2021

Choose a reason for hiding this comment

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

Note: this is currently always false as our testing infrastructure is using ES5 code.

@atscott atscott requested a review from IgorMinar October 1, 2021 21:04
Copy link
Contributor

@IgorMinar IgorMinar left a comment

Choose a reason for hiding this comment

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

this looks good to me. thanks @JoostK

…oid CSP violations

This commit removes the dynamic creation of named arrays for internal
runtime storage arrays as they may cause CSP violations in development
mode,  when an application's CSP configuration does not include
`unsafe-eval`.

Named arrays for view data can still be enabled in development mode
using the `ngDevMode=namedConstructors` query parameter when loading the
application.

The usage of native class syntax for named arrays does not have the
desired effect when the code is downleveled to ES5. Since ES5 targets
are becoming increasingly more rare this is considered less of a problem
than the CSP violation.

Fixes angular#43494
@JoostK JoostK added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 8, 2021
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir removed the action: presubmit The PR is in need of a google3 presubmit label Nov 9, 2021
@atscott
Copy link
Contributor

atscott commented Nov 9, 2021

This PR was merged into the repository by commit 4f8eaac.

@atscott atscott closed this in 4f8eaac Nov 9, 2021
atscott pushed a commit that referenced this pull request Nov 9, 2021
…oid CSP violations (#43587)

This commit removes the dynamic creation of named arrays for internal
runtime storage arrays as they may cause CSP violations in development
mode,  when an application's CSP configuration does not include
`unsafe-eval`.

Named arrays for view data can still be enabled in development mode
using the `ngDevMode=namedConstructors` query parameter when loading the
application.

The usage of native class syntax for named arrays does not have the
desired effect when the code is downleveled to ES5. Since ES5 targets
are becoming increasingly more rare this is considered less of a problem
than the CSP violation.

Fixes #43494

PR Close #43587
@angular-automatic-lock-bot
Copy link

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 Dec 10, 2021
dimakuba pushed a commit to dimakuba/angular that referenced this pull request Dec 28, 2021
…oid CSP violations (angular#43587)

This commit removes the dynamic creation of named arrays for internal
runtime storage arrays as they may cause CSP violations in development
mode,  when an application's CSP configuration does not include
`unsafe-eval`.

Named arrays for view data can still be enabled in development mode
using the `ngDevMode=namedConstructors` query parameter when loading the
application.

The usage of native class syntax for named arrays does not have the
desired effect when the code is downleveled to ES5. Since ES5 targets
are becoming increasingly more rare this is considered less of a problem
than the CSP violation.

Fixes angular#43494

PR Close angular#43587
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: core Issues related to the framework runtime cla: yes cross-cutting: CSP target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Angular dev mode errors with a strict CSP without unsafe-eval
4 participants