CLI option for disabling auto-registration of external plugins (#7470)

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java

@@ -432,7 +432,8 @@ public class ThreadBesuNodeRunner implements BesuNodeRunner {
       besuPluginContext.addService(PermissioningService.class, permissioningService);
       besuPluginContext.addService(PrivacyPluginService.class, new PrivacyPluginServiceImpl());
 
-      besuPluginContext.registerPlugins(new PluginConfiguration(pluginsPath));
+      besuPluginContext.registerPlugins(
+          new PluginConfiguration.Builder().pluginsDir(pluginsPath).build());
 
       // register built-in plugins
       new RocksDBPlugin().register(besuPluginContext);

acceptance-tests/test-plugins/src/test/java/org/hyperledger/besu/services/BesuPluginContextImplTest.java

@@ -64,7 +64,8 @@ public class BesuPluginContextImplTest {
   @Test
   public void verifyEverythingGoesSmoothly() {
     assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
-    contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));
+    contextImpl.registerPlugins(
+        PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build());
     assertThat(contextImpl.getRegisteredPlugins()).isNotEmpty();
 
     final Optional<TestPicoCLIPlugin> testPluginOptional =
@@ -86,7 +87,8 @@ public class BesuPluginContextImplTest {
     System.setProperty("testPicoCLIPlugin.testOption", "FAILREGISTER");
 
     assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
-    contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));
+    contextImpl.registerPlugins(
+        PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build());
     assertThat(contextImpl.getRegisteredPlugins()).isNotInstanceOfAny(TestPicoCLIPlugin.class);
 
     contextImpl.beforeExternalServices();
@@ -104,7 +106,8 @@ public class BesuPluginContextImplTest {
     System.setProperty("testPicoCLIPlugin.testOption", "FAILSTART");
 
     assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
-    contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));
+    contextImpl.registerPlugins(
+        PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build());
     assertThat(contextImpl.getRegisteredPlugins())
         .extracting("class")
         .contains(TestPicoCLIPlugin.class);
@@ -129,7 +132,8 @@ public class BesuPluginContextImplTest {
     System.setProperty("testPicoCLIPlugin.testOption", "FAILSTOP");
 
     assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
-    contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));
+    contextImpl.registerPlugins(
+        PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build());
     assertThat(contextImpl.getRegisteredPlugins())
         .extracting("class")
         .contains(TestPicoCLIPlugin.class);
@@ -151,7 +155,9 @@ public class BesuPluginContextImplTest {
   @Test
   public void lifecycleExceptions() throws Throwable {
     final ThrowableAssert.ThrowingCallable registerPlugins =
-        () -> contextImpl.registerPlugins(new PluginConfiguration(DEFAULT_PLUGIN_DIRECTORY));
+        () ->
+            contextImpl.registerPlugins(
+                PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build());
 
     assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::startPlugins);
     assertThatExceptionOfType(IllegalStateException.class).isThrownBy(contextImpl::stopPlugins);
@@ -173,7 +179,8 @@ public class BesuPluginContextImplTest {
 
   @Test
   public void shouldRegisterAllPluginsWhenNoPluginsOption() {
-    final PluginConfiguration config = createConfigurationForAllPlugins();
+    final PluginConfiguration config =
+        PluginConfiguration.builder().pluginsDir(DEFAULT_PLUGIN_DIRECTORY).build();
 
     assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
     contextImpl.registerPlugins(config);
@@ -227,12 +234,33 @@ public class BesuPluginContextImplTest {
     assertThat(contextImpl.getRegisteredPlugins()).isEmpty();
   }
 
-  private PluginConfiguration createConfigurationForAllPlugins() {
-    return new PluginConfiguration(null, DEFAULT_PLUGIN_DIRECTORY);
+  @Test
+  void shouldNotRegisterAnyPluginsIfExternalPluginsDisabled() {
+    PluginConfiguration config =
+        PluginConfiguration.builder()
+            .pluginsDir(DEFAULT_PLUGIN_DIRECTORY)
+            .externalPluginsEnabled(false)
+            .build();
+    contextImpl.registerPlugins(config);
+    assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isTrue();
+  }
+
+  @Test
+  void shouldRegisterPluginsIfExternalPluginsEnabled() {
+    PluginConfiguration config =
+        PluginConfiguration.builder()
+            .pluginsDir(DEFAULT_PLUGIN_DIRECTORY)
+            .externalPluginsEnabled(true)
+            .build();
+    contextImpl.registerPlugins(config);
+    assertThat(contextImpl.getRegisteredPlugins().isEmpty()).isFalse();
   }
 
   private PluginConfiguration createConfigurationForSpecificPlugin(final String pluginName) {
-    return new PluginConfiguration(List.of(new PluginInfo(pluginName)), DEFAULT_PLUGIN_DIRECTORY);
+    return PluginConfiguration.builder()
+        .requestedPlugins(List.of(new PluginInfo(pluginName)))
+        .pluginsDir(DEFAULT_PLUGIN_DIRECTORY)
+        .build();
   }
 
   private Optional<TestPicoCLIPlugin> findTestPlugin(

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

@@ -1122,7 +1122,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
       runner.startExternalServices();
 
       startPlugins(runner);
-      validatePluginOptions();
+      validatePrivacyPluginOptions();
       setReleaseMetrics();
       preSynchronization();
 
@@ -1347,7 +1347,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
     besuPluginContext.startPlugins();
   }
 
-  private void validatePluginOptions() {
+  private void validatePrivacyPluginOptions() {
     // plugins do not 'wire up' until start has been called
     // consequently you can only do some configuration checks
     // after start has been called on plugins
@@ -1491,6 +1491,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
     validateGraphQlOptions();
     validateApiOptions();
     validateConsensusSyncCompatibilityOptions();
+    validatePluginOptions();
     p2pTLSConfigOptions.checkP2PTLSOptionsDependencies(logger, commandLine);
   }
 
@@ -1511,6 +1512,10 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
     }
   }
 
+  private void validatePluginOptions() {
+    pluginsConfigurationOptions.validate(commandLine);
+  }
+
   private void validateApiOptions() {
     apiConfigurationOptions.validate(commandLine, logger);
   }

besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java

@@ -128,6 +128,9 @@ public interface DefaultCommandValues {
   /** The constant DEFAULT_PLUGINS_OPTION_NAME. */
   String DEFAULT_PLUGINS_OPTION_NAME = "--plugins";
 
+  /** The constant DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME. */
+  String DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME = "--Xplugins-external-enabled";
+
   /**
    * Gets default besu data path.
    *

besu/src/main/java/org/hyperledger/besu/cli/options/stable/PluginsConfigurationOptions.java

@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.cli.options.stable;
 
+import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME;
 import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_PLUGINS_OPTION_NAME;
 
 import org.hyperledger.besu.cli.converter.PluginInfoConverter;
@@ -28,6 +29,15 @@ import picocli.CommandLine;
 
 /** The Plugins Options options. */
 public class PluginsConfigurationOptions implements CLIOptions<PluginConfiguration> {
+
+  @CommandLine.Option(
+      names = {DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME},
+      description = "Enables external plugins (default: ${DEFAULT-VALUE})",
+      hidden = true,
+      defaultValue = "true",
+      arity = "1")
+  private Boolean externalPluginsEnabled = true;
+
   @CommandLine.Option(
       names = {DEFAULT_PLUGINS_OPTION_NAME},
       description = "Comma-separated list of plugin names",
@@ -42,7 +52,24 @@ public class PluginsConfigurationOptions implements CLIOptions<PluginConfigurati
 
   @Override
   public PluginConfiguration toDomainObject() {
-    return new PluginConfiguration(plugins);
+    return new PluginConfiguration.Builder()
+        .externalPluginsEnabled(externalPluginsEnabled)
+        .requestedPlugins(plugins)
+        .build();
+  }
+
+  /**
+   * Validate that there are no inconsistencies in the specified options.
+   *
+   * @param commandLine the full commandLine to check all the options specified by the user
+   */
+  public void validate(final CommandLine commandLine) {
+    String errorMessage =
+        String.format(
+            "%s option can only be used when %s is true",
+            DEFAULT_PLUGINS_OPTION_NAME, DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME);
+    CommandLineUtils.failIfOptionDoesntMeetRequirement(
+        commandLine, errorMessage, externalPluginsEnabled, List.of(DEFAULT_PLUGINS_OPTION_NAME));
   }
 
   @Override
@@ -61,6 +88,13 @@ public class PluginsConfigurationOptions implements CLIOptions<PluginConfigurati
         CommandLineUtils.getOptionValueOrDefault(
             commandLine, DEFAULT_PLUGINS_OPTION_NAME, new PluginInfoConverter());
 
-    return new PluginConfiguration(plugins);
+    boolean externalPluginsEnabled =
+        CommandLineUtils.getOptionValueOrDefault(
+            commandLine, DEFAULT_PLUGINS_EXTERNAL_ENABLED_OPTION_NAME, Boolean::parseBoolean);
+
+    return new PluginConfiguration.Builder()
+        .requestedPlugins(plugins)
+        .externalPluginsEnabled(externalPluginsEnabled)
+        .build();
   }
 }

besu/src/main/java/org/hyperledger/besu/services/BesuPluginContextImpl.java

@@ -132,20 +132,25 @@ public class BesuPluginContextImpl implements BesuContext, PluginVersionsProvide
         "Besu plugins have already been registered. Cannot register additional plugins.");
     state = Lifecycle.REGISTERING;
 
-    detectedPlugins = detectPlugins(config);
-    if (!config.getRequestedPlugins().isEmpty()) {
-      // Register only the plugins that were explicitly requested and validated
-      requestedPlugins = config.getRequestedPlugins();
-
-      // Match and validate the requested plugins against the detected plugins
-      List<BesuPlugin> registeringPlugins =
-          matchAndValidateRequestedPlugins(requestedPlugins, detectedPlugins);
-
-      registerPlugins(registeringPlugins);
+    if (config.isExternalPluginsEnabled()) {
+      detectedPlugins = detectPlugins(config);
+
+      if (config.getRequestedPlugins().isEmpty()) {
+        // If no plugins were specified, register all detected plugins
+        registerPlugins(detectedPlugins);
+      } else {
+        // Register only the plugins that were explicitly requested and validated
+        requestedPlugins = config.getRequestedPlugins();
+        // Match and validate the requested plugins against the detected plugins
+        List<BesuPlugin> registeringPlugins =
+            matchAndValidateRequestedPlugins(requestedPlugins, detectedPlugins);
+
+        registerPlugins(registeringPlugins);
+      }
     } else {
-      // If no plugins were specified, register all detected plugins
-      registerPlugins(detectedPlugins);
+      LOG.debug("External plugins are disabled. Skipping plugins registration.");
     }
+    state = Lifecycle.REGISTERED;
   }
 
   private List<BesuPlugin> matchAndValidateRequestedPlugins(
@@ -182,7 +187,6 @@ public class BesuPluginContextImpl implements BesuContext, PluginVersionsProvide
         registeredPlugins.add(plugin);
       }
     }
-    state = Lifecycle.REGISTERED;
   }
 
   private boolean registerPlugin(final BesuPlugin plugin) {

besu/src/test/java/org/hyperledger/besu/cli/PluginsOptionsTest.java

@@ -0,0 +1,118 @@
+/*
+ * Copyright contributors to Hyperledger Besu.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.cli;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.verify;
+
+import org.hyperledger.besu.ethereum.core.plugins.PluginConfiguration;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+
+public class PluginsOptionsTest extends CommandTestAbstract {
+
+  @Captor protected ArgumentCaptor<PluginConfiguration> pluginConfigurationArgumentCaptor;
+
+  @Test
+  public void shouldParsePluginOptionForSinglePlugin() {
+    parseCommand("--plugins", "pluginA");
+    verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture());
+    assertThat(pluginConfigurationArgumentCaptor.getValue().getRequestedPlugins())
+        .isEqualTo(List.of("pluginA"));
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void shouldParsePluginOptionForMultiplePlugins() {
+    parseCommand("--plugins", "pluginA,pluginB");
+    verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture());
+    assertThat(pluginConfigurationArgumentCaptor.getValue().getRequestedPlugins())
+        .isEqualTo(List.of("pluginA", "pluginB"));
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void shouldNotUsePluginOptionWhenNoPluginsSpecified() {
+    parseCommand();
+    verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture());
+    assertThat(pluginConfigurationArgumentCaptor.getValue().getRequestedPlugins())
+        .isEqualTo(List.of());
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void shouldNotParseAnyPluginsWhenPluginOptionIsEmpty() {
+    parseCommand("--plugins", "");
+    verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture());
+    assertThat(pluginConfigurationArgumentCaptor.getValue().getRequestedPlugins())
+        .isEqualTo(List.of());
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void shouldParsePluginsExternalEnabledOptionWhenFalse() {
+    parseCommand("--Xplugins-external-enabled=false");
+    verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture());
+
+    assertThat(pluginConfigurationArgumentCaptor.getValue().isExternalPluginsEnabled())
+        .isEqualTo(false);
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void shouldParsePluginsExternalEnabledOptionWhenTrue() {
+    parseCommand("--Xplugins-external-enabled=true");
+    verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture());
+
+    assertThat(pluginConfigurationArgumentCaptor.getValue().isExternalPluginsEnabled())
+        .isEqualTo(true);
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void shouldEnablePluginsExternalByDefault() {
+    parseCommand();
+    verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture());
+    assertThat(pluginConfigurationArgumentCaptor.getValue().isExternalPluginsEnabled())
+        .isEqualTo(true);
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
+  @Test
+  public void shouldFailWhenPluginsIsDisabledAndPluginsExplicitlyRequested() {
+    parseCommand("--Xplugins-external-enabled=false", "--plugins", "pluginA");
+    verify(mockBesuPluginContext).registerPlugins(pluginConfigurationArgumentCaptor.capture());
+
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8))
+        .contains("--plugins option can only be used when --Xplugins-external-enabled is true");
+  }
+}

besu/src/test/resources/everything_config.toml

@@ -242,3 +242,7 @@ Xp2p-tls-clienthello-sni=false
 
 #contracts
 Xevm-jumpdest-cache-weight-kb=32000
+
+# plugins
+Xplugins-external-enabled=true
+plugins=["none"]

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/plugins/PluginConfiguration.java

@@ -14,58 +14,25 @@
  */
 package org.hyperledger.besu.ethereum.core.plugins;
 
-import static java.util.Objects.requireNonNull;
-
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Collections;
 import java.util.List;
 
-/**
- * Configuration for managing plugins, including their information, detection type, and directory.
- */
 public class PluginConfiguration {
   private final List<PluginInfo> requestedPlugins;
   private final Path pluginsDir;
+  private final boolean externalPluginsEnabled;
 
-  /**
-   * Constructs a new PluginConfiguration with the specified plugin information and requestedPlugins
-   * directory.
-   *
-   * @param requestedPlugins List of {@link PluginInfo} objects representing the requestedPlugins.
-   * @param pluginsDir The directory where requestedPlugins are located.
-   */
-  public PluginConfiguration(final List<PluginInfo> requestedPlugins, final Path pluginsDir) {
+  public PluginConfiguration(
+      final List<PluginInfo> requestedPlugins,
+      final Path pluginsDir,
+      final boolean externalPluginsEnabled) {
     this.requestedPlugins = requestedPlugins;
     this.pluginsDir = pluginsDir;
+    this.externalPluginsEnabled = externalPluginsEnabled;
   }
 
-  /**
-   * Constructs a PluginConfiguration with specified plugins using the default directory.
-   *
-   * @param requestedPlugins List of plugins for consideration or registration. discoverable plugins
-   *     are.
-   */
-  public PluginConfiguration(final List<PluginInfo> requestedPlugins) {
-    this.requestedPlugins = requestedPlugins;
-    this.pluginsDir = PluginConfiguration.defaultPluginsDir();
-  }
-
-  /**
-   * Constructs a PluginConfiguration with the specified plugins directory
-   *
-   * @param pluginsDir The directory where plugins are located. Cannot be null.
-   */
-  public PluginConfiguration(final Path pluginsDir) {
-    this.requestedPlugins = null;
-    this.pluginsDir = requireNonNull(pluginsDir);
-  }
-
-  /**
-   * Returns the names of requested plugins, or an empty list if none.
-   *
-   * @return List of requested plugin names, never {@code null}.
-   */
   public List<String> getRequestedPlugins() {
     return requestedPlugins == null
         ? Collections.emptyList()
@@ -76,17 +43,46 @@ public class PluginConfiguration {
     return pluginsDir;
   }
 
-  /**
-   * Returns the default plugins directory based on system properties.
-   *
-   * @return The default {@link Path} to the plugin's directory.
-   */
+  public boolean isExternalPluginsEnabled() {
+    return externalPluginsEnabled;
+  }
+
   public static Path defaultPluginsDir() {
-    final String pluginsDirProperty = System.getProperty("besu.plugins.dir");
-    if (pluginsDirProperty == null) {
-      return Paths.get(System.getProperty("besu.home", "."), "plugins");
-    } else {
-      return Paths.get(pluginsDirProperty);
+    String pluginsDirProperty = System.getProperty("besu.plugins.dir");
+    return pluginsDirProperty == null
+        ? Paths.get(System.getProperty("besu.home", "."), "plugins")
+        : Paths.get(pluginsDirProperty);
+  }
+
+  public static Builder builder() {
+    return new Builder();
+  }
+
+  public static class Builder {
+    private List<PluginInfo> requestedPlugins;
+    private Path pluginsDir;
+    private boolean externalPluginsEnabled = true;
+
+    public Builder requestedPlugins(final List<PluginInfo> requestedPlugins) {
+      this.requestedPlugins = requestedPlugins;
+      return this;
+    }
+
+    public Builder pluginsDir(final Path pluginsDir) {
+      this.pluginsDir = pluginsDir;
+      return this;
+    }
+
+    public Builder externalPluginsEnabled(final boolean externalPluginsEnabled) {
+      this.externalPluginsEnabled = externalPluginsEnabled;
+      return this;
+    }
+
+    public PluginConfiguration build() {
+      if (pluginsDir == null) {
+        pluginsDir = PluginConfiguration.defaultPluginsDir();
+      }
+      return new PluginConfiguration(requestedPlugins, pluginsDir, externalPluginsEnabled);
     }
   }
 }