Skip to content

Commit

Permalink
Set shell: true when spawning npm commands
Browse files Browse the repository at this point in the history
  • Loading branch information
ecraig12345 committed Apr 24, 2024
1 parent 5cddb72 commit e04f80f
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 38 deletions.
7 changes: 7 additions & 0 deletions change/lage-5de6deae-24ba-4fc1-afbc-d8315a8e66b2.json
Original file line number Diff line number Diff line change
@@ -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"
}
3 changes: 2 additions & 1 deletion packages/lage/src/command/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { getPackageAndTask } from "../task/taskId";
* Generates a graph and spit it out in stdout
*
* Expected format:
* ```json
* [
* {
* "id": "bar##build",
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 4 additions & 5 deletions packages/lage/src/task/NpmScriptTask.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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,
},
});
Expand Down
3 changes: 2 additions & 1 deletion packages/lage/src/types/Workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ export interface Workspace {
root: string;
allPackages: PackageInfos;
npmClient: string;
npmCmd: string;
/** @deprecated no longer used */
npmCmd?: string;
}
28 changes: 0 additions & 28 deletions packages/lage/src/workspace/findNpmClient.ts

This file was deleted.

3 changes: 0 additions & 3 deletions packages/lage/src/workspace/getWorkspace.ts
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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,
};
}
1 change: 1 addition & 0 deletions scripts/config/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

0 comments on commit e04f80f

Please sign in to comment.