From 66cbcf2f2f6c6b0d08f822df3950f29c8a3b1f6c Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Thu, 9 Jun 2022 17:20:07 +0000
Subject: [PATCH] Issue #3274892 by phenaproxima, tedbow: Before applying the
update, check that any needed changes to the site directory are possible
---
automatic_updates.services.yml | 6 +
.../fake-site/vendor/composer/installed.json | 7 +-
.../tests/src/Functional/UpdaterFormTest.php | 5 +
.../sites/default/default.services.yml | 2 +
.../sites/default/default.settings.php | 6 +
.../fake_site/vendor/composer/installed.json | 10 +-
.../ScaffoldFilePermissionsValidator.php | 127 +++++++
.../fake-site/vendor/composer/installed.json | 7 +-
.../new_project_added/active.installed.json | 7 +-
.../new_project_added/staged.installed.json | 7 +-
.../no_errors/active.installed.json | 7 +-
.../no_errors/staged.installed.json | 7 +-
.../project_removed/active.installed.json | 7 +-
.../project_removed/staged.installed.json | 7 +-
.../version_changed/active.installed.json | 7 +-
.../version_changed/staged.installed.json | 7 +-
.../Functional/ReadinessValidationTest.php | 11 +-
tests/src/Functional/UpdaterFormTest.php | 5 +-
tests/src/Kernel/CronUpdaterTest.php | 1 +
.../ReadinessValidationManagerTest.php | 15 +-
.../ScaffoldFilePermissionsValidatorTest.php | 346 ++++++++++++++++++
.../StagedProjectsValidatorTest.php | 12 +-
22 files changed, 592 insertions(+), 24 deletions(-)
create mode 100644 package_manager/tests/fixtures/fake_site/sites/default/default.services.yml
create mode 100644 package_manager/tests/fixtures/fake_site/sites/default/default.settings.php
create mode 100644 src/Validator/ScaffoldFilePermissionsValidator.php
create mode 100644 tests/src/Kernel/ReadinessValidation/ScaffoldFilePermissionsValidatorTest.php
diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml
index a5e5545c3a..8ef9946555 100644
--- a/automatic_updates.services.yml
+++ b/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 }
diff --git a/automatic_updates_extensions/tests/fixtures/fake-site/vendor/composer/installed.json b/automatic_updates_extensions/tests/fixtures/fake-site/vendor/composer/installed.json
index dffe8ff002..630abfad5d 100644
--- a/automatic_updates_extensions/tests/fixtures/fake-site/vendor/composer/installed.json
+++ b/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",
diff --git a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php
index b4212ed7cc..8fd85a0a45 100644
--- a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php
+++ b/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");
diff --git a/package_manager/tests/fixtures/fake_site/sites/default/default.services.yml b/package_manager/tests/fixtures/fake_site/sites/default/default.services.yml
new file mode 100644
index 0000000000..95dde1725d
--- /dev/null
+++ b/package_manager/tests/fixtures/fake_site/sites/default/default.services.yml
@@ -0,0 +1,2 @@
+# This file should be staged because it's scaffolded into place by Drupal core.
+services: {}
diff --git a/package_manager/tests/fixtures/fake_site/sites/default/default.settings.php b/package_manager/tests/fixtures/fake_site/sites/default/default.settings.php
new file mode 100644
index 0000000000..0d23e84006
--- /dev/null
+++ b/package_manager/tests/fixtures/fake_site/sites/default/default.settings.php
@@ -0,0 +1,6 @@
+<?php
+
+/**
+ * @file
+ * This file should be staged because it's scaffolded into place by Drupal core.
+ */
diff --git a/package_manager/tests/fixtures/fake_site/vendor/composer/installed.json b/package_manager/tests/fixtures/fake_site/vendor/composer/installed.json
index ad9a32852d..bc3f936e44 100644
--- a/package_manager/tests/fixtures/fake_site/vendor/composer/installed.json
+++ b/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": ""
+ }
+ }
+ }
}
]
}
diff --git a/src/Validator/ScaffoldFilePermissionsValidator.php b/src/Validator/ScaffoldFilePermissionsValidator.php
new file mode 100644
index 0000000000..307ccb940c
--- /dev/null
+++ b/src/Validator/ScaffoldFilePermissionsValidator.php
@@ -0,0 +1,127 @@
+<?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',
+ ];
+ }
+
+}
diff --git a/tests/fixtures/fake-site/vendor/composer/installed.json b/tests/fixtures/fake-site/vendor/composer/installed.json
index c8ad1ba32e..75bdd28cd5 100644
--- a/tests/fixtures/fake-site/vendor/composer/installed.json
+++ b/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",
diff --git a/tests/fixtures/project_staged_validation/new_project_added/active.installed.json b/tests/fixtures/project_staged_validation/new_project_added/active.installed.json
index 56d81ccad1..1f6de85d07 100644
--- a/tests/fixtures/project_staged_validation/new_project_added/active.installed.json
+++ b/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",
diff --git a/tests/fixtures/project_staged_validation/new_project_added/staged.installed.json b/tests/fixtures/project_staged_validation/new_project_added/staged.installed.json
index b0c0c6d63e..b0bdf7c1a4 100644
--- a/tests/fixtures/project_staged_validation/new_project_added/staged.installed.json
+++ b/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",
diff --git a/tests/fixtures/project_staged_validation/no_errors/active.installed.json b/tests/fixtures/project_staged_validation/no_errors/active.installed.json
index 65a8de94c6..6b93669c33 100644
--- a/tests/fixtures/project_staged_validation/no_errors/active.installed.json
+++ b/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",
diff --git a/tests/fixtures/project_staged_validation/no_errors/staged.installed.json b/tests/fixtures/project_staged_validation/no_errors/staged.installed.json
index f395558489..ee87bd5e9e 100644
--- a/tests/fixtures/project_staged_validation/no_errors/staged.installed.json
+++ b/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",
diff --git a/tests/fixtures/project_staged_validation/project_removed/active.installed.json b/tests/fixtures/project_staged_validation/project_removed/active.installed.json
index 4ec3e126c9..6e2ecb6b9f 100644
--- a/tests/fixtures/project_staged_validation/project_removed/active.installed.json
+++ b/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",
diff --git a/tests/fixtures/project_staged_validation/project_removed/staged.installed.json b/tests/fixtures/project_staged_validation/project_removed/staged.installed.json
index 100c033c34..c44eb0104d 100644
--- a/tests/fixtures/project_staged_validation/project_removed/staged.installed.json
+++ b/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",
diff --git a/tests/fixtures/project_staged_validation/version_changed/active.installed.json b/tests/fixtures/project_staged_validation/version_changed/active.installed.json
index e7bb050139..cb7c3ca5ef 100644
--- a/tests/fixtures/project_staged_validation/version_changed/active.installed.json
+++ b/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",
diff --git a/tests/fixtures/project_staged_validation/version_changed/staged.installed.json b/tests/fixtures/project_staged_validation/version_changed/staged.installed.json
index 62a4b1dc72..e457476ace 100644
--- a/tests/fixtures/project_staged_validation/version_changed/staged.installed.json
+++ b/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",
diff --git a/tests/src/Functional/ReadinessValidationTest.php b/tests/src/Functional/ReadinessValidationTest.php
index 96eab4ae97..942d28f22b 100644
--- a/tests/src/Functional/ReadinessValidationTest.php
+++ b/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.
diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php
index f73f37443f..7bbb2254bc 100644
--- a/tests/src/Functional/UpdaterFormTest.php
+++ b/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();
diff --git a/tests/src/Kernel/CronUpdaterTest.php b/tests/src/Kernel/CronUpdaterTest.php
index 2e70847acd..93edcf19b7 100644
--- a/tests/src/Kernel/CronUpdaterTest.php
+++ b/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();
diff --git a/tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php b/tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php
index 4094b1f2de..8976c67436 100644
--- a/tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php
+++ b/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');
diff --git a/tests/src/Kernel/ReadinessValidation/ScaffoldFilePermissionsValidatorTest.php b/tests/src/Kernel/ReadinessValidation/ScaffoldFilePermissionsValidatorTest.php
new file mode 100644
index 0000000000..d77ce05b3a
--- /dev/null
+++ b/tests/src/Kernel/ReadinessValidation/ScaffoldFilePermissionsValidatorTest.php
@@ -0,0 +1,346 @@
+<?php
+
+namespace Drupal\Tests\automatic_updates\Kernel\ReadinessValidation;
+
+use Drupal\package_manager\Exception\StageValidationException;
+use Drupal\package_manager\ValidationResult;
+use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase;
+
+/**
+ * @covers \Drupal\automatic_updates\Validator\ScaffoldFilePermissionsValidator
+ *
+ * @group automatic_updates
+ */
+class ScaffoldFilePermissionsValidatorTest extends AutomaticUpdatesKernelTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = ['automatic_updates'];
+
+ /**
+ * The active directory of the virtual project.
+ *
+ * @var string
+ */
+ private $activeDir;
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+
+ $this->createTestProject();
+ $this->activeDir = $this->container->get('package_manager.path_locator')
+ ->getProjectRoot();
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function assertValidationResultsEqual(array $expected_results, array $actual_results): void {
+ $map = function (string $path): string {
+ return $this->activeDir . '/' . $path;
+ };
+ foreach ($expected_results as $i => $result) {
+ // Prepend the active directory to every path listed in the error result,
+ // and add the expected summary.
+ $messages = array_map($map, $result->getMessages());
+ $expected_results[$i] = ValidationResult::createError($messages, t('The following paths must be writable in order to update default site configuration files.'));
+ }
+ parent::assertValidationResultsEqual($expected_results, $actual_results);
+ }
+
+ /**
+ * Write-protects a set of paths in the active directory.
+ *
+ * @param string[] $paths
+ * The paths to write-protect, relative to the active directory.
+ */
+ private function writeProtect(array $paths): void {
+ foreach ($paths as $path) {
+ $path = $this->activeDir . '/' . $path;
+ chmod($path, 0400);
+ $this->assertFileIsNotWritable($path, "Failed to write-protect $path.");
+ }
+ }
+
+ /**
+ * Data provider for ::testPermissionsBeforeStart().
+ *
+ * @return array[]
+ * Sets of arguments to pass to the test method.
+ */
+ public function providerPermissionsBeforeStart(): array {
+ return [
+ 'write-protected scaffold file, writable site directory' => [
+ ['sites/default/default.settings.php'],
+ [
+ ValidationResult::createError(['sites/default/default.settings.php']),
+ ],
+ ],
+ // Whether the site directory is write-protected only matters during
+ // pre-apply, because it only presents a problem if scaffold files have
+ // been added or removed in the staging area. Which is a condition we can
+ // only detect during pre-apply.
+ 'write-protected scaffold file and site directory' => [
+ [
+ 'sites/default/default.settings.php',
+ 'sites/default',
+ ],
+ [
+ ValidationResult::createError(['sites/default/default.settings.php']),
+ ],
+ ],
+ 'write-protected site directory' => [
+ ['sites/default'],
+ [],
+ ],
+ ];
+ }
+
+ /**
+ * Tests that scaffold file permissions are checked before an update begins.
+ *
+ * @param string[] $write_protected_paths
+ * A list of paths, relative to the project root, which should be write
+ * protected before staged changes are applied.
+ * @param \Drupal\package_manager\ValidationResult[] $expected_results
+ * The expected validation results, if any.
+ *
+ * @dataProvider providerPermissionsBeforeStart
+ */
+ public function testPermissionsBeforeStart(array $write_protected_paths, array $expected_results): void {
+ $this->writeProtect($write_protected_paths);
+ $this->assertCheckerResultsFromManager($expected_results, TRUE);
+
+ try {
+ $this->container->get('automatic_updates.updater')
+ ->begin(['drupal' => '9.8.1']);
+
+ // If no exception was thrown, ensure that we weren't expecting an error.
+ $this->assertEmpty($expected_results);
+ }
+ catch (StageValidationException $e) {
+ $this->assertValidationResultsEqual($expected_results, $e->getResults());
+ }
+ }
+
+ /**
+ * Data provider for ::testScaffoldFilesChanged().
+ *
+ * @return array[]
+ * Sets of arguments to pass to the test method.
+ */
+ public function providerScaffoldFilesChanged(): array {
+ // The summary is always replaced by ::assertValidationResultsEqual(), so
+ // if there's more than one message in a result, just give it a mocked
+ // summary object to prevent an exception.
+ $summary = $this->prophesize('\Drupal\Core\StringTranslation\TranslatableMarkup')
+ ->reveal();
+
+ return [
+ // If no scaffold files are changed, it doesn't matter if the site
+ // directory is writable.
+ 'no scaffold changes, site directory not writable' => [
+ ['sites/default'],
+ [],
+ [],
+ [],
+ ],
+ 'no scaffold changes, site directory writable' => [
+ [],
+ [],
+ [],
+ [],
+ ],
+ // If scaffold files are added or deleted in the site directory, the site
+ // directory must be writable.
+ 'new scaffold file added to non-writable site directory' => [
+ ['sites/default'],
+ [],
+ [
+ '[web-root]/sites/default/new.txt' => '',
+ ],
+ [
+ ValidationResult::createError(['sites/default']),
+ ],
+ ],
+ 'new scaffold file added to writable site directory' => [
+ [],
+ [],
+ [
+ '[web-root]/sites/default/new.txt' => '',
+ ],
+ [],
+ ],
+ 'writable scaffold file removed from non-writable site directory' => [
+ ['sites/default'],
+ [
+ '[web-root]/sites/default/deleted.txt' => '',
+ ],
+ [],
+ [
+ ValidationResult::createError(['sites/default']),
+ ],
+ ],
+ 'writable scaffold file removed from writable site directory' => [
+ [],
+ [
+ '[web-root]/sites/default/deleted.txt' => '',
+ ],
+ [],
+ [],
+ ],
+ 'non-writable scaffold file removed from non-writable site directory' => [
+ [
+ // The file must be made write-protected before the site directory is,
+ // or the permissions change will fail.
+ 'sites/default/deleted.txt',
+ 'sites/default',
+ ],
+ [
+ '[web-root]/sites/default/deleted.txt' => '',
+ ],
+ [],
+ [
+ ValidationResult::createError(['sites/default', 'sites/default/deleted.txt'], $summary),
+ ],
+ ],
+ 'non-writable scaffold file removed from writable site directory' => [
+ ['sites/default/deleted.txt'],
+ [
+ '[web-root]/sites/default/deleted.txt' => '',
+ ],
+ [],
+ [
+ ValidationResult::createError(['sites/default/deleted.txt']),
+ ],
+ ],
+ // If only scaffold files outside the site directory changed, the
+ // validator doesn't care if the site directory is writable.
+ 'new scaffold file added outside non-writable site directory' => [
+ ['sites/default'],
+ [],
+ [
+ '[web-root]/foo.html' => '',
+ ],
+ [],
+ ],
+ 'new scaffold file added outside writable site directory' => [
+ [],
+ [],
+ [
+ '[web-root]/foo.html' => '',
+ ],
+ [],
+ ],
+ 'writable scaffold file removed outside non-writable site directory' => [
+ ['sites/default'],
+ [
+ '[web-root]/foo.txt' => '',
+ ],
+ [],
+ [],
+ ],
+ 'writable scaffold file removed outside writable site directory' => [
+ [],
+ [
+ '[web-root]/foo.txt' => '',
+ ],
+ [],
+ [],
+ ],
+ 'non-writable scaffold file removed outside non-writable site directory' => [
+ [
+ 'sites/default',
+ 'foo.txt',
+ ],
+ [
+ '[web-root]/foo.txt' => '',
+ ],
+ [],
+ [],
+ ],
+ 'non-writable scaffold file removed outside writable site directory' => [
+ ['foo.txt'],
+ [
+ '[web-root]/foo.txt' => '',
+ ],
+ [],
+ [],
+ ],
+ ];
+ }
+
+ /**
+ * Tests site directory permissions are checked before changes are applied.
+ *
+ * @param string[] $write_protected_paths
+ * A list of paths, relative to the project root, which should be write
+ * protected before staged changes are applied.
+ * @param string[] $active_scaffold_files
+ * An array simulating the extra.drupal-scaffold.file-mapping section of the
+ * active drupal/core package.
+ * @param string[] $staged_scaffold_files
+ * An array simulating the extra.drupal-scaffold.file-mapping section of the
+ * staged drupal/core package.
+ * @param \Drupal\package_manager\ValidationResult[] $expected_results
+ * The expected validation results, if any.
+ *
+ * @dataProvider providerScaffoldFilesChanged
+ */
+ public function testScaffoldFilesChanged(array $write_protected_paths, array $active_scaffold_files, array $staged_scaffold_files, array $expected_results): void {
+ // Create fake scaffold files so we can test scenarios in which a scaffold
+ // file that exists in the active directory is deleted in the staging area.
+ touch($this->activeDir . '/sites/default/deleted.txt');
+ touch($this->activeDir . '/foo.txt');
+
+ // Simulate updating Drupal core. This will copy the active directory into
+ // the (virtual) staging area.
+ // @see ::createTestProject()
+ // @see \Drupal\package_manager_test_fixture\EventSubscriber\FixtureStager::copyFilesFromFixture()
+ $updater = $this->container->get('automatic_updates.updater');
+ $updater->begin(['drupal' => '9.8.1']);
+ $updater->stage();
+
+ // Rewrite the active and staged installed.json files, inserting the given
+ // lists of scaffold files.
+ $installed = [
+ 'packages' => [
+ [
+ 'name' => 'drupal/core',
+ 'version' => \Drupal::VERSION,
+ 'extra' => [
+ 'drupal-scaffold' => [
+ 'file_mapping' => [],
+ ],
+ ],
+ ],
+ ],
+ ];
+ // Since the list of scaffold files is in a deeply nested array, reference
+ // it for readability.
+ $scaffold_files = &$installed['packages'][0]['extra']['drupal-scaffold']['file-mapping'];
+
+ // Change the list of scaffold files in the active and stage directories.
+ $scaffold_files = $active_scaffold_files;
+ file_put_contents($this->activeDir . '/vendor/composer/installed.json', json_encode($installed));
+ $scaffold_files = $staged_scaffold_files;
+ file_put_contents($updater->getStageDirectory() . '/vendor/composer/installed.json', json_encode($installed));
+
+ $this->writeProtect($write_protected_paths);
+
+ try {
+ $updater->apply();
+
+ // If no exception was thrown, ensure that we weren't expecting an error.
+ $this->assertEmpty($expected_results);
+ }
+ catch (StageValidationException $e) {
+ $this->assertValidationResultsEqual($expected_results, $e->getResults());
+ }
+ }
+
+}
diff --git a/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
index f393369997..7268428389 100644
--- a/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
+++ b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
@@ -62,14 +62,22 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
* Tests that exceptions are turned into validation errors.
*/
public function testEventConsumesExceptionResults(): void {
+ /** @var \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher */
+ $event_dispatcher = $this->container->get('event_dispatcher');
+
// Just before the staged changes are applied, delete the composer.json file
// to trigger an error. This uses the highest possible priority to guarantee
// it runs before any other subscribers.
$listener = function (): void {
unlink("$this->activeDir/composer.json");
};
- $this->container->get('event_dispatcher')
- ->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX);
+ $event_dispatcher->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX);
+
+ // Disable the scaffold file permissions validator because it will try to
+ // read composer.json from the active directory, which won't exist thanks to
+ // the event listener we just added.
+ $validator = $this->container->get('automatic_updates.validator.scaffold_file_permissions');
+ $event_dispatcher->removeSubscriber($validator);
$result = ValidationResult::createError([
"Composer could not find the config file: $this->activeDir/composer.json\n",
--
GitLab