diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml index 69c61565692e6a5f2974eb119ed8411b03d0e9dc..cce9d1c7260d870f3d2e8b230c58da57f3b8de7a 100644 --- a/automatic_updates.services.yml +++ b/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: diff --git a/src/Validator/WritableFileSystemValidator.php b/src/Validator/WritableFileSystemValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..ad0dd986a358c5192a35b10fdb17c6b3e99eb4cb --- /dev/null +++ b/src/Validator/WritableFileSystemValidator.php @@ -0,0 +1,87 @@ +<?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', + ]; + } + +} diff --git a/tests/modules/automatic_updates_test/src/TestController.php b/tests/modules/automatic_updates_test/src/TestController.php index 7772d1f01b4bbdca5844053c41026d89b48fc5b1..690edc978630c9c9631164ca9ec67d54a0c6b591 100644 --- a/tests/modules/automatic_updates_test/src/TestController.php +++ b/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); } /** diff --git a/tests/modules/automatic_updates_test_disable_validators/automatic_updates_test_disable_validators.info.yml b/tests/modules/automatic_updates_test_disable_validators/automatic_updates_test_disable_validators.info.yml new file mode 100644 index 0000000000000000000000000000000000000000..31c0d195f8debe32c65b9ff9b5eae5e77377f8e0 --- /dev/null +++ b/tests/modules/automatic_updates_test_disable_validators/automatic_updates_test_disable_validators.info.yml @@ -0,0 +1,4 @@ +name: 'Automatic Updates Test: Disable validators' +description: Allows certain update validators to be disabled during testing. +type: module +package: Testing diff --git a/tests/modules/automatic_updates_test_disable_validators/src/AutomaticUpdatesTestDisableValidatorsServiceProvider.php b/tests/modules/automatic_updates_test_disable_validators/src/AutomaticUpdatesTestDisableValidatorsServiceProvider.php new file mode 100644 index 0000000000000000000000000000000000000000..307b1a71246c236ef4b2ca47f4acc6077abcafc8 --- /dev/null +++ b/tests/modules/automatic_updates_test_disable_validators/src/AutomaticUpdatesTestDisableValidatorsServiceProvider.php @@ -0,0 +1,26 @@ +<?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']); + } + +} diff --git a/tests/src/Build/CoreUpdateTest.php b/tests/src/Build/CoreUpdateTest.php index 323a6b54985c32bd4f2d6fffa2cf1bd36725b0bb..3dddedf38b1885e993b69e06ef25f8b24a832e32 100644 --- a/tests/src/Build/CoreUpdateTest.php +++ b/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. */ diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php index 1bc7feb93c066e48e6353eda0334d2cecc127ae4..70a1ce0ed4e0274a6e68fd9cb3295529e4f90d97 100644 --- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php +++ b/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. diff --git a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php index 8f87e36c0db27642add9fd403b996e4d673535e0..76af4f096689ae46cda1fcf3f17f6bf7c146f097 100644 --- a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php +++ b/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'); } /**