Save the archive of core download --no-extract safely

features/core-download.feature

@@ -552,6 +552,82 @@ Feature: Download WordPress
       """
     And the return code should be 1
 
+  Scenario: Core download with no-extract does not name an archive after its URL
+    Given an empty directory
+    And that HTTP requests to http://example.com/payload.php.md5 will respond with:
+      """
+      HTTP/1.1 200 OK
+      Content-Type: text/plain
+
+      3c58c7ed6e95bcf8fadbadc3cc2ca52b
+      """
+    And that HTTP requests to http://example.com/payload.php will respond with:
+      """
+      HTTP/1.1 200 OK
+      Content-Type: application/octet-stream
+
+      PK This only needs to look like a zip archive.
+      """
+
+    # The mocked responses make the HTTP library emit warnings, so STDERR is not checked.
+    When I try `wp core download http://example.com/payload.php --no-extract`
+    Then STDOUT should contain:
+      """
+      Success: WordPress downloaded.
+      """
+    And the return code should be 0
+    And the payload.php file should not exist
+    And the {RUN_DIR} directory should contain:
+      """
+      .zip
+      """
+
+  @skip-windows
+  Scenario: Core download with no-extract replaces a symlink at the archive path instead of writing through it
+    Given an empty directory
+    And an empty cache
+    And a victim.txt file:
+      """
+      Not an archive.
+      """
+
+    When I run `ln -s victim.txt wordpress-4.5-de_DE.zip`
+    And I run `wp core download --version=4.5 --locale=de_DE --no-extract`
+    Then the victim.txt file should be:
+      """
+      Not an archive.
+      """
+    And the wordpress-4.5-de_DE.zip file should exist
+
+    When I run `rm wordpress-4.5-de_DE.zip`
+    And I run `ln -s victim.txt wordpress-4.5-de_DE.zip`
+    And I run `wp core download --version=4.5 --locale=de_DE --no-extract`
+    Then STDOUT should contain:
+      """
+      Using cached file
+      """
+    And the victim.txt file should be:
+      """
+      Not an archive.
+      """
+
+  @skip-windows
+  Scenario: Core download with no-extract keeps the permissions of an archive it replaces
+    Given an empty directory
+    And an empty cache
+    And a wordpress-4.5-de_DE.zip file:
+      """
+      An older copy of the archive.
+      """
+
+    When I run `chmod 600 wordpress-4.5-de_DE.zip`
+    And I run `wp core download --version=4.5 --locale=de_DE --no-extract`
+    And I run `php -r 'printf( "%o", fileperms( "wordpress-4.5-de_DE.zip" ) & 0777 );'`
+    Then STDOUT should be:
+      """
+      600
+      """
+
   Scenario: Core download with no-extract should not unzip the download file (already cached)
     Given an empty directory
 

src/Core_Command.php

@@ -292,8 +292,8 @@ public function download( $args, $assoc_args ) {
 					WP_CLI::warning( 'Extraction failed, downloading a new copy...' );
 					$bad_cache = true;
 				}
-			} elseif ( ! copy( $cache_file, $download_dir . basename( $cache_file ) ) ) {
-				WP_CLI::error( "Couldn't copy WordPress archive to '{$download_dir}" . basename( $cache_file ) . "'." );
+			} else {
+				self::save_archive( $cache_file, $download_dir . basename( $cache_file ) );
 			}
 		}
 
@@ -414,14 +414,11 @@ function () use ( $old_temp ) {
 					WP_CLI::error( "Couldn't extract WordPress archive. {$exception->getMessage()}" );
 				}
 			} else {
-				// Name the archive like the cached one, rather than after the temporary file.
-				$archive_name = $from_url ? basename( (string) Utils\parse_url( $download_url, PHP_URL_PATH ) ) : "wordpress-{$version}-{$locale}.{$extension}";
-				if ( '' === $archive_name || '.' === $archive_name ) {
-					$archive_name = basename( $temp );
-				}
-				if ( ! copy( $temp, $download_dir . $archive_name ) ) {
-					WP_CLI::error( "Couldn't copy WordPress archive to '{$download_dir}{$archive_name}'." );
-				}
+				// Name a release like the cached copy of it. An archive downloaded from a URL keeps
+				// the random name of the temporary file, so that the URL cannot choose the name or
+				// type of a file written into the target directory.
+				$archive_name = $from_url ? basename( $temp ) : "wordpress-{$version}-{$locale}.{$extension}";
+				self::save_archive( $temp, $download_dir . $archive_name );
 			}
 
 			// Do not use the cache for nightly builds or for downloaded URLs
@@ -2251,6 +2248,52 @@ private function remove_directory( $dir, $abspath_realpath_trailing ) {
 		return true;
 	}
 
+	/**
+	 * Save a downloaded archive at its destination in the download directory.
+	 *
+	 * The archive is written to a new file with a random name next to the destination first, and
+	 * then moved into place, so that a symlink at the destination is replaced instead of followed.
+	 *
+	 * @param string $source      Path of the archive to save.
+	 * @param string $destination Path to save the archive at.
+	 */
+	private static function save_archive( $source, $destination ): void {
+		$partial = dirname( $destination ) . '/.wp-cli-archive-' . bin2hex( random_bytes( 8 ) ) . '.part';
+
+		// Creating the file exclusively fails if anything, including a symlink, is already at its path.
+		// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged -- Failures are reported below.
+		$input = @fopen( $source, 'rb' );
+		// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged -- Failures are reported below.
+		$output = @fopen( $partial, 'xb' );
+
+		$copied = false !== $input && false !== $output && false !== stream_copy_to_stream( $input, $output );
+
+		if ( false !== $input ) {
+			fclose( $input );
+		}
+
+		if ( false !== $output ) {
+			fclose( $output );
+		}
+
+		// Keep the permissions of an archive that is replaced, as copying over it would have.
+		if ( $copied && ! is_link( $destination ) && is_file( $destination ) ) {
+			$mode = fileperms( $destination );
+			if ( false !== $mode ) {
+				chmod( $partial, $mode & 0777 );
+			}
+		}
+
+		// Moving the file into place replaces whatever is at the destination, rather than writing through a symlink there.
+		// phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged -- Failures are reported below.
+		if ( ! $copied || ! @rename( $partial, $destination ) ) {
+			if ( file_exists( $partial ) ) {
+				unlink( $partial );
+			}
+			WP_CLI::error( "Couldn't copy WordPress archive to '{$destination}'." );
+		}
+	}
+
 	private static function strip_content_dir( $zip_file ) {
 		$new_zip_file = Utils\make_temp_file( 'wp_', '.zip' );
 		register_shutdown_function(