Issue #3231996 by phenaproxima, kunal.sachdev, tedbow: Ensure file system is writeable

automatic_updates.services.yml

@@ -46,6 +46,13 @@ services:
       - '@automatic_updates.path_locator'
     tags:
       - { name: event_subscriber }
+  automatic_updates.validator.file_system_permissions:
+    class: Drupal\automatic_updates\Validator\WritableFileSystemValidator
+    arguments:
+      - '@automatic_updates.path_locator'
+      - '%app.root%'
+    tags:
+      - { name: event_subscriber }
   automatic_updates.path_locator:
     class: Drupal\automatic_updates\PathLocator
     arguments:

src/Validator/WritableFileSystemValidator.php

+<?php
+
+namespace Drupal\automatic_updates\Validator;
+
+use Drupal\automatic_updates\AutomaticUpdatesEvents;
+use Drupal\automatic_updates\Event\UpdateEvent;
+use Drupal\automatic_updates\PathLocator;
+use Drupal\automatic_updates\Validation\ValidationResult;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+/**
+ * Checks that the file system is writable.
+ */
+class WritableFileSystemValidator implements EventSubscriberInterface {
+
+  use StringTranslationTrait;
+
+  /**
+   * The path locator service.
+   *
+   * @var \Drupal\automatic_updates\PathLocator
+   */
+  protected $pathLocator;
+
+  /**
+   * The Drupal root.
+   *
+   * @var string
+   */
+  protected $appRoot;
+
+  /**
+   * Constructs a WritableFileSystemValidator object.
+   *
+   * @param \Drupal\automatic_updates\PathLocator $path_locator
+   *   The path locator service.
+   * @param string $app_root
+   *   The Drupal root.
+   */
+  public function __construct(PathLocator $path_locator, string $app_root) {
+    $this->pathLocator = $path_locator;
+    $this->appRoot = $app_root;
+  }
+
+  /**
+   * Checks that the file system is writable.
+   *
+   * @param \Drupal\automatic_updates\Event\UpdateEvent $event
+   *   The event object.
+   *
+   * @todo It might make sense to use a more sophisticated method of testing
+   *   writability than is_writable(), since it's not clear if that can return
+   *   false negatives/positives due to things like SELinux, exotic file
+   *   systems, and so forth.
+   */
+  public function checkPermissions(UpdateEvent $event): void {
+    $messages = [];
+
+    if (!is_writable($this->appRoot)) {
+      $messages[] = $this->t('The Drupal directory "@dir" is not writable.', [
+        '@dir' => $this->appRoot,
+      ]);
+    }
+
+    $dir = $this->pathLocator->getVendorDirectory();
+    if (!is_writable($dir)) {
+      $messages[] = $this->t('The vendor directory "@dir" is not writable.', ['@dir' => $dir]);
+    }
+
+    if ($messages) {
+      $result = ValidationResult::createError($messages, $this->t('The file system is not writable.'));
+      $event->addValidationResult($result);
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getSubscribedEvents() {
+    return [
+      AutomaticUpdatesEvents::READINESS_CHECK => 'checkPermissions',
+      AutomaticUpdatesEvents::PRE_START => 'checkPermissions',
+    ];
+  }
+
+}

tests/modules/automatic_updates_test/src/TestController.php

@@ -2,9 +2,11 @@
 
 namespace Drupal\automatic_updates_test;
 
+use Drupal\automatic_updates\Exception\UpdateException;
 use Drupal\automatic_updates\UpdateRecommender;
 use Drupal\Component\Utility\Environment;
 use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\Render\HtmlResponse;
 use Symfony\Component\HttpFoundation\BinaryFileResponse;
 use Symfony\Component\HttpFoundation\Response;
 
@@ -19,24 +21,37 @@ class TestController extends ControllerBase {
    * @param string $to_version
    *   The version of core to update to.
    *
-   * @return array
-   *   The renderable array of the page.
+   * @return \Symfony\Component\HttpFoundation\Response
+   *   The response object.
    */
-  public function update(string $to_version): array {
+  public function update(string $to_version): Response {
     // Let it take as long as it needs.
     Environment::setTimeLimit(0);
 
     /** @var \Drupal\automatic_updates\Updater $updater */
     $updater = \Drupal::service('automatic_updates.updater');
-    $updater->begin(['drupal' => $to_version]);
-    $updater->stage();
-    $updater->commit();
-    $updater->clean();
+    try {
+      $updater->begin(['drupal' => $to_version]);
+      $updater->stage();
+      $updater->commit();
+      $updater->clean();
 
-    $project = (new UpdateRecommender())->getProjectInfo();
-    return [
-      '#markup' => $project['existing_version'],
-    ];
+      $project = (new UpdateRecommender())->getProjectInfo();
+      $content = $project['existing_version'];
+      $status = 200;
+    }
+    catch (UpdateException $e) {
+      $messages = [];
+      foreach ($e->getValidationResults() as $result) {
+        if ($summary = $result->getSummary()) {
+          $messages[] = $summary;
+        }
+        $messages = array_merge($messages, $result->getMessages());
+      }
+      $content = implode('<br />', $messages);
+      $status = 500;
+    }
+    return new HtmlResponse($content, $status);
   }
 
   /**

tests/modules/automatic_updates_test_disable_validators/automatic_updates_test_disable_validators.info.yml

+name: 'Automatic Updates Test: Disable validators'
+description: Allows certain update validators to be disabled during testing.
+type: module
+package: Testing

tests/modules/automatic_updates_test_disable_validators/src/AutomaticUpdatesTestDisableValidatorsServiceProvider.php

+<?php
+
+namespace Drupal\automatic_updates_test_disable_validators;
+
+use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\Core\DependencyInjection\ServiceProviderBase;
+use Drupal\Core\Site\Settings;
+
+/**
+ * Allows specific validators to be disabled by site settings.
+ *
+ * This should only really be used by functional tests. Kernel tests should
+ * override their ::register() method to remove service definitions; build tests
+ * should stay out of the API/services layer unless absolutely necessary.
+ */
+class AutomaticUpdatesTestDisableValidatorsServiceProvider extends ServiceProviderBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function alter(ContainerBuilder $container) {
+    $validators = Settings::get('automatic_updates_disable_validators', []);
+    array_walk($validators, [$container, 'removeDefinition']);
+  }
+
+}

tests/src/Build/CoreUpdateTest.php

@@ -129,8 +129,15 @@ class CoreUpdateTest extends UpdateTestBase {
   public function testApi(string $template): void {
     $this->createTestSite($template);
 
-    $this->visit('/automatic-update-test/update/9.8.1');
-    $this->getMink()->assertSession()->pageTextContains('9.8.1');
+    $mink = $this->getMink();
+    $assert_session = $mink->assertSession();
+
+    // Ensure that the update is prevented if the web root and/or vendor
+    // directories are not writable.
+    $this->assertReadOnlyFileSystemError($template, '/automatic-update-test/update/9.8.1');
+
+    $mink->getSession()->reload();
+    $assert_session->pageTextContains('9.8.1');
   }
 
   /**
@@ -145,11 +152,19 @@ class CoreUpdateTest extends UpdateTestBase {
     $this->createTestSite($template);
 
     $mink = $this->getMink();
-    $page = $mink->getSession()->getPage();
+    $session = $mink->getSession();
+    $page = $session->getPage();
     $assert_session = $mink->assertSession();
+
     $this->visit('/admin/modules');
     $assert_session->pageTextContains('There is a security update available for your version of Drupal.');
     $page->clickLink('Update');
+
+    // Ensure that the update is prevented if the web root and/or vendor
+    // directories are not writable.
+    $this->assertReadOnlyFileSystemError($template, parse_url($session->getCurrentUrl(), PHP_URL_PATH));
+    $session->reload();
+
     $assert_session->pageTextNotContains('There is a security update available for your version of Drupal.');
     $page->pressButton('Update');
     $this->waitForBatchJob();
@@ -177,6 +192,42 @@ class CoreUpdateTest extends UpdateTestBase {
     $this->assertUpdateSuccessful();
   }
 
+  /**
+   * Asserts that the update is prevented if the filesystem isn't writable.
+   *
+   * @param string $template
+   *   The project template used to build the test site. See ::createTestSite()
+   *   for the possible values.
+   * @param string $url
+   *   A URL where we can see the error message which is raised when parts of
+   *   the file system are not writable. This URL will be visited twice: once
+   *   for the web root, and once for the vendor directory.
+   */
+  private function assertReadOnlyFileSystemError(string $template, string $url): void {
+    $directories = [
+      'Drupal' => rtrim($this->getWebRoot(), './'),
+    ];
+
+    // The location of the vendor directory depends on which project template
+    // was used to build the test site.
+    if ($template === 'drupal/recommended-project') {
+      $directories['vendor'] = $this->getWorkspaceDirectory() . '/vendor';
+    }
+    elseif ($template === 'drupal/legacy-project') {
+      $directories['vendor'] = $directories['Drupal'] . '/vendor';
+    }
+
+    $assert_session = $this->getMink()->assertSession();
+    foreach ($directories as $type => $path) {
+      chmod($path, 0555);
+      $this->assertDirectoryIsNotWritable($path);
+      $this->visit($url);
+      $assert_session->pageTextContains("The $type directory \"$path\" is not writable.");
+      chmod($path, 0755);
+      $this->assertDirectoryIsWritable($path);
+    }
+  }
+
   /**
    * Asserts that Drupal core was successfully updated.
    */

tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php

@@ -12,7 +12,29 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase {
   /**
    * {@inheritdoc}
    */
-  protected static $modules = ['update', 'update_test'];
+  protected static $modules = [
+    'automatic_updates_test_disable_validators',
+    'update',
+    'update_test',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function prepareSettings() {
+    parent::prepareSettings();
+
+    // Disable the filesystem permissions validator, since we cannot guarantee
+    // that the current code base will be writable in all testing situations. We
+    // test this validator in our build tests, since those do give us control
+    // over the filesystem permissions.
+    // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError()
+    $settings['settings']['automatic_updates_disable_validators'] = (object) [
+      'value' => ['automatic_updates.validator.file_system_permissions'],
+      'required' => TRUE,
+    ];
+    $this->writeSettings($settings);
+  }
 
   /**
    * Sets the current (running) version of core, as known to the Update module.

tests/src/Kernel/AutomaticUpdatesKernelTestBase.php

@@ -55,6 +55,13 @@ abstract class AutomaticUpdatesKernelTestBase extends KernelTestBase {
     if ($this->client) {
       $container->set('http_client', $this->client);
     }
+
+    // Disable the filesystem permissions validator, since we cannot guarantee
+    // that the current code base will be writable in all testing situations. We
+    // test this validator in our build tests, since those do give us control
+    // over the filesystem permissions.
+    // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError()
+    $container->removeDefinition('automatic_updates.validator.file_system_permissions');
   }
 
   /**