diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml index cb01c1f43524ae3c9c028aebbb92da4094fc5837..5ac683429c16d515a824097b334266a0b9348930 100644 --- a/automatic_updates.services.yml +++ b/automatic_updates.services.yml @@ -92,6 +92,12 @@ services: - '@package_manager.validator.multisite' tags: - { name: event_subscriber } + automatic_updates.validator.symlink: + class: Drupal\automatic_updates\Validator\PackageManagerReadinessCheck + arguments: + - '@package_manager.validator.symlink' + tags: + - { name: event_subscriber } automatic_updates.cron_frequency_validator: class: Drupal\automatic_updates\Validator\CronFrequencyValidator arguments: diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml index 60f7142c4af76d0d04bfeed502fa371e43375fe7..e83f61fd518f9e4f6dc048b78b3ae858083ad70d 100644 --- a/package_manager/package_manager.services.yml +++ b/package_manager/package_manager.services.yml @@ -186,6 +186,12 @@ services: - '@string_translation' tags: - { name: event_subscriber } + package_manager.validator.symlink: + class: Drupal\package_manager\Validator\SymlinkValidator + arguments: + - '@package_manager.path_locator' + tags: + - { name: event_subscriber } package_manager.test_site_excluder: class: Drupal\package_manager\PathExcluder\TestSiteExcluder arguments: diff --git a/package_manager/src/Validator/SymlinkValidator.php b/package_manager/src/Validator/SymlinkValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..c90190b1a20bd55b5d8ff96e64b2e85cec09682d --- /dev/null +++ b/package_manager/src/Validator/SymlinkValidator.php @@ -0,0 +1,117 @@ +<?php + +namespace Drupal\package_manager\Validator; + +use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\package_manager\Event\PreApplyEvent; +use Drupal\package_manager\Event\PreCreateEvent; +use Drupal\package_manager\Event\PreOperationStageEvent; +use Drupal\package_manager\PathLocator; +use Symfony\Component\Finder\Finder; + +/** + * Flags errors if the project root or staging area contain symbolic links. + * + * @todo Remove this when Composer Stager's PHP file copier handles symlinks + * without issues. + */ +class SymlinkValidator implements PreOperationStageValidatorInterface { + + use StringTranslationTrait; + + /** + * The path locator service. + * + * @var \Drupal\package_manager\PathLocator + */ + protected $pathLocator; + + /** + * Constructs a SymlinkValidator object. + * + * @param \Drupal\package_manager\PathLocator $path_locator + * The path locator service. + */ + public function __construct(PathLocator $path_locator) { + $this->pathLocator = $path_locator; + } + + /** + * {@inheritdoc} + */ + public function validateStagePreOperation(PreOperationStageEvent $event): void { + $dir = $this->pathLocator->getProjectRoot(); + + if ($this->hasLinks($dir)) { + $event->addError([ + $this->t('Symbolic links were found in the active directory, which are not supported at this time.'), + ]); + } + } + + /** + * Checks if the staging area has any symbolic links. + * + * @param \Drupal\package_manager\Event\PreApplyEvent $event + * The event object. + */ + public function preApply(PreApplyEvent $event): void { + $dir = $event->getStage()->getStageDirectory(); + + if ($this->hasLinks($dir)) { + $event->addError([ + $this->t('Symbolic links were found in the staging area, which are not supported at this time.'), + ]); + } + } + + /** + * Recursively checks if a directory has any symbolic links. + * + * @param string $dir + * The path of the directory to check. + * + * @return bool + * TRUE if the directory contains any symbolic links, FALSE otherwise. + */ + protected function hasLinks(string $dir): bool { + // Finder::filter() explicitly requires a closure, so create one from + // ::isLink() so that we can still override it for testing purposes. + $is_link = \Closure::fromCallable([$this, 'isLink']); + + // Finder::hasResults() is more efficient than count() because it will + // return early if there is a match. + return Finder::create() + ->in($dir) + ->filter($is_link) + ->ignoreUnreadableDirs() + ->hasResults(); + } + + /** + * Checks if a file or directory is a symbolic link. + * + * @param \SplFileInfo $file + * A value object for the file or directory. + * + * @return bool + * TRUE if the file or directory is a symbolic link, FALSE otherwise. + */ + protected function isLink(\SplFileInfo $file): bool { + return $file->isLink(); + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents() { + return [ + PreCreateEvent::class => 'validateStagePreOperation', + PreApplyEvent::class => [ + ['validateStagePreOperation'], + ['preApply'], + ], + ]; + } + +} diff --git a/package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php b/package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php index a9d60ac959bb16883715b95b9ac8b725013003cf..1548f5da649316aa009ef2bea4f092d4f1a07896 100644 --- a/package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php +++ b/package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php @@ -31,6 +31,9 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase { protected function setUp(): void { $this->composerRunner = $this->prophesize(ComposerRunnerInterface::class); parent::setUp(); + // Use a virtual project so that the test isn't affected by symlinks or + // other unexpected things that might be present in the running code base. + $this->createTestProject(); } /** diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php index 7296d19205a94f0453d6fbf4fe7357ae1006c427..bd79149923f3e6617f2664f28675dc4672ae3c27 100644 --- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php +++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php @@ -84,6 +84,20 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { $container->setDefinition($class, $definition->setPublic(FALSE)); $container->setAlias(PathFactoryInterface::class, $class); + // When a virtual project is used, the path locator and disk space validator + // are replaced with mocks. When staged changes are applied, the container + // is rebuilt, which destroys the mocked services and can cause unexpected + // side effects. The 'persist' tag prevents the mocks from being destroyed + // during a container rebuild. + // @see ::createTestProject() + $persist = [ + 'package_manager.path_locator', + 'package_manager.validator.disk_space', + ]; + foreach ($persist as $service_id) { + $container->getDefinition($service_id)->addTag('persist'); + } + foreach ($this->disableValidators as $service_id) { if ($container->hasDefinition($service_id)) { $container->getDefinition($service_id)->clearTag('event_subscriber'); @@ -211,7 +225,8 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { // Since the path locator now points to a virtual file system, we need to // replace the disk space validator with a test-only version that bypasses // system calls, like disk_free_space() and stat(), which aren't supported - // by vfsStream. + // by vfsStream. This validator will persist through container rebuilds. + // @see ::register() $validator = new TestDiskSpaceValidator( $this->container->get('package_manager.path_locator'), $this->container->get('string_translation') @@ -229,6 +244,8 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { /** * Mocks the path locator and injects it into the service container. * + * The mocked path locator will persist through container rebuilds. + * * @param string $project_root * The project root. * @param string|null $vendor_dir @@ -239,6 +256,8 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { * * @return \Drupal\package_manager\PathLocator * The mocked path locator. + * + * @see ::register() */ protected function mockPathLocator(string $project_root, string $vendor_dir = NULL, string $web_root = ''): PathLocator { if (empty($vendor_dir)) { diff --git a/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php b/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php index 35605045ea2b3b4b50816f875400338e6ec8c85d..80a3640bb23f60581869b5a2dfc07b9d540ccd50 100644 --- a/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php +++ b/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php @@ -17,6 +17,16 @@ class PendingUpdatesValidatorTest extends PackageManagerKernelTestBase { */ protected static $modules = ['system']; + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + // Use a virtual project so that the test isn't affected by symlinks or + // other unexpected things that might be present in the running code base. + $this->createTestProject(); + } + /** * Tests that no error is raised if there are no pending updates. */ diff --git a/package_manager/tests/src/Kernel/StageEventsTest.php b/package_manager/tests/src/Kernel/StageEventsTest.php index 16c28129f1ba53a65dc4ef6bda5c4f6ec5642908..6ce862a88195ffd27944b69b3c7af0dce677dc0e 100644 --- a/package_manager/tests/src/Kernel/StageEventsTest.php +++ b/package_manager/tests/src/Kernel/StageEventsTest.php @@ -44,6 +44,9 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc */ protected function setUp(): void { parent::setUp(); + // Use a virtual project so that the test isn't affected by symlinks or + // other unexpected things that might be present in the running code base. + $this->createTestProject(); $this->stage = $this->createStage(); } diff --git a/package_manager/tests/src/Kernel/StageOwnershipTest.php b/package_manager/tests/src/Kernel/StageOwnershipTest.php index c5f3bdac9b8536f55a81e8b01b37a7d3768cb757..c25a85930f484fbed9c1600ba7a69828783efb63 100644 --- a/package_manager/tests/src/Kernel/StageOwnershipTest.php +++ b/package_manager/tests/src/Kernel/StageOwnershipTest.php @@ -39,6 +39,9 @@ class StageOwnershipTest extends PackageManagerKernelTestBase { $this->installSchema('user', ['users_data']); $this->installEntitySchema('user'); $this->registerPostUpdateFunctions(); + // Use a virtual project so that the test isn't affected by symlinks or + // other unexpected things that might be present in the running code base. + $this->createTestProject(); } /** diff --git a/package_manager/tests/src/Kernel/StageTest.php b/package_manager/tests/src/Kernel/StageTest.php index 3f0ba3d0cf60aacd45d728332db6bb5339c6a041..584ad1e8ac54c5675456bf066432eb52ce256c78 100644 --- a/package_manager/tests/src/Kernel/StageTest.php +++ b/package_manager/tests/src/Kernel/StageTest.php @@ -30,6 +30,10 @@ class StageTest extends PackageManagerKernelTestBase { * {@inheritdoc} */ protected function setUp(): void { + // Disable the symlink validator, since this test doesn't use a virtual + // project, but the running code base may have symlinks that don't affect + // the test. + $this->disableValidators[] = 'package_manager.validator.symlink'; parent::setUp(); $this->installConfig('system'); diff --git a/package_manager/tests/src/Kernel/SymlinkValidatorTest.php b/package_manager/tests/src/Kernel/SymlinkValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..ea93febb6e92f471a0db35f76093fd0835921355 --- /dev/null +++ b/package_manager/tests/src/Kernel/SymlinkValidatorTest.php @@ -0,0 +1,135 @@ +<?php + +namespace Drupal\Tests\package_manager\Kernel; + +use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\package_manager\Exception\StageValidationException; +use Drupal\package_manager\ValidationResult; +use Drupal\package_manager\Validator\SymlinkValidator; + +/** + * @covers \Drupal\package_manager\Validator\SymlinkValidator + * + * @group package_manager + */ +class SymlinkValidatorTest extends PackageManagerKernelTestBase { + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + $this->createTestProject(); + } + + /** + * {@inheritdoc} + */ + public function register(ContainerBuilder $container) { + parent::register($container); + + $container->getDefinition('package_manager.validator.symlink') + ->setClass(TestSymlinkValidator::class); + } + + /** + * Tests that a symlink in the project root raises an error. + */ + public function testSymlinkInProjectRoot(): void { + $result = ValidationResult::createError([ + 'Symbolic links were found in the active directory, which are not supported at this time.', + ]); + + $active_dir = $this->container->get('package_manager.path_locator') + ->getProjectRoot(); + // @see \Drupal\Tests\package_manager\Kernel\TestSymlinkValidator::isLink() + touch($active_dir . '/modules/a_link'); + + try { + $this->createStage()->create(); + $this->fail('Expected a validation error.'); + } + catch (StageValidationException $e) { + $this->assertValidationResultsEqual([$result], $e->getResults()); + } + } + + /** + * Tests that a symlink in the staging area raises an error. + */ + public function testSymlinkInStagingArea(): void { + $result = ValidationResult::createError([ + 'Symbolic links were found in the staging area, which are not supported at this time.', + ]); + + $stage = $this->createStage(); + $stage->create(); + // Simulate updating a package. This will copy the active directory into + // the (virtual) staging area. + // @see ::createTestProject() + // @see \Drupal\package_manager_test_fixture\EventSubscriber\FixtureStager::copyFilesFromFixture() + $stage->require(['composer/semver:^3']); + + // @see \Drupal\Tests\package_manager\Kernel\TestSymlinkValidator::isLink() + touch($stage->getStageDirectory() . '/modules/a_link'); + + try { + $stage->apply(); + $this->fail('Expected a validation error.'); + } + catch (StageValidationException $e) { + $this->assertValidationResultsEqual([$result], $e->getResults()); + } + } + + /** + * Tests that symlinks in the project root and staging area raise an error. + */ + public function testSymlinkInProjectRootAndStagingArea(): void { + $expected_results = [ + ValidationResult::createError([ + 'Symbolic links were found in the active directory, which are not supported at this time.', + ]), + ValidationResult::createError([ + 'Symbolic links were found in the staging area, which are not supported at this time.', + ]), + ]; + + $stage = $this->createStage(); + $stage->create(); + // Simulate updating a package. This will copy the active directory into + // the (virtual) staging area. + // @see ::createTestProject() + // @see \Drupal\package_manager_test_fixture\EventSubscriber\FixtureStager::copyFilesFromFixture() + $stage->require(['composer/semver:^3']); + + $active_dir = $this->container->get('package_manager.path_locator') + ->getProjectRoot(); + // @see \Drupal\Tests\package_manager\Kernel\TestSymlinkValidator::isLink() + touch($active_dir . '/modules/a_link'); + touch($stage->getStageDirectory() . '/modules/a_link'); + + try { + $stage->apply(); + $this->fail('Expected a validation error.'); + } + catch (StageValidationException $e) { + $this->assertValidationResultsEqual($expected_results, $e->getResults()); + } + } + +} + +/** + * A test validator that considers anything named 'a_link' to be a symlink. + */ +class TestSymlinkValidator extends SymlinkValidator { + + /** + * {@inheritdoc} + */ + protected function isLink(\SplFileInfo $file): bool { + return $file->getBasename() === 'a_link' || parent::isLink($file); + } + +} diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php index 80e5e554aad58dbf5287b6e5ba6f8bf475790bf9..0582bcc863e550f2340b2fde02f07637f1fac2a3 100644 --- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php +++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php @@ -44,8 +44,12 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase { // if either the active and stage directories don't have a composer.lock // file, which is the case with some of our fixtures. 'package_manager.validator.lock_file', - // Always disable the Xdebug validator to allow test to run with Xdebug on. + // Always allow tests to run with Xdebug on. 'automatic_updates.validator.xdebug', + // Disable the symlink validator, since the running code base may contain + // symlinks that don't affect functional testing. + 'automatic_updates.validator.symlink', + 'package_manager.validator.symlink', ]; /** diff --git a/tests/src/Kernel/CronUpdaterTest.php b/tests/src/Kernel/CronUpdaterTest.php index 93edcf19b7d28af88e5a94d89f55f2322d4414c3..fbca84f4215c95776620bfef27a12df6b2fc881f 100644 --- a/tests/src/Kernel/CronUpdaterTest.php +++ b/tests/src/Kernel/CronUpdaterTest.php @@ -252,6 +252,15 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase { // Ensure that there is a security release to which we should update. $this->setReleaseMetadata(['drupal' => __DIR__ . "/../../fixtures/release-history/drupal.9.8.1-security.xml"]); + // Disable the symlink validators so that this test isn't affected by + // symlinks that might be present in the running code base. + $validators = [ + 'automatic_updates.validator.symlink', + 'package_manager.validator.symlink', + ]; + $validators = array_map([$this->container, 'get'], $validators); + array_walk($validators, [$this->container->get('event_dispatcher'), 'removeSubscriber']); + // If the pre- or post-destroy events throw an exception, it will not be // caught by the cron updater, but it *will* be caught by the main cron // service, which will log it as a cron error that we'll want to check for. diff --git a/tests/src/Kernel/ReadinessValidation/CronFrequencyValidatorTest.php b/tests/src/Kernel/ReadinessValidation/CronFrequencyValidatorTest.php index 07fd5e9351d2402bc538a6b78162eede0aba754b..efcc9d7bf8632403def3fb3f8479b48693b51b64 100644 --- a/tests/src/Kernel/ReadinessValidation/CronFrequencyValidatorTest.php +++ b/tests/src/Kernel/ReadinessValidation/CronFrequencyValidatorTest.php @@ -25,6 +25,13 @@ class CronFrequencyValidatorTest extends AutomaticUpdatesKernelTestBase { * {@inheritdoc} */ protected function setUp(): void { + // Disable the symlink validator so that the test isn't affected by symlinks + // or other unexpected things that might be present in the running code + // base. + // @todo Make this test use a virtual project in + // https://drupal.org/i/3285145. + $this->disableValidators[] = 'automatic_updates.validator.symlink'; + $this->disableValidators[] = 'package_manager.validator.symlink'; parent::setUp(); $this->setCoreVersion('9.8.0'); $this->setReleaseMetadata(['drupal' => __DIR__ . '/../../../fixtures/release-history/drupal.9.8.1-security.xml']); diff --git a/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php b/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php index f5c593c46daecf9be14aba3d7db9e60856322fda..882e86d52416841b6df572f7665f3a4902234218 100644 --- a/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php +++ b/tests/src/Kernel/ReadinessValidation/PackageManagerReadinessChecksTest.php @@ -51,6 +51,7 @@ class PackageManagerReadinessChecksTest extends AutomaticUpdatesKernelTestBase { 'File system validator' => ['package_manager.validator.file_system'], 'Composer settings validator' => ['package_manager.validator.composer_settings'], 'Multisite validator' => ['package_manager.validator.multisite'], + 'Symlink validator' => ['package_manager.validator.symlink'], ]; } diff --git a/tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php b/tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php index 8976c67436b8c730c4778e7a1e44fa23acdec14b..2aff4c4ec63f581762ea97bd9a00afc217e07582 100644 --- a/tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php +++ b/tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php @@ -34,6 +34,9 @@ class ReadinessValidationManagerTest extends AutomaticUpdatesKernelTestBase { $this->installEntitySchema('user'); $this->installSchema('user', ['users_data']); $this->createTestValidationResults(); + // Use a virtual project so that the test isn't affected by symlinks or + // other unexpected things that might be present in the running code base. + $this->createTestProject(); } /** diff --git a/tests/src/Kernel/ReadinessValidation/SettingsValidatorTest.php b/tests/src/Kernel/ReadinessValidation/SettingsValidatorTest.php index 30493cecf7d6ed2a66c349ff52761a86186a7057..52dfe3fd1ac7886502db86bf7a59f9b9823a58db 100644 --- a/tests/src/Kernel/ReadinessValidation/SettingsValidatorTest.php +++ b/tests/src/Kernel/ReadinessValidation/SettingsValidatorTest.php @@ -46,6 +46,10 @@ class SettingsValidatorTest extends AutomaticUpdatesKernelTestBase { * @dataProvider providerSettingsValidation */ public function testSettingsValidation(bool $setting, array $expected_results): void { + // Use a virtual project so that the test isn't affected by symlinks or + // other unexpected things that might be present in the running code base. + $this->createTestProject(); + $this->setSetting('update_fetch_with_http_fallback', $setting); $this->assertCheckerResultsFromManager($expected_results, TRUE); diff --git a/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php b/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php index d8cd1aa8cab0a311da1980a55f9b605aecea664f..54102e41cea6fbf6a62094fc3657331270cdc0b7 100644 --- a/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php +++ b/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php @@ -21,6 +21,16 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase { */ protected static $modules = ['automatic_updates']; + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + // Use a virtual project so that the test isn't affected by symlinks or + // other unexpected things that might be present in the running code base. + $this->createTestProject(); + } + /** * Data provider for ::testReadinessCheck(). *