From dc7e54d424c4bad98fb0b0e722c74c2e2480ade8 Mon Sep 17 00:00:00 2001 From: Adam G-H <32250-phenaproxima@users.noreply.drupalcode.org> Date: Tue, 30 Jan 2024 15:46:34 +0000 Subject: [PATCH] Issue #3417905: Prevent random build test failures by preventing stage IDs from starting with - --- package_manager/src/StageBase.php | 10 ++++++++-- src/ConsoleUpdateStage.php | 6 +----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/package_manager/src/StageBase.php b/package_manager/src/StageBase.php index 9caba25efb..c51efdfef6 100644 --- a/package_manager/src/StageBase.php +++ b/package_manager/src/StageBase.php @@ -6,7 +6,7 @@ namespace Drupal\package_manager; use Composer\Semver\VersionParser; use Drupal\Component\Datetime\TimeInterface; -use Drupal\Component\Utility\Crypt; +use Drupal\Component\Utility\Random; use Drupal\Core\Queue\QueueFactory; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\StringTranslation\TranslatableMarkup; @@ -344,7 +344,13 @@ abstract class StageBase implements LoggerAwareInterface { // to create a stage directory at around the same time. If an error occurs // while the event is being processed, the stage is marked as available. // @see ::dispatch() - $id = Crypt::randomBytesBase64(); + // We specifically generate a random 32-character alphanumeric name in order + // to guarantee that the the stage ID won't start with -, which could cause + // it to be interpreted as an option if it's used as a command-line + // argument. (For example, + // \Drupal\Component\Utility\Crypt::randomBytesBase64() would be vulnerable + // to this; the stage ID needs to be unique, but not cryptographically so.) + $id = (new Random())->name(32); // Re-acquire the tempstore to ensure that the lock is written by whoever is // actually logged in (or not) right now, since it's possible that the stage // was instantiated (i.e., __construct() was called) by a different session, diff --git a/src/ConsoleUpdateStage.php b/src/ConsoleUpdateStage.php index 4297f800b9..101d75fb03 100644 --- a/src/ConsoleUpdateStage.php +++ b/src/ConsoleUpdateStage.php @@ -280,11 +280,7 @@ class ConsoleUpdateStage extends UpdateStage { * The ID of the current stage. */ protected function triggerPostApply(string $stage_id): void { - // The stage ID needs to be quoted in order to prevent it from being parsed - // as a command-line option if it begins with -, which is a possibility - // because we use \Drupal\Component\Utility\Crypt::randomBytesBase64() to - // generate the stage ID, and the string it returns might begin with -. - $arguments = sprintf('post-apply "%s"', $stage_id); + $arguments = sprintf('post-apply %s', $stage_id); if ($this->isFromWeb) { $arguments .= ' --is-from-web'; } -- GitLab