diff --git a/.changeset/sour-buses-develop.md b/.changeset/sour-buses-develop.md new file mode 100644 index 000000000000..09a6355da4a2 --- /dev/null +++ b/.changeset/sour-buses-develop.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +fix: prevent users from passing multiple arguments to non array options diff --git a/packages/wrangler/src/__tests__/core/command-registration.test.ts b/packages/wrangler/src/__tests__/core/command-registration.test.ts index 538b38761cc4..2308f12e75cf 100644 --- a/packages/wrangler/src/__tests__/core/command-registration.test.ts +++ b/packages/wrangler/src/__tests__/core/command-registration.test.ts @@ -484,6 +484,17 @@ describe("Command Registration", () => { `[Error: Missing namespace definition for 'wrangler missing-namespace']` ); }); + + test("throws upon duplicated arguments on non-array options", async () => { + await expect( + runWrangler( + "my-test-command --str foo --str bar --num 123 --bool --arr first second --str baz" + ) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `[Error: The argument "--str" expects a single value, but received multiple: ["foo","bar","baz"].]` + ); + }); + test("throws upon alias to undefined command", async () => { defineAlias({ command: "wrangler my-alias-command", diff --git a/packages/wrangler/src/__tests__/index.test.ts b/packages/wrangler/src/__tests__/index.test.ts index ac55c7326f60..7b3fe0ed9dfb 100644 --- a/packages/wrangler/src/__tests__/index.test.ts +++ b/packages/wrangler/src/__tests__/index.test.ts @@ -140,6 +140,22 @@ describe("wrangler", () => { }); }); + describe("global options", () => { + it("should display an error if duplicated --env or --config arguments are provided", async () => { + await expect( + runWrangler("--env prod -e prod") + ).rejects.toThrowErrorMatchingInlineSnapshot( + `[Error: The argument "--env" expects a single value, but received multiple: ["prod","prod"].]` + ); + + await expect( + runWrangler("--config=wrangler.toml -c example") + ).rejects.toThrowErrorMatchingInlineSnapshot( + `[Error: The argument "--config" expects a single value, but received multiple: ["wrangler.toml","example"].]` + ); + }); + }); + describe("preview", () => { it("should throw an error if the deprecated command is used with positional arguments", async () => { await expect(runWrangler("preview GET")).rejects diff --git a/packages/wrangler/src/core/helpers.ts b/packages/wrangler/src/core/helpers.ts new file mode 100644 index 000000000000..17ed1783df2f --- /dev/null +++ b/packages/wrangler/src/core/helpers.ts @@ -0,0 +1,48 @@ +import { CommandLineArgsError } from "../errors"; + +/** + * A helper to demand one of a set of options + * via https://github.com/yargs/yargs/issues/1093#issuecomment-491299261 + */ +export function demandOneOfOption(...options: string[]) { + return function (argv: { [key: string]: unknown }) { + const count = options.filter((option) => argv[option]).length; + const lastOption = options.pop(); + + if (count === 0) { + throw new CommandLineArgsError( + `Exactly one of the arguments ${options.join( + ", " + )} and ${lastOption} is required` + ); + } else if (count > 1) { + throw new CommandLineArgsError( + `Arguments ${options.join( + ", " + )} and ${lastOption} are mutually exclusive` + ); + } + + return true; + }; +} + +/** + * A helper to ensure that an argument only receives a single value. + * + * This is a workaround for a limitation in yargs where non-array arguments can still receive multiple values + * even though the inferred type is not an array. + * + * @see https://github.com/yargs/yargs/issues/1318 + */ +export function demandSingleValue(key: string) { + return function (argv: { [key: string]: unknown }) { + if (Array.isArray(argv[key])) { + throw new CommandLineArgsError( + `The argument "--${key}" expects a single value, but received multiple: ${JSON.stringify(argv[key])}.` + ); + } + + return true; + }; +} diff --git a/packages/wrangler/src/core/index.ts b/packages/wrangler/src/core/index.ts index 70dc50053d0c..0ac285cb531c 100644 --- a/packages/wrangler/src/core/index.ts +++ b/packages/wrangler/src/core/index.ts @@ -1 +1,2 @@ export { defineAlias, defineCommand, defineNamespace } from "./define-command"; +export { demandOneOfOption, demandSingleValue } from "./helpers"; diff --git a/packages/wrangler/src/core/register-commands.ts b/packages/wrangler/src/core/register-commands.ts index d1ecfdffaab3..76921508da49 100644 --- a/packages/wrangler/src/core/register-commands.ts +++ b/packages/wrangler/src/core/register-commands.ts @@ -6,6 +6,7 @@ import { FatalError, UserError } from "../errors"; import { logger } from "../logger"; import { printWranglerBanner } from "../update-check"; import { CommandRegistrationError, DefinitionTreeRoot } from "./define-command"; +import { demandSingleValue } from "./helpers"; import type { CommonYargsArgv, SubHelp } from "../yargs-types"; import type { Command, @@ -125,6 +126,17 @@ function walkTreeAndRegister( if (def.type === "command") { yargs.options(def.args); + // Yargs offers an `array: true` option that will always coerces the value to an array + // e.g. `--name foo` becomes `{ name: ["foo"] }` instead of `{ name: "foo" }` + // However, non-array arguments can still receives multiple values + // e.g. `--name foo --name bar` becomes `{ name: ["foo", "bar"] }` regardless of the `array` setting + // @see https://github.com/yargs/yargs/issues/1318 + for (const [key, opt] of Object.entries(def.args)) { + if (!opt.array) { + yargs.check(demandSingleValue(key)); + } + } + for (const key of def.positionalArgs ?? []) { yargs.positional(key, def.args[key]); } diff --git a/packages/wrangler/src/errors.ts b/packages/wrangler/src/errors.ts index 68d10df821eb..e273d9572c98 100644 --- a/packages/wrangler/src/errors.ts +++ b/packages/wrangler/src/errors.ts @@ -28,6 +28,8 @@ export class FatalError extends UserError { } } +export class CommandLineArgsError extends UserError {} + /** * JsonFriendlyFatalError is used to output JSON when wrangler crashes, useful for --json mode. * diff --git a/packages/wrangler/src/generate/index.ts b/packages/wrangler/src/generate/index.ts index 1d9ab282da32..765a1b1b40a1 100644 --- a/packages/wrangler/src/generate/index.ts +++ b/packages/wrangler/src/generate/index.ts @@ -2,9 +2,9 @@ import fs from "node:fs"; import path from "node:path"; import { execa } from "execa"; import { getC3CommandFromEnv } from "../environment-variables/misc-variables"; -import { UserError } from "../errors"; +import { CommandLineArgsError, UserError } from "../errors"; import { cloneIntoDirectory, initializeGit } from "../git-client"; -import { CommandLineArgsError, printWranglerBanner } from "../index"; +import { printWranglerBanner } from "../index"; import { initHandler } from "../init"; import { logger } from "../logger"; import { getPackageManager } from "../package-manager"; diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts index 458eb44894ad..71a4654a7cb0 100644 --- a/packages/wrangler/src/index.ts +++ b/packages/wrangler/src/index.ts @@ -38,12 +38,17 @@ import { import { devHandler, devOptions } from "./dev"; import { workerNamespaceCommands } from "./dispatch-namespace"; import { docsHandler, docsOptions } from "./docs"; -import { JsonFriendlyFatalError, UserError } from "./errors"; +import { + CommandLineArgsError, + JsonFriendlyFatalError, + UserError, +} from "./errors"; import { generateHandler, generateOptions } from "./generate"; import { hyperdrive } from "./hyperdrive/index"; import { initHandler, initOptions } from "./init"; import "./kv"; import "./workflows"; +import { demandSingleValue } from "./core"; import { logBuildFailure, logger, LOGGER_LEVELS } from "./logger"; import * as metrics from "./metrics"; import { mTlsCertificateCommands } from "./mtls-certificate/cli"; @@ -159,35 +164,6 @@ export function getLegacyScriptName( : args.name ?? config.name; } -/** - * A helper to demand one of a set of options - * via https://github.com/yargs/yargs/issues/1093#issuecomment-491299261 - */ -export function demandOneOfOption(...options: string[]) { - return function (argv: { [key: string]: unknown }) { - const count = options.filter((option) => argv[option]).length; - const lastOption = options.pop(); - - if (count === 0) { - throw new CommandLineArgsError( - `Exactly one of the arguments ${options.join( - ", " - )} and ${lastOption} is required` - ); - } else if (count > 1) { - throw new CommandLineArgsError( - `Arguments ${options.join( - ", " - )} and ${lastOption} are mutually exclusive` - ); - } - - return true; - }; -} - -export class CommandLineArgsError extends UserError {} - export function createCLIParser(argv: string[]) { const experimentalGradualRollouts = // original flag -- using internal product name (Gradual Rollouts) -- kept for temp back-compat @@ -230,12 +206,14 @@ export function createCLIParser(argv: string[]) { type: "string", requiresArg: true, }) + .check(demandSingleValue("config")) .option("env", { alias: "e", describe: "Environment to use for operations and .env files", type: "string", requiresArg: true, }) + .check(demandSingleValue("env")) .option("experimental-json-config", { alias: "j", describe: `Experimental: support wrangler.json`, diff --git a/packages/wrangler/src/init.ts b/packages/wrangler/src/init.ts index bd56661fde0d..124e14a74225 100644 --- a/packages/wrangler/src/init.ts +++ b/packages/wrangler/src/init.ts @@ -12,7 +12,7 @@ import { readConfig } from "./config"; import { getDatabaseInfoFromId } from "./d1/utils"; import { confirm, select } from "./dialogs"; import { getC3CommandFromEnv } from "./environment-variables/misc-variables"; -import { FatalError, UserError } from "./errors"; +import { CommandLineArgsError, FatalError, UserError } from "./errors"; import { getGitVersioon, initializeGit, isInsideGitRepo } from "./git-client"; import { logger } from "./logger"; import { getPackageManager } from "./package-manager"; @@ -21,7 +21,7 @@ import { getBasePath } from "./paths"; import { requireAuth } from "./user"; import { createBatches } from "./utils/create-batches"; import * as shellquote from "./utils/shell-quote"; -import { CommandLineArgsError, printWranglerBanner } from "./index"; +import { printWranglerBanner } from "./index"; import type { RawConfig } from "./config"; import type { CustomDomainRoute, diff --git a/packages/wrangler/src/kv/index.ts b/packages/wrangler/src/kv/index.ts index 3f8e19600ae1..3e47314fe535 100644 --- a/packages/wrangler/src/kv/index.ts +++ b/packages/wrangler/src/kv/index.ts @@ -2,10 +2,14 @@ import { Blob } from "node:buffer"; import { arrayBuffer } from "node:stream/consumers"; import { StringDecoder } from "node:string_decoder"; import { readConfig } from "../config"; -import { defineAlias, defineCommand, defineNamespace } from "../core"; +import { + defineAlias, + defineCommand, + defineNamespace, + demandOneOfOption, +} from "../core"; import { confirm } from "../dialogs"; -import { UserError } from "../errors"; -import { CommandLineArgsError, demandOneOfOption } from "../index"; +import { CommandLineArgsError, UserError } from "../errors"; import { logger } from "../logger"; import * as metrics from "../metrics"; import { parseJSON, readFileSync, readFileSyncToBuffer } from "../parse"; diff --git a/packages/wrangler/src/pubsub/pubsub-commands.ts b/packages/wrangler/src/pubsub/pubsub-commands.ts index ccc4e5b190a3..8913a639f7e2 100644 --- a/packages/wrangler/src/pubsub/pubsub-commands.ts +++ b/packages/wrangler/src/pubsub/pubsub-commands.ts @@ -1,6 +1,6 @@ import { readConfig } from "../config"; import { confirm } from "../dialogs"; -import { CommandLineArgsError } from "../index"; +import { CommandLineArgsError } from "../errors"; import { logger } from "../logger"; import * as metrics from "../metrics"; import { parseHumanDuration } from "../parse"; diff --git a/packages/wrangler/src/queues/cli/commands/consumer/http-pull/add.ts b/packages/wrangler/src/queues/cli/commands/consumer/http-pull/add.ts index 808999dadfcc..961263e9e5d1 100644 --- a/packages/wrangler/src/queues/cli/commands/consumer/http-pull/add.ts +++ b/packages/wrangler/src/queues/cli/commands/consumer/http-pull/add.ts @@ -1,5 +1,5 @@ import { readConfig } from "../../../../../config"; -import { CommandLineArgsError } from "../../../../../index"; +import { CommandLineArgsError } from "../../../../../errors"; import { logger } from "../../../../../logger"; import { postConsumer } from "../../../../client"; import type { diff --git a/packages/wrangler/src/queues/cli/commands/consumer/worker/add.ts b/packages/wrangler/src/queues/cli/commands/consumer/worker/add.ts index 86606a5b8c45..f98794c28c0d 100644 --- a/packages/wrangler/src/queues/cli/commands/consumer/worker/add.ts +++ b/packages/wrangler/src/queues/cli/commands/consumer/worker/add.ts @@ -1,5 +1,5 @@ import { readConfig } from "../../../../../config"; -import { CommandLineArgsError } from "../../../../../index"; +import { CommandLineArgsError } from "../../../../../errors"; import { logger } from "../../../../../logger"; import { postConsumer } from "../../../../client"; import type { diff --git a/packages/wrangler/src/queues/cli/commands/create.ts b/packages/wrangler/src/queues/cli/commands/create.ts index 5d39f8dd1315..d3a06f1a0623 100644 --- a/packages/wrangler/src/queues/cli/commands/create.ts +++ b/packages/wrangler/src/queues/cli/commands/create.ts @@ -1,5 +1,5 @@ import { readConfig } from "../../../config"; -import { CommandLineArgsError } from "../../../index"; +import { CommandLineArgsError } from "../../../errors"; import { logger } from "../../../logger"; import { createQueue } from "../../client"; import { handleFetchError } from "../../utils"; diff --git a/packages/wrangler/src/r2/index.ts b/packages/wrangler/src/r2/index.ts index 8f936339bfea..faee2f2996a7 100644 --- a/packages/wrangler/src/r2/index.ts +++ b/packages/wrangler/src/r2/index.ts @@ -5,8 +5,8 @@ import * as stream from "node:stream"; import { ReadableStream } from "node:stream/web"; import prettyBytes from "pretty-bytes"; import { readConfig } from "../config"; -import { FatalError, UserError } from "../errors"; -import { CommandLineArgsError, printWranglerBanner } from "../index"; +import { CommandLineArgsError, FatalError, UserError } from "../errors"; +import { printWranglerBanner } from "../index"; import { logger } from "../logger"; import * as metrics from "../metrics"; import { requireAuth } from "../user"; diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index 10e0e78d11d9..12e571dae63c 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -6,8 +6,7 @@ import { getNodeCompat } from "miniflare"; import { findWranglerToml, readConfig } from "../config"; import { getEntry } from "../deployment-bundle/entry"; import { getVarsForDev } from "../dev/dev-vars"; -import { UserError } from "../errors"; -import { CommandLineArgsError } from "../index"; +import { CommandLineArgsError, UserError } from "../errors"; import { logger } from "../logger"; import { parseJSONC } from "../parse"; import { printWranglerBanner } from "../update-check";