Issue #3311200 by yash.rode, tedbow, Wim Leers, David Strauss, phenaproxima,...

Issue #3311200 by yash.rode, tedbow, Wim Leers, David Strauss, phenaproxima, TravisCarden: Cron updater should delete the existing stage if not available and the site is currently on an insecure version

src/CronUpdater.php

@@ -161,6 +161,28 @@ class CronUpdater extends Updater {
   private function performUpdate(string $target_version, ?int $timeout): void {
     $project_info = new ProjectInfo('drupal');
 
+    if (!$this->isAvailable()) {
+      if ($project_info->isInstalledVersionSafe() && !$this->isApplying()) {
+        $this->logger->notice('Cron will not perform any updates because there is an existing stage and the current version of the site is secure.');
+        return;
+      }
+      if (!$project_info->isInstalledVersionSafe() && $this->isApplying()) {
+        $this->logger->notice(
+          'Cron will not perform any updates as an existing staged update is applying. The site is currently on an insecure version of Drupal core but will attempt to update to a secure version next time cron is run. This update may be applied manually at the <a href="%url">update form</a>.',
+          ['%url' => Url::fromRoute('update.report_update')->setAbsolute()->toString()],
+        );
+        return;
+      }
+    }
+
+    // Delete the existing staging area if not available and the site is
+    // currently on an insecure version.
+    if (!$project_info->isInstalledVersionSafe() && !$this->isAvailable() && !$this->isApplying()) {
+      // @todo Improve this in https://www.drupal.org/i/3325654.
+      $this->logger->notice('The existing stage was not in the process of being applied, so it was destroyed to allow updating the site to a secure version during cron.');
+      $this->destroy(TRUE);
+    }
+
     $installed_version = $project_info->getInstalledVersion();
     if (empty($installed_version)) {
       $this->logger->error('Unable to determine the current version of Drupal core.');

tests/src/Build/CoreUpdateTest.php

@@ -4,6 +4,7 @@ declare(strict_types = 1);
 
 namespace Drupal\Tests\automatic_updates\Build;
 
+use Behat\Mink\Element\DocumentElement;
 use Drupal\automatic_updates\CronUpdater;
 use Drupal\automatic_updates\Updater;
 use Drupal\Composer\Composer;
@@ -146,20 +147,7 @@ class CoreUpdateTest extends UpdateTestBase {
     $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(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 to 9.8.1');
-    $this->waitForBatchJob();
-    $assert_session->pageTextContains('Ready to update');
+    $this->coreUpdateTillUpdateReady($page);
     $page->pressButton('Continue');
     $this->waitForBatchJob();
     $assert_session->pageTextContains('Update complete!');
@@ -230,6 +218,22 @@ class CoreUpdateTest extends UpdateTestBase {
     $this->webAssert->statusMessageNotExists('warning');
   }
 
+  /**
+   * Tests stage is destroyed if not available and site is on insecure version.
+   */
+  public function testStageDestroyedIfNotAvailable(): void {
+    $this->createTestProject('RecommendedProject');
+    $mink = $this->getMink();
+    $session = $mink->getSession();
+    $page = $session->getPage();
+    $assert_session = $mink->assertSession();
+    $this->coreUpdateTillUpdateReady($page);
+    $this->visit('/admin/reports/status');
+    $assert_session->pageTextContains('Your site is ready for automatic updates.');
+    $page->clickLink('Run cron');
+    $this->assertUpdateSuccessful('9.8.1');
+  }
+
   /**
    * Asserts that the update is prevented if the filesystem isn't writable.
    *
@@ -373,4 +377,28 @@ class CoreUpdateTest extends UpdateTestBase {
     $this->assertCoreVersion($expected_version);
   }
 
+  /**
+   * Performs core update till update ready form.
+   *
+   * @param \Behat\Mink\Element\DocumentElement $page
+   *   The page element.
+   */
+  private function coreUpdateTillUpdateReady(DocumentElement $page): void {
+    $session = $this->getMink()->getSession();
+    $this->visit('/admin/modules');
+    $assert_session = $this->getMink()->assertSession($session);
+    $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(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 to 9.8.1');
+    $this->waitForBatchJob();
+    $assert_session->pageTextContains('Ready to update');
+  }
+
 }

tests/src/Kernel/CronUpdaterTest.php

@@ -22,6 +22,7 @@ use Drupal\package_manager\Exception\StageValidationException;
 use Drupal\package_manager\ValidationResult;
 use Drupal\package_manager_bypass\Committer;
 use Drupal\Tests\automatic_updates\Traits\EmailNotificationsTestTrait;
+use Drupal\Tests\package_manager\Kernel\TestStage;
 use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;
 use Drupal\Tests\user\Traits\UserCreationTrait;
 use Prophecy\Argument;
@@ -344,6 +345,90 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
     }
   }
 
+  /**
+   * Tests stage is destroyed if not available and site is on insecure version.
+   */
+  public function testStageDestroyedIfNotAvailable(): void {
+    $stage = $this->createStage();
+    $stage->create();
+    $original_stage_directory = $stage->getStageDirectory();
+    $this->assertDirectoryExists($original_stage_directory);
+
+    $listener = function (PostRequireEvent $event) use (&$cron_stage_dir, $original_stage_directory): void {
+      $this->assertDirectoryDoesNotExist($original_stage_directory);
+      $cron_stage_dir = $this->container->get('package_manager.stager')->getInvocationArguments()[0][1]->resolve();
+      $this->assertSame($event->getStage()->getStageDirectory(), $cron_stage_dir);
+      $this->assertDirectoryExists($cron_stage_dir);
+    };
+    $this->container->get('event_dispatcher')->addListener(PostRequireEvent::class, $listener, PHP_INT_MAX);
+
+    $this->container->get('cron')->run();
+    $this->assertIsString($cron_stage_dir);
+    $this->assertNotEquals($original_stage_directory, $cron_stage_dir);
+    $this->assertDirectoryDoesNotExist($cron_stage_dir);
+    $this->assertTrue($this->logger->hasRecord('The existing stage was not in the process of being applied, so it was destroyed to allow updating the site to a secure version during cron.', (string) RfcLogLevel::NOTICE));
+  }
+
+  /**
+   * Tests stage is not destroyed if another update is applying.
+   */
+  public function testStageNotDestroyedIfApplying(): void {
+    $this->config('automatic_updates.settings')->set('cron', CronUpdater::ALL)->save();
+    $this->setReleaseMetadata([
+      'drupal' => __DIR__ . "/../../../package_manager/tests/fixtures/release-history/drupal.9.8.1-security.xml",
+    ]);
+    $this->setCoreVersion('9.8.0');
+    $stage = $this->createStage();
+    $stage->create();
+    $stage->require(['drupal/core:9.8.1']);
+    $stop_error = t('Stopping stage from applying');
+
+    // Add a PreApplyEvent event listener so we can attempt to run cron when
+    // another stage is applying.
+    $this->container->get('event_dispatcher')->addListener(PreApplyEvent::class, function (PreApplyEvent $event) use ($stop_error) {
+      // Ensure the stage that is applying the operation is not the cron
+      // updater.
+      $this->assertInstanceOf(TestStage::class, $event->getStage());
+      $this->container->get('cron')->run();
+      // We do not actually want to apply this operation it was just invoked to
+      // allow cron to be  attempted.
+      $event->addError([$stop_error]);
+    }, PHP_INT_MAX);
+
+    try {
+      $stage->apply();
+      $this->fail('Expected update to fail');
+    }
+    catch (StageValidationException $exception) {
+      $this->assertValidationResultsEqual([ValidationResult::createError([$stop_error])], $exception->getResults());
+    }
+
+    $this->assertTrue($this->logger->hasRecord("Cron will not perform any updates as an existing staged update is applying. The site is currently on an insecure version of Drupal core but will attempt to update to a secure version next time cron is run. This update may be applied manually at the <a href=\"%url\">update form</a>.", (string) RfcLogLevel::NOTICE));
+    $this->assertUpdateStagedTimes(1);
+  }
+
+  /**
+   * Tests stage is not destroyed if not available and site is on secure version.
+   */
+  public function testStageNotDestroyedIfSecure(): void {
+    $this->config('automatic_updates.settings')->set('cron', CronUpdater::ALL)->save();
+    $this->setReleaseMetadata([
+      'drupal' => __DIR__ . "/../../../package_manager/tests/fixtures/release-history/drupal.9.8.2.xml",
+    ]);
+    $this->setCoreVersion('9.8.1');
+    $stage = $this->createStage();
+    $stage->create();
+    $stage->require(['drupal/random']);
+    $this->assertUpdateStagedTimes(1);
+
+    // Trigger CronUpdater, the above should cause it to detect a stage that is
+    // applying.
+    $this->container->get('cron')->run();
+
+    $this->assertTrue($this->logger->hasRecord('Cron will not perform any updates because there is an existing stage and the current version of the site is secure.', (string) RfcLogLevel::NOTICE));
+    $this->assertUpdateStagedTimes(1);
+  }
+
   /**
    * Tests that CronUpdater::begin() unconditionally throws an exception.
    */