Merge pull request #171 from cloudflare/cina/refactor-subprocess-exec

Refactor subprocess execution to use `@actions/exec`

.changeset/proud-insects-add.md

@@ -0,0 +1,5 @@
+---
+"wrangler-action": patch
+---
+
+Fixed issues that caused the action to fail if any secret or var values contained shell metacharacters.

.changeset/warm-mugs-judge.md

@@ -0,0 +1,5 @@
+---
+"wrangler-action": patch
+---
+
+Bumped `DEFAULT_WRANGLER_VERSION` to 3.13.2

package-lock.json

@@ -1,15 +1,16 @@
 {
 	"name": "wrangler-action",
-	"version": "3.3.0",
+	"version": "3.3.1",
 	"lockfileVersion": 3,
 	"requires": true,
 	"packages": {
 		"": {
 			"name": "wrangler-action",
-			"version": "3.3.0",
+			"version": "3.3.1",
 			"license": "MIT OR Apache-2.0",
 			"dependencies": {
-				"@actions/core": "^1.10.0"
+				"@actions/core": "^1.10.0",
+				"@actions/exec": "^1.1.1"
 			},
 			"devDependencies": {
 				"@changesets/changelog-github": "^0.4.8",
@@ -31,6 +32,14 @@
 				"uuid": "^8.3.2"
 			}
 		},
+		"node_modules/@actions/exec": {
+			"version": "1.1.1",
+			"resolved": "https://registry.npmjs.org/@actions/exec/-/exec-1.1.1.tgz",
+			"integrity": "sha512-+sCcHHbVdk93a0XT19ECtO/gIXoxvdsgQLzb2fE2/5sIZmWQuluYyjPQtrtTHdU1YzTZ7bAPN4sITq2xi1679w==",
+			"dependencies": {
+				"@actions/io": "^1.0.1"
+			}
+		},
 		"node_modules/@actions/http-client": {
 			"version": "2.1.0",
 			"resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-2.1.0.tgz",
@@ -39,6 +48,11 @@
 				"tunnel": "^0.0.6"
 			}
 		},
+		"node_modules/@actions/io": {
+			"version": "1.1.3",
+			"resolved": "https://registry.npmjs.org/@actions/io/-/io-1.1.3.tgz",
+			"integrity": "sha512-wi9JjgKLYS7U/z8PPbco+PvTb/nRWjeoFlJ1Qer83k/3C5PHQi28hiVdeE2kHXmIL99mQFawx8qt/JPjZilJ8Q=="
+		},
 		"node_modules/@babel/code-frame": {
 			"version": "7.22.5",
 			"resolved": "https://registry.npmjs.org/@babel/code-frame/-/code-frame-7.22.5.tgz",

package.json

@@ -29,7 +29,8 @@
 		"check": "prettier --check ."
 	},
 	"dependencies": {
-		"@actions/core": "^1.10.0"
+		"@actions/core": "^1.10.0",
+		"@actions/exec": "^1.1.1"
 	},
 	"devDependencies": {
 		"@changesets/changelog-github": "^0.4.8",

src/exec.ts

@@ -0,0 +1,54 @@
+import {
+	exec as _childProcessExec,
+	type ExecException,
+} from "node:child_process";
+import { EOL } from "node:os";
+import { promisify } from "node:util";
+
+export { exec } from "@actions/exec";
+
+const childProcessExec = promisify(_childProcessExec);
+
+type ExecAsyncException = ExecException & {
+	stderr: string;
+	stdout: string;
+};
+
+function isExecAsyncException(err: unknown): err is ExecAsyncException {
+	return err instanceof Error && "code" in err && "stderr" in err;
+}
+
+export async function execShell(
+	command: string,
+	{
+		silent = false,
+		...options
+	}: Parameters<typeof childProcessExec>[1] & { silent?: boolean } = {},
+) {
+	if (!silent) {
+		process.stdout.write("[command]" + command + EOL);
+	}
+
+	try {
+		const promise = childProcessExec(command, {
+			...options,
+		});
+
+		const { child } = promise;
+
+		if (!silent) {
+			child.stdout?.on("data", (data: Buffer) => process.stdout.write(data));
+			child.stderr?.on("data", (data: Buffer) => process.stderr.write(data));
+		}
+
+		await promise;
+		return child.exitCode;
+	} catch (err: any) {
+		if (isExecAsyncException(err)) {
+			process.stderr.write(err.stderr);
+			throw new Error(`Process failed with exit code ${err.code}`);
+		}
+
+		throw err;
+	}
+}

src/index.ts

@@ -8,13 +8,11 @@ import {
 	startGroup as originalStartGroup,
 	setFailed,
 } from "@actions/core";
-import { exec, execSync } from "node:child_process";
-import * as util from "node:util";
+import { exec, execShell } from "./exec";
 import { checkWorkingDirectory, semverCompare } from "./utils";
 import { getPackageManager } from "./packageManagers";
-const execAsync = util.promisify(exec);
 
-const DEFAULT_WRANGLER_VERSION = "3.5.1";
+const DEFAULT_WRANGLER_VERSION = "3.13.2";
 
 /**
  * A configuration object that contains all the inputs & immutable state for the action.
@@ -62,8 +60,8 @@ function endGroup(): void {
 
 async function main() {
 	try {
-		installWrangler();
 		authenticationSetup();
+		await installWrangler();
 		await execCommands(getMultilineInput("preCommands"), "pre");
 		await uploadSecrets();
 		await wranglerCommands();
@@ -75,36 +73,28 @@ async function main() {
 	}
 }
 
-async function runProcess(
-	command: Parameters<typeof execAsync>[0],
-	options: Parameters<typeof execAsync>[1],
-) {
-	try {
-		const result = await execAsync(command, options);
-
-		result.stdout && info(result.stdout.toString());
-		result.stderr && error(result.stderr.toString(), true);
-
-		return result;
-	} catch (err: any) {
-		err.stdout && info(err.stdout.toString());
-		err.stderr && error(err.stderr.toString(), true);
-		throw new Error(`\`${command}\` returned non-zero exit code.`);
-	}
-}
-
-function installWrangler() {
+async function installWrangler() {
 	if (config["WRANGLER_VERSION"].startsWith("1")) {
 		throw new Error(
 			`Wrangler v1 is no longer supported by this action. Please use major version 2 or greater`,
 		);
 	}
+
 	startGroup("📥 Installing Wrangler");
-	const command = `${packageManager.install} wrangler@${config["WRANGLER_VERSION"]}`;
-	info(`Running command: ${command}`);
-	execSync(command, { cwd: config["workingDirectory"], env: process.env });
-	info(`✅ Wrangler installed`, true);
-	endGroup();
+	try {
+		await exec(
+			packageManager.install,
+			[`wrangler@${config["WRANGLER_VERSION"]}`],
+			{
+				cwd: config["workingDirectory"],
+				silent: config["QUIET_MODE"],
+			},
+		);
+
+		info(`✅ Wrangler installed`, true);
+	} finally {
+		endGroup();
+	}
 }
 
 function authenticationSetup() {
@@ -119,28 +109,21 @@ async function execCommands(commands: string[], cmdType: string) {
 
 	startGroup(`🚀 Running ${cmdType}Commands`);
 	try {
-		const arrPromises = commands.map(async (command) => {
+		for (const command of commands) {
 			const cmd = command.startsWith("wrangler")
 				? `${packageManager.exec} ${command}`
 				: command;
 
-			info(`🚀 Executing command: ${cmd}`);
-
-			return await runProcess(cmd, {
+			await execShell(cmd, {
 				cwd: config["workingDirectory"],
-				env: process.env,
+				silent: config["QUIET_MODE"],
 			});
-		});
-
-		await Promise.all(arrPromises);
+		}
 	} finally {
 		endGroup();
 	}
 }
 
-/**
- * A helper function to get the secret from the environment variables.
- */
 function getSecret(secret: string) {
 	if (!secret) {
 		throw new Error("Secret name cannot be blank.");
@@ -148,33 +131,43 @@ function getSecret(secret: string) {
 
 	const value = process.env[secret];
 	if (!value) {
-		throw new Error(`Value for secret ${secret} not found.`);
+		throw new Error(`Value for secret ${secret} not found in environment.`);
 	}
 
 	return value;
 }
 
-async function legacyUploadSecrets(
+function getEnvVar(envVar: string) {
+	if (!envVar) {
+		throw new Error("Var name cannot be blank.");
+	}
+
+	const value = process.env[envVar];
+	if (!value) {
+		throw new Error(`Value for var ${envVar} not found in environment.`);
+	}
+
+	return value;
+}
+
+function legacyUploadSecrets(
 	secrets: string[],
 	environment?: string,
 	workingDirectory?: string,
 ) {
-	const arrPromises = secrets
-		.map((secret) => {
-			const command = `echo ${getSecret(secret)} | ${
-				packageManager.exec
-			} wrangler secret put ${secret}`;
-			return environment ? command.concat(` --env ${environment}`) : command;
-		})
-		.map(
-			async (command) =>
-				await execAsync(command, {
-					cwd: workingDirectory,
-					env: process.env,
-				}),
-		);
-
-	await Promise.all(arrPromises);
+	return Promise.all(
+		secrets.map((secret) => {
+			const args = ["wrangler", "secret", "put", secret];
+			if (environment) {
+				args.push("--env", environment);
+			}
+			return exec(packageManager.exec, args, {
+				cwd: workingDirectory,
+				silent: config["QUIET_MODE"],
+				input: Buffer.from(getSecret(secret)),
+			});
+		}),
+	);
 }
 
 async function uploadSecrets() {
@@ -189,51 +182,34 @@ async function uploadSecrets() {
 	startGroup("🔑 Uploading secrets...");
 
 	try {
-		if (semverCompare(config["WRANGLER_VERSION"], "3.4.0"))
+		if (semverCompare(config["WRANGLER_VERSION"], "3.4.0")) {
 			return legacyUploadSecrets(secrets, environment, workingDirectory);
+		}
 
-		const secretObj = secrets.reduce((acc: any, secret: string) => {
-			acc[secret] = getSecret(secret);
-			return acc;
-		}, {});
+		const args = ["wrangler", "secret:bulk"];
 
-		const environmentSuffix = !environment.length
-			? ""
-			: ` --env ${environment}`;
+		if (environment) {
+			args.push("--env", environment);
+		}
 
-		const secretCmd = `echo "${JSON.stringify(secretObj).replaceAll(
-			'"',
-			'\\"',
-		)}" | ${packageManager.exec} wrangler secret:bulk ${environmentSuffix}`;
-
-		execSync(secretCmd, {
+		await exec(packageManager.exec, args, {
 			cwd: workingDirectory,
-			env: process.env,
-			stdio: "ignore",
+			silent: config["QUIET_MODE"],
+			input: Buffer.from(
+				JSON.stringify(
+					Object.fromEntries(
+						secrets.map((secret) => [secret, getSecret(secret)]),
+					),
+				),
+			),
 		});
-
-		info(`✅ Uploaded secrets`);
 	} catch (err) {
-		error(`❌ Upload failed`);
 		throw new Error(`Failed to upload secrets.`);
 	} finally {
 		endGroup();
 	}
 }
 
-function getVarArgs() {
-	const vars = config["VARS"];
-	const envVarArray = vars.map((envVar: string) => {
-		if (process.env[envVar] && process.env[envVar]?.length !== 0) {
-			return `${envVar}:${process.env[envVar]!}`;
-		} else {
-			throw new Error(`Value for var ${envVar} not found in environment.`);
-		}
-	});
-
-	return envVarArray.length > 0 ? `--var ${envVarArray.join(" ").trim()}` : "";
-}
-
 async function wranglerCommands() {
 	startGroup("🚀 Running Wrangler Commands");
 	try {
@@ -248,27 +224,29 @@ async function wranglerCommands() {
 			commands.push(deployCommand);
 		}
 
-		const arrPromises = commands.map(async (command) => {
-			if (environment.length > 0 && !command.includes(`--env`)) {
-				command = command.concat(` --env ${environment}`);
+		for (let command of commands) {
+			const args = [];
+
+			if (environment && !command.includes("--env")) {
+				args.push("--env", environment);
 			}
 
-			const cmd = `${packageManager.exec} wrangler ${command} ${
+			if (
+				config["VARS"].length &&
 				(command.startsWith("deploy") || command.startsWith("publish")) &&
-				!command.includes(`--var`)
-					? getVarArgs()
-					: ""
-			}`.trim();
+				!command.includes("--var")
+			) {
+				args.push("--var");
+				for (const v of config["VARS"]) {
+					args.push(`${v}:${getEnvVar(v)}`);
+				}
+			}
 
-			info(`🚀 Executing command: ${cmd}`);
-
-			return await runProcess(cmd, {
+			await exec(`${packageManager.exec} wrangler ${command}`, args, {
 				cwd: config["workingDirectory"],
-				env: process.env,
+				silent: config["QUIET_MODE"],
 			});
-		});
-
-		await Promise.all(arrPromises);
+		}
 	} finally {
 		endGroup();
 	}