From 9bfc8acef93018df32e78842854da99532dcc086 Mon Sep 17 00:00:00 2001
From: Wim Leers <44946-wimleers@users.noreply.drupalcode.org>
Date: Mon, 6 Feb 2023 19:56:25 +0000
Subject: [PATCH] Issue #3331168 by Wim Leers, tedbow: Limit trusted Composer
plugins to a known list, allow user to add more
---
.../install/package_manager.settings.yml | 1 +
.../config/schema/package_manager.schema.yml | 14 +
package_manager/package_manager.api.php | 8 +
package_manager/package_manager.install | 11 +
package_manager/package_manager.module | 7 +
package_manager/package_manager.services.yml | 8 +
.../src/Event/PreOperationStageEvent.php | 2 +-
.../Validator/ComposerPluginsValidator.php | 227 ++++++++++++
.../src/FixtureManipulator.php | 47 ++-
.../Kernel/ComposerPluginsValidatorTest.php | 331 ++++++++++++++++++
tests/src/Functional/UpdatePathTest.php | 4 +
11 files changed, 654 insertions(+), 6 deletions(-)
create mode 100644 package_manager/src/Validator/ComposerPluginsValidator.php
create mode 100644 package_manager/tests/src/Kernel/ComposerPluginsValidatorTest.php
diff --git a/package_manager/config/install/package_manager.settings.yml b/package_manager/config/install/package_manager.settings.yml
index bbe571a9cb..49b030016d 100644
--- a/package_manager/config/install/package_manager.settings.yml
+++ b/package_manager/config/install/package_manager.settings.yml
@@ -2,3 +2,4 @@ file_syncer: php
executables:
composer: ~
rsync: ~
+additional_trusted_composer_plugins: []
diff --git a/package_manager/config/schema/package_manager.schema.yml b/package_manager/config/schema/package_manager.schema.yml
index 1d02937177..618bb4c26c 100644
--- a/package_manager/config/schema/package_manager.schema.yml
+++ b/package_manager/config/schema/package_manager.schema.yml
@@ -1,3 +1,11 @@
+package_name:
+ type: string
+ label: 'Package name'
+ constraints:
+ Regex:
+ # @see https://getcomposer.org/schema.json
+ pattern: '/^[a-z0-9]([_.-]?[a-z0-9]+)*/[a-z0-9](([_.]|-{1,2})?[a-z0-9]+)*$/'
+
package_manager.settings:
type: config_object
label: 'Package Manager settings'
@@ -11,3 +19,9 @@ package_manager.settings:
sequence:
type: string
label: 'Absolute path to executable, or NULL'
+ additional_trusted_composer_plugins:
+ type: sequence
+ label: 'Additional trusted composer plugins'
+ sequence:
+ type: package_name
+ label: 'Trusted composer plugin'
diff --git a/package_manager/package_manager.api.php b/package_manager/package_manager.api.php
index 12d02c7dd4..83feb8d1e3 100644
--- a/package_manager/package_manager.api.php
+++ b/package_manager/package_manager.api.php
@@ -135,6 +135,14 @@
* package) in the active directory after a stage directory has been created
* ,Package Manager will refuse to make any further changes to the stage
* directory or apply the staged changes to the active directory.
+ * - Composer plugins are able to perform arbitrary file system operations, and
+ * hence could perform actions that make it impossible for Package Manager to
+ * guarantee the Drupal site will continue to work correctly. For that reason,
+ * Package Manager will refuse to make any further changes if untrusted
+ * composer plugins are installed or staged. Additional composer plugins are
+ * vetted over time. If you know what you are doing, it is possible to trust
+ * additional composer plugins by modifying package_manager.settings's
+ * "additional_trusted_composer_plugins" setting.
* - The Drupal site must not have any pending database updates.
* - Composer must use HTTPS to download packages and metadata (i.e., Composer's
* secure-http configuration option must be enabled). This is the default
diff --git a/package_manager/package_manager.install b/package_manager/package_manager.install
index 7ae42818d1..fa864d70da 100644
--- a/package_manager/package_manager.install
+++ b/package_manager/package_manager.install
@@ -41,3 +41,14 @@ function package_manager_requirements(string $phase) {
}
return $requirements;
}
+
+/**
+ * Sets default value for new additional_trusted_composer_plugins configuration.
+ *
+ * @see https://www.drupal.org/project/automatic_updates/issues/3331168
+ */
+function package_manager_update_9001(): void {
+ /** @var \Drupal\Core\Config\Config $config */
+ $config = \Drupal::service('config.factory')->getEditable('package_manager.settings');
+ $config->set('additional_trusted_composer_plugins', [])->save();
+}
diff --git a/package_manager/package_manager.module b/package_manager/package_manager.module
index 880d39a59b..889f3e2966 100644
--- a/package_manager/package_manager.module
+++ b/package_manager/package_manager.module
@@ -33,6 +33,7 @@ function package_manager_help($route_name, RouteMatchInterface $route_match) {
$output .= '<ul>';
$output .= ' <li>' . t('It does not support Drupal multi-site installations.') . '</li>';
$output .= ' <li>' . t('It does not support symlinks. If you have any, see <a href="#package-manager-faq-composer-not-found">What if it says I have symlinks in my codebase?</a>.') . '</li>';
+ $output .= ' <li>' . t('It only allows supported composer plugins. If you have any, see <a href="#package-manager-faq-unsupported-composer-plugin">What if it says I have unsupported composer plugins in my codebase?</a>.') . '</li>';
$output .= ' <li>' . t('It does not automatically perform version control operations, e.g., with Git. Site administrators are responsible for committing updates.') . '</li>';
$output .= ' <li>' . t('It can only maintain one copy of the site at any given time. If a copy of the site already exists, another one cannot be created until the existing copy is destroyed.') . '</li>';
$output .= ' <li>' . t('It associates the temporary copy of the site with the user or session that originally created it, and only that user or session can make changes to it.') . '</li>';
@@ -80,6 +81,12 @@ function package_manager_help($route_name, RouteMatchInterface $route_match) {
$output .= '<p>' . t('The new configuration will take effect on the next Composer install or update event. Do this to apply it immediately:') . '</p>';
$output .= '<pre><code>composer install</code></pre>';
+ $output .= '<h4 id="package-manager-faq-unsupported-composer-plugin">' . t('What if it says I have unsupported composer plugins in my codebase?') . '</h4>';
+ $output .= '<p>' . t('A fresh Drupal installation only uses supported composer plugins, but some modules or themes may depend on additional composer plugins. Please <a href=":new-issue">create a new issue</a> when you encounter this.', [
+ ':new-issue' => 'https://www.drupal.org/node/add/project-issue/automatic_updates',
+ ]) . '</p>';
+ $output .= '<p>' . t('It is possible to <em>trust</em> additional composer plugins, but this requires significant expertise: understanding the code of that composer plugin, what the effects on the file system are and how it affects the Package Manager module. Some composer plugins could result in a broken site!') . '</p>';
+
$output .= '<h5>' . t('Custom code') . '</h5>';
$output .= '<p>' . t('Symlinks are seldom truly necessary and should be avoided in your own code. No solution currently exists to get around them--they must be removed in order to use Automatic Updates.') . '</p>';
diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml
index b6ba0accff..f4cf9785d4 100644
--- a/package_manager/package_manager.services.yml
+++ b/package_manager/package_manager.services.yml
@@ -217,6 +217,14 @@ services:
- '@string_translation'
tags:
- { name: event_subscriber }
+ package_manager.validator.composer_plugins:
+ class: Drupal\package_manager\Validator\ComposerPluginsValidator
+ arguments:
+ - '@config.factory'
+ - '@package_manager.composer_inspector'
+ - '@package_manager.path_locator'
+ tags:
+ - { name: event_subscriber }
package_manager.validator.patches:
class: Drupal\package_manager\Validator\ComposerPatchesValidator
tags:
diff --git a/package_manager/src/Event/PreOperationStageEvent.php b/package_manager/src/Event/PreOperationStageEvent.php
index c493c2a44a..b5c9a51545 100644
--- a/package_manager/src/Event/PreOperationStageEvent.php
+++ b/package_manager/src/Event/PreOperationStageEvent.php
@@ -49,7 +49,7 @@ abstract class PreOperationStageEvent extends StageEvent {
* is more than one message.
*/
public function addError(array $messages, ?TranslatableMarkup $summary = NULL): void {
- $this->results[] = ValidationResult::createError($messages, $summary);
+ $this->results[] = ValidationResult::createError(array_values($messages), $summary);
}
/**
diff --git a/package_manager/src/Validator/ComposerPluginsValidator.php b/package_manager/src/Validator/ComposerPluginsValidator.php
new file mode 100644
index 0000000000..aa2ab693d5
--- /dev/null
+++ b/package_manager/src/Validator/ComposerPluginsValidator.php
@@ -0,0 +1,227 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\package_manager\Validator;
+
+use Composer\Package\Package;
+use Drupal\Component\Render\FormattableMarkup;
+use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\package_manager\ComposerInspector;
+use Drupal\package_manager\Event\PreApplyEvent;
+use Drupal\package_manager\Event\PreCreateEvent;
+use Drupal\package_manager\Event\PreOperationStageEvent;
+use Drupal\package_manager\Event\StatusCheckEvent;
+use Drupal\package_manager\PathExcluder\VendorHardeningExcluder;
+use Drupal\package_manager\PathLocator;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+/**
+ * Validates the allowed composer plugins, both in active and stage.
+ *
+ * Composer plugins can make far-reaching changes on the filesystem. That is why
+ * they can cause Package Manager (more specifically the infrastructure it uses:
+ * php-tuf/composer-stager) to not work reliably; potentially even break a site!
+ *
+ * This validator restricts the use of composer plugins:
+ * - using arbitrary composer plugins is discouraged by composer, but disallowed
+ * by this module (it is too risky):
+ * @code config.allowed-plugins = true @endcode is forbidden.
+ * - installed composer plugins that are not allowed (in composer.json's
+ * @code config.allowed-plugins @endcode) are not executed by composer, so
+ * these are safe
+ * - installed composer plugins that are allowed need to be either explicitly
+ * supported by this validator (and they may need their own validation to
+ * ensure their configuration is safe, for example Drupal core's vendor
+ * hardening plugin), or explicitly trusted, by adding it to the
+ * @code package_manager.settings @endcode configuration's
+ * @code additional_trusted_composer_plugins @endcode list.
+ *
+ * @todo Determine how other Composer plugins will be supported in
+ * https://drupal.org/i/3339417.
+ *
+ * @see https://getcomposer.org/doc/04-schema.md#type
+ * @see https://getcomposer.org/doc/articles/plugins.md
+ *
+ * @internal
+ * This is an internal part of Package Manager and may be changed or removed
+ * at any time without warning. External code should not interact with this
+ * class.
+ */
+final class ComposerPluginsValidator implements EventSubscriberInterface {
+
+ use StringTranslationTrait;
+
+ /**
+ * Composer plugins known to modify other packages, but that are validated.
+ *
+ * (The validation guarantees they are safe to use.)
+ *
+ * Keys are composer plugin package names, values are associated validators or
+ * excluders necessary to make those composer plugins work reliably with the
+ * Package Manager module.
+ *
+ * @var string[][]
+ */
+ private const SUPPORTED_PLUGINS_THAT_DO_MODIFY = [
+ // cSpell:disable
+ 'cweagans/composer-patches' => ComposerPatchesValidator::class,
+ 'drupal/core-vendor-hardening' => VendorHardeningExcluder::class,
+ // cSpell:enable
+ ];
+
+ /**
+ * The composer plugins are known not to modify other packages.
+ *
+ * @var string[]
+ */
+ private const SUPPORTED_PLUGINS_THAT_DO_NOT_MODIFY = [
+ // cSpell:disable
+ 'composer/installers',
+ 'dealerdirect/phpcodesniffer-composer-installer',
+ 'drupal/core-composer-scaffold',
+ 'drupal/core-project-message',
+ 'phpstan/extension-installer',
+ // cSpell:enable
+ ];
+
+ /**
+ * The additional trusted composer plugin package names.
+ *
+ * Note: these are normalized package names.
+ *
+ * @var string[]
+ * @see \Composer\Package\PackageInterface::getName()
+ * @see \Composer\Package\PackageInterface::getPrettyName()
+ */
+ protected array $additionalTrustedComposerPlugins;
+
+ /**
+ * The Composer inspector service.
+ *
+ * @var \Drupal\package_manager\ComposerInspector
+ */
+ protected ComposerInspector $inspector;
+
+ /**
+ * The path locator service.
+ *
+ * @var \Drupal\package_manager\PathLocator
+ */
+ protected PathLocator $pathLocator;
+
+ /**
+ * Constructs a new ComposerPluginsValidator.
+ *
+ * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+ * The config factory.
+ * @param \Drupal\package_manager\ComposerInspector $inspector
+ * The Composer inspector service.
+ * @param \Drupal\package_manager\PathLocator $path_locator
+ * The path locator service.
+ */
+ public function __construct(ConfigFactoryInterface $config_factory, ComposerInspector $inspector, PathLocator $path_locator) {
+ $settings = $config_factory->get('package_manager.settings');
+ $this->additionalTrustedComposerPlugins = array_map(
+ [__CLASS__, 'normalizePackageName'],
+ $settings->get('additional_trusted_composer_plugins')
+ );
+ $this->inspector = $inspector;
+ $this->pathLocator = $path_locator;
+ }
+
+ /**
+ * Normalizes a package name.
+ *
+ * @param string $package_name
+ * A package name.
+ *
+ * @return string
+ * The normalized package name.
+ */
+ private static function normalizePackageName(string $package_name): string {
+ // Normalize the configured package names using Composer's own logic.
+ return (new Package($package_name, 'irrelevant', 'irrelevant'))->getName();
+ }
+
+ /**
+ * @return string[]
+ */
+ private function getSupportedPlugins(): array {
+ return array_merge(
+ array_keys(self::SUPPORTED_PLUGINS_THAT_DO_MODIFY),
+ self::SUPPORTED_PLUGINS_THAT_DO_NOT_MODIFY,
+ $this->additionalTrustedComposerPlugins,
+ );
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function validateStagePreOperation(PreOperationStageEvent $event): void {
+ $stage = $event->getStage();
+
+ // When about to copy the changes from the stage directory to the active
+ // directory, use the stage directory's composer instead of the active.
+ // Because composer plugins may be added or removed; the only thing that
+ // matters is the set of composer plugins that *will* apply — if a composer
+ // plugin is being removed, that's fine.
+ $dir = $event instanceof PreApplyEvent
+ ? $stage->getStageDirectory()
+ : $this->pathLocator->getProjectRoot();
+ try {
+ // @see https://getcomposer.org/doc/06-config.md#allow-plugins
+ $value = $this->inspector->getConfig('allow-plugins', $dir);
+ }
+ catch (\Exception $exception) {
+ $event->addErrorFromThrowable($exception, $this->t('Unable to determine Composer <code>allow-plugins/code> setting.'));
+ return;
+ }
+
+ if ($value === 1) {
+ $event->addError([$this->t('All composer plugins are allowed because <code>config.allow-plugins</code> is configured to <code>true</code>. This is an unacceptable security risk.')]);
+ return;
+ }
+
+ // Only packages with `true` as a value are actually executed by composer.
+ assert(is_array($value));
+ $allowed_plugins = array_keys(array_filter($value));
+ // Normalized allowed plugins: keys are normalized package names, values are
+ // the original package names.
+ $normalized_allowed_plugins = array_combine(
+ array_map([__CLASS__, 'normalizePackageName'], $allowed_plugins),
+ $allowed_plugins
+ );
+ $unsupported_plugins = array_diff_key($normalized_allowed_plugins, array_flip($this->getSupportedPlugins()));
+ if ($unsupported_plugins) {
+ $unsupported_plugins_messages = array_map(
+ fn (string $raw_allowed_plugin_name) => new FormattableMarkup(
+ "<code>@package_name</code>",
+ [
+ '@package_name' => $raw_allowed_plugin_name,
+ ]
+ ),
+ $unsupported_plugins
+ );
+ $summary = $this->formatPlural(
+ count($unsupported_plugins),
+ 'An unsupported Composer plugin was detected.',
+ 'Unsupported Composer plugins were detected.',
+ );
+ $event->addError($unsupported_plugins_messages, $summary);
+ }
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public static function getSubscribedEvents(): array {
+ return [
+ PreCreateEvent::class => 'validateStagePreOperation',
+ PreApplyEvent::class => 'validateStagePreOperation',
+ StatusCheckEvent::class => 'validateStagePreOperation',
+ ];
+ }
+
+}
diff --git a/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php b/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
index ddd9ab635a..d79016f6dd 100644
--- a/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
+++ b/package_manager/tests/modules/fixture_manipulator/src/FixtureManipulator.php
@@ -144,7 +144,7 @@ class FixtureManipulator {
* This function is internal and should not be called directly. Use
* ::addPackage(), ::modifyPackage(), and ::removePackage() instead.
*
- * @param string $name
+ * @param string $pretty_name
* The name of the package to add, update, or remove.
* @param array|null $package
* The package information to be set in installed.json and installed.php, or
@@ -155,7 +155,10 @@ class FixtureManipulator {
* @param bool|null $is_dev_requirement
* Whether or not the package is a developer requirement.
*/
- private function setPackage(string $name, ?array $package, bool $should_exist, ?bool $is_dev_requirement = NULL): void {
+ private function setPackage(string $pretty_name, ?array $package, bool $should_exist, ?bool $is_dev_requirement = NULL): void {
+ // @see \Composer\Package\BasePackage::__construct()
+ $name = strtolower($pretty_name);
+
if ($should_exist && isset($is_dev_requirement)) {
throw new \LogicException('Changing an existing project to a dev requirement is not supported');
}
@@ -178,14 +181,14 @@ class FixtureManipulator {
// Ensure that we actually expect to find the package already installed (or
// not).
$expected_package_message = $should_exist
- ? "Expected package '$name' to be installed, but it wasn't."
- : "Expected package '$name' to not be installed, but it was.";
+ ? "Expected package '$pretty_name' to be installed, but it wasn't."
+ : "Expected package '$pretty_name' to not be installed, but it was.";
if ($should_exist !== isset($position)) {
throw new \LogicException($expected_package_message);
}
if ($package) {
- $package = ['name' => $name] + $package;
+ $package = ['name' => $pretty_name] + $package;
$install_json_package = array_diff_key($package, array_flip(['install_path']));
}
@@ -299,6 +302,40 @@ class FixtureManipulator {
return $this;
}
+ /**
+ * Modifies a package's installed info.
+ *
+ * See ::addPackage() for information on how the `install_path` key is
+ * handled, if $package has it.
+ *
+ * @param array $additional_config
+ * The configuration to add.
+ */
+ public function addConfig(array $additional_config): self {
+ if (empty($additional_config)) {
+ throw new \InvalidArgumentException('No config to add.');
+ }
+
+ if (!$this->committingChanges) {
+ $this->queueManipulation('addConfig', func_get_args());
+ return $this;
+ }
+
+ $file = $this->dir . '/composer.json';
+ self::ensureFilePathIsWritable($file);
+
+ $data = file_get_contents($file);
+ $data = json_decode($data, TRUE, 512, JSON_THROW_ON_ERROR);
+
+ $config = $data['config'] ?? [];
+ $data['config'] = NestedArray::mergeDeep($config, $additional_config);
+
+ file_put_contents($file, json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
+ self::ensureFilePathIsWritable($file);
+
+ return $this;
+ }
+
/**
* Commits the changes to the directory.
*/
diff --git a/package_manager/tests/src/Kernel/ComposerPluginsValidatorTest.php b/package_manager/tests/src/Kernel/ComposerPluginsValidatorTest.php
new file mode 100644
index 0000000000..5552d76deb
--- /dev/null
+++ b/package_manager/tests/src/Kernel/ComposerPluginsValidatorTest.php
@@ -0,0 +1,331 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\Tests\package_manager\Kernel;
+
+use Drupal\Component\Utility\NestedArray;
+use Drupal\Core\StringTranslation\TranslatableMarkup;
+use Drupal\fixture_manipulator\ActiveFixtureManipulator;
+use Drupal\package_manager\Event\PreApplyEvent;
+use Drupal\package_manager\Event\PreCreateEvent;
+use Drupal\package_manager\ValidationResult;
+
+/**
+ * @covers \Drupal\package_manager\Validator\ComposerPluginsValidator
+ * @group package_manager
+ * @internal
+ */
+class ComposerPluginsValidatorTest extends PackageManagerKernelTestBase {
+
+ /**
+ * Tests `config.allow-plugins: true` fails validation during pre-create.
+ */
+ public function testInsecureConfigurationFailsValidationPreCreate(): void {
+ $active_manipulator = new ActiveFixtureManipulator();
+ $active_manipulator->addConfig(['allow-plugins' => TRUE]);
+ $active_manipulator->commitChanges();
+
+ $expected_results = [
+ ValidationResult::createError(
+ [
+ new TranslatableMarkup('All composer plugins are allowed because <code>config.allow-plugins</code> is configured to <code>true</code>. This is an unacceptable security risk.'),
+ ],
+ ),
+ ];
+ $this->assertStatusCheckResults($expected_results);
+ $this->assertResults($expected_results, PreCreateEvent::class);
+ }
+
+ /**
+ * Tests `config.allow-plugins: true` fails validation during pre-apply.
+ */
+ public function testInsecureConfigurationFailsValidationPreApply(): void {
+ $stage_manipulator = $this->getStageFixtureManipulator();
+ $stage_manipulator->addConfig(['allow-plugins' => TRUE]);
+
+ $expected_results = [
+ ValidationResult::createError(
+ [
+ new TranslatableMarkup('All composer plugins are allowed because <code>config.allow-plugins</code> is configured to <code>true</code>. This is an unacceptable security risk.'),
+ ],
+ ),
+ ];
+ $this->assertResults($expected_results, PreApplyEvent::class);
+ }
+
+ /**
+ * Tests composer plugins are validated during pre-create.
+ *
+ * @dataProvider providerSimpleValidCases
+ * @dataProvider providerSimpleInvalidCases
+ * @dataProvider providerComplexInvalidCases
+ */
+ public function testValidationDuringPreCreate(array $composer_config_to_add, array $packages_to_add, array $expected_results): void {
+ $active_manipulator = new ActiveFixtureManipulator();
+ if ($composer_config_to_add) {
+ $active_manipulator->addConfig($composer_config_to_add);
+ }
+ foreach ($packages_to_add as $package) {
+ $active_manipulator->addPackage($package);
+ }
+ $active_manipulator->commitChanges();
+
+ $this->assertStatusCheckResults($expected_results);
+ $this->assertResults($expected_results, PreCreateEvent::class);
+ }
+
+ /**
+ * Tests composer plugins are validated during pre-apply.
+ *
+ * @dataProvider providerSimpleValidCases
+ * @dataProvider providerSimpleInvalidCases
+ * @dataProvider providerComplexInvalidCases
+ */
+ public function testValidationDuringPreApply(array $composer_config_to_add, array $packages_to_add, array $expected_results): void {
+ $stage_manipulator = $this->getStageFixtureManipulator();
+ if ($composer_config_to_add) {
+ $stage_manipulator->addConfig($composer_config_to_add);
+ }
+ foreach ($packages_to_add as $package) {
+ $stage_manipulator->addPackage($package);
+ }
+
+ // Ensure \Drupal\package_manager\Validator\SupportedReleaseValidator does
+ // not complain.
+ $release_fixture_folder = __DIR__ . '/../../fixtures/release-history';
+ $this->setReleaseMetadata([
+ 'semver_test' => "$release_fixture_folder/semver_test.1.1.xml",
+ ]);
+
+ $this->assertResults($expected_results, PreApplyEvent::class);
+ }
+
+ /**
+ * Tests additional composer plugins can be trusted during pre-create.
+ *
+ * @dataProvider providerSimpleInvalidCases
+ * @dataProvider providerComplexInvalidCases
+ */
+ public function testValidationAfterTrustingDuringPreCreate(array $composer_config_to_add, array $packages_to_add, array $expected_results): void {
+ $expected_results_without_composer_plugin_violations = array_filter(
+ $expected_results,
+ fn (ValidationResult $v) => !$v->getSummary() || !str_contains(strtolower($v->getSummary()->getUntranslatedString()), 'unsupported composer plugin'),
+ );
+
+ // Trust all added packages.
+ $this->config('package_manager.settings')
+ ->set('additional_trusted_composer_plugins', array_map(fn (array $package) => $package['name'], $packages_to_add))
+ ->save();
+
+ // Reuse the test logic that does not trust additional packages, but with
+ // updated expected results.
+ $this->testValidationDuringPreCreate($composer_config_to_add, $packages_to_add, $expected_results_without_composer_plugin_violations);
+ }
+
+ /**
+ * Tests additional composer plugins can be trusted during pre-apply.
+ *
+ * @dataProvider providerSimpleInvalidCases
+ * @dataProvider providerComplexInvalidCases
+ */
+ public function testValidationAfterTrustingDuringPreApply(array $composer_config_to_add, array $packages_to_add, array $expected_results): void {
+ $expected_results_without_composer_plugin_violations = array_filter(
+ $expected_results,
+ fn (ValidationResult $v) => !$v->getSummary() || !str_contains(strtolower($v->getSummary()->getUntranslatedString()), 'unsupported composer plugin'),
+ );
+
+ // Trust all added packages.
+ $this->config('package_manager.settings')
+ ->set('additional_trusted_composer_plugins', array_map(fn (array $package) => $package['name'], $packages_to_add))
+ ->save();
+
+ // Reuse the test logic that does not trust additional packages, but with
+ // updated expected results.
+ $this->testValidationDuringPreApply($composer_config_to_add, $packages_to_add, $expected_results_without_composer_plugin_violations);
+ }
+
+ public function providerSimpleValidCases(): \Generator {
+ yield 'no composer plugins' => [
+ [],
+ [
+ [
+ 'name' => "drupal/semver_test",
+ 'version' => '8.1.0',
+ 'type' => 'drupal-module',
+ 'install_path' => '../../modules/semver_test',
+ ],
+ ],
+ [],
+ ];
+
+ // @todo Uncomment this in https://www.drupal.org/project/automatic_updates/issues/3252299
+ // phpcs:disable
+ /*
+ yield 'one supported composer plugin' => [
+ [
+ [
+ 'name' => 'cweagans/composer-patches',
+ 'version' => '1.0.0',
+ 'type' => 'composer-plugin',
+ ],
+ ],
+ [
+ // Note: this is not a complaint about using cweagans/composer-patches
+ // but a complaint about *how* it is used.
+ // @see \Drupal\package_manager\Validator\ComposerPatchesValidator
+ ValidationResult::createError([
+ new TranslatableMarkup('The <code>cweagans/composer-patches</code> plugin is installed, but the <code>composer-exit-on-patch-failure</code> key is not set to <code>true</code> in the <code>extra</code> section of composer.json.'),
+ ]),
+ ],
+ ];
+ */
+ // phpcs:enable
+
+ yield 'another supported composer plugin' => [
+ [
+ 'allow-plugins' => [
+ 'drupal/core-vendor-hardening' => TRUE,
+ ],
+ ],
+ [
+ [
+ 'name' => 'drupal/core-vendor-hardening',
+ 'version' => '9.8.0',
+ 'type' => 'composer-plugin',
+ ],
+ ],
+ [],
+ ];
+
+ yield 'one UNsupported but disallowed plugin — pretty package name' => [
+ [
+ 'allow-plugins' => [
+ 'composer/plugin-A' => FALSE,
+ ],
+ ],
+ [
+ [
+ 'name' => 'composer/plugin-A',
+ 'version' => '6.1',
+ 'type' => 'composer-plugin',
+ ],
+ ],
+ [],
+ ];
+
+ yield 'one UNsupported but disallowed plugin — normalized package name' => [
+ [
+ 'allow-plugins' => [
+ 'composer/plugin-b' => FALSE,
+ ],
+ ],
+ [
+ [
+ 'name' => 'composer/plugin-b',
+ 'version' => '20.1',
+ 'type' => 'composer-plugin',
+ ],
+ ],
+ [],
+ ];
+
+ yield 'one UNsupported but disallowed plugin' => [
+ [
+ 'allow-plugins' => [
+ // Definitely NOT `composer/plugin-c`!
+ 'drupal/core-project-message' => TRUE,
+ ],
+ ],
+ [
+ [
+ 'name' => 'composer/plugin-c',
+ 'version' => '16.4',
+ 'type' => 'composer-plugin',
+ ],
+ ],
+ [],
+ ];
+ }
+
+ public function providerSimpleInvalidCases(): \Generator {
+ yield 'one UNsupported composer plugin — pretty package name' => [
+ [
+ 'allow-plugins' => [
+ 'NOT-cweagans/NOT-composer-patches' => TRUE,
+ ],
+ ],
+ [
+ [
+ 'name' => 'NOT-cweagans/NOT-composer-patches',
+ 'version' => '6.1',
+ 'type' => 'composer-plugin',
+ ],
+ ],
+ [
+ ValidationResult::createError(
+ [
+ new TranslatableMarkup('<code>NOT-cweagans/NOT-composer-patches</code>'),
+ ],
+ new TranslatableMarkup('An unsupported Composer plugin was detected.'),
+ ),
+ ],
+ ];
+
+ yield 'one UNsupported composer plugin — normalized package name' => [
+ [
+ 'allow-plugins' => [
+ 'also-not-cweagans/also-not-composer-patches' => TRUE,
+ ],
+ ],
+ [
+ [
+ 'name' => 'also-not-cweagans/also-not-composer-patches',
+ 'version' => '20.1',
+ 'type' => 'composer-plugin',
+ ],
+ ],
+ [
+ ValidationResult::createError(
+ [
+ new TranslatableMarkup('<code>also-not-cweagans/also-not-composer-patches</code>'),
+ ],
+ new TranslatableMarkup('An unsupported Composer plugin was detected.'),
+ ),
+ ],
+ ];
+ }
+
+ /**
+ * Generates complex invalid test cases based on the simple test cases.
+ *
+ * @return \Generator
+ */
+ public function providerComplexInvalidCases(): \Generator {
+ $valid_cases = iterator_to_array($this->providerSimpleValidCases());
+ $invalid_cases = iterator_to_array($this->providerSimpleInvalidCases());
+ $all_config = NestedArray::mergeDeepArray(
+ // First key-value pair for each simple test case: the packages it adds.
+ array_map(fn (array $c) => $c[0], $valid_cases + $invalid_cases)
+ );
+ $all_packages = NestedArray::mergeDeepArray(
+ // Second key-value pair for each simple test case: the packages it adds.
+ array_map(fn (array $c) => $c[1], $valid_cases + $invalid_cases)
+ );
+
+ yield 'complex combination' => [
+ $all_config,
+ $all_packages,
+ [
+ ValidationResult::createError(
+ [
+ new TranslatableMarkup('<code>NOT-cweagans/NOT-composer-patches</code>'),
+ new TranslatableMarkup('<code>also-not-cweagans/also-not-composer-patches</code>'),
+ ],
+ new TranslatableMarkup('Unsupported Composer plugins were detected.'),
+ ),
+ ],
+ ];
+ }
+
+}
diff --git a/tests/src/Functional/UpdatePathTest.php b/tests/src/Functional/UpdatePathTest.php
index 329e34fb1e..3d57d1cb1e 100644
--- a/tests/src/Functional/UpdatePathTest.php
+++ b/tests/src/Functional/UpdatePathTest.php
@@ -61,6 +61,8 @@ class UpdatePathTest extends UpdatePathTestBase {
$this->assertSame(CronUpdater::SECURITY, $this->config('automatic_updates.settings')->get('cron'));
+ $this->assertSame(NULL, $this->config('package_manager.settings')->get('additional_trusted_composer_plugins'));
+
$this->runUpdates();
$this->assertSame(CronUpdater::DISABLED, $this->config('automatic_updates.settings')->get('cron'));
@@ -75,6 +77,8 @@ class UpdatePathTest extends UpdatePathTestBase {
$this->assertSame($expected_values, $key_value->getMultiple(array_values($map)));
$this->assertSame(StatusCheckMailer::ERRORS_ONLY, $this->config('automatic_updates.settings')->get('status_check_mail'));
+ $this->assertSame([], $this->config('package_manager.settings')->get('additional_trusted_composer_plugins'));
+
// Ensure that the router was rebuilt and routes have the expected changes.
$routes = $this->container->get('router')->getRouteCollection();
$routes = array_map([$routes, 'get'], [
--
GitLab