Issue #3274892 by phenaproxima, tedbow: Before applying the update, check that...

Issue #3274892 by phenaproxima, tedbow: Before applying the update, check that any needed changes to the site directory are possible

automatic_updates.services.yml

@@ -125,3 +125,9 @@ services:
     class: Drupal\automatic_updates\EventSubscriber\ConfigSubscriber
     tags:
       - { name: event_subscriber }
+  automatic_updates.validator.scaffold_file_permissions:
+    class: Drupal\automatic_updates\Validator\ScaffoldFilePermissionsValidator
+    arguments:
+      - '@package_manager.path_locator'
+    tags:
+      - { name: event_subscriber }

automatic_updates_extensions/tests/fixtures/fake-site/vendor/composer/installed.json

@@ -9,7 +9,12 @@
         },
         {
             "name": "drupal/core",
-            "version": "9.8.0"
+            "version": "9.8.0",
+            "extra": {
+                "drupal-scaffold": {
+                    "file-mapping": {}
+                }
+            }
         },
         {
             "name": "drupal/my_module",

automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php

@@ -135,6 +135,11 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
    * @dataProvider providerSuccessfulUpdate
    */
   public function testSuccessfulUpdate(bool $maintenance_mode_on, string $project_name, string $installed_version, string $target_version): void {
+    // Disable the scaffold file permissions validator because it will try to
+    // read composer.json from the staging area, which won't exist because
+    // Package Manager is bypassed.
+    $this->disableValidators(['automatic_updates.validator.scaffold_file_permissions']);
+
     $this->updateProject = $project_name;
     $this->setReleaseMetadata(__DIR__ . '/../../../../tests/fixtures/release-history/drupal.9.8.2.xml');
     $this->setReleaseMetadata(__DIR__ . "/../../fixtures/release-history/$project_name.1.1.xml");

package_manager/tests/fixtures/fake_site/sites/default/default.services.yml

+# This file should be staged because it's scaffolded into place by Drupal core.
+services: {}

package_manager/tests/fixtures/fake_site/sites/default/default.settings.php

+<?php
+
+/**
+ * @file
+ * This file should be staged because it's scaffolded into place by Drupal core.
+ */

package_manager/tests/fixtures/fake_site/vendor/composer/installed.json

@@ -2,7 +2,15 @@
   "packages": [
     {
       "name": "drupal/core",
-      "version": "9.8.0"
+      "version": "9.8.0",
+      "extra": {
+        "drupal-scaffold": {
+          "file-mapping": {
+            "[web-root]/sites/default/default.settings.php": "",
+            "[web-root]/sites/default/default.services.yml": ""
+          }
+        }
+      }
     }
   ]
 }

src/Validator/ScaffoldFilePermissionsValidator.php

+<?php
+
+namespace Drupal\automatic_updates\Validator;
+
+use Drupal\automatic_updates\Event\ReadinessCheckEvent;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\package_manager\ComposerUtility;
+use Drupal\package_manager\Event\PreApplyEvent;
+use Drupal\package_manager\Event\PreCreateEvent;
+use Drupal\package_manager\Event\PreOperationStageEvent;
+use Drupal\package_manager\PathLocator;
+use Drupal\package_manager\Validator\PreOperationStageValidatorInterface;
+
+/**
+ * Validates that scaffold files have appropriate permissions.
+ */
+class ScaffoldFilePermissionsValidator implements PreOperationStageValidatorInterface {
+
+  use StringTranslationTrait;
+
+  /**
+   * The path locator service.
+   *
+   * @var \Drupal\package_manager\PathLocator
+   */
+  protected $pathLocator;
+
+  /**
+   * Constructs a SiteDirectoryPermissionsValidator 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 {
+    $paths = [];
+
+    // Figure out the absolute path of `sites/default`.
+    $site_dir = $this->pathLocator->getProjectRoot();
+    $web_root = $this->pathLocator->getWebRoot();
+    if ($web_root) {
+      $site_dir .= '/' . $web_root;
+    }
+    $site_dir .= '/sites/default';
+
+    $stage = $event->getStage();
+    $active_scaffold_files = $this->getDefaultSiteFilesFromScaffold($stage->getActiveComposer());
+
+    // If the active directory and staging area have different files scaffolded
+    // into `sites/default` (i.e., files were added, renamed, or deleted), the
+    // site directory itself must be writable for the changes to be applied.
+    if ($event instanceof PreApplyEvent) {
+      $staged_scaffold_files = $this->getDefaultSiteFilesFromScaffold($stage->getStageComposer());
+
+      if ($active_scaffold_files !== $staged_scaffold_files) {
+        $paths[] = $site_dir;
+      }
+    }
+    // The scaffolded files themselves must be writable, so that any changes to
+    // them in the staging area can be synced back to the active directory.
+    foreach ($active_scaffold_files as $scaffold_file) {
+      $paths[] = $site_dir . '/' . $scaffold_file;
+    }
+
+    // Flag messages about anything in $paths which exists, but isn't writable.
+    $non_writable_files = array_filter($paths, function (string $path): bool {
+      return file_exists($path) && !is_writable($path);
+    });
+    if ($non_writable_files) {
+      // Re-key the messages in order to prevent false negative comparisons in
+      // tests.
+      $non_writable_files = array_values($non_writable_files);
+      $event->addError($non_writable_files, $this->t('The following paths must be writable in order to update default site configuration files.'));
+    }
+  }
+
+  /**
+   * Returns the list of file names scaffolded into `sites/default`.
+   *
+   * @param \Drupal\package_manager\ComposerUtility $composer
+   *   A Composer utility helper for a directory.
+   *
+   * @return string[]
+   *   The names of files that are scaffolded into `sites/default`, stripped
+   *   of the preceding path. For example,
+   *   `[web-root]/sites/default/default.settings.php` will be
+   *   `default.settings.php`. Will be sorted alphabetically. If the target
+   *   directory doesn't have the `drupal/core` package installed, the returned
+   *   array will be empty.
+   */
+  protected function getDefaultSiteFilesFromScaffold(ComposerUtility $composer): array {
+    $installed = $composer->getInstalledPackages();
+
+    if (array_key_exists('drupal/core', $installed)) {
+      $extra = $installed['drupal/core']->getExtra();
+      // We expect Drupal core to provide a list of scaffold files.
+      $files = $extra['drupal-scaffold']['file-mapping'];
+    }
+    else {
+      $files = [];
+    }
+    $files = array_keys($files);
+    $files = preg_grep('/sites\/default\//', $files);
+    $files = array_map('basename', $files);
+    sort($files);
+
+    return $files;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getSubscribedEvents() {
+    return [
+      ReadinessCheckEvent::class => 'validateStagePreOperation',
+      PreCreateEvent::class => 'validateStagePreOperation',
+      PreApplyEvent::class => 'validateStagePreOperation',
+    ];
+  }
+
+}

tests/fixtures/fake-site/vendor/composer/installed.json

@@ -16,7 +16,12 @@
         },
         {
             "name": "drupal/core",
-            "version": "9.8.0"
+            "version": "9.8.0",
+            "extra": {
+                "drupal-scaffold": {
+                    "file-mapping": {}
+                }
+            }
         },
         {
             "name": "drupal/core-dev",

tests/fixtures/project_staged_validation/new_project_added/active.installed.json

@@ -8,7 +8,12 @@
     {
       "name": "drupal/core",
       "version": "9.8.0",
-      "type": "drupal-core"
+      "type": "drupal-core",
+      "extra": {
+        "drupal-scaffold": {
+          "file-mapping": {}
+        }
+      }
     },
     {
       "name": "drupal/test_module",

tests/fixtures/project_staged_validation/new_project_added/staged.installed.json

@@ -8,7 +8,12 @@
     {
       "name": "drupal/core",
       "version": "9.8.1",
-      "type": "drupal-core"
+      "type": "drupal-core",
+      "extra": {
+        "drupal-scaffold": {
+          "file-mapping": {}
+        }
+      }
     },
     {
       "name": "drupal/test_module",

tests/fixtures/project_staged_validation/no_errors/active.installed.json

@@ -8,7 +8,12 @@
     {
       "name": "drupal/core",
       "version": "9.8.0",
-      "type": "drupal-core"
+      "type": "drupal-core",
+      "extra": {
+        "drupal-scaffold": {
+          "file-mapping": {}
+        }
+      }
     },
     {
       "name": "drupal/test_module",

tests/fixtures/project_staged_validation/no_errors/staged.installed.json

@@ -8,7 +8,12 @@
     {
       "name": "drupal/core",
       "version": "9.8.1",
-      "type": "drupal-core"
+      "type": "drupal-core",
+      "extra": {
+        "drupal-scaffold": {
+          "file-mapping": {}
+        }
+      }
     },
     {
       "name": "drupal/test_module",

tests/fixtures/project_staged_validation/project_removed/active.installed.json

@@ -8,7 +8,12 @@
     {
       "name": "drupal/core",
       "version": "9.8.0",
-      "type": "drupal-core"
+      "type": "drupal-core",
+      "extra": {
+        "drupal-scaffold": {
+          "file-mapping": {}
+        }
+      }
     },
     {
       "name": "drupal/test_theme",

tests/fixtures/project_staged_validation/project_removed/staged.installed.json

@@ -8,7 +8,12 @@
     {
       "name": "drupal/core",
       "version": "9.8.1",
-      "type": "drupal-core"
+      "type": "drupal-core",
+      "extra": {
+        "drupal-scaffold": {
+          "file-mapping": {}
+        }
+      }
     },
     {
       "name": "drupal/test_module2",

tests/fixtures/project_staged_validation/version_changed/active.installed.json

@@ -8,7 +8,12 @@
     {
       "name": "drupal/core",
       "version": "9.8.0",
-      "type": "drupal-core"
+      "type": "drupal-core",
+      "extra": {
+        "drupal-scaffold": {
+          "file-mapping": {}
+        }
+      }
     },
     {
       "name": "drupal/test_module",

tests/fixtures/project_staged_validation/version_changed/staged.installed.json

@@ -8,7 +8,12 @@
     {
       "name": "drupal/core",
       "version": "9.8.1",
-      "type": "drupal-core"
+      "type": "drupal-core",
+      "extra": {
+        "drupal-scaffold": {
+          "file-mapping": {}
+        }
+      }
     },
     {
       "name": "drupal/test_module",

tests/src/Functional/ReadinessValidationTest.php

@@ -402,10 +402,13 @@ class ReadinessValidationTest extends AutomaticUpdatesFunctionalTestBase {
       'package_manager_test_fixture',
     ]);
     // Because all actual staging operations are bypassed by
-    // package_manager_bypass (enabled by the parent class), disable this
-    // validator because it will complain if there's no actual Composer data to
-    // inspect.
-    $this->disableValidators(['automatic_updates.staged_projects_validator']);
+    // package_manager_bypass (enabled by the parent class), disable these
+    // validators because they will complain if there's no actual Composer data
+    // to inspect.
+    $this->disableValidators([
+      'automatic_updates.staged_projects_validator',
+      'automatic_updates.validator.scaffold_file_permissions',
+    ]);
 
     // The error should be persistently visible, even after the checker stops
     // flagging it.

tests/src/Functional/UpdaterFormTest.php

@@ -43,9 +43,10 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
    */
   protected function setUp(): void {
     // In this test class, all actual staging operations are bypassed by
-    // package_manager_bypass, which means this validator will complain because
-    // there is no actual Composer data for it to inspect.
+    // package_manager_bypass, which means these validators will complain
+    // because there is no actual Composer data for them to inspect.
     $this->disableValidators[] = 'automatic_updates.staged_projects_validator';
+    $this->disableValidators[] = 'automatic_updates.validator.scaffold_file_permissions';
 
     parent::setUp();
 

tests/src/Kernel/CronUpdaterTest.php

@@ -57,6 +57,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
     // they attempt to compare the active and stage directories.
     $this->disableValidators[] = 'automatic_updates.validator.staged_database_updates';
     $this->disableValidators[] = 'automatic_updates.staged_projects_validator';
+    $this->disableValidators[] = 'automatic_updates.validator.scaffold_file_permissions';
     parent::setUp();
 
     $this->logger = new TestLogger();

tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php

@@ -238,11 +238,16 @@ class ReadinessValidationManagerTest extends AutomaticUpdatesKernelTestBase {
     // results should be stored.
     $this->assertValidationResultsEqual($results, $manager->getResults());
 
-    // Don't validate staged projects because actual staging operations are
-    // bypassed by package_manager_bypass, which will make this validator
-    // complain that there is no actual Composer data for it to inspect.
-    $validator = $this->container->get('automatic_updates.staged_projects_validator');
-    $this->container->get('event_dispatcher')->removeSubscriber($validator);
+    // Don't validate staged projects or scaffold file permissions because
+    // actual staging operations are bypassed by package_manager_bypass, which
+    // will make these validators complain that there is no actual Composer data
+    // for them to inspect.
+    $validators = array_map([$this->container, 'get'], [
+      'automatic_updates.staged_projects_validator',
+      'automatic_updates.validator.scaffold_file_permissions',
+    ]);
+    $event_dispatcher = $this->container->get('event_dispatcher');
+    array_walk($validators, [$event_dispatcher, 'removeSubscriber']);
 
     /** @var \Drupal\automatic_updates\Updater $updater */
     $updater = $this->container->get('automatic_updates.updater');