Skip to content

Conversation

clydin
Copy link
Member

@clydin clydin commented Apr 29, 2022

In an effort to improve supply chain security, the NodePackageInstallTask will now use the package
manager's --ignore-scripts option by default. Without the option, all direct and transitive
dependencies would have their scripts executed during the task's package manager installation operation.
The change only affects the package manager behavior controlled by the Schematics NodePackageInstallTask.

First-party Angular schematics do not currently require any direct or transitive dependency
install/postinstall scripts to execute. Only two dependencies within a v14.0 new project would
potentially be affected by this: nice-napi (transitive from piscina) and esbuild. The nice-napi
functionality of piscina is unused within the Angular CLI with no plans to use it in the future.
Even if it was used, the install script runs node-gyp-build which would only have an effect
(based on the current version 1.0.2) on platforms that are not Windows, darwin-x64, or linux-x64.
In the event this functionality is eventually used, the Angular CLI could be setup to automatically execute
this particular script for unsupported platforms. For esbuild, the postinstall functionality
performs an optional native binary bootstrap optimization but would only be performed if not
using Windows or Yarn. As such, it would not be performed for many users regardless of the change in
this commit. If noticeable performance regressions on platforms where the optimization was previously
performed are reported, the script could also be setup to be automatically run by the Angular CLI during
project creation and/or first build.

BREAKING CHANGE: Schematics NodePackageInstallTask will not execute package scripts by default
The NodePackageInstallTask will now use the package manager's --ignore-scripts option by default.
The --ignore-scripts option will prevent package scripts from executing automatically during an install.
If a schematic installs packages that need their install/postinstall scripts to be executed, the
NodePackageInstallTask now contains an allowScripts boolean option which can be enabled to provide the
previous behavior for that individual task. As with previous behavior, the allowScripts option will
prevent the individual task's usage of the --ignore-scripts option but will not override the package
manager's existing configuration.

@clydin clydin added the target: major This PR is targeted for the next major release label Apr 29, 2022
@clydin clydin force-pushed the ng-new/no-postinstalls branch from bd01053 to 1a75404 Compare April 29, 2022 19:57
@clydin clydin marked this pull request as ready for review April 29, 2022 19:59
@clydin clydin added the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 29, 2022
@clydin clydin requested a review from dgp1130 April 29, 2022 19:59
@clydin clydin added this to the v14 milestone Apr 29, 2022
Copy link
Collaborator

@dgp1130 dgp1130 left a comment

Choose a reason for hiding this comment

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

Typo: nice-napi in the commit message.

Is there anything we should consider doing to make sure future changes to transitive dependencies don't introduce something which requires a postinstall?

@dgp1130
Copy link
Collaborator

dgp1130 commented Apr 29, 2022

@twerske / @MarkTechson - I wonder if this would be worth mentioning the v14 blog post / release announcement as one way we're improving security for Angular applications?

@clydin clydin removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 29, 2022
@clydin clydin force-pushed the ng-new/no-postinstalls branch from 1a75404 to da61a2b Compare April 29, 2022 21:59
@clydin clydin added action: merge The PR is ready for merge by the caretaker and removed action: merge The PR is ready for merge by the caretaker labels Apr 29, 2022
… default in `NodePackageInstallTask`

In an effort to improve supply chain security, the `NodePackageInstallTask` will now use the package
manager's `--ignore-scripts` option by default. Without the option, all direct and transitive
dependencies would have their scripts executed during the task's package manager installation operation.
The change only affects the package manager behavior controlled by the Schematics `NodePackageInstallTask`.

First-party Angular schematics do not currently require any direct or transitive dependency
`install`/`postinstall` scripts to execute. Only two dependencies within a v14.0 new project would
potentially be affected by this: `nice-napi` (transitive from `piscina`) and `esbuild`. The `nice-napi`
functionality of `piscina` is unused within the Angular CLI with no plans to use it in the future.
Even if it was used, the `install` script runs `node-gyp-build` which would only have an effect
(based on the current version 1.0.2) on platforms that are not Windows, darwin-x64, or linux-x64.
In the event this functionality is eventually used, the Angular CLI could be setup to automatically execute
this particular script for unsupported platforms. For `esbuild`, the `postinstall` functionality
performs an optional native binary bootstrap optimization but would only be performed if not
using Windows or Yarn. As such, it would not be performed for many users regardless of the change in
this commit. If noticeable performance regressions on platforms where the optimization was previously
performed are reported, the script could also be setup to be automatically run by the Angular CLI during
project creation and/or first build.

BREAKING CHANGE: Schematics `NodePackageInstallTask` will not execute package scripts by default
The `NodePackageInstallTask` will now use the package manager's `--ignore-scripts` option by default.
The `--ignore-scripts` option will prevent package scripts from executing automatically during an install.
If a schematic installs packages that need their `install`/`postinstall` scripts to be executed, the
`NodePackageInstallTask` now contains an `allowScripts` boolean option which can be enabled to provide the
previous behavior for that individual task. As with previous behavior, the `allowScripts` option will
prevent the individual task's usage of the `--ignore-scripts` option but will not override the package
manager's existing configuration.
@clydin clydin force-pushed the ng-new/no-postinstalls branch from da61a2b to 3378942 Compare April 29, 2022 23:03
@clydin
Copy link
Member Author

clydin commented Apr 29, 2022

Is there anything we should consider doing to make sure future changes to transitive dependencies don't introduce something which requires a postinstall?

Added a new E2E test that checks for install scripts within a new project's dependencies and fails if any unexpected packages are detected.

@clydin clydin added the action: merge The PR is ready for merge by the caretaker label Apr 29, 2022
@dgp1130 dgp1130 merged commit 0e6425f into angular:main Apr 29, 2022
@clydin clydin deleted the ng-new/no-postinstalls branch April 29, 2022 23:39
@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 May 30, 2022
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 flag: breaking change target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants