From 3cfa2c5b255ad2c1458065880658ac21f8cf1f73 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 11:46:26 +0000 Subject: [PATCH 1/7] Initial plan From ef1653ae04043e59eb094e2c5515547dbe76920c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 11:58:38 +0000 Subject: [PATCH 2/7] Add path traversal hardening: validate package name and check destination path Agent-Logs-Url: https://github.com/wp-cli/package-command/sessions/159d5271-2365-4bfe-8f16-6fd715d1a133 Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --- features/package-install.feature | 21 +++++++++++++++++++++ src/Package_Command.php | 27 ++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/features/package-install.feature b/features/package-install.feature index ff76ccbe..5b48f071 100644 --- a/features/package-install.feature +++ b/features/package-install.feature @@ -1417,6 +1417,27 @@ Feature: Install WP-CLI packages """ And STDOUT should be empty + Scenario: Reject a ZIP package whose composer.json name contains path-traversal components + Given an empty directory + And a create-traversal-zip.php file: + """ + open( 'traversal.zip', ZipArchive::CREATE ); + $zip->addFromString( 'traversal/composer.json', '{"name":"..","description":"path traversal test","type":"wp-cli-package"}' ); + $zip->addFromString( 'traversal/vendor/autoload.php', '' ); + $zip->close(); + """ + When I run `php create-traversal-zip.php` + + When I try `wp package install traversal.zip` + Then the return code should be 1 + And STDERR should contain: + """ + Error: Invalid package name '..': + """ + And STDOUT should be empty + @github-api Scenario: Install package with --no-interaction fails fast on Git authentication errors Given an empty directory diff --git a/src/Package_Command.php b/src/Package_Command.php index 1de39a8b..42f00fd3 100644 --- a/src/Package_Command.php +++ b/src/Package_Command.php @@ -8,6 +8,7 @@ use Composer\Installer; use Composer\Json\JsonFile; use Composer\Package\BasePackage; +use Composer\Package\Loader\ValidatingArrayLoader; use Composer\Package\PackageInterface; use Composer\Package\Version\VersionSelector; use Composer\Repository; @@ -302,6 +303,10 @@ public function install( $args, $assoc_args ) { // Move to a location based on the package name $local_dir = rtrim( WP_CLI::get_runner()->get_packages_dir_path(), '/' ) . '/local/'; $actual_dir_package = $local_dir . str_replace( '/', '-', $package_name ); + // Guard against path traversal: ensure destination stays within local_dir. + if ( ! self::is_child_path( $actual_dir_package, $local_dir ) ) { + throw new Exception( 'Invalid package: resolved destination path escapes the packages directory.' ); + } Extractor::copy_overwrite_files( $dir_package, $actual_dir_package ); Extractor::rmdir( $dir_package ); // Behold, the extracted package @@ -1182,13 +1187,33 @@ private static function get_package_name_and_version_from_dir_package( $dir_pack WP_CLI::error( sprintf( "Invalid package: no name in composer.json file '%s'.", $composer_file ) ); } $package_name = $composer_data['name']; - $version = self::DEFAULT_DEV_BRANCH_CONSTRAINTS; + $naming_error = ValidatingArrayLoader::hasPackageNamingError( $package_name ); + if ( null !== $naming_error ) { + WP_CLI::error( sprintf( "Invalid package name '%s': %s", $package_name, $naming_error ) ); + } + $version = self::DEFAULT_DEV_BRANCH_CONSTRAINTS; if ( ! empty( $composer_data['version'] ) ) { $version = $composer_data['version']; } return [ $package_name, $version ]; } + /** + * Checks whether a path is a direct child of (or equal to) a given parent directory. + * + * Uses string-based normalisation so it works even when the paths do not + * exist on the filesystem yet. + * + * @param string $path Path to test. + * @param string $parent_dir Parent directory to test against. + * @return bool True when $path is inside $parent_dir. + */ + private static function is_child_path( $path, $parent_dir ) { + $normalized_path = rtrim( Path::normalize( $path ), '/' ) . '/'; + $normalized_parent = rtrim( Path::normalize( $parent_dir ), '/' ) . '/'; + return 0 === strpos( $normalized_path, $normalized_parent ); + } + /** * Gets the WP-CLI packages composer.json object. */ From 0d3523180e781a5131bc4bbde2d3e1469d219778 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 11:59:33 +0000 Subject: [PATCH 3/7] Rename test helper PHP file for clarity Agent-Logs-Url: https://github.com/wp-cli/package-command/sessions/159d5271-2365-4bfe-8f16-6fd715d1a133 Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --- features/package-install.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/features/package-install.feature b/features/package-install.feature index 5b48f071..d827959b 100644 --- a/features/package-install.feature +++ b/features/package-install.feature @@ -1419,7 +1419,7 @@ Feature: Install WP-CLI packages Scenario: Reject a ZIP package whose composer.json name contains path-traversal components Given an empty directory - And a create-traversal-zip.php file: + And a create-malicious-package-zip.php file: """ addFromString( 'traversal/vendor/autoload.php', '' ); $zip->close(); """ - When I run `php create-traversal-zip.php` + When I run `php create-malicious-package-zip.php` When I try `wp package install traversal.zip` Then the return code should be 1 From 0655ac35df1d3739984a6b200b3f9f7adfc1caa9 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Mon, 27 Apr 2026 14:22:54 +0200 Subject: [PATCH 4/7] Apply suggestion from @swissspidy --- features/package-install.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/package-install.feature b/features/package-install.feature index d827959b..a3fd8223 100644 --- a/features/package-install.feature +++ b/features/package-install.feature @@ -1430,7 +1430,7 @@ Feature: Install WP-CLI packages """ When I run `php create-malicious-package-zip.php` - When I try `wp package install traversal.zip` + And I try `wp package install traversal.zip` Then the return code should be 1 And STDERR should contain: """ From 538cad5fb899107c020b44294afdd390893ab49a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 13:40:57 +0000 Subject: [PATCH 5/7] Fix docblock wording and add no-artifact assertions to traversal test Agent-Logs-Url: https://github.com/wp-cli/package-command/sessions/d84b0dfc-3c57-40c3-82e8-fbe52598eb58 Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --- features/package-install.feature | 5 +++++ src/Package_Command.php | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/features/package-install.feature b/features/package-install.feature index a3fd8223..7f67768c 100644 --- a/features/package-install.feature +++ b/features/package-install.feature @@ -1437,6 +1437,11 @@ Feature: Install WP-CLI packages Error: Invalid package name '..': """ And STDOUT should be empty + And the {PACKAGE_PATH}vendor/autoload.php file should not contain: + """ + COMPROMISED + """ + And the {PACKAGE_PATH}local directory should not exist @github-api Scenario: Install package with --no-interaction fails fast on Git authentication errors diff --git a/src/Package_Command.php b/src/Package_Command.php index 42f00fd3..0b5e4301 100644 --- a/src/Package_Command.php +++ b/src/Package_Command.php @@ -1199,7 +1199,7 @@ private static function get_package_name_and_version_from_dir_package( $dir_pack } /** - * Checks whether a path is a direct child of (or equal to) a given parent directory. + * Checks whether a path is inside (or equal to) a given parent directory. * * Uses string-based normalisation so it works even when the paths do not * exist on the filesystem yet. From c4f7ef41c3b2bc33839a0a48d12e6e485b96fcc3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 14:23:05 +0000 Subject: [PATCH 6/7] Fix test: assert vendor/autoload.php does not exist instead of checking contents Agent-Logs-Url: https://github.com/wp-cli/package-command/sessions/7bc70ad5-73a0-45a9-b337-7be724bfffcb Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --- features/package-install.feature | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/features/package-install.feature b/features/package-install.feature index 7f67768c..262e93d1 100644 --- a/features/package-install.feature +++ b/features/package-install.feature @@ -1437,10 +1437,7 @@ Feature: Install WP-CLI packages Error: Invalid package name '..': """ And STDOUT should be empty - And the {PACKAGE_PATH}vendor/autoload.php file should not contain: - """ - COMPROMISED - """ + And the {PACKAGE_PATH}vendor/autoload.php file should not exist And the {PACKAGE_PATH}local directory should not exist @github-api From ab278f6400c95166f9ec29e158f38f298fe5b696 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 27 Apr 2026 20:33:55 +0000 Subject: [PATCH 7/7] Fix is_child_path: resolve .. segments and use case-insensitive comparison on Windows Agent-Logs-Url: https://github.com/wp-cli/package-command/sessions/c458f3cb-4cae-4b57-b796-a60a092eea37 Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> --- src/Package_Command.php | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/Package_Command.php b/src/Package_Command.php index 0b5e4301..7e779643 100644 --- a/src/Package_Command.php +++ b/src/Package_Command.php @@ -1201,19 +1201,43 @@ private static function get_package_name_and_version_from_dir_package( $dir_pack /** * Checks whether a path is inside (or equal to) a given parent directory. * - * Uses string-based normalisation so it works even when the paths do not - * exist on the filesystem yet. + * Resolves '.' and '..' segments without touching the filesystem so it + * works even when the paths do not exist yet. Uses a case-insensitive + * comparison on Windows where the filesystem is case-insensitive. * * @param string $path Path to test. * @param string $parent_dir Parent directory to test against. * @return bool True when $path is inside $parent_dir. */ private static function is_child_path( $path, $parent_dir ) { - $normalized_path = rtrim( Path::normalize( $path ), '/' ) . '/'; - $normalized_parent = rtrim( Path::normalize( $parent_dir ), '/' ) . '/'; + $normalized_path = self::resolve_dot_segments( rtrim( str_replace( '\\', '/', $path ), '/' ) ) . '/'; + $normalized_parent = self::resolve_dot_segments( rtrim( str_replace( '\\', '/', $parent_dir ), '/' ) ) . '/'; + if ( DIRECTORY_SEPARATOR === '\\' ) { + return 0 === stripos( $normalized_path, $normalized_parent ); + } return 0 === strpos( $normalized_path, $normalized_parent ); } + /** + * Resolves '.' and '..' segments in a path without touching the filesystem. + * + * @param string $path Forward-slash path to resolve. + * @return string Resolved path. + */ + private static function resolve_dot_segments( $path ) { + $is_absolute = isset( $path[0] ) && '/' === $path[0]; + $result = []; + foreach ( explode( '/', $path ) as $part ) { + if ( '..' === $part ) { + array_pop( $result ); + } elseif ( '.' !== $part && '' !== $part ) { + $result[] = $part; + } + } + $resolved = implode( '/', $result ); + return $is_absolute ? '/' . $resolved : $resolved; + } + /** * Gets the WP-CLI packages composer.json object. */