fix: validate and reject duplicate single-value CLI flags

packages/utils/src/command.ts

@@ -46,6 +46,32 @@ export interface CliCommand<OwnArgs = Record<never, never>, ParentArgs = Record<
   handler?: (args: OwnArgs & ParentArgs) => Promise<R>;
 }
 
+/**
+ * Validates that single-value options don't have duplicate values
+ * @param argv Parsed arguments from yargs
+ * @param options Command option definitions
+ * @throws Error if a single-value flag is duplicated
+ */
+// biome-ignore lint/suspicious/noExplicitAny: We need to use `any` type here
+function validateNoDuplicateSingleValueFlags(argv: any, options?: CliCommandOptions<any>): void {
+  if (!options) return;
+
+  for (const [optionName, optionDef] of Object.entries(options)) {
+    // Skip array-type options as they're allowed to have multiple values
+    if (optionDef.type === "array") continue;
+
+    const value = argv[optionName];
+
+    // Check if this single-value option received multiple values (became an array)
+    if (Array.isArray(value)) {
+      throw new Error(
+        `Option '--${optionName}' is a single-value flag but was provided multiple times. ` +
+          `Received values: [${value.join(", ")}]. Please provide only one value.`
+      );
+    }
+  }
+}
+
 /**
  * Register a CliCommand type to yargs. Recursively registers subcommands too.
  * @param yargs
@@ -59,6 +85,12 @@ export function registerCommandToYargs(yargs: Argv, cliCommand: CliCommand<any,
     describe: cliCommand.describe,
     builder: (yargsBuilder) => {
       yargsBuilder.options(cliCommand.options ?? {});
+
+      // Add middleware to validate no duplicate single-value flags
+      yargsBuilder.middleware((argv) => {
+        validateNoDuplicateSingleValueFlags(argv, cliCommand.options);
+      }, true);
+
       for (const subcommand of cliCommand.subcommands ?? []) {
         registerCommandToYargs(yargsBuilder, subcommand);
       }

packages/utils/test/unit/command.test.ts

@@ -0,0 +1,275 @@
+import {describe, expect, it} from "vitest";
+import yargs from "yargs/yargs";
+import {hideBin} from "yargs/helpers";
+import {registerCommandToYargs, CliCommand} from "../../src/command.js";
+
+describe("registerCommandToYargs", () => {
+  describe("duplicate single-value flag validation", () => {
+    it("Should accept single-value string flag with one value", async () => {
+      const command: CliCommand<{url: string}> = {
+        command: "test",
+        describe: "Test command",
+        options: {
+          url: {
+            type: "string",
+            description: "Single value URL",
+          },
+        },
+        handler: async (args) => {
+          expect(args.url).toBe("https://example.com");
+        },
+      };
+
+      const parser = yargs(hideBin(["node", "script", "test", "--url", "https://example.com"]));
+      registerCommandToYargs(parser, command);
+
+      // Should not throw
+      await expect(parser.parse()).resolves.toBeDefined();
+    });
+
+    it("Should accept single-value number flag with one value", async () => {
+      const command: CliCommand<{port: number}> = {
+        command: "test",
+        describe: "Test command",
+        options: {
+          port: {
+            type: "number",
+            description: "Port number",
+          },
+        },
+        handler: async (args) => {
+          expect(args.port).toBe(8080);
+        },
+      };
+
+      const parser = yargs(hideBin(["node", "script", "test", "--port", "8080"]));
+      registerCommandToYargs(parser, command);
+
+      // Should not throw
+      await expect(parser.parse()).resolves.toBeDefined();
+    });
+
+    it("Should accept single-value boolean flag with one value", async () => {
+      const command: CliCommand<{verbose: boolean}> = {
+        command: "test",
+        describe: "Test command",
+        options: {
+          verbose: {
+            type: "boolean",
+            description: "Verbose output",
+          },
+        },
+        handler: async (args) => {
+          expect(args.verbose).toBe(true);
+        },
+      };
+
+      const parser = yargs(hideBin(["node", "script", "test", "--verbose"]));
+      registerCommandToYargs(parser, command);
+
+      // Should not throw
+      await expect(parser.parse()).resolves.toBeDefined();
+    });
+
+    it("Should accept array-type flag with multiple values", async () => {
+      const command: CliCommand<{bootnodes: string[]}> = {
+        command: "test",
+        describe: "Test command",
+        options: {
+          bootnodes: {
+            type: "array",
+            description: "Multiple bootnodes",
+          },
+        },
+        handler: async (args) => {
+          expect(args.bootnodes).toEqual(["node1", "node2"]);
+        },
+      };
+
+      const parser = yargs(hideBin(["node", "script", "test", "--bootnodes", "node1", "--bootnodes", "node2"]));
+      registerCommandToYargs(parser, command);
+
+      // Should not throw - arrays are allowed to have multiple values
+      await expect(parser.parse()).resolves.toBeDefined();
+    });
+
+    it("Should throw error when single-value string flag is duplicated", () => {
+      const command: CliCommand<{url: string}> = {
+        command: "test",
+        describe: "Test command",
+        options: {
+          url: {
+            type: "string",
+            description: "Single value URL",
+          },
+        },
+        handler: async () => {
+          // Should not reach here
+          throw new Error("Handler should not be called");
+        },
+      };
+
+      const parser = yargs(hideBin(["node", "script", "test", "--url", "https://example1.com", "--url", "https://example2.com"]));
+      registerCommandToYargs(parser, command);
+
+      // Should throw with descriptive error
+      expect(() => parser.parseSync()).toThrow(
+        "Option '--url' is a single-value flag but was provided multiple times"
+      );
+      expect(() => parser.parseSync()).toThrow("https://example1.com");
+      expect(() => parser.parseSync()).toThrow("https://example2.com");
+    });
+
+    it("Should throw error when single-value number flag is duplicated", () => {
+      const command: CliCommand<{port: number}> = {
+        command: "test",
+        describe: "Test command",
+        options: {
+          port: {
+            type: "number",
+            description: "Port number",
+          },
+        },
+        handler: async () => {
+          throw new Error("Handler should not be called");
+        },
+      };
+
+      const parser = yargs(hideBin(["node", "script", "test", "--port", "8080", "--port", "9000"]));
+      registerCommandToYargs(parser, command);
+
+      // Should throw with descriptive error
+      expect(() => parser.parseSync()).toThrow(
+        "Option '--port' is a single-value flag but was provided multiple times"
+      );
+    });
+
+    it("Should validate multiple flags independently", async () => {
+      const command: CliCommand<{url: string; port: number; verbose: boolean}> = {
+        command: "test",
+        describe: "Test command",
+        options: {
+          url: {
+            type: "string",
+            description: "Single value URL",
+          },
+          port: {
+            type: "number",
+            description: "Port number",
+          },
+          verbose: {
+            type: "boolean",
+            description: "Verbose output",
+          },
+        },
+        handler: async (args) => {
+          expect(args.url).toBe("https://example.com");
+          expect(args.port).toBe(8080);
+          expect(args.verbose).toBe(true);
+        },
+      };
+
+      const parser = yargs(
+        hideBin(["node", "script", "test", "--url", "https://example.com", "--port", "8080", "--verbose"])
+      );
+      registerCommandToYargs(parser, command);
+
+      // Should not throw when all flags have single values
+      await expect(parser.parse()).resolves.toBeDefined();
+    });
+
+    it("Should throw error for the first duplicated flag encountered", () => {
+      const command: CliCommand<{url: string; checkpointSyncUrl: string}> = {
+        command: "test",
+        describe: "Test command",
+        options: {
+          url: {
+            type: "string",
+            description: "URL",
+          },
+          checkpointSyncUrl: {
+            type: "string",
+            description: "Checkpoint sync URL",
+          },
+        },
+        handler: async () => {
+          throw new Error("Handler should not be called");
+        },
+      };
+
+      const parser = yargs(
+        hideBin([
+          "node",
+          "script",
+          "test",
+          "--url",
+          "https://example1.com",
+          "--url",
+          "https://example2.com",
+          "--checkpointSyncUrl",
+          "https://checkpoint1.com",
+          "--checkpointSyncUrl",
+          "https://checkpoint2.com",
+        ])
+      );
+      registerCommandToYargs(parser, command);
+
+      // Should throw error mentioning at least one of the duplicated flags
+      expect(() => parser.parseSync()).toThrow("is a single-value flag but was provided multiple times");
+    });
+
+    it("Should handle command with no options gracefully", async () => {
+      const command: CliCommand = {
+        command: "test",
+        describe: "Test command",
+        handler: async () => {
+          // Empty handler
+        },
+      };
+
+      const parser = yargs(hideBin(["node", "script", "test"]));
+      registerCommandToYargs(parser, command);
+
+      // Should not throw
+      await expect(parser.parse()).resolves.toBeDefined();
+    });
+
+    it("Should reproduce the exact bug from the issue - checkpointSyncUrl", () => {
+      const command: CliCommand<{checkpointSyncUrl: string}> = {
+        command: "beacon",
+        describe: "Run a beacon chain node",
+        options: {
+          checkpointSyncUrl: {
+            type: "string",
+            description: "Server url hosting Beacon Node APIs to fetch weak subjectivity state",
+          },
+        },
+        handler: async () => {
+          throw new Error("Handler should not be called");
+        },
+      };
+
+      // Exact scenario from the bug report
+      const parser = yargs(
+        hideBin([
+          "node",
+          "lodestar",
+          "beacon",
+          "--checkpointSyncUrl",
+          "https://checkpoint-sync.fusaka-devnet-4.ethpandaops.io",
+          "--checkpointSyncUrl",
+          "https://checkpoint-sync.fusaka-devnet-4.ethpandaops.io",
+        ])
+      );
+      registerCommandToYargs(parser, command);
+
+      // Should throw error instead of silently creating an array
+      expect(() => parser.parseSync()).toThrow(
+        "Option '--checkpointSyncUrl' is a single-value flag but was provided multiple times"
+      );
+      expect(() => parser.parseSync()).toThrow(
+        "https://checkpoint-sync.fusaka-devnet-4.ethpandaops.io"
+      );
+    });
+  });
+});