From eca7f303bacfa02ea5de820e8d13cd2bae5d8806 Mon Sep 17 00:00:00 2001 From: Rahul Sethi <5822355+RamIdeas@users.noreply.github.com> Date: Sun, 1 Oct 2023 12:26:55 +0100 Subject: [PATCH 1/5] use `shell-quote` package instead of approximating behaviour with .split(" ") and .join(" ") --- .../create-cloudflare/e2e-tests/cli.test.ts | 7 ++- packages/create-cloudflare/package.json | 8 ++- packages/create-cloudflare/src/common.ts | 3 +- .../src/helpers/__tests__/command.test.ts | 5 +- .../create-cloudflare/src/helpers/command.ts | 11 ++-- packages/wrangler/package.json | 2 + .../src/__tests__/helpers/run-wrangler.ts | 6 +- packages/wrangler/src/init.ts | 7 ++- pnpm-lock.yaml | 60 ++++++++++++------- 9 files changed, 70 insertions(+), 39 deletions(-) diff --git a/packages/create-cloudflare/e2e-tests/cli.test.ts b/packages/create-cloudflare/e2e-tests/cli.test.ts index bca593e59454..8464af1e91bd 100644 --- a/packages/create-cloudflare/e2e-tests/cli.test.ts +++ b/packages/create-cloudflare/e2e-tests/cli.test.ts @@ -1,6 +1,7 @@ import { existsSync, rmSync, mkdtempSync, realpathSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; +import shellquote from "shell-quote"; import { beforeEach, afterEach, describe, test, expect } from "vitest"; import { version } from "../package.json"; import { frameworkToTest } from "./frameworkToTest"; @@ -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") as string[]; 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" + ) as string[]; const { output } = await runC3({ argv }); expect(output).toEqual(version); }); diff --git a/packages/create-cloudflare/package.json b/packages/create-cloudflare/package.json index 69566ccfb07e..60e87029a56a 100644 --- a/packages/create-cloudflare/package.json +++ b/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,6 +70,7 @@ "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", @@ -74,9 +78,7 @@ "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": ">=16.13.0" diff --git a/packages/create-cloudflare/src/common.ts b/packages/create-cloudflare/src/common.ts index e10abcda6315..ad29fd2436f3 100644 --- a/packages/create-cloudflare/src/common.ts +++ b/packages/create-cloudflare/src/common.ts @@ -1,6 +1,7 @@ import { existsSync, mkdirSync, readdirSync } from "fs"; import { basename, dirname, resolve } from "path"; import { chdir } from "process"; +import shellquote from "shell-quote"; import { getFrameworkCli } from "frameworks/index"; import { processArgument } from "helpers/args"; import { @@ -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)}\`` )}`, }); diff --git a/packages/create-cloudflare/src/helpers/__tests__/command.test.ts b/packages/create-cloudflare/src/helpers/__tests__/command.test.ts index 5c5373b095c0..53d9838550fb 100644 --- a/packages/create-cloudflare/src/helpers/__tests__/command.test.ts +++ b/packages/create-cloudflare/src/helpers/__tests__/command.test.ts @@ -1,5 +1,6 @@ import { spawn } from "cross-spawn"; import { detectPackageManager } from "helpers/packages"; +import shellquote from "shell-quote"; import { beforeEach, afterEach, describe, expect, test, vi } from "vitest"; import whichPMRuns from "which-pm-runs"; import { @@ -57,7 +58,7 @@ describe("Command Helpers", () => { }); const expectSpawnWith = (cmd: string) => { - const [command, ...args] = cmd.split(" "); + const [command, ...args] = shellquote.parse(cmd) as string[]; 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) as string[]; expect(spawn).toHaveBeenCalledWith(command, args, { stdio: "pipe", diff --git a/packages/create-cloudflare/src/helpers/command.ts b/packages/create-cloudflare/src/helpers/command.ts index 1a37af3e82dd..351763113e6d 100644 --- a/packages/create-cloudflare/src/helpers/command.ts +++ b/packages/create-cloudflare/src/helpers/command.ts @@ -1,6 +1,7 @@ import { existsSync, rmSync } from "fs"; import path from "path"; import { spawn } from "cross-spawn"; +import shellquote from "shell-quote"; import { endSection, stripAnsi } from "./cli"; import { brandColor, dim } from "./colors"; import { spinner } from "./interactive"; @@ -47,12 +48,12 @@ export const runCommand = async ( opts: RunOptions = {} ): Promise => { if (typeof command === "string") { - command = command.trim().replace(/\s+/g, " ").split(" "); + command = shellquote.parse(command) as string[]; } 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, }); diff --git a/packages/wrangler/package.json b/packages/wrangler/package.json index ebde5fee13d9..510bf1f58f5b 100644 --- a/packages/wrangler/package.json +++ b/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", diff --git a/packages/wrangler/src/__tests__/helpers/run-wrangler.ts b/packages/wrangler/src/__tests__/helpers/run-wrangler.ts index c9a4edaa2a8b..20d2945ad334 100644 --- a/packages/wrangler/src/__tests__/helpers/run-wrangler.ts +++ b/packages/wrangler/src/__tests__/helpers/run-wrangler.ts @@ -1,12 +1,14 @@ +import shellquote from "shell-quote"; import { main } from "../../index"; 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) as string[]; + await main(argv); } catch (err) { if (err instanceof Error) { err.message = normalizeSlashes(stripTimings(err.message)); diff --git a/packages/wrangler/src/init.ts b/packages/wrangler/src/init.ts index c7d17306f16b..5073d799448c 100644 --- a/packages/wrangler/src/init.ts +++ b/packages/wrangler/src/init.ts @@ -4,6 +4,7 @@ import path, { dirname } from "node:path"; import TOML from "@iarna/toml"; import { execa } from "execa"; import { findUp } from "find-up"; +import shellquote from "shell-quote"; import { version as wranglerVersion } from "../package.json"; import { fetchResult } from "./cfetch"; @@ -184,7 +185,7 @@ export async function initHandler(args: InitArgs) { } const c3Arguments = [ - ...getC3CommandFromEnv().split(" "), + ...(shellquote.parse(getC3CommandFromEnv()) as string[]), fromDashScriptName, ...(yesFlag && isNpm(packageManager) ? ["-y"] : []), // --yes arg for npx ...(isNpm(packageManager) ? ["--"] : []), @@ -252,7 +253,9 @@ export async function initHandler(args: InitArgs) { c3Arguments.unshift("-y"); // arg for npx } - c3Arguments.unshift(...getC3CommandFromEnv().split(" ")); + c3Arguments.unshift( + ...(shellquote.parse(getC3CommandFromEnv()) as string[]) + ); // Deprecate the `init --from-dash` command const replacementC3Command = `\`${packageManager.type} ${c3Arguments.join( diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2ae87f943ba4..cac0143d0c60 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -477,6 +477,9 @@ importers: '@types/semver': specifier: ^7.5.1 version: 7.5.1 + '@types/shell-quote': + specifier: ^1.7.2 + version: 1.7.2 '@types/which-pm-runs': specifier: ^1.0.0 version: 1.0.0 @@ -525,6 +528,9 @@ importers: semver: specifier: ^7.5.1 version: 7.5.1 + shell-quote: + specifier: ^1.8.1 + version: 1.8.1 typescript: specifier: ^5.0.2 version: 5.0.3 @@ -587,7 +593,7 @@ importers: version: 8.49.0 eslint-config-turbo: specifier: latest - version: 1.10.13(eslint@8.49.0) + version: 1.10.14(eslint@8.49.0) eslint-plugin-import: specifier: 2.26.x version: 2.26.0(@typescript-eslint/parser@6.7.2)(eslint@8.49.0) @@ -1047,6 +1053,9 @@ importers: '@types/serve-static': specifier: ^1.13.10 version: 1.13.10 + '@types/shell-quote': + specifier: ^1.7.2 + version: 1.7.2 '@types/signal-exit': specifier: ^3.0.1 version: 3.0.1 @@ -1194,6 +1203,9 @@ importers: serve-static: specifier: ^1.15.0 version: 1.15.0(supports-color@9.2.2) + shell-quote: + specifier: ^1.8.1 + version: 1.8.1 shellac: specifier: ^0.8.0 version: 0.8.0 @@ -1284,7 +1296,7 @@ packages: engines: {node: '>=6.9.0'} dev: true - /@babel/core@7.22.20(supports-color@9.2.2): + /@babel/core@7.22.20: resolution: {integrity: sha512-Y6jd1ahLubuYweD/zJH+vvOY141v4f9igNQAQ+MBgq9JlHS2iTsZKn1aMsb3vGccZsXI16VzTBw52Xx0DWmtnA==} engines: {node: '>=6.9.0'} dependencies: @@ -1293,10 +1305,10 @@ packages: '@babel/generator': 7.22.15 '@babel/helper-compilation-targets': 7.22.15 '@babel/helper-module-transforms': 7.22.20(@babel/core@7.22.20) - '@babel/helpers': 7.22.15(supports-color@9.2.2) + '@babel/helpers': 7.22.15 '@babel/parser': 7.22.16 '@babel/template': 7.22.15 - '@babel/traverse': 7.22.20(supports-color@9.2.2) + '@babel/traverse': 7.22.20 '@babel/types': 7.22.19 convert-source-map: 1.8.0 debug: 4.3.4(supports-color@9.2.2) @@ -1500,7 +1512,7 @@ packages: peerDependencies: '@babel/core': ^7.0.0 dependencies: - '@babel/core': 7.22.20(supports-color@9.2.2) + '@babel/core': 7.22.20 '@babel/helper-environment-visitor': 7.22.20 '@babel/helper-module-imports': 7.22.15 '@babel/helper-simple-access': 7.22.5 @@ -1620,12 +1632,12 @@ packages: - supports-color dev: true - /@babel/helpers@7.22.15(supports-color@9.2.2): + /@babel/helpers@7.22.15: resolution: {integrity: sha512-7pAjK0aSdxOwR+CcYAqgWOGy5dcfvzsTIfFTb2odQqW47MDfv14UaJDY6eng8ylM2EaeKXdxaSWESbkmaQHTmw==} engines: {node: '>=6.9.0'} dependencies: '@babel/template': 7.22.15 - '@babel/traverse': 7.22.20(supports-color@9.2.2) + '@babel/traverse': 7.22.20 '@babel/types': 7.22.19 transitivePeerDependencies: - supports-color @@ -2372,7 +2384,7 @@ packages: peerDependencies: '@babel/core': ^7.0.0-0 dependencies: - '@babel/core': 7.22.20(supports-color@9.2.2) + '@babel/core': 7.22.20 '@babel/helper-plugin-utils': 7.22.5 dev: true @@ -2382,7 +2394,7 @@ packages: peerDependencies: '@babel/core': ^7.0.0-0 dependencies: - '@babel/core': 7.22.20(supports-color@9.2.2) + '@babel/core': 7.22.20 '@babel/helper-plugin-utils': 7.22.5 dev: true @@ -2710,7 +2722,7 @@ packages: '@babel/types': 7.22.5 dev: true - /@babel/traverse@7.22.20(supports-color@9.2.2): + /@babel/traverse@7.22.20: resolution: {integrity: sha512-eU260mPZbU7mZ0N+X10pxXhQFMGTeLb9eFS0mxehS8HZp9o1uSnFeWQuG1UPrlxgA7QoUzFhOnilHDp0AXCyHw==} engines: {node: '>=6.9.0'} dependencies: @@ -5868,6 +5880,10 @@ packages: '@types/node': 20.1.7 dev: true + /@types/shell-quote@1.7.2: + resolution: {integrity: sha512-p3SZxGp6LXB6RPdMpJmquKjaxQlN/ijyBLTKGPg9IJK6J2g2sJsMmtXP9kNR+Axxi6MDKN2e/76HCOUmvkIpcw==} + dev: true + /@types/signal-exit@3.0.1: resolution: {integrity: sha512-OSitN9PP9E/c4tlt1Qdj3CAz5uHD9Da5rhUqlaKyQRCX1T7Zdpbk6YdeZbR2eiE2ce+NMBgVnMxGqpaPSNQDUQ==} dev: true @@ -6502,7 +6518,7 @@ packages: peerDependencies: vite: ^4.2.0 dependencies: - '@babel/core': 7.22.20(supports-color@9.2.2) + '@babel/core': 7.22.20 '@babel/plugin-transform-react-jsx-self': 7.22.5(@babel/core@7.22.20) '@babel/plugin-transform-react-jsx-source': 7.22.5(@babel/core@7.22.20) react-refresh: 0.14.0 @@ -7833,7 +7849,7 @@ packages: date-fns: 2.30.0 lodash: 4.17.21 rxjs: 7.8.0 - shell-quote: 1.8.0 + shell-quote: 1.8.1 spawn-command: 0.0.2-1 supports-color: 8.1.1 tree-kill: 1.2.2 @@ -7849,7 +7865,7 @@ packages: date-fns: 2.30.0 lodash: 4.17.21 rxjs: 7.8.0 - shell-quote: 1.8.0 + shell-quote: 1.8.1 spawn-command: 0.0.2-1 supports-color: 8.1.1 tree-kill: 1.2.2 @@ -8867,13 +8883,13 @@ packages: source-map: 0.6.1 dev: true - /eslint-config-turbo@1.10.13(eslint@8.49.0): - resolution: {integrity: sha512-Ffa0SxkRCPMtfUX/HDanEqsWoLwZTQTAXO9W4IsOtycb2MzJDrVcLmoFW5sMwCrg7gjqbrC4ZJoD+1SPPzIVqg==} + /eslint-config-turbo@1.10.14(eslint@8.49.0): + resolution: {integrity: sha512-ZeB+IcuFXy1OICkLuAplVa0euoYbhK+bMEQd0nH9+Lns18lgZRm33mVz/iSoH9VdUzl/1ZmFmoK+RpZc+8R80A==} peerDependencies: eslint: '>6.6.0' dependencies: eslint: 8.49.0 - eslint-plugin-turbo: 1.10.13(eslint@8.49.0) + eslint-plugin-turbo: 1.10.14(eslint@8.49.0) dev: false /eslint-import-resolver-node@0.3.7: @@ -9208,8 +9224,8 @@ packages: - typescript dev: true - /eslint-plugin-turbo@1.10.13(eslint@8.49.0): - resolution: {integrity: sha512-el4AAmn0zXmvHEyp1h0IQMfse10Vy8g5Vbg4IU3+vD9CSj5sDbX07iFVt8sCKg7og9Q5FAa9mXzlCf7t4vYgzg==} + /eslint-plugin-turbo@1.10.14(eslint@8.49.0): + resolution: {integrity: sha512-sBdBDnYr9AjT1g4lR3PBkZDonTrMnR4TvuGv5W0OiF7z9az1rI68yj2UHJZvjkwwcGu5mazWA1AfB0oaagpmfg==} peerDependencies: eslint: '>6.6.0' dependencies: @@ -13301,7 +13317,7 @@ packages: minimatch: 3.1.2 pidtree: 0.3.1 read-pkg: 3.0.0 - shell-quote: 1.8.0 + shell-quote: 1.8.1 string.prototype.padend: 3.1.3 dev: true @@ -14333,7 +14349,7 @@ packages: /react-devtools-core@4.23.0: resolution: {integrity: sha512-KkzneT1LczFtebbTJlvRphIRvzuHLhI9ghfrseVv9ktBs+l2cXy8Svw5U16lzQnwU9okVEcURmGPgH79WWrlaw==} dependencies: - shell-quote: 1.8.0 + shell-quote: 1.8.1 ws: 7.5.6 transitivePeerDependencies: - bufferutil @@ -15295,8 +15311,8 @@ packages: resolution: {integrity: sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A==} engines: {node: '>=8'} - /shell-quote@1.8.0: - resolution: {integrity: sha512-QHsz8GgQIGKlRi24yFc6a6lN69Idnx634w49ay6+jA5yFh7a1UY+4Rp6HPx/L/1zcEDPEij8cIsiqR6bQsE5VQ==} + /shell-quote@1.8.1: + resolution: {integrity: sha512-6j1W9l1iAs/4xYBI1SYOVZyFcCis9b4KCLQ8fgAGG07QvzaRLVVRQvAy85yNmmZSjYjg4MWh4gNvlPujU/5LpA==} dev: true /shellac@0.8.0: From a99f872bf16a20bde4a46074a6d56665f05c4c65 Mon Sep 17 00:00:00 2001 From: Rahul Sethi <5822355+RamIdeas@users.noreply.github.com> Date: Sun, 1 Oct 2023 13:46:08 +0100 Subject: [PATCH 2/5] wrap shell-quote.parse to match our needs: - return string[], not including objects - passthrough glob patterns - throw when using non-simple commands --- .../create-cloudflare/e2e-tests/cli.test.ts | 6 ++-- packages/create-cloudflare/src/common.ts | 2 +- .../src/helpers/__tests__/command.test.ts | 6 ++-- .../create-cloudflare/src/helpers/command.ts | 4 +-- .../src/helpers/shell-quote.ts | 34 +++++++++++++++++++ .../src/__tests__/helpers/run-wrangler.ts | 4 +-- packages/wrangler/src/init.ts | 8 ++--- packages/wrangler/src/utils/shell-quote.ts | 34 +++++++++++++++++++ 8 files changed, 82 insertions(+), 16 deletions(-) create mode 100644 packages/create-cloudflare/src/helpers/shell-quote.ts create mode 100644 packages/wrangler/src/utils/shell-quote.ts diff --git a/packages/create-cloudflare/e2e-tests/cli.test.ts b/packages/create-cloudflare/e2e-tests/cli.test.ts index 8464af1e91bd..0b1122bb370f 100644 --- a/packages/create-cloudflare/e2e-tests/cli.test.ts +++ b/packages/create-cloudflare/e2e-tests/cli.test.ts @@ -1,7 +1,7 @@ import { existsSync, rmSync, mkdtempSync, realpathSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; -import shellquote from "shell-quote"; +import * as shellquote from "../src/helpers/shell-quote"; import { beforeEach, afterEach, describe, test, expect } from "vitest"; import { version } from "../package.json"; import { frameworkToTest } from "./frameworkToTest"; @@ -31,7 +31,7 @@ describe.skipIf(frameworkToTest || isQuarantineMode())( }); test("--version with positionals", async () => { - const argv = shellquote.parse("foo bar baz --version") as string[]; + const argv = shellquote.parse("foo bar baz --version"); const { output } = await runC3({ argv }); expect(output).toEqual(version); }); @@ -39,7 +39,7 @@ describe.skipIf(frameworkToTest || isQuarantineMode())( test("--version with flags", async () => { const argv = shellquote.parse( "foo --type webFramework --no-deploy --version" - ) as string[]; + ); const { output } = await runC3({ argv }); expect(output).toEqual(version); }); diff --git a/packages/create-cloudflare/src/common.ts b/packages/create-cloudflare/src/common.ts index ad29fd2436f3..bef9ac9ea7e0 100644 --- a/packages/create-cloudflare/src/common.ts +++ b/packages/create-cloudflare/src/common.ts @@ -1,7 +1,7 @@ import { existsSync, mkdirSync, readdirSync } from "fs"; import { basename, dirname, resolve } from "path"; import { chdir } from "process"; -import shellquote from "shell-quote"; +import * as shellquote from "./helpers/shell-quote"; import { getFrameworkCli } from "frameworks/index"; import { processArgument } from "helpers/args"; import { diff --git a/packages/create-cloudflare/src/helpers/__tests__/command.test.ts b/packages/create-cloudflare/src/helpers/__tests__/command.test.ts index 53d9838550fb..c737248f5da7 100644 --- a/packages/create-cloudflare/src/helpers/__tests__/command.test.ts +++ b/packages/create-cloudflare/src/helpers/__tests__/command.test.ts @@ -1,6 +1,6 @@ import { spawn } from "cross-spawn"; import { detectPackageManager } from "helpers/packages"; -import shellquote from "shell-quote"; +import * as shellquote from "../shell-quote"; import { beforeEach, afterEach, describe, expect, test, vi } from "vitest"; import whichPMRuns from "which-pm-runs"; import { @@ -58,7 +58,7 @@ describe("Command Helpers", () => { }); const expectSpawnWith = (cmd: string) => { - const [command, ...args] = shellquote.parse(cmd) as string[]; + const [command, ...args] = shellquote.parse(cmd); expect(spawn).toHaveBeenCalledWith(command, args, { stdio: "inherit", @@ -67,7 +67,7 @@ describe("Command Helpers", () => { }; const expectSilentSpawnWith = (cmd: string) => { - const [command, ...args] = shellquote.parse(cmd) as string[]; + const [command, ...args] = shellquote.parse(cmd); expect(spawn).toHaveBeenCalledWith(command, args, { stdio: "pipe", diff --git a/packages/create-cloudflare/src/helpers/command.ts b/packages/create-cloudflare/src/helpers/command.ts index 351763113e6d..5163cbb97055 100644 --- a/packages/create-cloudflare/src/helpers/command.ts +++ b/packages/create-cloudflare/src/helpers/command.ts @@ -1,7 +1,7 @@ import { existsSync, rmSync } from "fs"; import path from "path"; import { spawn } from "cross-spawn"; -import shellquote from "shell-quote"; +import * as shellquote from "./shell-quote"; import { endSection, stripAnsi } from "./cli"; import { brandColor, dim } from "./colors"; import { spinner } from "./interactive"; @@ -48,7 +48,7 @@ export const runCommand = async ( opts: RunOptions = {} ): Promise => { if (typeof command === "string") { - command = shellquote.parse(command) as string[]; + command = shellquote.parse(command); } return printAsyncStatus({ diff --git a/packages/create-cloudflare/src/helpers/shell-quote.ts b/packages/create-cloudflare/src/helpers/shell-quote.ts new file mode 100644 index 000000000000..5903cbb92daf --- /dev/null +++ b/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[] { + 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; +} diff --git a/packages/wrangler/src/__tests__/helpers/run-wrangler.ts b/packages/wrangler/src/__tests__/helpers/run-wrangler.ts index 20d2945ad334..6cfbb994bc18 100644 --- a/packages/wrangler/src/__tests__/helpers/run-wrangler.ts +++ b/packages/wrangler/src/__tests__/helpers/run-wrangler.ts @@ -1,4 +1,4 @@ -import shellquote from "shell-quote"; +import * as shellquote from "../../utils/shell-quote"; import { main } from "../../index"; import { normalizeSlashes, stripTimings } from "./mock-console"; @@ -7,7 +7,7 @@ import { normalizeSlashes, stripTimings } from "./mock-console"; */ export async function runWrangler(cmd = "") { try { - const argv = shellquote.parse(cmd) as string[]; + const argv = shellquote.parse(cmd); await main(argv); } catch (err) { if (err instanceof Error) { diff --git a/packages/wrangler/src/init.ts b/packages/wrangler/src/init.ts index 5073d799448c..823d61c8112f 100644 --- a/packages/wrangler/src/init.ts +++ b/packages/wrangler/src/init.ts @@ -4,7 +4,7 @@ import path, { dirname } from "node:path"; import TOML from "@iarna/toml"; import { execa } from "execa"; import { findUp } from "find-up"; -import shellquote from "shell-quote"; +import * as shellquote from "./utils/shell-quote"; import { version as wranglerVersion } from "../package.json"; import { fetchResult } from "./cfetch"; @@ -185,7 +185,7 @@ export async function initHandler(args: InitArgs) { } const c3Arguments = [ - ...(shellquote.parse(getC3CommandFromEnv()) as string[]), + ...shellquote.parse(getC3CommandFromEnv()), fromDashScriptName, ...(yesFlag && isNpm(packageManager) ? ["-y"] : []), // --yes arg for npx ...(isNpm(packageManager) ? ["--"] : []), @@ -253,9 +253,7 @@ export async function initHandler(args: InitArgs) { c3Arguments.unshift("-y"); // arg for npx } - c3Arguments.unshift( - ...(shellquote.parse(getC3CommandFromEnv()) as string[]) - ); + c3Arguments.unshift(...shellquote.parse(getC3CommandFromEnv())); // Deprecate the `init --from-dash` command const replacementC3Command = `\`${packageManager.type} ${c3Arguments.join( diff --git a/packages/wrangler/src/utils/shell-quote.ts b/packages/wrangler/src/utils/shell-quote.ts new file mode 100644 index 000000000000..5903cbb92daf --- /dev/null +++ b/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[] { + 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; +} From d79639cffb1b50088be67b0e839dbf69735fb915 Mon Sep 17 00:00:00 2001 From: Rahul Sethi <5822355+RamIdeas@users.noreply.github.com> Date: Sun, 1 Oct 2023 13:46:46 +0100 Subject: [PATCH 3/5] fix tests with incorrect CLI args formatting --- packages/wrangler/src/__tests__/d1/execute.test.ts | 2 +- packages/wrangler/src/__tests__/deploy.test.ts | 2 +- packages/wrangler/src/__tests__/kv.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/wrangler/src/__tests__/d1/execute.test.ts b/packages/wrangler/src/__tests__/d1/execute.test.ts index d77ff76cd7e6..257375d9cc44 100644 --- a/packages/wrangler/src/__tests__/d1/execute.test.ts +++ b/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.` ); diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index fe3a4a41241c..bdaaa89cda24 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/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( diff --git a/packages/wrangler/src/__tests__/kv.test.ts b/packages/wrangler/src/__tests__/kv.test.ts index d9a39e934c7e..4073689dcbf1 100644 --- a/packages/wrangler/src/__tests__/kv.test.ts +++ b/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( From 4bafbacad7e979c0a1427f3c460bf1f432439f04 Mon Sep 17 00:00:00 2001 From: Rahul Sethi <5822355+RamIdeas@users.noreply.github.com> Date: Mon, 2 Oct 2023 09:25:55 +0100 Subject: [PATCH 4/5] fix linting --- packages/wrangler/src/__tests__/helpers/run-wrangler.ts | 2 +- packages/wrangler/src/init.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/wrangler/src/__tests__/helpers/run-wrangler.ts b/packages/wrangler/src/__tests__/helpers/run-wrangler.ts index 6cfbb994bc18..957807e4c403 100644 --- a/packages/wrangler/src/__tests__/helpers/run-wrangler.ts +++ b/packages/wrangler/src/__tests__/helpers/run-wrangler.ts @@ -1,5 +1,5 @@ -import * as shellquote from "../../utils/shell-quote"; import { main } from "../../index"; +import * as shellquote from "../../utils/shell-quote"; import { normalizeSlashes, stripTimings } from "./mock-console"; /** diff --git a/packages/wrangler/src/init.ts b/packages/wrangler/src/init.ts index 823d61c8112f..2ff4d1540d38 100644 --- a/packages/wrangler/src/init.ts +++ b/packages/wrangler/src/init.ts @@ -4,7 +4,6 @@ import path, { dirname } from "node:path"; import TOML from "@iarna/toml"; import { execa } from "execa"; import { findUp } from "find-up"; -import * as shellquote from "./utils/shell-quote"; import { version as wranglerVersion } from "../package.json"; import { fetchResult } from "./cfetch"; @@ -18,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"; From ea0e5e73ba9f389ad60462e4fc4097a3f3f36a02 Mon Sep 17 00:00:00 2001 From: Rahul Sethi <5822355+RamIdeas@users.noreply.github.com> Date: Mon, 2 Oct 2023 10:07:02 +0100 Subject: [PATCH 5/5] fix linting in c3 --- packages/create-cloudflare/e2e-tests/cli.test.ts | 2 +- packages/create-cloudflare/src/common.ts | 2 +- .../create-cloudflare/src/helpers/__tests__/command.test.ts | 2 +- packages/create-cloudflare/src/helpers/command.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/create-cloudflare/e2e-tests/cli.test.ts b/packages/create-cloudflare/e2e-tests/cli.test.ts index 0b1122bb370f..906d2b5586c2 100644 --- a/packages/create-cloudflare/e2e-tests/cli.test.ts +++ b/packages/create-cloudflare/e2e-tests/cli.test.ts @@ -1,9 +1,9 @@ import { existsSync, rmSync, mkdtempSync, realpathSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; -import * as shellquote from "../src/helpers/shell-quote"; 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"; diff --git a/packages/create-cloudflare/src/common.ts b/packages/create-cloudflare/src/common.ts index bef9ac9ea7e0..d543b77c7fe2 100644 --- a/packages/create-cloudflare/src/common.ts +++ b/packages/create-cloudflare/src/common.ts @@ -1,7 +1,6 @@ import { existsSync, mkdirSync, readdirSync } from "fs"; import { basename, dirname, resolve } from "path"; import { chdir } from "process"; -import * as shellquote from "./helpers/shell-quote"; import { getFrameworkCli } from "frameworks/index"; import { processArgument } from "helpers/args"; import { @@ -29,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(); diff --git a/packages/create-cloudflare/src/helpers/__tests__/command.test.ts b/packages/create-cloudflare/src/helpers/__tests__/command.test.ts index c737248f5da7..5aaeb2761091 100644 --- a/packages/create-cloudflare/src/helpers/__tests__/command.test.ts +++ b/packages/create-cloudflare/src/helpers/__tests__/command.test.ts @@ -1,6 +1,5 @@ import { spawn } from "cross-spawn"; import { detectPackageManager } from "helpers/packages"; -import * as shellquote from "../shell-quote"; import { beforeEach, afterEach, describe, expect, test, vi } from "vitest"; import whichPMRuns from "which-pm-runs"; import { @@ -10,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; diff --git a/packages/create-cloudflare/src/helpers/command.ts b/packages/create-cloudflare/src/helpers/command.ts index 5163cbb97055..35957f2f59a4 100644 --- a/packages/create-cloudflare/src/helpers/command.ts +++ b/packages/create-cloudflare/src/helpers/command.ts @@ -1,11 +1,11 @@ import { existsSync, rmSync } from "fs"; import path from "path"; import { spawn } from "cross-spawn"; -import * as shellquote from "./shell-quote"; 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"; /**