From 0ed875191aa538de4d08c5d08a2b3c4ab5e75615 Mon Sep 17 00:00:00 2001
From: tedbow <tedbow@240860.no-reply.drupal.org>
Date: Wed, 3 Nov 2021 16:25:21 +0000
Subject: [PATCH] Issue #3246420 by phenaproxima, tedbow: Create a validator to
ensure that active composer.lock file has not changed since the stage was
created when applying updates
---
package_manager/package_manager.services.yml | 8 +
.../src/EventSubscriber/LockFileValidator.php | 145 ++++++++++++++
.../src/Kernel/DiskSpaceValidatorTest.php | 11 ++
.../src/Kernel/LockFileValidatorTest.php | 183 ++++++++++++++++++
.../Kernel/PackageManagerKernelTestBase.php | 10 +-
.../WritableFileSystemValidatorTest.php | 6 +-
.../StagedProjectsValidatorTest.php | 12 ++
tests/src/Kernel/UpdaterTest.php | 3 +
8 files changed, 374 insertions(+), 4 deletions(-)
create mode 100644 package_manager/src/EventSubscriber/LockFileValidator.php
create mode 100644 package_manager/tests/src/Kernel/LockFileValidatorTest.php
diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml
index 07241aab23..60a8d6d519 100644
--- a/package_manager/package_manager.services.yml
+++ b/package_manager/package_manager.services.yml
@@ -108,6 +108,14 @@ services:
- '@string_translation'
tags:
- { name: event_subscriber }
+ package_manager.validator.lock_file:
+ class: Drupal\package_manager\EventSubscriber\LockFileValidator
+ arguments:
+ - '@state'
+ - '@package_manager.path_locator'
+ - '@string_translation'
+ tags:
+ - { name: event_subscriber }
package_manager.validator.file_system:
class: Drupal\package_manager\EventSubscriber\WritableFileSystemValidator
arguments:
diff --git a/package_manager/src/EventSubscriber/LockFileValidator.php b/package_manager/src/EventSubscriber/LockFileValidator.php
new file mode 100644
index 0000000000..154e137a8c
--- /dev/null
+++ b/package_manager/src/EventSubscriber/LockFileValidator.php
@@ -0,0 +1,145 @@
+<?php
+
+namespace Drupal\package_manager\EventSubscriber;
+
+use Drupal\Core\State\StateInterface;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\Core\StringTranslation\TranslationInterface;
+use Drupal\package_manager\Event\PostDestroyEvent;
+use Drupal\package_manager\Event\PreApplyEvent;
+use Drupal\package_manager\Event\PreCreateEvent;
+use Drupal\package_manager\Event\PreRequireEvent;
+use Drupal\package_manager\Event\StageEvent;
+use Drupal\package_manager\PathLocator;
+use Drupal\package_manager\ValidationResult;
+
+/**
+ * Checks that the active lock file is unchanged during stage operations.
+ */
+class LockFileValidator implements StageValidatorInterface {
+
+ use StringTranslationTrait;
+
+ /**
+ * The state key under which to store the hash of the active lock file.
+ *
+ * @var string
+ */
+ protected const STATE_KEY = 'package_manager.lock_hash';
+
+ /**
+ * The state service.
+ *
+ * @var \Drupal\Core\State\StateInterface
+ */
+ protected $state;
+
+ /**
+ * The path locator service.
+ *
+ * @var \Drupal\package_manager\PathLocator
+ */
+ protected $pathLocator;
+
+ /**
+ * Constructs a LockFileValidator object.
+ *
+ * @param \Drupal\Core\State\StateInterface $state
+ * The state service.
+ * @param \Drupal\package_manager\PathLocator $path_locator
+ * The path locator service.
+ * @param \Drupal\Core\StringTranslation\TranslationInterface $translation
+ * The string translation service.
+ */
+ public function __construct(StateInterface $state, PathLocator $path_locator, TranslationInterface $translation) {
+ $this->state = $state;
+ $this->pathLocator = $path_locator;
+ $this->setStringTranslation($translation);
+ }
+
+ /**
+ * Returns the current hash of the active directory's lock file.
+ *
+ * @return string|false
+ * The hash of the active directory's lock file, or FALSE if the lock file
+ * does not exist.
+ */
+ protected function getHash() {
+ $file = $this->pathLocator->getActiveDirectory() . DIRECTORY_SEPARATOR . 'composer.lock';
+ // We want to directly hash the lock file itself, rather than look at its
+ // content-hash value, which is actually a hash of the relevant parts of
+ // composer.json. We're trying to verify that the actual installed packages
+ // have not changed; we don't care about the constraints in composer.json.
+ try {
+ return hash_file('sha256', $file);
+ }
+ catch (\Throwable $exception) {
+ return FALSE;
+ }
+ }
+
+ /**
+ * Stores the current lock file hash.
+ */
+ public function storeHash(PreCreateEvent $event): void {
+ $hash = $this->getHash();
+ if ($hash) {
+ $this->state->set(static::STATE_KEY, $hash);
+ }
+ else {
+ $result = ValidationResult::createError([
+ $this->t('Could not hash the active lock file.'),
+ ]);
+ $event->addValidationResult($result);
+ }
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function validateStage(StageEvent $event): void {
+ // Ensure we can get a current hash of the lock file.
+ $hash = $this->getHash();
+ if (empty($hash)) {
+ $error = $this->t('Could not hash the active lock file.');
+ }
+
+ // Ensure we also have a stored hash of the lock file.
+ $stored_hash = $this->state->get(static::STATE_KEY);
+ if (empty($stored_hash)) {
+ $error = $this->t('Could not retrieve stored hash of the active lock file.');
+ }
+
+ // If we have both hashes, ensure they match.
+ if ($hash && $stored_hash && hash_equals($stored_hash, $hash) == FALSE) {
+ $error = $this->t('Stored lock file hash does not match the active lock file.');
+ }
+
+ // @todo Let the validation result carry all the relevant messages in
+ // https://www.drupal.org/i/3247479.
+ if (isset($error)) {
+ $result = ValidationResult::createError([$error]);
+ $event->addValidationResult($result);
+ }
+ }
+
+ /**
+ * Deletes the stored lock file hash.
+ */
+ public function deleteHash(): void {
+ $this->state->delete(static::STATE_KEY);
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public static function getSubscribedEvents() {
+ return [
+ PreCreateEvent::class => 'storeHash',
+ PreRequireEvent::class => 'validateStage',
+ PreApplyEvent::class => 'validateStage',
+ PostDestroyEvent::class => 'deleteHash',
+ ];
+ }
+
+}
diff --git a/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php b/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php
index 8e3a46c306..bb969e1a71 100644
--- a/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php
+++ b/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php
@@ -27,6 +27,17 @@ class DiskSpaceValidatorTest extends PackageManagerKernelTestBase {
->setClass(TestDiskSpaceValidator::class);
}
+ /**
+ * {@inheritdoc}
+ */
+ protected function disableValidators(ContainerBuilder $container): void {
+ parent::disableValidators($container);
+
+ // Disable the lock file validator, since in this test we are validating an
+ // imaginary file system which doesn't have any lock files.
+ $container->removeDefinition('package_manager.validator.lock_file');
+ }
+
/**
* Data provider for ::testDiskSpaceValidation().
*
diff --git a/package_manager/tests/src/Kernel/LockFileValidatorTest.php b/package_manager/tests/src/Kernel/LockFileValidatorTest.php
new file mode 100644
index 0000000000..298c103ad7
--- /dev/null
+++ b/package_manager/tests/src/Kernel/LockFileValidatorTest.php
@@ -0,0 +1,183 @@
+<?php
+
+namespace Drupal\Tests\package_manager\Kernel;
+
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\package_manager\Event\PreApplyEvent;
+use Drupal\package_manager\Event\PreCreateEvent;
+use Drupal\package_manager\Event\PreRequireEvent;
+use Drupal\package_manager\EventSubscriber\LockFileValidator;
+use Drupal\package_manager\PathLocator;
+use Drupal\package_manager\ValidationResult;
+use org\bovigo\vfs\vfsStream;
+
+/**
+ * @coversDefaultClass \Drupal\package_manager\EventSubscriber\LockFileValidator
+ *
+ * @group package_manager
+ */
+class LockFileValidatorTest extends PackageManagerKernelTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+
+ $vendor = vfsStream::newDirectory('vendor');
+ $this->vfsRoot->addChild($vendor);
+ $stage = vfsStream::newDirectory('stage');
+ $this->vfsRoot->addChild($stage);
+
+ $path_locator = $this->prophesize(PathLocator::class);
+ $path_locator->getActiveDirectory()->willReturn($this->vfsRoot->url());
+ $path_locator->getProjectRoot()->willReturn($this->vfsRoot->url());
+ $path_locator->getStageDirectory()->willReturn($stage->url());
+ $path_locator->getWebRoot()->willReturn('');
+ $path_locator->getVendorDirectory()->willReturn($vendor->url());
+ $this->container->set('package_manager.path_locator', $path_locator->reveal());
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function disableValidators(ContainerBuilder $container): void {
+ parent::disableValidators($container);
+
+ // Disable the disk space validator, since it tries to inspect the file
+ // system in ways that vfsStream doesn't support, like calling stat() and
+ // disk_free_space().
+ $container->removeDefinition('package_manager.validator.disk_space');
+ }
+
+ /**
+ * Tests that if no active lock file exists, a stage cannot be created.
+ *
+ * @covers ::storeHash
+ */
+ public function testCreateWithNoLock(): void {
+ $no_lock = ValidationResult::createError(['Could not hash the active lock file.']);
+ $this->assertResults([$no_lock], PreCreateEvent::class);
+ }
+
+ /**
+ * Tests that if an active lock file exists, a stage can be created.
+ *
+ * @covers ::storeHash
+ * @covers ::deleteHash
+ */
+ public function testCreateWithLock(): void {
+ $this->createActiveLockFile();
+ $this->assertResults([]);
+
+ // Change the lock file to ensure the stored hash of the previous version
+ // has been deleted.
+ $this->vfsRoot->getChild('composer.lock')->setContent('"changed"');
+ $this->assertResults([]);
+ }
+
+ /**
+ * Tests validation when the lock file has changed.
+ *
+ * @dataProvider providerValidateStageEvents
+ */
+ public function testLockFileChanged(string $event_class): void {
+ $this->createActiveLockFile();
+
+ // Add a listener with an extremely high priority to the same event that
+ // should raise the validation error. Because the validator uses the default
+ // priority of 0, this listener changes lock file before the validator
+ // runs.
+ $this->addListener($event_class, function () {
+ $this->vfsRoot->getChild('composer.lock')->setContent('"changed"');
+ });
+ $result = ValidationResult::createError([
+ 'Stored lock file hash does not match the active lock file.',
+ ]);
+ $this->assertResults([$result], $event_class);
+ }
+
+ /**
+ * Tests validation when the lock file is deleted.
+ *
+ * @dataProvider providerValidateStageEvents
+ */
+ public function testLockFileDeleted(string $event_class): void {
+ $this->createActiveLockFile();
+
+ // Add a listener with an extremely high priority to the same event that
+ // should raise the validation error. Because the validator uses the default
+ // priority of 0, this listener deletes lock file before the validator
+ // runs.
+ $this->addListener($event_class, function () {
+ $this->vfsRoot->removeChild('composer.lock');
+ });
+ $result = ValidationResult::createError([
+ 'Could not hash the active lock file.',
+ ]);
+ $this->assertResults([$result], $event_class);
+ }
+
+ /**
+ * Tests validation when a stored hash of the active lock file is unavailable.
+ *
+ * @dataProvider providerValidateStageEvents
+ */
+ public function testNoStoredHash(string $event_class): void {
+ $this->createActiveLockFile();
+
+ $reflector = new \ReflectionClassConstant(LockFileValidator::class, 'STATE_KEY');
+ $state_key = $reflector->getValue();
+
+ // Add a listener with an extremely high priority to the same event that
+ // should raise the validation error. Because the validator uses the default
+ // priority of 0, this listener deletes stored hash before the validator
+ // runs.
+ $this->addListener($event_class, function () use ($state_key) {
+ $this->container->get('state')->delete($state_key);
+ });
+ $result = ValidationResult::createError([
+ 'Could not retrieve stored hash of the active lock file.',
+ ]);
+ $this->assertResults([$result], $event_class);
+ }
+
+ /**
+ * Data provider for test methods that validate the staging area.
+ *
+ * @return array[]
+ * Sets of arguments to pass to the test method.
+ */
+ public function providerValidateStageEvents(): array {
+ return [
+ 'pre-require' => [
+ PreRequireEvent::class,
+ ],
+ 'pre-apply' => [
+ PreApplyEvent::class,
+ ],
+ ];
+ }
+
+ /**
+ * Creates a 'composer.lock' file in the active directory.
+ */
+ private function createActiveLockFile(): void {
+ $lock_file = vfsStream::newFile('composer.lock')->setContent('{}');
+ $this->vfsRoot->addChild($lock_file);
+ }
+
+ /**
+ * Adds an event listener with the highest possible priority.
+ *
+ * @param string $event_class
+ * The event class to listen for.
+ * @param callable $listener
+ * The listener to add.
+ */
+ private function addListener(string $event_class, callable $listener): void {
+ $this->container->get('event_dispatcher')
+ ->addListener($event_class, $listener, PHP_INT_MAX);
+ }
+
+}
diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
index 7e5fdfdf87..431220461f 100644
--- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
+++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
@@ -50,10 +50,11 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
*
* @param \Drupal\package_manager\ValidationResult[] $expected_results
* The expected validation results.
- * @param string $event_class
- * The class of the event which should return the results.
+ * @param string|null $event_class
+ * (optional) The class of the event which should return the results. Must
+ * be passed if $expected_results is not empty.
*/
- protected function assertResults(array $expected_results, string $event_class): void {
+ protected function assertResults(array $expected_results, string $event_class = NULL): void {
$stage = new TestStage(
$this->container->get('package_manager.path_locator'),
$this->container->get('package_manager.beginner'),
@@ -62,6 +63,7 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
$this->container->get('package_manager.cleaner'),
$this->container->get('event_dispatcher'),
);
+
try {
$stage->create();
$stage->require(['drupal/core:9.8.1']);
@@ -72,9 +74,11 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
$this->assertEmpty($expected_results);
}
catch (StageException $e) {
+ $this->assertNotEmpty($expected_results);
$this->assertValidationResultsEqual($expected_results, $e->getResults());
// TestStage::dispatch() attaches the event object to the exception so
// that we can analyze it.
+ $this->assertNotEmpty($event_class);
$this->assertInstanceOf($event_class, $e->event);
}
}
diff --git a/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php b/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php
index 9ba1284b2d..0b2398d6a1 100644
--- a/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php
+++ b/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php
@@ -43,6 +43,10 @@ class WritableFileSystemValidatorTest extends PackageManagerKernelTestBase {
// system in ways that vfsStream doesn't support, like calling stat() and
// disk_free_space().
$container->removeDefinition('package_manager.validator.disk_space');
+
+ // Disable the lock file validator, since the mock file system we create in
+ // this test doesn't have any lock files to validate.
+ $container->removeDefinition('package_manager.validator.lock_file');
}
/**
@@ -112,7 +116,7 @@ class WritableFileSystemValidatorTest extends PackageManagerKernelTestBase {
$path_locator->getVendorDirectory()->willReturn($vendor->url());
$this->container->set('package_manager.path_locator', $path_locator->reveal());
- /** @var \Drupal\Tests\package_manager\Kernel\TestValidator $validator */
+ /** @var \Drupal\Tests\package_manager\Kernel\TestWritableFileSystemValidator $validator */
$validator = $this->container->get('package_manager.validator.file_system');
$validator->appRoot = $root->url();
diff --git a/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
index 3bb3721f94..82e0feed52 100644
--- a/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
+++ b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
@@ -2,6 +2,7 @@
namespace Drupal\Tests\automatic_updates\Kernel\ReadinessValidation;
+use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\package_manager\Event\PreApplyEvent;
use Drupal\package_manager\PathLocator;
use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase;
@@ -21,6 +22,17 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
'package_manager',
];
+ /**
+ * {@inheritdoc}
+ */
+ protected function disableValidators(ContainerBuilder $container): void {
+ parent::disableValidators($container);
+
+ // This test deals with fake sites that don't necessarily have lock files,
+ // so disable lock file validation.
+ $container->removeDefinition('package_manager.validator.lock_file');
+ }
+
/**
* Runs the validator under test against an arbitrary pair of directories.
*
diff --git a/tests/src/Kernel/UpdaterTest.php b/tests/src/Kernel/UpdaterTest.php
index add52e11db..541e16e354 100644
--- a/tests/src/Kernel/UpdaterTest.php
+++ b/tests/src/Kernel/UpdaterTest.php
@@ -48,6 +48,9 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase {
$kernel = $this->container->get('kernel');
$kernel->rebuildContainer();
$this->container = $kernel->getContainer();
+ // Keep using the mocked path locator.
+ $this->container->set('package_manager.path_locator', $locator->reveal());
+
// When we call Updater::stage(), the stored project versions should be
// read from state and passed to Composer Stager's Stager service, in the
// form of a Composer command. We set up a mock here to ensure that those
--
GitLab