From d647227bbcced55f22549fc6bee75edaa45d36c4 Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Fri, 15 Sep 2023 01:39:51 -0500 Subject: [PATCH 1/3] Refactor subprocess execution to use @actions/exec Instead of using a mix of `child_process.exec`, `child_process.execSync` and a promisified version of `child_process.exec`, we now (mostly) just use `@actions/exec`. That runs `child_process.spawn` under the hood and handles a lot of character escaping for us. We can also now pass Buffers directly into the subprocess as stdin instead of relying on shell piping. This ends up fixing a few problems we had where secrets and env var values containing shell metacharacters were being misinterpreted. Unfortunately, `@actions/exec` doesn't support running with a shell. That means we still have to roll our own wrapper around `child_process.exec` to avoid a breaking change to `preCommands` and `postCommands`, since users might be expecting these to run within a shell. Also worth noting that we're no longer hiding stdout and stderr from the secret uploading step. We were previously doing this out of an abundance of caution, but it made debugging issues very difficult if secret upload failed for some reason. I feel ok doing this since we're no longer echoing & piping the secret values, wrangler doesn't ever output secret values, and as a last line of defense GitHub masks any secret values that accidentally get logged. --- package-lock.json | 20 ++++- package.json | 3 +- src/exec.ts | 54 ++++++++++++++ src/index.ts | 186 ++++++++++++++++++++-------------------------- 4 files changed, 155 insertions(+), 108 deletions(-) create mode 100644 src/exec.ts diff --git a/package-lock.json b/package-lock.json index d34023c..e32cc36 100644 --- a/package-lock.json +++ b/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", diff --git a/package.json b/package.json index 1ef6bd7..8b22bc2 100644 --- a/package.json +++ b/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", diff --git a/src/exec.ts b/src/exec.ts new file mode 100644 index 0000000..e20a2e1 --- /dev/null +++ b/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[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; + } +} diff --git a/src/index.ts b/src/index.ts index 5e38275..3541832 100755 --- a/src/index.ts +++ b/src/index.ts @@ -8,11 +8,9 @@ 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"; @@ -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[0], - options: Parameters[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(); } From 76d614f400bd92237ed23c3df559f2c31b14a790 Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Fri, 15 Sep 2023 01:57:15 -0500 Subject: [PATCH 2/3] Add changeset --- .changeset/proud-insects-add.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/proud-insects-add.md diff --git a/.changeset/proud-insects-add.md b/.changeset/proud-insects-add.md new file mode 100644 index 0000000..189d272 --- /dev/null +++ b/.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. From 473d9cbd296528b41c653af10062faba6540a7ab Mon Sep 17 00:00:00 2001 From: Cina Saffary Date: Wed, 18 Oct 2023 17:04:41 -0500 Subject: [PATCH 3/3] Bump `DEFAULT_WRANGLER_VERSION` to 3.13.2 --- .changeset/warm-mugs-judge.md | 5 +++++ src/index.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/warm-mugs-judge.md diff --git a/.changeset/warm-mugs-judge.md b/.changeset/warm-mugs-judge.md new file mode 100644 index 0000000..484fa18 --- /dev/null +++ b/.changeset/warm-mugs-judge.md @@ -0,0 +1,5 @@ +--- +"wrangler-action": patch +--- + +Bumped `DEFAULT_WRANGLER_VERSION` to 3.13.2 diff --git a/src/index.ts b/src/index.ts index 3541832..e5a18a5 100755 --- a/src/index.ts +++ b/src/index.ts @@ -12,7 +12,7 @@ import { exec, execShell } from "./exec"; import { checkWorkingDirectory, semverCompare } from "./utils"; import { getPackageManager } from "./packageManagers"; -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.