From dd6d34e5e2e98e49d758129ba21e17fff13661c9 Mon Sep 17 00:00:00 2001 From: Kamran Abdul Aziz Date: Wed, 26 Aug 2026 14:58:07 +0530 Subject: [PATCH] Force removal of test directories in cleanup `rm -r` without `-f` prompts for confirmation on write-protected files, such as the read-only git object files in a nested checkout, leaving cleanup.php waiting for input that can never arrive. The two sibling commands in the same array already use `rm -rf`, so the missing flag looks like an oversight rather than a choice. The same applies to the default `WPT_RM_TEST_DIR_CMD`, which is executed on the remote host over SSH. Anyone who has overridden that variable should include `-f` in their own command as well, so the README now says so in both places it documents the variable. That default also builds a shell command by concatenating `WPT_TEST_DIR` directly, so a path containing a space was split into several targets and a path containing a semicolon ran as a second command on the remote host. It is now passed through `escapeshellarg()`. The value is escaped again when the whole command is handed to `ssh`, which unwraps correctly. This weakness predates the change, but forcing removal makes it worth closing here rather than leaving it on a line this commit already touches. Genuine failures such as permission errors still return a non-zero exit code and are still reported by `perform_operations()`. One behaviour does change: removing a path that does not exist now succeeds quietly, where before it reported a failure. Cleaning an already clean directory is not an error, so that seems like the better outcome, but it is a change. Fixes #301 --- README.md | 5 +++-- cleanup.php | 4 ++-- functions.php | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 73917db..e99704d 100644 --- a/README.md +++ b/README.md @@ -221,7 +221,8 @@ export WPT_PHP_EXECUTABLE=${WPT_PHP_EXECUTABLE-php} export WPT_PHPUNIT_CMD="" # (Optionally) define the command execution to remove the test directory -# Use if `rm -r` can't be called directly for some reason. +# Use if `rm -rf` can't be called directly for some reason. Include the `-f` +# flag, or removal stops to ask about write-protected files such as git objects. export WPT_RM_TEST_DIR_CMD="" # SSH connection string (can also be an alias). @@ -358,7 +359,7 @@ export WPT_PHPUNIT_CMD="" **Remove directory command** -(Optionally) define the command execution to remove the test directory. Use if `rm -r` can't be called directly for some reason. +(Optionally) define the command execution to remove the test directory. Use if `rm -rf` can't be called directly for some reason. Include the `-f` flag, or removal stops to ask about write-protected files such as git objects, and cleanup waits for input that never arrives. ``` export WPT_RM_TEST_DIR_CMD="" diff --git a/cleanup.php b/cleanup.php index dddc5fb..ed09326 100644 --- a/cleanup.php +++ b/cleanup.php @@ -37,13 +37,13 @@ * The following actions are performed: * - Forcefully deletes only the .git directory and the node_modules cache. * - Forcefully remove the `node_modules/.cache` directory. - * - Remove the entire preparation directory. + * - Forcefully remove the entire preparation directory. */ perform_operations( array( 'rm -rf ' . escapeshellarg( $runner_vars['WPT_PREPARE_DIR'] . '/.git' ), 'rm -rf ' . escapeshellarg( $runner_vars['WPT_PREPARE_DIR'] . '/node_modules/.cache' ), - 'rm -r ' . escapeshellarg( $runner_vars['WPT_PREPARE_DIR'] ), + 'rm -rf ' . escapeshellarg( $runner_vars['WPT_PREPARE_DIR'] ), ) ); diff --git a/functions.php b/functions.php index b8367c0..6291b48 100644 --- a/functions.php +++ b/functions.php @@ -87,7 +87,7 @@ function setup_runner_env_vars() { 'WPT_SSH_CONNECT' => trim( getenv( 'WPT_SSH_CONNECT' ) ), 'WPT_SSH_OPTIONS' => '' !== $ssh_options ? $ssh_options : '-o StrictHostKeyChecking=no', 'WPT_PHP_EXECUTABLE' => '' !== $php_exec ? $php_exec : 'php', - 'WPT_RM_TEST_DIR_CMD' => '' !== $rm_test_dir ? $rm_test_dir : 'rm -r ' . $runner_configuration['WPT_TEST_DIR'], + 'WPT_RM_TEST_DIR_CMD' => '' !== $rm_test_dir ? $rm_test_dir : 'rm -rf ' . escapeshellarg( $runner_configuration['WPT_TEST_DIR'] ), 'WPT_REPORT_API_KEY' => trim( getenv( 'WPT_REPORT_API_KEY' ) ), 'WPT_DEBUG' => (bool) getenv( 'WPT_DEBUG' ), )