diff --git a/TESTING_INSTRUCTIONS.md b/TESTING_INSTRUCTIONS.md new file mode 100644 index 000000000..63759da6b --- /dev/null +++ b/TESTING_INSTRUCTIONS.md @@ -0,0 +1,23 @@ +# Test PR #1292 — Personal Data Exporter Check (PHPMD Complexity Fix) + +## Install +1. Download `plugin-check-pr-1292.zip` +2. WP Admin → Plugins → Add New → Upload Plugin +3. Upload the zip, click Install Now, then Activate + +## What was changed +CI PHPMD complexity failure resolved. Extracted token-matching logic into dedicated helpers to reduce NPath complexity below 200 threshold. + +1. **Extracted `is_personal_data_function_call()`** — checks `T_STRING` match + global call + `(`. +2. **Extracted `is_wpdb_method_call()`** — checks `$wpdb` + `->` + `T_STRING` method + array match. +3. **Extracted `is_exporter_filter_registration()`** — checks `add_filter` + global call + `(` + string arg + match. + +All previous functionality (token scanner, test-exclusion, experimental trait, @since fix, docs row, 4th test) remains intact. + +## Verify +1. Run `composer phpmd` — 0 complexity errors. +2. Run `npm run test-php` — 476 tests pass. +3. Run `composer lint` and `composer phpstan` — 0 errors. + +## Requirements +WordPress 6.3+ · PHP 7.4+ \ No newline at end of file diff --git a/docs/checks.md b/docs/checks.md index 11662a607..16ba353f7 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -16,6 +16,7 @@ | plugin_uninstall | plugin_repo | Checks related to plugin uninstallation. | [Learn more](https://developer.wordpress.org/plugins/plugin-basics/uninstall-methods/#method-2-uninstall-php) | | external_admin_menu_links | plugin_repo | Detects external URLs used in top-level WordPress admin menu, which disrupts the expected user experience. | [Learn more](https://developer.wordpress.org/plugins/wordpress-org/detailed-plugin-guidelines/#11-plugins-should-not-hijack-the-admin) | | wp_functions_compatibility | plugin_repo | Checks whether WordPress functions used by the plugin are compatible with the declared minimum supported WordPress version ("Requires at least"). | [Learn more](https://developer.wordpress.org/plugins/plugin-basics/header-requirements/#header-fields) | +| personal_data_exporter | plugin_repo | Detects plugins that store personal data without registering a personal data exporter for GDPR compliance. | [Learn more](https://developer.wordpress.org/plugins/privacy/adding-the-personal-data-exporter-to-your-plugin/) | | plugin_review_phpcs | plugin_repo | Runs PHP_CodeSniffer to detect certain best practices plugins should follow for submission on WordPress.org, including heredoc usage detection. | [Learn more](https://developer.wordpress.org/plugins/plugin-basics/best-practices/) | | direct_db_queries | security, plugin_repo | Checks the usage of direct database queries, which should be avoided. | [Learn more](https://developer.wordpress.org/apis/database/) | | direct_db | security, plugin_repo | Checks the escaping in direct database queries. | [Learn more](https://developer.wordpress.org/apis/database/) | diff --git a/includes/Checker/Checks/Plugin_Repo/Personal_Data_Exporter_Check.php b/includes/Checker/Checks/Plugin_Repo/Personal_Data_Exporter_Check.php new file mode 100644 index 000000000..15b7f7491 --- /dev/null +++ b/includes/Checker/Checks/Plugin_Repo/Personal_Data_Exporter_Check.php @@ -0,0 +1,451 @@ +insert()` and + * `update_user_meta()` are common for non-personal data (view counters, plugin + * settings, caches) and may produce false positives on real plugins. Opt in + * with `--include-experimental`. + * + * @since 2.0.0 + * @link https://developer.wordpress.org/plugins/privacy/adding-the-personal-data-exporter-to-your-plugin/ + */ +class Personal_Data_Exporter_Check extends Abstract_File_Check { + + use Amend_Check_Result; + use Experimental_Check; + + /** + * Personal-data storage function names that trigger the check. + * + * @since 2.0.0 + * @var string[] + */ + const PERSONAL_DATA_FUNCTIONS = array( + 'add_user_meta', + 'update_user_meta', + 'add_comment_meta', + 'update_comment_meta', + ); + + /** + * `$wpdb` method names that trigger the check. + * + * @since 2.0.0 + * @var string[] + */ + const WPDB_METHODS = array( 'insert', 'update', 'replace' ); + + /** + * Filter name that registers a personal data exporter. + * + * @since 2.0.0 + * @var string + */ + const EXPORTER_FILTER = 'wp_privacy_personal_data_exporters'; + + /** + * Gets the categories for the check. + * + * Every check must have at least one category. + * + * @since 2.0.0 + * + * @return array The categories for the check. + */ + public function get_categories() { + return array( Check_Categories::CATEGORY_PLUGIN_REPO ); + } + + /** + * Amends the given result by running the check on the given list of files. + * + * @since 2.0.0 + * + * @param Check_Result $result The check result to amend, including the plugin context to check. + * @param array $files List of absolute file paths. + */ + protected function check_files( Check_Result $result, array $files ) { + $php_files = self::filter_files_by_extension( $files, 'php' ); + $php_files = $this->filter_out_test_paths( $php_files, $result->plugin()->path() ); + + $this->check_for_missing_exporter( $result, $php_files ); + } + + /** + * Checks whether the plugin handles personal data but omits the exporter filter. + * + * The check is intentionally a two-step process: + * 1. Confirm the plugin has at least one personal-data storage call. + * 2. Only then verify whether it registers the exporter filter. + * + * This avoids false positives for plugins that do not touch personal data at all. + * + * @since 2.0.0 + * + * @param Check_Result $result The check result to amend. + * @param array $php_files List of absolute PHP file paths. + */ + protected function check_for_missing_exporter( Check_Result $result, array $php_files ) { + if ( empty( $php_files ) ) { + return; + } + + // Step 1: detect personal data signals across all plugin PHP files. + $signal_file = $this->find_file_with_personal_data_signal( $php_files ); + + if ( null === $signal_file ) { + // No personal data handling detected — nothing to warn about. + return; + } + + // Step 2: check if the plugin already registers a personal data exporter. + if ( $this->plugin_registers_exporter( $php_files ) ) { + // Exporter is registered — no issue. + return; + } + + // Personal data is handled but no exporter is registered: emit a warning. + $this->add_result_warning_for_file( + $result, + __( 'Personal data was detected in this plugin but no data exporter has been registered. Plugins that store personal data should implement a data exporter via the wp_privacy_personal_data_exporters filter so that site administrators can fulfill data export requests.', 'plugin-check' ), + 'missing_personal_data_exporter', + $signal_file, + 0, + 0, + 'https://developer.wordpress.org/plugins/privacy/adding-the-personal-data-exporter-to-your-plugin/', + 5 + ); + } + + /** + * Filters out files inside the plugin's own `tests/` subdirectory. + * + * The match is anchored to the plugin root so a host project's + * `tests/phpunit/...` test runner is not affected. + * + * @since 2.0.0 + * + * @param array $php_files Absolute file paths. + * @param string $plugin_root Absolute path to the plugin's main file. + * @return array Filtered list. + */ + private function filter_out_test_paths( array $php_files, string $plugin_root ): array { + $plugin_dir = wp_normalize_path( dirname( $plugin_root ) ); + + return array_values( + array_filter( + $php_files, + static function ( string $file ) use ( $plugin_dir ): bool { + $normalized = wp_normalize_path( $file ); + + // File must be inside the plugin directory to be subject to filtering. + if ( 0 !== strpos( $normalized, $plugin_dir . '/' ) ) { + return true; + } + + $relative = substr( $normalized, strlen( $plugin_dir ) + 1 ); + + // Skip top-level "tests" or "tests/anything" inside the plugin. + if ( 0 === strpos( $relative, 'tests' ) && ( strlen( $relative ) === 5 || '/' === $relative[5] ) ) { + return false; + } + + return true; + } + ) + ); + } + + /** + * Returns the first file containing a personal-data storage call, or null. + * + * Uses `token_get_all()` to skip comments and ensure the call is a real + * function/variable call rather than text inside a string literal or + * docblock. + * + * @since 2.0.0 + * + * @param array $php_files Absolute file paths. + * @return string|null First matching file path, or null if none found. + */ + private function find_file_with_personal_data_signal( array $php_files ): ?string { + foreach ( $php_files as $file ) { + $source = file_get_contents( $file ); + if ( false === $source || '' === $source ) { + continue; + } + + $tokens = token_get_all( $source ); + $count = count( $tokens ); + + for ( $i = 0; $i < $count; $i++ ) { + if ( $this->is_personal_data_function_call( $tokens, $i ) ) { + return $file; + } + + if ( $this->is_wpdb_method_call( $tokens, $i ) ) { + return $file; + } + } + } + + return null; + } + + /** + * Checks if the token at the given index is a personal data function call. + * + * @since 2.0.0 + * + * @param array $tokens Token stream. + * @param int $index Current token index. + * @return bool + */ + private function is_personal_data_function_call( array $tokens, int $index ): bool { + $token = $tokens[ $index ]; + if ( ! is_array( $token ) || T_STRING !== $token[0] ) { + return false; + } + $name = strtolower( $token[1] ); + if ( ! in_array( $name, self::PERSONAL_DATA_FUNCTIONS, true ) ) { + return false; + } + if ( ! $this->is_global_function_call( $tokens, $index ) ) { + return false; + } + $next = $this->get_next_significant_token_index( $tokens, $index ); + if ( null === $next || '(' !== $tokens[ $next ] ) { + return false; + } + return true; + } + + /** + * Checks if the token at the given index is a `$wpdb` method call (insert, update, replace). + * + * @since 2.0.0 + * + * @param array $tokens Token stream. + * @param int $index Current token index. + * @return bool + */ + private function is_wpdb_method_call( array $tokens, int $index ): bool { + $token = $tokens[ $index ]; + if ( ! is_array( $token ) || T_VARIABLE !== $token[0] || '$wpdb' !== $token[1] ) { + return false; + } + + $arrow_index = $this->get_next_significant_token_index( $tokens, $index ); + $method_index = $this->get_next_significant_token_index( $tokens, $arrow_index ); + + if ( null === $arrow_index || null === $method_index ) { + return false; + } + + $arrow_token = $tokens[ $arrow_index ]; + $method_token = $tokens[ $method_index ]; + + return is_array( $arrow_token ) && T_OBJECT_OPERATOR === $arrow_token[0] + && is_array( $method_token ) && T_STRING === $method_token[0] + && in_array( strtolower( $method_token[1] ), self::WPDB_METHODS, true ); + } + + /** + * Determines whether any of the given files register the personal data exporter. + * + * Looks for `add_filter( 'wp_privacy_personal_data_exporters', … )` using + * the PHP tokenizer to avoid matching commented-out registrations. + * + * @since 2.0.0 + * + * @param array $php_files Absolute file paths. + * @return bool True if the filter is registered. + */ + private function plugin_registers_exporter( array $php_files ): bool { + foreach ( $php_files as $file ) { + $source = file_get_contents( $file ); + if ( false === $source || '' === $source ) { + continue; + } + + $tokens = token_get_all( $source ); + $count = count( $tokens ); + + for ( $i = 0; $i < $count; $i++ ) { + if ( $this->is_exporter_filter_registration( $tokens, $i ) ) { + return true; + } + } + } + + return false; + } + + /** + * Checks if the token at the given index is an `add_filter` call registering + * the personal data exporter filter. + * + * @since 2.0.0 + * + * @param array $tokens Token stream. + * @param int $index Current token index. + * @return bool + */ + private function is_exporter_filter_registration( array $tokens, int $index ): bool { + $token = $tokens[ $index ]; + if ( ! is_array( $token ) || T_STRING !== $token[0] || 'add_filter' !== strtolower( $token[1] ) ) { + return false; + } + if ( ! $this->is_global_function_call( $tokens, $index ) ) { + return false; + } + + $open_paren = $this->get_next_significant_token_index( $tokens, $index ); + $arg_index = $this->get_next_significant_token_index( $tokens, $open_paren ); + + if ( null === $open_paren || '(' !== $tokens[ $open_paren ] || null === $arg_index ) { + return false; + } + + $arg = $tokens[ $arg_index ]; + if ( ! is_array( $arg ) || T_CONSTANT_ENCAPSED_STRING !== $arg[0] ) { + return false; + } + + return trim( $arg[1], "\"' \t\n\r\0\x0B" ) === self::EXPORTER_FILTER; + } + + /** + * Checks whether a tokenized T_STRING is a global function call. + * + * Mirrors the helper from WP_Functions_Compatibility_Check — excludes + * method calls (`->method`), static calls (`Class::method`), and + * namespaced calls (`\My\function`). + * + * @since 2.0.0 + * + * @param array $tokens Token stream. + * @param int $index Current token index. + * @return bool + */ + private function is_global_function_call( array $tokens, int $index ): bool { + $previous_index = $this->get_previous_significant_token_index( $tokens, $index ); + if ( null === $previous_index ) { + return true; + } + + $previous_token = $tokens[ $previous_index ]; + + if ( is_array( $previous_token ) ) { + if ( in_array( $previous_token[0], array( T_FUNCTION, T_NEW, T_OBJECT_OPERATOR, T_DOUBLE_COLON ), true ) ) { + return false; + } + + if ( T_NS_SEPARATOR === $previous_token[0] ) { + $before_namespace_index = $this->get_previous_significant_token_index( $tokens, $previous_index ); + if ( null === $before_namespace_index ) { + return true; + } + + $before_namespace_token = $tokens[ $before_namespace_index ]; + if ( is_array( $before_namespace_token ) && in_array( $before_namespace_token[0], array( T_STRING, T_NAMESPACE ), true ) ) { + return false; + } + } + } + + return true; + } + + /** + * Finds the next significant token index. + * + * @since 2.0.0 + * + * @param array $tokens Token stream. + * @param int $index Current token index. + * @return int|null + */ + private function get_next_significant_token_index( array $tokens, int $index ): ?int { + $count = count( $tokens ); + for ( $i = $index + 1; $i < $count; $i++ ) { + $token = $tokens[ $i ]; + if ( is_array( $token ) && T_WHITESPACE === $token[0] ) { + continue; + } + + return $i; + } + + return null; + } + + /** + * Finds the previous significant token index. + * + * @since 2.0.0 + * + * @param array $tokens Token stream. + * @param int $index Current token index. + * @return int|null + */ + private function get_previous_significant_token_index( array $tokens, int $index ): ?int { + for ( $i = $index - 1; $i >= 0; $i-- ) { + $token = $tokens[ $i ]; + + if ( is_array( $token ) && in_array( $token[0], array( T_WHITESPACE, T_COMMENT, T_DOC_COMMENT ), true ) ) { + continue; + } + + return $i; + } + + return null; + } + + /** + * Gets the description for the check. + * + * Every check must have a short description explaining what the check does. + * + * @since 2.0.0 + * + * @return string Description. + */ + public function get_description(): string { + return __( 'Detects plugins that store personal data without registering a personal data exporter for GDPR compliance.', 'plugin-check' ); + } + + /** + * Gets the documentation URL for the check. + * + * Every check must have a URL with further information about the check. + * + * @since 2.0.0 + * + * @return string The documentation URL. + */ + public function get_documentation_url(): string { + return 'https://developer.wordpress.org/plugins/privacy/adding-the-personal-data-exporter-to-your-plugin/'; + } +} diff --git a/includes/Checker/Default_Check_Repository.php b/includes/Checker/Default_Check_Repository.php index c22371044..81d760924 100644 --- a/includes/Checker/Default_Check_Repository.php +++ b/includes/Checker/Default_Check_Repository.php @@ -102,6 +102,7 @@ private function register_default_checks() { 'direct_file_access' => new Checks\Plugin_Repo\Direct_File_Access_Check(), 'external_admin_menu_links' => new Checks\Plugin_Repo\External_Admin_Menu_Links_Check(), 'wp_functions_compatibility' => new Checks\Plugin_Repo\WP_Functions_Compatibility_Check(), + 'personal_data_exporter' => new Checks\Plugin_Repo\Personal_Data_Exporter_Check(), ) ); diff --git a/plugin-check-pr-1292.zip b/plugin-check-pr-1292.zip new file mode 100644 index 000000000..90300bf35 Binary files /dev/null and b/plugin-check-pr-1292.zip differ diff --git a/tests/phpunit/testdata/plugins/test-plugin-personal-data-exporter-with-errors/load.php b/tests/phpunit/testdata/plugins/test-plugin-personal-data-exporter-with-errors/load.php new file mode 100644 index 000000000..22fa167b1 --- /dev/null +++ b/tests/phpunit/testdata/plugins/test-plugin-personal-data-exporter-with-errors/load.php @@ -0,0 +1,26 @@ +insert() directly but does not register a data exporter. + * Requires at least: 6.3 + * Requires PHP: 7.4 + * Version: 1.0.0 + * Author: WordPress Performance Team + * Author URI: https://make.wordpress.org/performance/ + * License: GPLv2 or later + * License URI: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * Text Domain: test-plugin-personal-data-exporter-wpdb + * + * @package test-plugin-personal-data-exporter-wpdb + */ + +/** + * Persists a custom value tied to a user via a direct database write. + * + * @param int $user_id User ID. + */ +function test_pde_wpdb_save_user_data( $user_id ) { + global $wpdb; + $wpdb->insert( + $wpdb->usermeta, + array( + 'user_id' => $user_id, + 'meta_key' => 'test_pde_wpdb_value', + 'meta_value' => 'test_value', + ) + ); +} +add_action( 'user_register', 'test_pde_wpdb_save_user_data' ); diff --git a/tests/phpunit/testdata/plugins/test-plugin-personal-data-exporter-without-errors/load.php b/tests/phpunit/testdata/plugins/test-plugin-personal-data-exporter-without-errors/load.php new file mode 100644 index 000000000..d9758620e --- /dev/null +++ b/tests/phpunit/testdata/plugins/test-plugin-personal-data-exporter-without-errors/load.php @@ -0,0 +1,80 @@ + __( 'Test PDE OK Plugin Data', 'test-plugin-personal-data-exporter-ok' ), + 'callback' => 'test_pde_ok_exporter', + ); + return $exporters; +} +add_filter( 'wp_privacy_personal_data_exporters', 'test_pde_ok_register_exporter' ); + +/** + * Exports personal data for a user. + * + * @param string $email_address Email address of the user. + * @param int $page Pagination page number. + * @return array Export data. + */ +function test_pde_ok_exporter( $email_address, $page = 1 ) { + $user = get_user_by( 'email', $email_address ); + if ( ! $user ) { + return array( + 'data' => array(), + 'done' => true, + ); + } + + $preference = get_user_meta( $user->ID, 'test_pde_ok_preference', true ); + $data = array(); + + if ( $preference ) { + $data[] = array( + 'group_id' => 'test-pde-ok', + 'group_label' => __( 'Test PDE OK Data', 'test-plugin-personal-data-exporter-ok' ), + 'item_id' => 'test-pde-ok-' . $user->ID, + 'data' => array( + array( + 'name' => __( 'Preference', 'test-plugin-personal-data-exporter-ok' ), + 'value' => $preference, + ), + ), + ); + } + + return array( + 'data' => $data, + 'done' => true, + ); +} diff --git a/tests/phpunit/tests/Checker/Checks/Personal_Data_Exporter_Check_Tests.php b/tests/phpunit/tests/Checker/Checks/Personal_Data_Exporter_Check_Tests.php new file mode 100644 index 000000000..e58b913f9 --- /dev/null +++ b/tests/phpunit/tests/Checker/Checks/Personal_Data_Exporter_Check_Tests.php @@ -0,0 +1,117 @@ +run( $check_result ); + + $warnings = $check_result->get_warnings(); + + $this->assertNotEmpty( $warnings ); + + $found = false; + foreach ( $warnings as $file_warnings ) { + foreach ( $file_warnings as $line_warnings ) { + foreach ( $line_warnings as $col_warnings ) { + foreach ( $col_warnings as $warning ) { + if ( isset( $warning['code'] ) && 'missing_personal_data_exporter' === $warning['code'] ) { + $found = true; + break 4; + } + } + } + } + } + + $this->assertTrue( $found, 'Expected missing_personal_data_exporter warning was not found.' ); + } + + public function test_plugin_with_personal_data_and_exporter_has_no_warning() { + $check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-personal-data-exporter-without-errors/load.php' ); + $check_result = new Check_Result( $check_context ); + + $check = new Personal_Data_Exporter_Check(); + $check->run( $check_result ); + + $found = false; + foreach ( $check_result->get_warnings() as $file_warnings ) { + foreach ( $file_warnings as $line_warnings ) { + foreach ( $line_warnings as $col_warnings ) { + foreach ( $col_warnings as $warning ) { + if ( isset( $warning['code'] ) && 'missing_personal_data_exporter' === $warning['code'] ) { + $found = true; + break 4; + } + } + } + } + } + + $this->assertFalse( $found, 'Unexpected missing_personal_data_exporter warning was found.' ); + } + + public function test_plugin_with_no_personal_data_has_no_warning() { + $check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-safe-redirect/load.php' ); + $check_result = new Check_Result( $check_context ); + + $check = new Personal_Data_Exporter_Check(); + $check->run( $check_result ); + + $found = false; + foreach ( $check_result->get_warnings() as $file_warnings ) { + foreach ( $file_warnings as $line_warnings ) { + foreach ( $line_warnings as $col_warnings ) { + foreach ( $col_warnings as $warning ) { + if ( isset( $warning['code'] ) && 'missing_personal_data_exporter' === $warning['code'] ) { + $found = true; + break 4; + } + } + } + } + } + + $this->assertFalse( $found, 'Unexpected missing_personal_data_exporter warning on a plugin with no personal data.' ); + } + + public function test_plugin_with_wpdb_insert_no_exporter_triggers_warning() { + $check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-personal-data-exporter-with-wpdb-insert/load.php' ); + $check_result = new Check_Result( $check_context ); + + $check = new Personal_Data_Exporter_Check(); + $check->run( $check_result ); + + $warnings = $check_result->get_warnings(); + + $this->assertNotEmpty( $warnings ); + + $found = false; + foreach ( $warnings as $file_warnings ) { + foreach ( $file_warnings as $line_warnings ) { + foreach ( $line_warnings as $col_warnings ) { + foreach ( $col_warnings as $warning ) { + if ( isset( $warning['code'] ) && 'missing_personal_data_exporter' === $warning['code'] ) { + $found = true; + break 4; + } + } + } + } + } + + $this->assertTrue( $found, 'Expected missing_personal_data_exporter warning for $wpdb->insert was not found.' ); + } +} diff --git a/wp-tests-config.php b/wp-tests-config.php new file mode 100644 index 000000000..2db47cda5 --- /dev/null +++ b/wp-tests-config.php @@ -0,0 +1,23 @@ +