Issue #3231996: Ensure file system is writeable

src/Validator/WritableFileSystemValidator.php

@@ -6,15 +6,9 @@ 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\File\Exception\FileException;
-use Drupal\Core\File\FileSystemInterface;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
-use Drupal\Core\StringTranslation\TranslatableMarkup;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
-/**
- * Validates that important file system paths are writable.
- */
 class WritableFileSystemValidator implements EventSubscriberInterface {
 
   use StringTranslationTrait;
@@ -27,74 +21,52 @@ class WritableFileSystemValidator implements EventSubscriberInterface {
   protected $pathLocator;
 
   /**
-   * The file system service.
+   * The Drupal root.
    *
-   * @var \Drupal\Core\File\FileSystemInterface
+   * @var string
    */
-  protected $fileSystem;
+  protected $appRoot;
 
   /**
-   * Constructs a WriteableFileSystemValidator object.
+   * Constructs a WritableFileSystemValidator object.
    *
    * @param \Drupal\automatic_updates\PathLocator $path_locator
    *   The path locator service.
-   * @param \Drupal\Core\File\FileSystemInterface $file_system
-   *   The file system service.
+   * @param string $app_root
+   *   The Drupal root.
    */
-  public function __construct(PathLocator $path_locator, FileSystemInterface $file_system) {
+  public function __construct(PathLocator $path_locator, string $app_root) {
     $this->pathLocator = $path_locator;
-    $this->fileSystem = $file_system;
+    $this->appRoot = $app_root;
   }
 
   /**
-   * Validates that the file system is writable.
+   * 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 {
-    $dir = implode(DIRECTORY_SEPARATOR, [
-      $this->pathLocator->getProjectRoot(),
-      $this->pathLocator->getWebRoot(),
-    ]);
-    $message = $this->t('The Drupal directory "@dir" is not writable.', [
-      '@dir' => $dir,
-    ]);
-    $this->checkPath($dir, 'core/core.api.php', $event, $message);
-
-    $dir = $this->pathLocator->getVendorDirectory();
-    $message = $this->t('The vendor directory "@dir" is not writable.', [
-      '@dir' => $dir,
-    ]);
-    $this->checkPath($dir, 'composer/LICENSE', $event, $message);
-  }
+    $messages = [];
 
-  /**
-   * Tests that a path is writable by writing and deleting a temporary file.
-   *
-   * @param string $path_to_check
-   *   The path (directory) to test.
-   * @param string $file_to_copy
-   *   The path of the file, relative to $path_to_check, which should be copied
-   *   temporarily.
-   * @param \Drupal\automatic_updates\Event\UpdateEvent $event
-   *   The event object.
-   * @param \Drupal\Core\StringTranslation\TranslatableMarkup $summary
-   *   The error summary if the path is not writable.
-   */
-  protected function checkPath(string $path_to_check, string $file_to_copy, UpdateEvent $event, TranslatableMarkup $summary): void {
-    $source = $path_to_check . DIRECTORY_SEPARATOR . $file_to_copy;
-    $destination = uniqid($source);
+    if (!is_writable($this->appRoot)) {
+      $messages[] = $this->t('The Drupal directory "@dir" is not writable.', [
+        '@dir' => $this->appRoot,
+      ]);
+    }
 
-    try {
-      // If we can copy the file and then delete the copy, the file system is
-      // writable.
-      $this->fileSystem->copy($source, $destination, FileSystemInterface::EXISTS_REPLACE);
-      $this->fileSystem->delete($destination);
+    $dir = $this->pathLocator->getVendorDirectory();
+    if (!is_writable($dir)) {
+      $messages[] = $this->t('The vendor directory "@dir" is not writable.', ['@dir' => $dir]);
     }
-    catch (FileException $e) {
-      $message = $e->getMessage();
-      $result = ValidationResult::createError([$message], $summary);
+
+    if ($messages) {
+      $result = ValidationResult::createError($messages, $this->t('The file system is not writable.'));
       $event->addValidationResult($result);
     }
   }
@@ -105,6 +77,7 @@ class WritableFileSystemValidator implements EventSubscriberInterface {
   public static function getSubscribedEvents() {
     return [
       AutomaticUpdatesEvents::READINESS_CHECK => 'checkPermissions',
+      AutomaticUpdatesEvents::PRE_START => 'checkPermissions',
     ];
   }