From a8e6af281414e904a1cd664057e88068558698d1 Mon Sep 17 00:00:00 2001 From: Alain Schlesser Date: Sat, 31 Oct 2026 10:27:54 +0100 Subject: [PATCH 1/3] Save the archive of core download --no-extract safely An archive downloaded from a URL was named after the URL since #355, which let whoever provides the URL choose the name and type of a file written into the target directory. Keep the random name of the temporary file for these again, with the extension detected from the archive. Save the archive by writing a new file with a random name next to its destination and moving it into place, so that a symlink at the destination is replaced instead of written through. Co-Authored-By: Claude Opus 5.5 --- features/core-download.feature | 59 ++++++++++++++++++++++++++++++++++ src/Core_Command.php | 55 +++++++++++++++++++++++++------ 2 files changed, 104 insertions(+), 10 deletions(-) diff --git a/features/core-download.feature b/features/core-download.feature index 1242fc80..e3ad2304 100644 --- a/features/core-download.feature +++ b/features/core-download.feature @@ -552,6 +552,65 @@ 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. + """ + Scenario: Core download with no-extract should not unzip the download file (already cached) Given an empty directory diff --git a/src/Core_Command.php b/src/Core_Command.php index 81eebfc7..f133d5ac 100644 --- a/src/Core_Command.php +++ b/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,44 @@ 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 ); + } + + // 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( From 3c4ae6110b13eeb4993ecffe1db552188fe42ae2 Mon Sep 17 00:00:00 2001 From: Alain Schlesser Date: Sat, 31 Oct 2026 10:42:55 +0100 Subject: [PATCH 2/3] Keep the permissions of an archive that is replaced Co-Authored-By: Claude Opus 5.5 --- src/Core_Command.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Core_Command.php b/src/Core_Command.php index f133d5ac..e35f342d 100644 --- a/src/Core_Command.php +++ b/src/Core_Command.php @@ -2276,6 +2276,14 @@ private static function save_archive( $source, $destination ): void { 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 ) ) { From 512b572cc48aa95ef6c5db3e73faec0fbc66f578 Mon Sep 17 00:00:00 2001 From: Alain Schlesser Date: Sat, 31 Oct 2026 10:53:47 +0100 Subject: [PATCH 3/3] Cover keeping the permissions of a replaced archive with a scenario Co-Authored-By: Claude Opus 5.5 --- features/core-download.feature | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/features/core-download.feature b/features/core-download.feature index e3ad2304..41820e70 100644 --- a/features/core-download.feature +++ b/features/core-download.feature @@ -611,6 +611,23 @@ Feature: Download WordPress 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