Skip to content

Commit 0e6425f

Browse files
clydindgp1130
authored andcommitted
feat(@angular-devkit/schematics): disable package script execution by 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.
1 parent 21622b8 commit 0e6425f

File tree

11 files changed

+147
-1
lines changed

11 files changed

+147
-1
lines changed

.circleci/config.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ jobs:
325325
name: Execute E2E Tests
326326
command: |
327327
if (Test-Path env:CIRCLE_PULL_REQUEST) {
328-
node tests\legacy-cli\run_e2e.js "--glob={tests/basic/**,tests/i18n/extract-ivy*.ts,tests/build/profile.ts,tests/test/test-sourcemap.ts}" --nb-shards=$env:CIRCLE_NODE_TOTAL --shard=$env:CIRCLE_NODE_INDEX
328+
node tests\legacy-cli\run_e2e.js "--glob={tests/basic/**,tests/i18n/extract-ivy*.ts,tests/build/profile.ts,tests/test/test-sourcemap.ts,tests/misc/check-postinstalls.ts}" --nb-shards=$env:CIRCLE_NODE_TOTAL --shard=$env:CIRCLE_NODE_INDEX
329329
} else {
330330
node tests\legacy-cli\run_e2e.js --nb-shards=$env:CIRCLE_NODE_TOTAL --shard=$env:CIRCLE_NODE_INDEX
331331
}

goldens/public-api/angular_devkit/schematics/tasks/index.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ export class NodePackageInstallTask implements TaskConfigurationGenerator<NodePa
1111
constructor(workingDirectory?: string);
1212
constructor(options: NodePackageInstallTaskOptions);
1313
// (undocumented)
14+
allowScripts: boolean;
15+
// (undocumented)
1416
hideOutput: boolean;
1517
// (undocumented)
1618
packageManager?: string;

packages/angular_devkit/schematics/tasks/package-manager/executor.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ export default function (
100100
args.push(taskPackageManagerProfile.quietArgument);
101101
}
102102

103+
if (!options.allowScripts) {
104+
args.push('--ignore-scripts');
105+
}
106+
103107
if (factoryOptions.registry) {
104108
args.push(`--registry="${factoryOptions.registry}"`);
105109
}

packages/angular_devkit/schematics/tasks/package-manager/install-task.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@ interface NodePackageInstallTaskOptions {
1515
workingDirectory?: string;
1616
quiet?: boolean;
1717
hideOutput?: boolean;
18+
allowScripts?: boolean;
1819
}
1920

2021
export class NodePackageInstallTask implements TaskConfigurationGenerator<NodePackageTaskOptions> {
2122
quiet = true;
2223
hideOutput = true;
24+
allowScripts = false;
2325
workingDirectory?: string;
2426
packageManager?: string;
2527
packageName?: string;
@@ -45,6 +47,9 @@ export class NodePackageInstallTask implements TaskConfigurationGenerator<NodePa
4547
if (options.packageName != undefined) {
4648
this.packageName = options.packageName;
4749
}
50+
if (options.allowScripts !== undefined) {
51+
this.allowScripts = options.allowScripts;
52+
}
4853
}
4954
}
5055

@@ -58,6 +63,7 @@ export class NodePackageInstallTask implements TaskConfigurationGenerator<NodePa
5863
workingDirectory: this.workingDirectory,
5964
packageManager: this.packageManager,
6065
packageName: this.packageName,
66+
allowScripts: this.allowScripts,
6167
},
6268
};
6369
}

packages/angular_devkit/schematics/tasks/package-manager/options.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,5 @@ export interface NodePackageTaskOptions {
2323
workingDirectory?: string;
2424
packageName?: string;
2525
packageManager?: string;
26+
allowScripts?: boolean;
2627
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"schematics": {
3+
"test": {
4+
"factory": "./index.js",
5+
"schema": "./schema.json",
6+
"description": "test schematic that creates a directory with a local test package that should not run its post-install script"
7+
}
8+
}
9+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
const tasks = require("@angular-devkit/schematics/tasks");
2+
3+
exports.default = ({ allowScripts, ignoreScripts = false }) => {
4+
return (tree, context) => {
5+
tree.create('/install-test/package.json', JSON.stringify({
6+
name: 'install-test',
7+
version: '0.0.0',
8+
scripts: {
9+
postinstall: `node run-post.js`,
10+
}
11+
}));
12+
tree.create('/install-test/.npmrc', `ignore-scripts=${ignoreScripts}`);
13+
tree.create('/install-test/run-post.js', 'require("fs").writeFileSync(__dirname + "/post-script-ran", "12345");')
14+
context.addTask(new tasks.NodePackageInstallTask({ workingDirectory: 'install-test', allowScripts }));
15+
};
16+
};
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "allow-scripts",
3+
"version": "0.0.1",
4+
"schematics": "./collection.json"
5+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
{
2+
"$schema": "http://json-schema.org/draft-07/schema",
3+
"type": "object",
4+
"additionalProperties": false,
5+
"properties": {
6+
"allowScripts": {
7+
"type": "boolean"
8+
},
9+
"ignoreScripts": {
10+
"type": "boolean"
11+
}
12+
}
13+
}
14+
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { copyAssets } from '../../utils/assets';
2+
import { expectFileNotToExist, expectFileToExist, rimraf } from '../../utils/fs';
3+
import { ng } from '../../utils/process';
4+
5+
export default async function () {
6+
// Copy test schematic into test project to ensure schematic dependencies are available
7+
await copyAssets('schematic-allow-scripts', 'schematic-allow-scripts');
8+
9+
// By default should not run the postinstall from the added package.json in the schematic
10+
await ng('generate', './schematic-allow-scripts:test');
11+
await expectFileToExist('install-test/package.json');
12+
await expectFileNotToExist('install-test/post-script-ran');
13+
14+
// Cleanup for next test case
15+
await rimraf('install-test');
16+
17+
// Should run the postinstall if the allowScripts task option is enabled
18+
// For testing purposes, this schematic exposes the task option via a schematic option
19+
await ng('generate', './schematic-allow-scripts:test', '--allow-scripts');
20+
await expectFileToExist('install-test/package.json');
21+
await expectFileToExist('install-test/post-script-ran');
22+
23+
// Cleanup for next test case
24+
await rimraf('install-test');
25+
26+
// Package manager configuration should take priority
27+
// The `ignoreScripts` schematic option sets the value of the `ignore-scripts` option in a test project `.npmrc`
28+
await ng('generate', './schematic-allow-scripts:test', '--allow-scripts', '--ignore-scripts');
29+
await expectFileToExist('install-test/package.json');
30+
await expectFileNotToExist('install-test/post-script-ran');
31+
}
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import glob from 'glob';
2+
import { promisify } from 'util';
3+
import { readFile } from '../../utils/fs';
4+
5+
const globAsync = promisify(glob);
6+
7+
const CURRENT_SCRIPT_PACKAGES: ReadonlySet<string> = new Set([
8+
'esbuild (postinstall)',
9+
'nice-napi (install)',
10+
]);
11+
12+
const POTENTIAL_SCRIPTS: ReadonlyArray<string> = ['preinstall', 'install', 'postinstall'];
13+
14+
// Some packages include test and/or example code that causes false positives
15+
const FALSE_POSITIVE_PATHS: ReadonlySet<string> = new Set([
16+
'node_modules/jasmine-spec-reporter/examples/protractor/package.json',
17+
'node_modules/resolve/test/resolver/multirepo/package.json',
18+
]);
19+
20+
export default async function () {
21+
const manifestPaths = await globAsync('node_modules/**/package.json');
22+
const newPackages: string[] = [];
23+
24+
for (const manifestPath of manifestPaths) {
25+
if (FALSE_POSITIVE_PATHS.has(manifestPath)) {
26+
continue;
27+
}
28+
29+
let manifest;
30+
try {
31+
manifest = JSON.parse(await readFile(manifestPath));
32+
} catch {
33+
continue;
34+
}
35+
36+
if (!manifest.scripts) {
37+
continue;
38+
}
39+
40+
for (const script of POTENTIAL_SCRIPTS) {
41+
if (!manifest.scripts[script]) {
42+
continue;
43+
}
44+
45+
const packageScript = `${manifest.name} (${script})`;
46+
47+
if (!CURRENT_SCRIPT_PACKAGES.has(packageScript)) {
48+
newPackages.push(packageScript + `[${manifestPath}]`);
49+
}
50+
}
51+
}
52+
53+
if (newPackages.length) {
54+
throw new Error(
55+
'New install script package(s) detected:\n' + JSON.stringify(newPackages, null, 2),
56+
);
57+
}
58+
}

0 commit comments

Comments
 (0)