Set shell: true when spawning npm commands (#742)

microsoft · Apr 24, 2024 · 60cdce1 · 60cdce1
1 parent 5cddb72
commit 60cdce1
Show file tree

Hide file tree

Showing 8 changed files with 18 additions and 39 deletions.
diff --git a/change/lage-5de6deae-24ba-4fc1-afbc-d8315a8e66b2.json b/change/lage-5de6deae-24ba-4fc1-afbc-d8315a8e66b2.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Set shell: true when spawning npm commands. This is required for Windows after a [Node security fix](https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2).",
+  "packageName": "lage",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/package.json b/package.json
@@ -22,7 +22,8 @@
     "format": "prettier --write .",
     "format:check": "prettier --check .",
     "prepare": "husky install",
-    "postinstall": "patch-package"
+    "postinstall": "patch-package",
+    "lage:local": "node ./packages/lage/bin/lage.js"
   },
   "devDependencies": {
     "@swc/core": "^1.3.14",

diff --git a/packages/lage/src/command/info.ts b/packages/lage/src/command/info.ts
@@ -13,6 +13,7 @@ import { getPackageAndTask } from "../task/taskId";
  * Generates a graph and spit it out in stdout
  *
  * Expected format:
+ * ```json
  * [
  *   {
  *       "id": "bar##build",
@@ -44,8 +45,8 @@ import { getPackageAndTask } from "../task/taskId";
  *   },
  *   ...
  * ]
+ * ```
  */
-
 export async function info(cwd: string, config: Config) {
   const workspace = getWorkspace(cwd, config);
   const packages = getPipelinePackages(workspace, config);

diff --git a/packages/lage/src/task/NpmScriptTask.ts b/packages/lage/src/task/NpmScriptTask.ts
@@ -1,15 +1,13 @@
 import { TaskLogger } from "../logger/TaskLogger";
 import { ChildProcess } from "child_process";
 import { PackageInfo } from "workspace-tools";
-import { findNpmClient } from "../workspace/findNpmClient";
 import { spawn } from "child_process";
 import path from "path";
 import { getNpmCommand } from "./getNpmCommand";
 import { Config } from "../types/Config";
 import { LogLevel } from "../logger/LogLevel";
 
 export class NpmScriptTask {
-  static npmCmd = "";
   static activeProcesses = new Set<ChildProcess>();
   static gracefulKillTimeout = 2500;
 
@@ -34,22 +32,23 @@ export class NpmScriptTask {
   }
 
   constructor(public task: string, public info: PackageInfo, private config: Config, private logger: TaskLogger) {
-    NpmScriptTask.npmCmd = NpmScriptTask.npmCmd || findNpmClient(config.npmClient);
     this.npmArgs = getNpmCommand(config.node, config.args, task);
   }
 
   run() {
     const { info, logger, npmArgs } = this;
-    const { npmCmd } = NpmScriptTask;
+    const npmCmd = this.config.npmClient;
     return new Promise<void>((resolve, reject) => {
       logger.verbose(`Running ${[npmCmd, ...npmArgs].join(" ")}`);
 
       const cp = spawn(npmCmd, npmArgs, {
         cwd: path.dirname(info.packageJsonPath),
         stdio: "pipe",
+        // This is required for Windows due to https://nodejs.org/en/blog/vulnerability/april-2024-security-releases-2
+        shell: true,
         env: {
+          ...(process.stdout.isTTY && !this.config.reporter.includes("json") && { FORCE_COLOR: "1" }), // allow user env to override this
           ...process.env,
-          ...(process.stdout.isTTY && !this.config.reporter.includes("json") && { FORCE_COLOR: "1" }),
           LAGE_PACKAGE_NAME: info.name,
         },
       });

diff --git a/packages/lage/src/types/Workspace.ts b/packages/lage/src/types/Workspace.ts
@@ -4,5 +4,6 @@ export interface Workspace {
   root: string;
   allPackages: PackageInfos;
   npmClient: string;
-  npmCmd: string;
+  /** @deprecated no longer used */
+  npmCmd?: string;
 }
diff --git a/packages/lage/src/workspace/findNpmClient.ts b/packages/lage/src/workspace/findNpmClient.ts
diff --git a/packages/lage/src/workspace/getWorkspace.ts b/packages/lage/src/workspace/getWorkspace.ts
@@ -1,7 +1,6 @@
 import { Workspace } from "../types/Workspace";
 import { getWorkspaceRoot, getPackageInfos } from "workspace-tools";
 import { Config } from "../types/Config";
-import { findNpmClient } from "./findNpmClient";
 
 export function getWorkspace(cwd: string, config: Pick<Config, "since" | "ignore" | "npmClient">): Workspace {
   const root = getWorkspaceRoot(cwd);
@@ -11,12 +10,10 @@ export function getWorkspace(cwd: string, config: Pick<Config, "since" | "ignore
 
   const { npmClient } = config;
   const allPackages = getPackageInfos(root);
-  const npmCmd = findNpmClient(npmClient);
 
   return {
     root,
     allPackages,
     npmClient,
-    npmCmd,
   };
 }
diff --git a/scripts/config/jest.config.js b/scripts/config/jest.config.js
@@ -28,6 +28,7 @@ const config = {
   watchPathIgnorePatterns: ["/node_modules/"],
   moduleNameMapper,
   ...(process.env.LAGE_PACKAGE_NAME && { maxWorkers: 1 }),
+  testTimeout: process.platform === "win32" ? 10000 : 5000,
   setupFilesAfterEnv: [path.join(__dirname, "jest-setup-after-env.js")],
 };
 module.exports = config;