Chore: Prevent repeated cli boolean flags for being accepted (#8898)

* Chore: Prevent repeated cli boolean flags for being accepted

Signed-off-by: Preeti <35308865+pr9t@users.noreply.github.com>

* allow unmatched options

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Preeti <35308865+pr9t@users.noreply.github.com>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Justin Florentine <justin+github@florentine.us>

Author: 3 people

CHANGELOG.md

@@ -8,6 +8,7 @@
 - Remove deprecated `Quantity.getValue` method (deprecated since 2019) [#8968](https://github.com/hyperledger/besu/pull/8968)
 - Support for block creation on networks running a pre-Byzantium fork is removed, after being deprecated for a few months. If still running a pre-Byzantium network, it needs to be updated to continue to produce blocks [#9005](https://github.com/hyperledger/besu/pull/9005)
 - Remove support for Ethereum protocol version `eth/67`. [#9008](https://github.com/hyperledger/besu/pull/9008). 
+- Abort startup if boolean command line options are specified more than once [#8898](https://github.com/hyperledger/besu/pull/8898)
 
 ### Upcoming Breaking Changes
 

app/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

@@ -253,8 +253,11 @@
 import picocli.CommandLine.Command;
 import picocli.CommandLine.ExecutionException;
 import picocli.CommandLine.IExecutionStrategy;
+import picocli.CommandLine.Model.ITypeInfo;
+import picocli.CommandLine.Model.OptionSpec;
 import picocli.CommandLine.Option;
 import picocli.CommandLine.ParameterException;
+import picocli.CommandLine.ParseResult;
 
 /** Represents the main Besu CLI command that runs the Besu Ethereum client full node. */
 @SuppressWarnings("FieldCanBeLocal") // because Picocli injected fields report false positives
@@ -832,6 +835,17 @@ private int executeCommandLine(
       final BesuParameterExceptionHandler parameterExceptionHandler,
       final BesuExecutionExceptionHandler executionExceptionHandler,
       final String... args) {
+
+    try {
+      // Parse and run duplicate-check
+      // As this happens before the plugins registration and plugins can add options, we must
+      // allow unmatched options
+      final ParseResult pr = commandLine.setUnmatchedArgumentsAllowed(true).parseArgs(args);
+      rejectDuplicateScalarOptions(pr); // your generic validator
+    } catch (ParameterException e) {
+      // ← Send it to the standard handler: prints one line & exits status 1
+      return parameterExceptionHandler.handleParseException(e, args);
+    }
     return commandLine
         .setExecutionStrategy(executionStrategy)
         .setParameterExceptionHandler(parameterExceptionHandler)
@@ -846,7 +860,8 @@ private int executeCommandLine(
   public void toCommandLine() {
     commandLine =
         new CommandLine(this, new BesuCommandCustomFactory(besuPluginContext))
-            .setCaseInsensitiveEnumValuesAllowed(true);
+            .setCaseInsensitiveEnumValuesAllowed(true)
+            .setToggleBooleanFlags(false);
   }
 
   @Override
@@ -938,6 +953,33 @@ private void configurePrecompileCaching() {
                 .inc());
   }
 
+  /** Reject any option that is not multi-valued but appears more than once. */
+  private static void rejectDuplicateScalarOptions(final ParseResult pr) {
+    for (OptionSpec spec : pr.matchedOptions()) {
+
+      // skip help/version flags
+      if (spec.usageHelp() || spec.versionHelp()) {
+        continue;
+      }
+
+      // ── determine if this option can repeat
+      ITypeInfo type = spec.typeInfo();
+      boolean multiValued =
+          spec.arity().max() > 1 || type.isMultiValue() || type.isCollection() || type.isArray();
+
+      if (multiValued) {
+        continue; // lists are allowed to repeat
+      }
+
+      // ── single-valued: abort if it appears more than once ───────────
+      if (pr.matchedOption(spec.longestName()).stringValues().size() > 1) {
+        throw new ParameterException(
+            pr.commandSpec().commandLine(),
+            String.format("Option '%s' should be specified only once", spec.longestName()));
+      }
+    }
+  }
+
   private void checkPermissionsAndPrintPaths(final String userName) {
     // Check permissions for the data path
     checkPermissions(dataDir(), userName, false);

app/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java

@@ -39,6 +39,7 @@
 import static org.hyperledger.besu.plugin.services.storage.DataStorageFormat.BONSAI;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.contains;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.isNotNull;
@@ -2770,4 +2771,21 @@ void chainPruningEnabledWithPOA() throws IOException {
         "--version-compatibility-protection=false");
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
+
+  @Test
+  public void shouldLogErrorIfDuplicateBooleanOptionUsed() {
+    parseCommand("--p2p-enabled=true", "--p2p-enabled=false");
+    assertThat(commandErrorOutput.toString(UTF_8))
+        .contains("Option '--p2p-enabled' should be specified only once");
+  }
+
+  @Test
+  public void shouldNotWarnForRepeatableOptionBootnodes() {
+    parseCommand(
+        "enode://" + VALID_NODE_ID + "@192.168.0.1:4567",
+        "enode://" + VALID_NODE_ID + "@192.168.0.2:4567");
+
+    // Verify that the logger does NOT warn about duplication
+    verify(mockLogger, never()).warn(contains("bootnodes"));
+  }
 }