From 1864f56bac8b2455577e39db7ebdf241d164aa93 Mon Sep 17 00:00:00 2001 From: Yash Rode <57207-yash.rode@users.noreply.drupalcode.org> Date: Wed, 8 Feb 2023 03:17:19 +0000 Subject: [PATCH] Issue #3311229 by Wim Leers, yash.rode, tedbow, TravisCarden: Validate compliance with composer minimum stability during PreRequireEvent --- package_manager/package_manager.services.yml | 7 ++ package_manager/src/ComposerInspector.php | 54 ++++++++- .../ComposerMinimumStabilityValidator.php | 104 ++++++++++++++++++ .../Validator/ComposerPluginsValidator.php | 8 +- .../tests/fixtures/fake_site/composer.json | 3 +- .../src/Kernel/ComposerInspectorTest.php | 6 +- .../ComposerMinimumStabilityValidatorTest.php | 41 +++++++ 7 files changed, 212 insertions(+), 11 deletions(-) create mode 100644 package_manager/src/Validator/ComposerMinimumStabilityValidator.php create mode 100644 package_manager/tests/src/Kernel/ComposerMinimumStabilityValidatorTest.php diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml index f4cf9785d4..a3cf7823ff 100644 --- a/package_manager/package_manager.services.yml +++ b/package_manager/package_manager.services.yml @@ -113,6 +113,13 @@ services: - '@package_manager.path_locator' tags: - { name: event_subscriber } + package_manger.validator.composer_minimum_stability: + class: Drupal\package_manager\Validator\ComposerMinimumStabilityValidator + arguments: + - '@package_manager.path_locator' + - '@package_manager.composer_inspector' + tags: + - { name: event_subscriber } package_manager.validator.multisite: class: Drupal\package_manager\Validator\MultisiteValidator arguments: diff --git a/package_manager/src/ComposerInspector.php b/package_manager/src/ComposerInspector.php index 83145e0ac5..3cab289969 100644 --- a/package_manager/src/ComposerInspector.php +++ b/package_manager/src/ComposerInspector.php @@ -4,6 +4,8 @@ declare(strict_types = 1); namespace Drupal\package_manager; +use PhpTuf\ComposerStager\Domain\Exception\RuntimeException; +use PhpTuf\ComposerStager\Domain\Service\ProcessOutputCallback\ProcessOutputCallbackInterface; use PhpTuf\ComposerStager\Domain\Service\ProcessRunner\ComposerRunnerInterface; /** @@ -49,12 +51,54 @@ final class ComposerInspector { * @param string $working_dir * The working directory in which to run Composer. * - * @return mixed|null - * The output data. + * @return string|null + * The output data. Note that the caller must know the shape of the + * requested key's value: if it's a string, no further processing is needed, + * but if it is a boolean, an array or a map, JSON decoding should be + * applied. + * + * @see \Composer\Command\ConfigCommand::execute() */ - public function getConfig(string $key, string $working_dir) { - $this->runner->run(['config', $key, "--working-dir=$working_dir", '--json'], $this->jsonCallback); - return $this->jsonCallback->getOutputData(); + public function getConfig(string $key, string $working_dir) : ?string { + // For whatever reason, PHPCS thinks that $output is not used, even though + // it very clearly *is*. So, shut PHPCS up for the duration of this method. + // phpcs:disable DrupalPractice.CodeAnalysis.VariableAnalysis.UnusedVariable + $callback = new class () implements ProcessOutputCallbackInterface { + + /** + * The command output. + * + * @var string + */ + public string $output = ''; + + /** + * {@inheritdoc} + */ + public function __invoke(string $type, string $buffer): void { + $this->output .= trim($buffer); + } + + }; + // phpcs:enable + try { + $this->runner->run(['config', $key, "--working-dir=$working_dir"], $callback); + } + catch (RuntimeException $e) { + // Assume any error from `composer config` is about an undefined key-value + // pair which may have a known default value. + // @todo Remove this once https://github.com/composer/composer/issues/11302 lands and ships in a composer release. + switch ($key) { + // @see https://getcomposer.org/doc/04-schema.md#minimum-stability + case 'minimum-stability': + return 'stable'; + + default: + // Otherwise, re-throw the exception. + throw $e; + } + } + return $callback->output; } /** diff --git a/package_manager/src/Validator/ComposerMinimumStabilityValidator.php b/package_manager/src/Validator/ComposerMinimumStabilityValidator.php new file mode 100644 index 0000000000..aad401503d --- /dev/null +++ b/package_manager/src/Validator/ComposerMinimumStabilityValidator.php @@ -0,0 +1,104 @@ +<?php + +declare(strict_types = 1); + +namespace Drupal\package_manager\Validator; + +use Composer\Semver\Semver; +use Composer\Semver\VersionParser; +use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\package_manager\ComposerInspector; +use Drupal\package_manager\Event\PreRequireEvent; +use Drupal\package_manager\PathLocator; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +/** + * Checks that the (about to be) installed packages meet the minimum stability. + * + * @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 ComposerMinimumStabilityValidator implements EventSubscriberInterface { + + use StringTranslationTrait; + + /** + * The path locator service. + * + * @var \Drupal\package_manager\PathLocator + */ + protected PathLocator $pathLocator; + + /** + * The Composer inspector service. + * + * @var \Drupal\package_manager\ComposerInspector + */ + protected ComposerInspector $inspector; + + /** + * Constructs a ComposerMinimumStabilityValidator object. + * + * @param \Drupal\package_manager\PathLocator $path_locator + * The path locator service. + * @param \Drupal\package_manager\ComposerInspector $inspector + * The Composer inspector service. + */ + public function __construct(PathLocator $path_locator, ComposerInspector $inspector) { + $this->pathLocator = $path_locator; + $this->inspector = $inspector; + } + + /** + * Validates composer minimum stability. + * + * @param \Drupal\package_manager\Event\PreRequireEvent $event + * The stage event. + */ + public function validateMinimumStability(PreRequireEvent $event): void { + $dir = $this->pathLocator->getProjectRoot(); + $minimum_stability = $this->inspector->getConfig('minimum-stability', $dir); + $requested_packages = $event->getRuntimePackages(); + + foreach ($requested_packages as $package_name => $version) { + // In the root composer.json, a stability flag can also be specified. They + // take the form @code constraint@stability @endcode. A stability flag + // allow the project owner to deviate from the minimum-stability setting. + // @see https://getcomposer.org/doc/04-schema.md#package-links + // @see \Composer\Package\Loader\RootPackageLoader::extractStabilityFlags() + if (str_contains($version, '@')) { + continue; + } + $stability = VersionParser::parseStability($version); + + // Because drupal/core prefers to not depend on composer/composer we need + // to compare two versions that are identical except for stability to + // determine if the package stability is less that the minimum stability. + if (Semver::satisfies("1.0.0-$stability", "< 1.0.0-$minimum_stability")) { + $event->addError([ + $this->t("<code>@package_name</code>'s requested version @package_version is less stable (@package_stability) than the minimum stability (@minimum_stability) required in @file.", + [ + '@package_name' => $package_name, + '@package_version' => $version, + '@package_stability' => $stability, + '@minimum_stability' => $minimum_stability, + '@file' => $this->pathLocator->getProjectRoot() . '/composer.json', + ] + ), + ]); + } + } + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents(): array { + return [ + PreRequireEvent::class => 'validateMinimumStability', + ]; + } + +} diff --git a/package_manager/src/Validator/ComposerPluginsValidator.php b/package_manager/src/Validator/ComposerPluginsValidator.php index aa2ab693d5..c8a99e14e1 100644 --- a/package_manager/src/Validator/ComposerPluginsValidator.php +++ b/package_manager/src/Validator/ComposerPluginsValidator.php @@ -6,6 +6,7 @@ namespace Drupal\package_manager\Validator; use Composer\Package\Package; use Drupal\Component\Render\FormattableMarkup; +use Drupal\Component\Serialization\Json; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\package_manager\ComposerInspector; @@ -15,6 +16,7 @@ use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\StatusCheckEvent; use Drupal\package_manager\PathExcluder\VendorHardeningExcluder; use Drupal\package_manager\PathLocator; +use PhpTuf\ComposerStager\Domain\Exception\RuntimeException; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** @@ -172,10 +174,10 @@ final class ComposerPluginsValidator implements EventSubscriberInterface { : $this->pathLocator->getProjectRoot(); try { // @see https://getcomposer.org/doc/06-config.md#allow-plugins - $value = $this->inspector->getConfig('allow-plugins', $dir); + $value = Json::decode($this->inspector->getConfig('allow-plugins', $dir)); } - catch (\Exception $exception) { - $event->addErrorFromThrowable($exception, $this->t('Unable to determine Composer <code>allow-plugins/code> setting.')); + catch (RuntimeException $exception) { + $event->addErrorFromThrowable($exception, $this->t('Unable to determine Composer <code>allow-plugins</code> setting.')); return; } diff --git a/package_manager/tests/fixtures/fake_site/composer.json b/package_manager/tests/fixtures/fake_site/composer.json index 17e40a3ec4..4c53c626db 100644 --- a/package_manager/tests/fixtures/fake_site/composer.json +++ b/package_manager/tests/fixtures/fake_site/composer.json @@ -17,7 +17,8 @@ "foo": 1.23, "bar": 134, "foo-bar": null - } + }, + "baz": null }, "repositories": { "packagist.org": false, diff --git a/package_manager/tests/src/Kernel/ComposerInspectorTest.php b/package_manager/tests/src/Kernel/ComposerInspectorTest.php index 8f7181e593..112f217884 100644 --- a/package_manager/tests/src/Kernel/ComposerInspectorTest.php +++ b/package_manager/tests/src/Kernel/ComposerInspectorTest.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; +use Drupal\Component\Serialization\Json; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\KernelTests\KernelTestBase; use PhpTuf\ComposerStager\Domain\Exception\RuntimeException; @@ -26,7 +27,7 @@ class ComposerInspectorTest extends KernelTestBase { public function testConfig(): void { $dir = __DIR__ . '/../../fixtures/fake_site'; $inspector = $this->container->get('package_manager.composer_inspector'); - $this->assertSame(1, $inspector->getConfig('secure-http', $dir)); + $this->assertSame(1, Json::decode($inspector->getConfig('secure-http', $dir))); $this->assertSame([ 'boo' => 'boo boo', @@ -37,7 +38,8 @@ class ComposerInspectorTest extends KernelTestBase { "bar" => 134, "foo-bar" => NULL, ], - ], $inspector->getConfig('extra', $dir)); + 'baz' => NULL, + ], Json::decode($inspector->getConfig('extra', $dir))); $this->expectException(RuntimeException::class); $inspector->getConfig('non-existent-config', $dir); diff --git a/package_manager/tests/src/Kernel/ComposerMinimumStabilityValidatorTest.php b/package_manager/tests/src/Kernel/ComposerMinimumStabilityValidatorTest.php new file mode 100644 index 0000000000..5cb13665d0 --- /dev/null +++ b/package_manager/tests/src/Kernel/ComposerMinimumStabilityValidatorTest.php @@ -0,0 +1,41 @@ +<?php + +declare(strict_types = 1); + +namespace Drupal\Tests\package_manager\Kernel; + +use Drupal\package_manager\Exception\StageValidationException; +use Drupal\package_manager\ValidationResult; + +/** + * @covers \Drupal\package_manager\Validator\ComposerMinimumStabilityValidator + * @group package_manager + * @internal + */ +class ComposerMinimumStabilityValidatorTest extends PackageManagerKernelTestBase { + + /** + * Tests error if requested version is less stable than the minimum: stable. + */ + public function testPreRequireEvent(): void { + $stage = $this->createStage(); + $stage->create(); + $result = ValidationResult::createError([ + t("<code>drupal/core</code>'s requested version 9.8.1-beta1 is less stable (beta) than the minimum stability (stable) required in <PROJECT_ROOT>/composer.json."), + ]); + try { + $stage->require(['drupal/core:9.8.1-beta1']); + $this->fail('Able to require a package even though it did not meet minimum stability.'); + } + catch (StageValidationException $exception) { + $this->assertValidationResultsEqual([$result], $exception->getResults()); + } + $stage->destroy(); + + // Specifying a stability flag bypasses this check. + $stage1 = $this->createStage(); + $stage1->create(); + $stage1->require(['drupal/core:9.8.1-beta1@dev']); + } + +} -- GitLab