From 78e1c8ded71c4b619f66264f34c454fbfebdd744 Mon Sep 17 00:00:00 2001 From: Konstantin Obenland Date: Tue, 21 Jul 2026 10:39:06 -0500 Subject: [PATCH] Plugin Directory: Tidy up Block Plugin Checker output and add tests Escape checker result values (readme/header license, block names, file paths) consistently at the point they are built into messages, and add a kses backstop when the block validator renders them. Apply the readme parser's existing sanitize_text() to the License header so it matches the other headers. Also make get_browser_url() return a string when there is no repo, guard an undefined array key in the results loop, tidy an inconsistent esc_html() call in the readme validator, and make the local page seed idempotent. Adds Block_Plugin_Checker_Test covering the individual checks and message rendering. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01UNnLdnE7etihGNvsWY1qGE --- .../plugin-directory/bin/after-start.sh | 33 +- .../cli/class-block-plugin-checker.php | 59 +-- .../plugin-directory/readme/class-parser.php | 2 +- .../readme/class-validator.php | 2 +- .../shortcodes/class-block-validator.php | 35 +- .../tests/Block_Plugin_Checker_Test.php | 455 ++++++++++++++++++ .../tests/Test_Readme_Parser.php | 29 ++ 7 files changed, 574 insertions(+), 41 deletions(-) create mode 100644 wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Block_Plugin_Checker_Test.php diff --git a/environments/plugin-directory/bin/after-start.sh b/environments/plugin-directory/bin/after-start.sh index f5a0a9c0fc..872d1f8e40 100755 --- a/environments/plugin-directory/bin/after-start.sh +++ b/environments/plugin-directory/bin/after-start.sh @@ -22,18 +22,37 @@ $WPENV cli sudo sh -c \ # Set up permalinks. $WP wp rewrite structure '/%postname%/' --hard -# Create pages that exist on wordpress.org/plugins (if they don't already exist). +# Create pages that exist on wordpress.org/plugins. +# +# Idempotent: match by slug and update, otherwise create. after-start.sh runs on +# every `wp-env start`, and a plain `wp post create` would append -2, -3, … to the +# slug each time, leaving the canonical slug pointing at a stale, empty page. echo "Creating pages..." + +# ensure_page <slug> <content> [parent_id]; prints the page ID. +ensure_page() { + local title="$1" slug="$2" content="$3" parent="$4" id + local args=( --post_type=page --post_status=publish --post_title="$title" --post_name="$slug" --post_content="$content" --porcelain ) + [ -n "$parent" ] && args+=( --post_parent="$parent" ) + id=$($WP wp post list --post_type=page --name="$slug" --field=ID 2>/dev/null | tr -d '\r\n ') + if [ -n "$id" ]; then + $WP wp post update "$id" --post_content="$content" > /dev/null 2>&1 + else + id=$($WP wp post create "${args[@]}" 2>/dev/null | tr -d '\r\n ') + fi + printf '%s' "$id" +} + # Parent: /developers/ -$WP wp post create --post_type=page --post_status=publish --post_title='Developer Information' --post_name='developers' --porcelain > /dev/null 2>&1 && echo " Created page: /developers/" || true -DEVELOPERS_ID=$($WP wp post list --post_type=page --name=developers --field=ID 2>/dev/null) +DEVELOPERS_ID=$(ensure_page 'Developer Information' 'developers' '' '') if [ -n "$DEVELOPERS_ID" ]; then # Children of /developers/ - $WP wp post create --post_type=page --post_status=publish --post_title='Add your Plugin' --post_name='add' --post_parent=$DEVELOPERS_ID --porcelain > /dev/null 2>&1 && echo " Created page: /developers/add/" || true - $WP wp post create --post_type=page --post_status=publish --post_title='Readme Validator' --post_content='[readme-validator]' --post_name='readme-validator' --post_parent=$DEVELOPERS_ID --porcelain > /dev/null 2>&1 && echo " Created page: /developers/readme-validator/" || true - $WP wp post create --post_type=page --post_status=publish --post_title='Block Plugin Checker' --post_content='[block-validator]' --post_name='block-plugin-validator' --post_parent=$DEVELOPERS_ID --porcelain > /dev/null 2>&1 && echo " Created page: /developers/block-plugin-validator/" || true - $WP wp post create --post_type=page --post_status=publish --post_title='Release Management' --post_content='[release-confirmation]' --post_name='releases' --post_parent=$DEVELOPERS_ID --porcelain > /dev/null 2>&1 && echo " Created page: /developers/releases/" || true + ensure_page 'Add your Plugin' 'add' '' "$DEVELOPERS_ID" > /dev/null + ensure_page 'Readme Validator' 'readme-validator' '[readme-validator]' "$DEVELOPERS_ID" > /dev/null + ensure_page 'Block Plugin Checker' 'block-plugin-validator' '[block-validator]' "$DEVELOPERS_ID" > /dev/null + ensure_page 'Release Management' 'releases' '[release-confirmation]' "$DEVELOPERS_ID" > /dev/null + echo " Pages ready under /developers/" fi # Create stub database tables that exist outside WordPress on production. diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/cli/class-block-plugin-checker.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/cli/class-block-plugin-checker.php index 78109c435b..7515cdc5c6 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/cli/class-block-plugin-checker.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/cli/class-block-plugin-checker.php @@ -96,6 +96,9 @@ public function get_browser_url( $file ) { return str_replace( 'https://plugins.svn.wordpress.org', 'https://plugins.trac.wordpress.org/browser', $this->repo_url ) . '/' . $file; } } + + // No repo to link to (e.g. a ZIP upload) — return an empty string rather than null. + return ''; } /** @@ -160,7 +163,7 @@ public function run_check_plugin_repo( $url ) { $this->record_result( __FUNCTION__, 'error', - sprintf( __( 'Invalid url: %s', 'wporg-plugins' ), $url ), + sprintf( __( 'Invalid url: %s', 'wporg-plugins' ), esc_html( $url ) ), $url ); return $this->results; @@ -170,7 +173,7 @@ public function run_check_plugin_repo( $url ) { $this->record_result( __FUNCTION__, 'error', - sprintf( __( 'URL must be GitHub or plugins.svn.wordpress.org: %s', 'wporg-plugins' ), $url ), + sprintf( __( 'URL must be GitHub or plugins.svn.wordpress.org: %s', 'wporg-plugins' ), esc_html( $url ) ), $url ); return $this->results; @@ -182,7 +185,7 @@ public function run_check_plugin_repo( $url ) { $this->record_result( __FUNCTION__, 'error', - sprintf( __( 'URL must be a plugin repository: %s', 'wporg-plugins' ), $url ), + sprintf( __( 'URL must be a plugin repository: %s', 'wporg-plugins' ), esc_html( $url ) ), $url ); return $this->results; @@ -212,7 +215,7 @@ public function run_check_plugin_repo( $url ) { $this->record_result( __FUNCTION__, 'error', - sprintf( __( 'URL must be a plugin repository: %s', 'wporg-plugins' ), $url ), + sprintf( __( 'URL must be a plugin repository: %s', 'wporg-plugins' ), esc_html( $url ) ), $url ); return $this->results; @@ -248,7 +251,7 @@ function export_plugin( $svn_url ) { $this->record_result( __FUNCTION__, 'error', - sprintf( __( 'Error fetching repository %s: %s', 'wporg-plugins' ), $svn_url, $export['errors'][0]['error_code'] ?? 'unknown error' ), + sprintf( __( 'Error fetching repository %s: %s', 'wporg-plugins' ), esc_html( $svn_url ), esc_html( $export['errors'][0]['error_code'] ?? 'unknown error' ) ), $export['errors'] ?? array() ); return false; @@ -421,10 +424,10 @@ public function find_php_functions( $base_dir ) { 'error', sprintf( __( 'PHP error %s in %s', 'wporg-plugins' ), - $php_calls->get_error_message(), + esc_html( $php_calls->get_error_message() ), sprintf( '<a href="%s">%s</a>', - $this->get_browser_url( $filename ) . '#L' . $php_calls->get_error_data(), - $this->relative_filename( $filename ) + esc_url( $this->get_browser_url( $filename ) . '#L' . $php_calls->get_error_data() ), + esc_html( $this->relative_filename( $filename ) ) ) ), [ $filename, $php_calls->get_error_data() ] @@ -514,7 +517,7 @@ function check_license() { __FUNCTION__, 'info', // translators: %s is the license. - sprintf( __( 'Found a license in readme.txt: %s.', 'wporg-plugins' ), $this->readme->license ), + sprintf( __( 'Found a license in readme.txt: %s.', 'wporg-plugins' ), esc_html( $this->readme->license ) ), $this->readme->license ); } elseif ( ! empty( $this->headers->License ) ) { @@ -522,7 +525,7 @@ function check_license() { __FUNCTION__, 'info', // translators: %s is the license. - sprintf( __( 'Found a license in plugin headers: %s.', 'wporg-plugins' ), $this->headers->License ), + sprintf( __( 'Found a license in plugin headers: %s.', 'wporg-plugins' ), esc_html( $this->headers->License ) ), $this->headers->License ); } @@ -598,8 +601,8 @@ function check_for_duplicate_block_name() { sprintf( // translators: %1$s is the block slug, %2$s is the found plugin title. __( 'Block name %1$s already exists in the plugin "%2$s."', 'wporg-plugins' ), - '<code>' . $block->name . '</code>', - $query->posts[0]->post_title + '<code>' . esc_html( $block->name ) . '</code>', + esc_html( $query->posts[0]->post_title ) ), [ 'block_name' => $block->name, 'slug' => $post->post_name ] ); @@ -621,7 +624,7 @@ function check_for_standard_block_name() { __FUNCTION__, 'error', // translators: %s is the block name. - sprintf( __( 'Block name %s is invalid. Please use lowercase alphanumeric characters.', 'wporg-plugins' ), '<code>' . $block->name . '</code>' ) + sprintf( __( 'Block name %s is invalid. Please use lowercase alphanumeric characters.', 'wporg-plugins' ), '<code>' . esc_html( $block->name ) . '</code>' ) ); } else { $disallowed_ns = array( 'cgb/', 'create-block/', 'example/', 'block/', 'core/', 'gutenberg-examples/' ); @@ -633,8 +636,8 @@ function check_for_standard_block_name() { sprintf( // translators: %1$s is the block name, %2$s is the namespace. __( 'Block %1$s uses namespace %2$s. Please use a unique namespace.', 'wporg-plugins' ), - '<code>' . $block->name . '</code>', - '<code>' . $ns . '</code>' + '<code>' . esc_html( $block->name ) . '</code>', + '<code>' . esc_html( $ns ) . '</code>' ) ); break; @@ -665,7 +668,7 @@ function check_for_multiple_namespaces() { // translators: %1$d is the number of namespaces, %2$s is the list of namespaces. __( 'Found blocks with %1$d different namespaces: %2$s.', 'wporg-plugins' ), count( $namespaces ), - '<code>' . implode( ', ', $namespaces ) . '</code>' + '<code>' . esc_html( implode( ', ', $namespaces ) ) . '</code>' ), $namespaces ); @@ -732,7 +735,7 @@ function check_for_unique_namespace() { 'error', // translators: %1$s is the namespace, %2$s is a link to the plugin. sprintf( __( 'Please use a unique block namespace. Namespace %1$s is already used by %2$s.' ), - '<code>' . $this->get_namespace( $block ) . '</code>', + '<code>' . esc_html( $this->get_namespace( $block ) ) . '</code>', '<a href="' . esc_url( get_permalink( $post ) ) . '">' . esc_html( $post->post_title ) . '</a>' ) ); @@ -745,7 +748,7 @@ function check_for_unique_namespace() { 'warning', // translators: %1$s is the namespace, %2$s is a link to the plugin. sprintf( __( 'Please use a unique block namespace. Namespace %1$s is already used by %2$s.' ), - '<code>' . $this->get_namespace( $block ) . '</code>', + '<code>' . esc_html( $this->get_namespace( $block ) ) . '</code>', '<a href="' . esc_url( get_permalink( $post ) ) . '">' . esc_html( $post->post_title ) . '</a>' ) ); @@ -793,7 +796,7 @@ function check_for_block_json() { __FUNCTION__, 'info', // translators: %s is the block name. - sprintf( __( 'Found a block.json file for block %s.', 'wporg-plugins' ), '<code>' . $block_name . '</code>' ), + sprintf( __( 'Found a block.json file for block %s.', 'wporg-plugins' ), '<code>' . esc_html( $block_name ) . '</code>' ), $this->block_json_files[ $block_name ] ); } @@ -834,7 +837,7 @@ function( $block ) { sprintf( /* translators: %s is a list of block names. */ __( 'More than one top-level block was found: %s', 'wporg-plugins' ), - implode( ', ', $list ) + esc_html( implode( ', ', $list ) ) ) ); } @@ -852,7 +855,7 @@ function check_for_block_script_files() { __FUNCTION__, 'info', // translators: %s is the block name. - sprintf( __( 'Found file %s.', 'wporg-plugins' ), '<code>' . $script . '</code>' ), + sprintf( __( 'Found file %s.', 'wporg-plugins' ), '<code>' . esc_html( $script ) . '</code>' ), compact( 'kind', 'script' ) ); } else { @@ -868,7 +871,7 @@ function check_for_block_script_files() { $this->record_result( __FUNCTION__, 'error', - sprintf( $message, '<code>' . $script . '</code>' ), + sprintf( $message, '<code>' . esc_html( $script ) . '</code>' ), compact( 'kind', 'script' ) ); } @@ -912,8 +915,8 @@ function check_block_json_is_valid_json() { sprintf( // translators: %1$s is the file name, %2$s is the json error message. __( 'Error attempting to parse json in %1$s: %2$s', 'wporg-plugins' ), - '<code><a href="' . $this->get_browser_url( $block_json_file ) . '">' . $this->relative_filename( $block_json_file ) . '</a></code>', - $message + '<code><a href="' . esc_url( $this->get_browser_url( $block_json_file ) ) . '">' . esc_html( $this->relative_filename( $block_json_file ) ) . '</a></code>', + esc_html( $message ) ), $this->relative_filename( $block_json_file ) ); @@ -932,7 +935,7 @@ function check_block_json_is_valid() { __FUNCTION__, 'info', // translators: %s is the file name. - sprintf( __( 'JSON file %s is valid.', 'wporg-plugins' ), '<code>' . $this->relative_filename( $block_json_file ) . '</code>' ), + sprintf( __( 'JSON file %s is valid.', 'wporg-plugins' ), '<code>' . esc_html( $this->relative_filename( $block_json_file ) ) . '</code>' ), $this->relative_filename( $block_json_file ) ); continue; @@ -947,7 +950,7 @@ function check_block_json_is_valid() { $this->record_result( __FUNCTION__, ( 'error' === $code ? 'warning' : $code ), // TODO: be smarter about mapping these - '<code><a href="' . $this->get_browser_url( $block_json_file ) . '">' . $this->relative_filename( $block_json_file ) . '</a></code>: ' . $message, + '<code><a href="' . esc_url( $this->get_browser_url( $block_json_file ) ) . '">' . esc_html( $this->relative_filename( $block_json_file ) ) . '</a></code>: ' . wp_kses( $message, array( 'code' => array() ) ), array( $this->relative_filename( $block_json_file ), $result->get_error_data( $code ), @@ -1073,7 +1076,7 @@ function check_php_function_calls() { sprintf( // translators: %s is the function name. __( 'Found PHP call %s. This may cause problems.', 'wporg-plugins' ), - '<a href="' . $this->get_browser_url( $call[2] ) . '#L' . $call[1] . '"><code>' . $call[0] . '()</code></a>' + '<a href="' . esc_url( $this->get_browser_url( $call[2] ) . '#L' . $call[1] ) . '"><code>' . esc_html( $call[0] ) . '()</code></a>' ), $call ); @@ -1084,7 +1087,7 @@ function check_php_function_calls() { sprintf( // translators: %s is the function name. __( 'Found PHP call %s. This is likely to prevent your plugin from working as expected.', 'wporg-plugins' ), - '<a href="' . $this->get_browser_url( $call[2] ) . '#L' . $call[1] . '"><code>' . $call[0] . '()</code></a>' + '<a href="' . esc_url( $this->get_browser_url( $call[2] ) . '#L' . $call[1] ) . '"><code>' . esc_html( $call[0] ) . '()</code></a>' ), $call ); diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-parser.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-parser.php index b43dc2d0ed..d886f322d8 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-parser.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-parser.php @@ -373,7 +373,7 @@ protected function parse_readme_contents( $contents ) { $headers['license'] = trim( str_replace( $url[1], '', $headers['license'] ), " -*\t\n\r\n()<>" ); } - $this->license = $headers['license']; + $this->license = $this->sanitize_text( $headers['license'] ); } if ( ! empty( $headers['license_uri'] ) ) { $this->license_uri = $headers['license_uri']; diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-validator.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-validator.php index f7c50e8ea0..db422881ae 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-validator.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-validator.php @@ -359,7 +359,7 @@ public function translate_code_to_message( $error_code, $data = false ) { return sprintf( /* translators: %s: list of tags not supported */ __( 'One or more tags were ignored. The following tags are not permitted: %s', 'wporg-plugins' ), - '<code>' . implode( '</code>, <code>', $data ) . '</code>' + '<code>' . implode( '</code>, <code>', array_map( 'esc_html', $data ) ) . '</code>' ); case 'low_usage_tags': return sprintf( diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/shortcodes/class-block-validator.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/shortcodes/class-block-validator.php index 4dd7226c52..bb05e447d5 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/shortcodes/class-block-validator.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/shortcodes/class-block-validator.php @@ -336,19 +336,20 @@ protected static function display_results( $checker ) { $output .= "<p class='small'>{$labels['description']}</p>\n"; } $output .= "<div class='notice notice-{$type} notice-alt'>\n"; - foreach ( (array) $results_by_type[ $type ] as $item ) { + foreach ( (array) ( $results_by_type[ $type ] ?? array() ) as $item ) { // Only get details if this is a warning or error. $details = ( 'info' === $type ) ? false : self::get_detailed_help( $item->check_name, $item ); + $message = self::sanitize_message( $item->message ); if ( $details ) { $details = '<p>' . implode( '</p><p>', (array) $details ) . '</p>'; - $output .= "<details class='{$item->check_name}'><summary>{$item->message}</summary>{$details}</details>"; + $output .= "<details class='{$item->check_name}'><summary>{$message}</summary>{$details}</details>"; } else { - $output .= "<p>{$item->message}</p>"; + $output .= "<p>{$message}</p>"; } } // Collapse block.json warnings into one details at the end of warnings list. if ( 'error' === $type && ! empty( $block_json_issues ) ) { - $messages = wp_list_pluck( $block_json_issues, 'message' ); + $messages = array_map( array( __CLASS__, 'sanitize_message' ), wp_list_pluck( $block_json_issues, 'message' ) ); $details = '<p>' . implode( '</p><p>', (array) $messages ) . '</p>'; $output .= sprintf( '<details class="check_block_json_is_valid"><summary>%1$s</summary>%2$s</details>', @@ -368,6 +369,32 @@ protected static function display_results( $checker ) { echo $output; } + /** + * Restrict a checker result message to the markup the checks actually use. + * + * Messages recorded by Block_Plugin_Checker intentionally contain a little + * markup, but they also interpolate plugin-controlled values such as readme + * headers, block names and file paths. Those values are escaped where they + * are interpolated; this is the backstop for anything that is missed. + * + * @param string $message A message recorded by Block_Plugin_Checker. + * + * @return string The message, safe to output as HTML. + */ + protected static function sanitize_message( $message ) { + return wp_kses( + $message, + array( + 'a' => array( 'href' => true ), + 'br' => array(), + 'code' => array(), + 'em' => array(), + 'strong' => array(), + ), + array( 'http', 'https' ) + ); + } + /** * Get a more detailed help message for a given check. * diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Block_Plugin_Checker_Test.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Block_Plugin_Checker_Test.php new file mode 100644 index 0000000000..b357a8f695 --- /dev/null +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Block_Plugin_Checker_Test.php @@ -0,0 +1,455 @@ +<?php +/** + * Tests for the Block Plugin Checker's individual checks and the way its messages + * are rendered. + * + * These exercise the checks that can run purely off injected state — no repo + * export, database, or filesystem scan. The filesystem/DB checks + * (check_readme_exists, check_for_block_script_files, check_for_duplicate_block_name, + * check_for_unique_namespace, the size checks) and check_for_translation_function + * are covered elsewhere or need fixtures. + * + * Escaping is checked inline on the checks that emit plugin-controlled data + * (license, block name, block.json path), since that is where the values flow in. + * + * @package WordPressdotorg\Plugin_Directory\Tests + */ + +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\TestCase; +use WordPressdotorg\Plugin_Directory\CLI\Block_Plugin_Checker; +use WordPressdotorg\Plugin_Directory\Shortcodes\Block_Validator; + +/** + * Unit tests for the individual Block Plugin Checker checks and message rendering. + * + * @group block-validator + */ +#[CoversClass( Block_Plugin_Checker::class )] +#[CoversClass( Block_Validator::class )] +class Block_Plugin_Checker_Test extends TestCase { + + /** + * Build a checker and inject the given protected properties, bypassing the + * constructor (which adds filters and expects a slug). + * + * @param array $props Property name => value to set on the checker. + * @return Block_Plugin_Checker + */ + private function checker( array $props = array() ): Block_Plugin_Checker { + $reflection = new ReflectionClass( Block_Plugin_Checker::class ); + $checker = $reflection->newInstanceWithoutConstructor(); + + foreach ( $props as $name => $value ) { + $property = $reflection->getProperty( $name ); + $property->setAccessible( true ); + $property->setValue( $checker, $value ); + } + + return $checker; + } + + /** + * Pull the message strings out of a check's results. + * + * @param Block_Plugin_Checker $checker The checker that has run a check. + * @param string|null $type Result type to filter on, or null for any. + * @param string|null $check Check name to filter on, or null for any. + * @return string[] + */ + private function messages( Block_Plugin_Checker $checker, ?string $type = null, ?string $check = null ): array { + return array_map( + static fn( $result ) => $result->message, + $checker->get_results( $type, $check ) + ); + } + + /* + * Tests for check_license. + */ + + /** + * Nothing in the readme or the plugin headers earns a warning. + */ + public function test_missing_license_warns() { + $checker = $this->checker( array( 'readme' => (object) array( 'license' => '' ) ) ); + $checker->check_license(); + + $this->assertCount( 1, $checker->get_results( 'warning', 'check_license' ) ); + } + + /** + * A readme License that passes validation is reported back verbatim — so any + * markup riding along in it must be escaped. + */ + public function test_readme_license_is_reported_and_escaped() { + $checker = $this->checker( array( 'readme' => (object) array( 'license' => 'GPLv2 <img src=x onerror=alert(document.domain)>' ) ) ); + $checker->check_license(); + + $messages = $this->messages( $checker, 'info', 'check_license' ); + $this->assertCount( 1, $messages ); + $this->assertStringNotContainsString( '<img', $messages[0] ); + $this->assertStringContainsString( '<img', $messages[0] ); + } + + /** + * The same, for a License read from the plugin file headers instead of the readme. + */ + public function test_plugin_header_license_is_reported_and_escaped() { + $checker = $this->checker( + array( + 'readme' => (object) array( 'license' => '' ), + 'headers' => (object) array( 'License' => 'GPLv2 <img src=x onerror=alert(document.domain)>' ), + ) + ); + $checker->check_license(); + + $messages = $this->messages( $checker, 'info', 'check_license' ); + $this->assertCount( 1, $messages ); + $this->assertStringNotContainsString( '<img', $messages[0] ); + } + + /* + * Tests for check_plugin_headers. + */ + + /** + * A plugin with no readable headers is not a WordPress plugin. + */ + public function test_missing_headers_error() { + $checker = $this->checker( array( 'headers' => null ) ); + $checker->check_plugin_headers(); + + $this->assertCount( 1, $checker->get_results( 'error', 'check_plugin_headers' ) ); + } + + /* + * Tests for check_block_tag. + */ + + /** + * Data provider for {@see test_block_tag()}. + * + * @return array<string, array{0: string[], 1: string}> + */ + public static function block_tag_provider(): array { + return array( + 'present' => array( array( 'block' ), 'info' ), + 'present mixed case' => array( array( 'Block' ), 'info' ), + 'other tags only' => array( array( 'gallery', 'media' ), 'warning' ), + 'no tags' => array( array(), 'warning' ), + ); + } + + /** + * The "block" tag is required (case-insensitively); its absence is a warning. + * + * @param string[] $tags Tags parsed from the readme. + * @param string $expected Expected result type. + */ + #[DataProvider( 'block_tag_provider' )] + public function test_block_tag( array $tags, string $expected ) { + $checker = $this->checker( array( 'readme' => (object) array( 'tags' => $tags ) ) ); + $checker->check_block_tag(); + + $this->assertCount( 1, $checker->get_results( $expected, 'check_block_tag' ) ); + } + + /* + * Tests for check_for_blocks. + */ + + /** + * A block plugin with no blocks is a fatal error. + */ + public function test_no_blocks_error() { + $checker = $this->checker( array( 'blocks' => array() ) ); + $checker->check_for_blocks(); + + $this->assertCount( 1, $checker->get_results( 'error', 'check_for_blocks' ) ); + } + + /** + * A reasonable number of blocks is reported as an informational note. + */ + public function test_blocks_are_counted() { + $checker = $this->checker( + array( + 'blocks' => array( + 'sample/one' => (object) array( 'name' => 'sample/one' ), + 'sample/two' => (object) array( 'name' => 'sample/two' ), + ), + ) + ); + $checker->check_for_blocks(); + + $this->assertCount( 1, $checker->get_results( 'info', 'check_for_blocks' ) ); + } + + /* + * Tests for check_for_standard_block_name. + */ + + /** + * A conventional namespaced block name raises nothing. + */ + public function test_valid_block_name_passes() { + $checker = $this->checker( array( 'blocks' => array( (object) array( 'name' => 'my-plugin/my-block' ) ) ) ); + $checker->check_for_standard_block_name(); + + $this->assertEmpty( $checker->get_results( 'error', 'check_for_standard_block_name' ) ); + } + + /** + * A malformed block name is echoed back inside <code>, so the name must be escaped. + */ + public function test_invalid_block_name_errors_and_is_escaped() { + $checker = $this->checker( array( 'blocks' => array( (object) array( 'name' => 'sample/<img src=x onerror=alert(document.domain)>' ) ) ) ); + $checker->check_for_standard_block_name(); + + $messages = $this->messages( $checker, 'error', 'check_for_standard_block_name' ); + $this->assertCount( 1, $messages ); + $this->assertStringNotContainsString( '<img', $messages[0] ); + $this->assertStringContainsString( '<code>', $messages[0] ); + } + + /** + * Reserved namespaces from block templates (core/, create-block/, …) are rejected. + */ + public function test_reserved_namespace_errors() { + $checker = $this->checker( array( 'blocks' => array( (object) array( 'name' => 'core/my-block' ) ) ) ); + $checker->check_for_standard_block_name(); + + $this->assertCount( 1, $checker->get_results( 'error', 'check_for_standard_block_name' ) ); + } + + /* + * Tests for check_for_multiple_namespaces. + */ + + /** + * Blocks sharing one namespace are fine. + */ + public function test_single_namespace_passes() { + $checker = $this->checker( + array( + 'blocks' => array( + (object) array( 'name' => 'sample/one' ), + (object) array( 'name' => 'sample/two' ), + ), + ) + ); + $checker->check_for_multiple_namespaces(); + + $this->assertEmpty( $checker->get_results( 'error', 'check_for_multiple_namespaces' ) ); + } + + /** + * Blocks spread across namespaces suggest an unrelated suite bundled together. + */ + public function test_multiple_namespaces_error() { + $checker = $this->checker( + array( + 'blocks' => array( + (object) array( 'name' => 'sample/one' ), + (object) array( 'name' => 'other/two' ), + ), + ) + ); + $checker->check_for_multiple_namespaces(); + + $this->assertCount( 1, $checker->get_results( 'error', 'check_for_multiple_namespaces' ) ); + } + + /* + * Tests for check_for_block_json. + */ + + /** + * A block backed by a block.json file is noted. + */ + public function test_block_with_block_json_is_noted() { + $checker = $this->checker( + array( + 'blocks' => array( 'sample/main' => (object) array( 'name' => 'sample/main' ) ), + 'block_json_files' => array( 'sample/main' => '/tmp/plugin/block.json' ), + ) + ); + $checker->check_for_block_json(); + + $this->assertCount( 1, $checker->get_results( 'info', 'check_for_block_json' ) ); + } + + /** + * No block.json files anywhere is a warning. + */ + public function test_block_without_block_json_warns() { + $checker = $this->checker( + array( + 'blocks' => array( 'sample/main' => (object) array( 'name' => 'sample/main' ) ), + 'block_json_files' => array(), + ) + ); + $checker->check_for_block_json(); + + $this->assertCount( 1, $checker->get_results( 'warning', 'check_for_block_json' ) ); + } + + /* + * Tests for check_for_single_parent. + */ + + /** + * One top-level block (others declaring it as parent) is the expected shape. + */ + public function test_single_top_level_block_passes() { + $checker = $this->checker( + array( + 'blocks' => array( + 'sample/list' => (object) array( 'name' => 'sample/list' ), + 'sample/list-item' => (object) array( + 'name' => 'sample/list-item', + 'parent' => array( 'sample/list' ), + ), + ), + ) + ); + $checker->check_for_single_parent(); + + $this->assertEmpty( $checker->get_results( 'warning', 'check_for_single_parent' ) ); + } + + /** + * Two independent top-level blocks earns a warning. + */ + public function test_multiple_top_level_blocks_warn() { + $checker = $this->checker( + array( + 'blocks' => array( + 'sample/one' => (object) array( 'name' => 'sample/one' ), + 'sample/two' => (object) array( 'name' => 'sample/two' ), + ), + ) + ); + $checker->check_for_single_parent(); + + $this->assertCount( 1, $checker->get_results( 'warning', 'check_for_single_parent' ) ); + } + + /* + * Tests for check_block_json_is_valid and check_block_json_is_valid_json. + */ + + /** + * A block.json that validates is reported as valid. + */ + public function test_valid_block_json_is_noted() { + $checker = $this->checker( + array( + 'path_to_plugin' => '/tmp/plugin', + 'block_json_validation' => array( '/tmp/plugin/block.json' => true ), + ) + ); + $checker->check_block_json_is_valid(); + + $this->assertCount( 1, $checker->get_results( 'info', 'check_block_json_is_valid' ) ); + } + + /** + * The file path is built into a Trac link. A quote in the path would otherwise + * close the href attribute and turn the rest of the path into live markup. + */ + public function test_block_json_error_path_cannot_break_out_of_href() { + $bad_path = '/tmp/plugin/report" onmouseover=alert(1) x="/block.json'; + $checker = $this->checker( + array( + 'path_to_plugin' => '/tmp/plugin', + 'repo_url' => 'https://plugins.svn.wordpress.org/sample/trunk', + 'block_json_validation' => array( $bad_path => new WP_Error( 'error', 'block.json[name] is required.' ) ), + ) + ); + $checker->check_block_json_is_valid(); + + $messages = $this->messages( $checker, 'warning', 'check_block_json_is_valid' ); + $this->assertCount( 1, $messages ); + + /* The quote survives only as an entity, never as attribute syntax. */ + $this->assertStringNotContainsString( '" onmouseover', $messages[0] ); + $this->assertStringContainsString( '" onmouseover', $messages[0] ); + + /* The only quotes left are the two delimiting the href. */ + $this->assertSame( 2, substr_count( $messages[0], '"' ) ); + } + + /** + * A JSON parse failure is surfaced as a fatal error. + */ + public function test_block_json_parse_error() { + $error = new WP_Error( 'json_parse_error', 'Syntax error' ); + $checker = $this->checker( + array( + 'path_to_plugin' => '/tmp/plugin', + 'block_json_validation' => array( '/tmp/plugin/block.json' => $error ), + ) + ); + $checker->check_block_json_is_valid_json(); + + $this->assertCount( 1, $checker->get_results( 'error', 'check_block_json_is_valid_json' ) ); + } + + /* + * Tests for check_php_function_calls. + */ + + /** + * Data provider for {@see test_php_function_calls_are_flagged()}. + * + * @return array<string, array{0: string, 1: string}> + */ + public static function php_call_provider(): array { + return array( + 'wp_add_inline_script' => array( 'wp_add_inline_script', 'warning' ), + 'wp_localize_script' => array( 'wp_localize_script', 'warning' ), + 'add_shortcode' => array( 'add_shortcode', 'error' ), + 'header' => array( 'header', 'error' ), + 'wp_redirect' => array( 'wp_redirect', 'error' ), + ); + } + + /** + * Certain PHP calls are flagged as warnings, others as errors. + * + * @param string $function_name Function name recorded from the plugin's PHP. + * @param string $expected Expected result type. + */ + #[DataProvider( 'php_call_provider' )] + public function test_php_function_calls_are_flagged( string $function_name, string $expected ) { + $checker = $this->checker( array( 'php_function_calls' => array( array( $function_name, 5, 'plugin.php' ) ) ) ); + $checker->check_php_function_calls(); + + $this->assertCount( 1, $checker->get_results( $expected, 'check_php_function_calls' ) ); + } + + /* + * Tests for the Block_Validator sanitize_message render-time backstop. + */ + + /** + * The backstop drops dangerous markup but keeps the formatting the checks emit. + */ + public function test_sanitize_message_allows_only_intended_markup() { + $method = new ReflectionMethod( Block_Validator::class, 'sanitize_message' ); + $method->setAccessible( true ); + + $dirty = '<code><a href="https://example.org/block.json">block.json</a></code>: ' + . '<img src=x onerror=alert(document.domain)><script>alert(1)</script><a href="javascript:alert(1)">x</a>'; + $clean = $method->invoke( null, $dirty ); + + $this->assertStringNotContainsString( '<img', $clean ); + $this->assertStringNotContainsString( '<script', $clean ); + $this->assertStringNotContainsString( 'javascript:', $clean ); + $this->assertStringContainsString( '<code>', $clean ); + $this->assertStringContainsString( '<a href="https://example.org/block.json">', $clean ); + } +} diff --git a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Test_Readme_Parser.php b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Test_Readme_Parser.php index 2207ae4521..14293b9cf7 100644 --- a/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Test_Readme_Parser.php +++ b/wordpress.org/public_html/wp-content/plugins/plugin-directory/tests/Test_Readme_Parser.php @@ -129,4 +129,33 @@ public function test_license_with_embedded_url( string $header, string $expected $this->assertSame( $expected_license, $parser->license ); $this->assertSame( $expected_uri, $parser->license_uri ); } + + /** + * Data provider for {@see test_license_is_sanitized()}. + * + * @return array<string, array{0: string}> + */ + public static function license_markup_provider(): array { + return array( + 'image payload' => array( 'License: GPLv2 <img src=x onerror=alert(document.domain)>' ), + 'script payload' => array( 'License: GPLv2 <script>alert(1)</script>' ), + 'payload containing url' => array( 'License: GPLv2 <img src=https://example.com/x onerror=alert(1)>' ), + 'unclosed tag' => array( 'License: GPLv2 <img src=x onerror=alert(1)' ), + ); + } + + /** + * A `License:` value that contains a GPL-compatible token passes validation, so it + * gets reported back to reviewers verbatim. It must not carry markup with it. + * + * @param string $header Full header line under test. + */ + #[DataProvider( 'license_markup_provider' )] + public function test_license_is_sanitized( string $header ): void { + $parser = new Parser( self::readme_with( $header ) ); + + $this->assertStringStartsWith( 'GPLv2', $parser->license ); + $this->assertStringNotContainsString( '<', $parser->license ); + $this->assertStringNotContainsString( 'onerror', $parser->license ); + } }