Issue #3277035 by phenaproxima, tedbow, chrisfromredfin: Create a validator to...

Issue #3277035 by phenaproxima, tedbow, chrisfromredfin: Create a validator to check for  symlinks anywhere in the project

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:

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:

package_manager/src/Validator/SymlinkValidator.php

+<?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'],
+      ],
+    ];
+  }
+
+}

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();
   }
 
   /**

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)) {

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.
    */

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();
   }
 

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();
   }
 
   /**

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');

package_manager/tests/src/Kernel/SymlinkValidatorTest.php

+<?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);
+  }
+
+}

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',
   ];
 
   /**

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.

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']);

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'],
     ];
   }
 

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();
   }
 
   /**

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);

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().
    *