use shell-quote package (#4080)

* use `shell-quote` package

instead of approximating behaviour with .split(" ") and .join(" ")

* wrap shell-quote.parse to match our needs:

- return string[], not including objects
- passthrough glob patterns
- throw when using non-simple commands

* fix tests with incorrect CLI args formatting

* fix linting

* fix linting in c3

packages/create-cloudflare/e2e-tests/cli.test.ts

@@ -3,6 +3,7 @@ import { tmpdir } from "os";
 import { join } from "path";
 import { beforeEach, afterEach, describe, test, expect } from "vitest";
 import { version } from "../package.json";
+import * as shellquote from "../src/helpers/shell-quote";
 import { frameworkToTest } from "./frameworkToTest";
 import { isQuarantineMode, keys, runC3 } from "./helpers";
 
@@ -30,13 +31,15 @@ describe.skipIf(frameworkToTest || isQuarantineMode())(
 		});
 
 		test("--version with positionals", async () => {
-			const argv = "foo bar baz --version".split(" ");
+			const argv = shellquote.parse("foo bar baz --version");
 			const { output } = await runC3({ argv });
 			expect(output).toEqual(version);
 		});
 
 		test("--version with flags", async () => {
-			const argv = "foo --type webFramework --no-deploy --version".split(" ");
+			const argv = shellquote.parse(
+				"foo --type webFramework --no-deploy --version"
+			);
 			const { output } = await runC3({ argv });
 			expect(output).toEqual(version);
 		});

packages/create-cloudflare/package.json

@@ -42,6 +42,7 @@
 	"devDependencies": {
 		"@babel/parser": "^7.21.3",
 		"@babel/types": "^7.21.4",
+		"@clack/core": "^0.3.2",
 		"@clack/prompts": "^0.6.3",
 		"@cloudflare/eslint-config-worker": "*",
 		"@cloudflare/workers-tsconfig": "workspace:*",
@@ -51,6 +52,8 @@
 		"@types/dns2": "^2.0.3",
 		"@types/esprima": "^4.0.3",
 		"@types/node": "^18.15.3",
+		"@types/semver": "^7.5.1",
+		"@types/shell-quote": "^1.7.2",
 		"@types/which-pm-runs": "^1.0.0",
 		"@types/yargs": "^17.0.22",
 		"@typescript-eslint/eslint-plugin": "^5.55.0",
@@ -67,16 +70,15 @@
 		"pnpm": "^8.6.11",
 		"recast": "^0.22.0",
 		"semver": "^7.5.1",
+		"shell-quote": "^1.8.1",
 		"typescript": "^5.0.2",
 		"undici": "5.20.0",
 		"vite-tsconfig-paths": "^4.0.8",
 		"vitest": "^0.30.0",
 		"which-pm-runs": "^1.1.0",
 		"wrangler": "workspace:*",
 		"yargs": "^17.7.1",
-		"yarn": "^1.22.19",
-		"@clack/core": "^0.3.2",
-		"@types/semver": "^7.5.1"
+		"yarn": "^1.22.19"
 	},
 	"engines": {
 		"node": ">=18.14.1"

packages/create-cloudflare/src/common.ts

@@ -28,6 +28,7 @@ import { detectPackageManager } from "helpers/packages";
 import { poll } from "helpers/poll";
 import { version as wranglerVersion } from "wrangler/package.json";
 import { version } from "../package.json";
+import * as shellquote from "./helpers/shell-quote";
 import type { C3Args, PagesGeneratorContext } from "types";
 
 const { name, npm } = detectPackageManager();
@@ -119,7 +120,7 @@ export const runDeploy = async (ctx: PagesGeneratorContext) => {
 		env: { CLOUDFLARE_ACCOUNT_ID: ctx.account.id, NODE_ENV: "production" },
 		startText: "Deploying your application",
 		doneText: `${brandColor("deployed")} ${dim(
-			`via \`${baseDeployCmd.join(" ")}\``
+			`via \`${shellquote.quote(baseDeployCmd)}\``
 		)}`,
 	});
 

packages/create-cloudflare/src/helpers/__tests__/command.test.ts

@@ -9,6 +9,7 @@ import {
 	npmInstall,
 	runCommand,
 } from "../command";
+import * as shellquote from "../shell-quote";
 
 // We can change how the mock spawn works by setting these variables
 let spawnResultCode = 0;
@@ -57,7 +58,7 @@ describe("Command Helpers", () => {
 	});
 
 	const expectSpawnWith = (cmd: string) => {
-		const [command, ...args] = cmd.split(" ");
+		const [command, ...args] = shellquote.parse(cmd);
 
 		expect(spawn).toHaveBeenCalledWith(command, args, {
 			stdio: "inherit",
@@ -66,7 +67,7 @@ describe("Command Helpers", () => {
 	};
 
 	const expectSilentSpawnWith = (cmd: string) => {
-		const [command, ...args] = cmd.split(" ");
+		const [command, ...args] = shellquote.parse(cmd);
 
 		expect(spawn).toHaveBeenCalledWith(command, args, {
 			stdio: "pipe",

packages/create-cloudflare/src/helpers/command.ts

@@ -5,6 +5,7 @@ import { endSection, stripAnsi } from "./cli";
 import { brandColor, dim } from "./colors";
 import { spinner } from "./interactive";
 import { detectPackageManager } from "./packages";
+import * as shellquote from "./shell-quote";
 import type { PagesGeneratorContext } from "types";
 
 /**
@@ -47,12 +48,12 @@ export const runCommand = async (
 	opts: RunOptions = {}
 ): Promise<string> => {
 	if (typeof command === "string") {
-		command = command.trim().replace(/\s+/g, " ").split(" ");
+		command = shellquote.parse(command);
 	}
 
 	return printAsyncStatus({
 		useSpinner: opts.useSpinner ?? opts.silent,
-		startText: opts.startText || command.join(" ").trim(),
+		startText: opts.startText || shellquote.quote(command),
 		doneText: opts.doneText,
 		promise() {
 			const [executable, ...args] = command;
@@ -181,7 +182,7 @@ export const runFrameworkGenerator = async (
 	cmd: string
 ) => {
 	if (ctx.framework?.args?.length) {
-		cmd = `${cmd} ${ctx.framework.args.join(" ")}`;
+		cmd = `${cmd} ${shellquote.quote(ctx.framework.args)}`;
 	}
 
 	endSection(
@@ -191,7 +192,7 @@ export const runFrameworkGenerator = async (
 
 	if (process.env.VITEST) {
 		const flags = ctx.framework?.config.testFlags ?? [];
-		cmd = `${cmd} ${flags.join(" ")}`;
+		cmd = `${cmd} ${shellquote.quote(flags)}`;
 	}
 
 	await runCommand(cmd);
@@ -228,7 +229,7 @@ export const installPackages = async (
 			break;
 	}
 
-	await runCommand(`${npm} ${cmd} ${saveFlag} ${packages.join(" ")}`, {
+	await runCommand(`${npm} ${cmd} ${saveFlag} ${shellquote.quote(packages)}`, {
 		...config,
 		silent: true,
 	});

packages/create-cloudflare/src/helpers/shell-quote.ts

@@ -0,0 +1,34 @@
+import shellquote from "shell-quote";
+
+export const quote = shellquote.quote;
+
+export function parse(cmd: string, env?: Record<string, string>): string[] {
+	const entries = shellquote.parse(cmd, env);
+	const argv: string[] = [];
+
+	for (const entry of entries) {
+		// use string entries, as is
+		if (typeof entry === "string") {
+			argv.push(entry);
+			continue;
+		}
+
+		// ignore comments
+		if ("comment" in entry) {
+			continue;
+		}
+
+		// we don't want to resolve globs, passthrough the pattern unexpanded
+		if (entry.op === "glob") {
+			argv.push(entry.pattern);
+			continue;
+		}
+
+		// any other entry.op is a ControlOperator (e.g. && or ||) we don't want to support
+		throw new Error(
+			`Only simple commands are supported, please don't use the "${entry.op}" operator in "${cmd}".`
+		);
+	}
+
+	return argv;
+}

packages/wrangler/package.json

@@ -136,6 +136,7 @@
 		"@types/prompts": "^2.0.14",
 		"@types/react": "^17.0.37",
 		"@types/serve-static": "^1.13.10",
+		"@types/shell-quote": "^1.7.2",
 		"@types/signal-exit": "^3.0.1",
 		"@types/source-map-support": "^0.5.7",
 		"@types/supports-color": "^8.1.1",
@@ -185,6 +186,7 @@
 		"remove-accents-esm": "^0.0.1",
 		"semiver": "^1.1.0",
 		"serve-static": "^1.15.0",
+		"shell-quote": "^1.8.1",
 		"shellac": "^0.8.0",
 		"signal-exit": "^3.0.7",
 		"strip-ansi": "^7.0.1",

packages/wrangler/src/__tests__/d1/execute.test.ts

@@ -18,7 +18,7 @@ describe("execute", () => {
 		});
 
 		await expect(
-			runWrangler("d1 execute --command 'select 1;'")
+			runWrangler("d1 execute db --command 'select 1;'")
 		).rejects.toThrowError(
 			`In a non-interactive environment, it's necessary to set a CLOUDFLARE_API_TOKEN environment variable for wrangler to work. Please go to https://developers.cloudflare.com/fundamentals/api/get-started/create-token/ for instructions on how to create an api token, and assign its value to CLOUDFLARE_API_TOKEN.`
 		);

packages/wrangler/src/__tests__/deploy.test.ts

@@ -4323,7 +4323,7 @@ addEventListener('fetch', event => {});`
 			mockSubDomainRequest();
 			mockUploadWorkerRequest();
 			await runWrangler(
-				"deploy --dry-run --outdir dist --define abc:'https://www.abc.net.au/news/'"
+				`deploy --dry-run --outdir dist --define "abc:'https://www.abc.net.au/news/'"`
 			);
 
 			expect(fs.readFileSync("dist/index.js", "utf-8")).toContain(

packages/wrangler/src/__tests__/helpers/run-wrangler.ts

@@ -1,12 +1,14 @@
 import { main } from "../../index";
+import * as shellquote from "../../utils/shell-quote";
 import { normalizeSlashes, stripTimings } from "./mock-console";
 
 /**
  * A helper to 'run' wrangler commands for tests.
  */
-export async function runWrangler(cmd?: string) {
+export async function runWrangler(cmd = "") {
 	try {
-		await main(cmd?.split(" ") ?? []);
+		const argv = shellquote.parse(cmd);
+		await main(argv);
 	} catch (err) {
 		if (err instanceof Error) {
 			err.message = normalizeSlashes(stripTimings(err.message));

packages/wrangler/src/__tests__/kv.test.ts

@@ -528,7 +528,7 @@ describe("wrangler", () => {
 					},
 				});
 				await runWrangler(
-					`kv:key put dKey dVal --namespace-id some-namespace-id --metadata {"mKey":"mValue"}`
+					`kv:key put dKey dVal --namespace-id some-namespace-id --metadata '{"mKey":"mValue"}'`
 				);
 				expect(requests.count).toEqual(1);
 				expect(std.out).toMatchInlineSnapshot(

packages/wrangler/src/init.ts

@@ -17,6 +17,7 @@ import { getPackageManager } from "./package-manager";
 import { parsePackageJSON, parseTOML, readFileSync } from "./parse";
 import { getBasePath } from "./paths";
 import { requireAuth } from "./user";
+import * as shellquote from "./utils/shell-quote";
 import { CommandLineArgsError, printWranglerBanner } from "./index";
 
 import type { RawConfig } from "./config";
@@ -184,7 +185,7 @@ export async function initHandler(args: InitArgs) {
 		}
 
 		const c3Arguments = [
-			...getC3CommandFromEnv().split(" "),
+			...shellquote.parse(getC3CommandFromEnv()),
 			fromDashScriptName,
 			...(yesFlag && isNpm(packageManager) ? ["-y"] : []), // --yes arg for npx
 			...(isNpm(packageManager) ? ["--"] : []),
@@ -252,7 +253,7 @@ export async function initHandler(args: InitArgs) {
 				c3Arguments.unshift("-y"); // arg for npx
 			}
 
-			c3Arguments.unshift(...getC3CommandFromEnv().split(" "));
+			c3Arguments.unshift(...shellquote.parse(getC3CommandFromEnv()));
 
 			// Deprecate the `init --from-dash` command
 			const replacementC3Command = `\`${packageManager.type} ${c3Arguments.join(

packages/wrangler/src/utils/shell-quote.ts

@@ -0,0 +1,34 @@
+import shellquote from "shell-quote";
+
+export const quote = shellquote.quote;
+
+export function parse(cmd: string, env?: Record<string, string>): string[] {
+	const entries = shellquote.parse(cmd, env);
+	const argv: string[] = [];
+
+	for (const entry of entries) {
+		// use string entries, as is
+		if (typeof entry === "string") {
+			argv.push(entry);
+			continue;
+		}
+
+		// ignore comments
+		if ("comment" in entry) {
+			continue;
+		}
+
+		// we don't want to resolve globs, passthrough the pattern unexpanded
+		if (entry.op === "glob") {
+			argv.push(entry.pattern);
+			continue;
+		}
+
+		// any other entry.op is a ControlOperator (e.g. && or ||) we don't want to support
+		throw new Error(
+			`Only simple commands are supported, please don't use the "${entry.op}" operator in "${cmd}".`
+		);
+	}
+
+	return argv;
+}