From 0301892d7e27bd884d1d9199db5420ad51505c5e Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 22 Aug 2023 10:15:38 +0200 Subject: [PATCH 01/11] Composer: update WordPressCS + PHPCS dependencies WordPressCS 3.0.0 has been released and requires a minimum PHPCS version of 3.7.2. This commit updates the WordPressCS and PHPCS dependencies and the documentation referring to those, in all the relevant places. Includes: * Removing the DealerDirect plugin from `require` as it comes with WordPressCS 3.0.0 automatically (via PHPCSUtils) and not having the explicit dependency may prevent conflicts with allowed versions in the future. * Updating the PHP version on which the CS check for the VIPCS native code is being run. With WPCS 3.0, this check is no longer limited to PHP 7.4. * Removing a work-around for WPCS < 3.0 vs PHP 8.0 * Updating the VIPCS native `.phpcs.xml.dist` ruleset. Ref: https://github.com/WordPress/WordPress-Coding-Standards/releases/tag/3.0.0 --- .github/workflows/basics.yml | 2 +- .github/workflows/quicktest.yml | 13 ++++++------- .github/workflows/test.yml | 9 +++------ .phpcs.xml.dist | 2 +- README.md | 6 +++--- composer.json | 4 ++-- 6 files changed, 16 insertions(+), 20 deletions(-) diff --git a/.github/workflows/basics.yml b/.github/workflows/basics.yml index 9392d9a8..9ce6955c 100644 --- a/.github/workflows/basics.yml +++ b/.github/workflows/basics.yml @@ -29,7 +29,7 @@ jobs: - name: Install PHP uses: shivammathur/setup-php@v2 with: - php-version: '7.4' + php-version: 'latest' coverage: none tools: cs2pr diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml index bab7311f..350c09d2 100644 --- a/.github/workflows/quicktest.yml +++ b/.github/workflows/quicktest.yml @@ -28,17 +28,17 @@ jobs: include: - php: '5.4' phpcs_version: 'dev-master' - wpcs_version: '2.3.*' + wpcs_version: '3.0.*' - php: '5.4' - phpcs_version: '3.7.1' - wpcs_version: '2.3.*' + phpcs_version: '3.7.2' + wpcs_version: '3.0.*' - php: 'latest' phpcs_version: 'dev-master' - wpcs_version: '2.3.*' + wpcs_version: '3.0.*' - php: 'latest' - phpcs_version: '3.7.1' - wpcs_version: '2.3.*' + phpcs_version: '3.7.2' + wpcs_version: '3.0.*' name: "QTest${{ matrix.phpcs_version == 'dev-master' && ' + Lint' || '' }}: PHP ${{ matrix.php }} - PHPCS ${{ matrix.phpcs_version }}" @@ -48,7 +48,6 @@ jobs: # On stable PHPCS versions, allow for PHP deprecation notices. # Unit tests don't need to fail on those for stable releases where those issues won't get fixed anymore. - # Note: the "elif" condition is temporary and should be removed once VIPCS updates to WPCS 3.0+. - name: Setup ini config id: set_ini run: | diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c0f698fd..0de1b5eb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -72,13 +72,13 @@ jobs: # no additional versions are included in the array. matrix: php: ['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2'] - phpcs_version: ['3.7.1', 'dev-master'] - wpcs_version: ['2.3.*'] + phpcs_version: ['3.7.2', 'dev-master'] + wpcs_version: ['3.0.*'] include: - php: '8.3' phpcs_version: 'dev-master' - wpcs_version: '2.3.*' + wpcs_version: '3.0.*' name: "Test: PHP ${{ matrix.php }} - PHPCS ${{ matrix.phpcs_version }} - WPCS ${{ matrix.wpcs_version }}" @@ -90,14 +90,11 @@ jobs: # On stable PHPCS versions, allow for PHP deprecation notices. # Unit tests don't need to fail on those for stable releases where those issues won't get fixed anymore. - # Note: the "elif" condition is temporary and should be removed once VIPCS updates to WPCS 3.0+. - name: Setup ini config id: set_ini run: | if [[ "${{ matrix.phpcs_version }}" != "dev-master" ]]; then echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED' >> $GITHUB_OUTPUT - elif [[ "${{ matrix.php }}" == "8.1" ]]; then - echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED' >> $GITHUB_OUTPUT else echo 'PHP_INI=error_reporting=-1' >> $GITHUB_OUTPUT fi diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index 0b06835d..c7bf32d6 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -20,7 +20,7 @@ - + diff --git a/README.md b/README.md index 1bcb6465..1006b175 100644 --- a/README.md +++ b/README.md @@ -16,9 +16,9 @@ Go to https://docs.wpvip.com/technical-references/code-review/phpcs-report/ to l ## Minimal requirements * PHP 5.4+ -* [PHPCS 3.7.1+](https://github.com/squizlabs/PHP_CodeSniffer/releases) +* [PHPCS 3.7.2+](https://github.com/squizlabs/PHP_CodeSniffer/releases) * [PHPCSUtils 1.0.8+](https://github.com/PHPCSStandards/PHPCSUtils) -* [WPCS 2.3.0+](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases) +* [WPCS 3.0.0+](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases) * [VariableAnalysis 2.11.17+](https://github.com/sirbrillig/phpcs-variable-analysis/releases) ## Installation @@ -35,7 +35,7 @@ composer g config allow-plugins.dealerdirect/phpcodesniffer-composer-installer t composer g require automattic/vipwpcs ``` -This will install the latest compatible versions of PHPCS, PHPCSUtils, WPCS and VariableAnalysis and register the external standards with PHP_CodeSniffer. +This will install the latest compatible versions of PHPCS, PHPCSUtils, PHPCSExtra, WPCS and VariableAnalysis and register the external standards with PHP_CodeSniffer. Please refer to the [installation instructions for installing PHP_CodeSniffer for WordPress.com VIP](https://docs.wpvip.com/how-tos/code-review/php_codesniffer/) for more details. diff --git a/composer.json b/composer.json index 005944eb..47ff77c0 100644 --- a/composer.json +++ b/composer.json @@ -19,8 +19,8 @@ "php": ">=5.4", "phpcsstandards/phpcsutils": "^1.0.8", "sirbrillig/phpcs-variable-analysis": "^2.11.17", - "squizlabs/php_codesniffer": "^3.7.1", - "wp-coding-standards/wpcs": "^2.3" + "squizlabs/php_codesniffer": "^3.7.2", + "wp-coding-standards/wpcs": "^3.0" }, "require-dev": { "php-parallel-lint/php-parallel-lint": "^1.3.2", From d30a3033ad107effd3d12856622f4058b64e3815 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 7 Mar 2023 01:05:46 +0100 Subject: [PATCH 02/11] WPCS 3.0 | Revert previously applied work-arounds These are no longer needed with WPCS 3.0.0. Ref: 746 --- .phpcs.xml.dist | 1 - tests/RulesetTest.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index c7bf32d6..a0049b7b 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -19,7 +19,6 @@ - diff --git a/tests/RulesetTest.php b/tests/RulesetTest.php index 41a95074..f6fed8fe 100644 --- a/tests/RulesetTest.php +++ b/tests/RulesetTest.php @@ -148,7 +148,7 @@ private function collect_phpcs_result() { } $shell = sprintf( - '%1$s%2$s --severity=1 --standard=%3$s --report=json --runtime-set minimum_supported_wp_version 0 ./%3$s/ruleset-test.inc', + '%1$s%2$s --severity=1 --standard=%3$s --report=json ./%3$s/ruleset-test.inc', $php, // Current PHP executable if available. $this->phpcs_bin, $this->ruleset From 4198a6dcbe5c83093272a32c280913023f248f3f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 22 Aug 2023 13:54:20 +0200 Subject: [PATCH 03/11] Security/EscapingVoidReturnFunctions: switch to using WPCS PrintingFunctionsTrait Note: this does mean that the sniff will now also support a `public` `customPrintingFunctions` property which can be adjusted in a custom ruleset. Co-authored-by: Gary Jones --- .../Sniffs/Security/EscapingVoidReturnFunctionsSniff.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php index e9fbc0c3..3b808d9e 100644 --- a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php @@ -9,6 +9,7 @@ namespace WordPressVIPMinimum\Sniffs\Security; use PHP_CodeSniffer\Util\Tokens; +use WordPressCS\WordPress\Helpers\PrintingFunctionsTrait; use WordPressVIPMinimum\Sniffs\Sniff; /** @@ -16,10 +17,14 @@ * * E.g. esc_html( _e( 'foo' ) ); * - * @package VIPCS\WordPressVIPMinimum + * @package VIPCS\WordPressVIPMinimum + * + * @uses \WordPressCS\WordPress\Helpers\PrintingFunctionsTrait::$customPrintingFunctions */ class EscapingVoidReturnFunctionsSniff extends Sniff { + use PrintingFunctionsTrait; + /** * Returns an array of tokens this test wants to listen for. * @@ -59,7 +64,7 @@ public function process_token( $stackPtr ) { return; } - if ( isset( $this->printingFunctions[ $this->tokens[ $next_token ]['content'] ] ) ) { + if ( $this->is_printing_function( $this->tokens[ $next_token ]['content'] ) ) { $message = 'Attempting to escape `%s()` which is printing its output.'; $data = [ $this->tokens[ $next_token ]['content'] ]; $this->phpcsFile->addError( $message, $stackPtr, 'Found', $data ); From f124466a060ba6fc9795ef392b081957aa5da598 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 22 Aug 2023 13:54:52 +0200 Subject: [PATCH 04/11] AbstractVariableRestrictionsSniff: use WPCS ContextHelper::is_in_isset_or_empty() Co-authored-by: Gary Jones --- .../Sniffs/AbstractVariableRestrictionsSniff.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php b/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php index 61d3ed7d..73e0efb3 100644 --- a/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php @@ -11,6 +11,7 @@ use PHPCSUtils\Utils\GetTokensAsString; use PHPCSUtils\Utils\MessageHelper; +use WordPressCS\WordPress\Helpers\ContextHelper; /** * Restricts usage of some variables. @@ -144,7 +145,7 @@ public function process_token( $stackPtr ) { } } - if ( $this->is_in_isset_or_empty( $stackPtr ) === true ) { + if ( ContextHelper::is_in_isset_or_empty( $this->phpcsFile, $stackPtr ) === true ) { // Checking whether a variable exists is not the same as using it. return; } From 1a8b51c1cd199c434d4db4063d5ec534f1661596 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 22 Aug 2023 13:55:11 +0200 Subject: [PATCH 05/11] AbstractVariableRestrictionsSniff: use WPCS RulesetPropertyHelper::merge_custom_array() Co-authored-by: Gary Jones --- .../Sniffs/AbstractVariableRestrictionsSniff.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php b/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php index 73e0efb3..ebbfc97f 100644 --- a/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php @@ -12,6 +12,7 @@ use PHPCSUtils\Utils\GetTokensAsString; use PHPCSUtils\Utils\MessageHelper; use WordPressCS\WordPress\Helpers\ContextHelper; +use WordPressCS\WordPress\Helpers\RulesetPropertyHelper; /** * Restricts usage of some variables. @@ -129,7 +130,7 @@ public function process_token( $stackPtr ) { $token = $this->tokens[ $stackPtr ]; - $this->excluded_groups = static::merge_custom_array( $this->exclude ); + $this->excluded_groups = RulesetPropertyHelper::merge_custom_array( $this->exclude ); if ( array_diff_key( $this->groups_cache, $this->excluded_groups ) === [] ) { // All groups have been excluded. // Don't remove the listener as the exclude property can be changed inline. From 29920a067dbe4bd085ba305461a2c4a7d8342c82 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 22 Aug 2023 13:57:16 +0200 Subject: [PATCH 06/11] AbstractArrayAssignmentRestrictions children: adjust for changes in received `$val` The `$val` parameter received by the `callback()` method will no longer automatically have been stripped of quotes. This adjusts the `callback()` methods of the sniff which extend the WPCS `AbstractArrayAssignmentRestrictions` sniff to take this into account. --- WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php | 2 +- WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php | 4 +++- WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php b/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php index 588074ba..9fe38793 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php @@ -52,6 +52,6 @@ public function getGroups() { public function callback( $key, $val, $line, $group ) { $key = strtolower( $key ); - return ( $key === 'nopaging' && ( $val === 'true' || $val === 1 ) ); + return ( $key === 'nopaging' && ( $val === 'true' || $val === '1' ) ); } } diff --git a/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php b/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php index 3204da2d..69a324e8 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php @@ -9,6 +9,7 @@ namespace WordPressVIPMinimum\Sniffs\Performance; +use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff; /** @@ -31,7 +32,7 @@ public function getGroups() { return [ 'orderby' => [ 'type' => 'error', - 'message' => 'Detected forbidden query_var "%s" of "%s". Use vip_get_random_posts() instead.', + 'message' => 'Detected forbidden query_var "%s" of %s. Use vip_get_random_posts() instead.', 'keys' => [ 'orderby', ], @@ -50,6 +51,7 @@ public function getGroups() { * @return bool FALSE if no match, TRUE if matches. */ public function callback( $key, $val, $line, $group ) { + $val = TextStrings::stripQuotes( $val ); return strtolower( $val ) === 'rand'; } } diff --git a/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php b/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php index 7067f686..f216d908 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php @@ -7,6 +7,7 @@ namespace WordPressVIPMinimum\Sniffs\Performance; +use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff; /** @@ -45,6 +46,7 @@ public function getGroups() { * @return bool FALSE if no match, TRUE if matches. */ public function callback( $key, $val, $line, $group ) { + $val = TextStrings::stripQuotes( $val ); return ( strpos( $val, 'NOT REGEXP' ) === 0 || strpos( $val, 'REGEXP' ) === 0 ); From 71dbddd8f4a4d1b429ba55c79569aa63ea662892 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 26 Jul 2022 04:46:51 +0200 Subject: [PATCH 07/11] VIPMinimum ruleset: replace strict comparison sniff Includes adding PHPCSExtra to the `composer.json` configuration as that is now a direct dependency of VIPCS as VIPCS now uses one of its sniffs in the ruleset. Co-authored-by: Gary Jones --- WordPress-VIP-Go/ruleset-test.inc | 2 +- WordPress-VIP-Go/ruleset.xml | 2 +- WordPressVIPMinimum/ruleset-test.inc | 2 +- WordPressVIPMinimum/ruleset.xml | 4 +++- composer.json | 1 + 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index d151ad67..7503201a 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -165,7 +165,7 @@ rawurlencode(); // Ok. extract( array( 'a' => 1 ) ); // Error. $obj->extract(); // Ok. -// WordPress.PHP.StrictComparisons.LooseComparison +// Universal.Operators.StrictComparisons true == $true; // Warning. false === $true; // Ok. diff --git a/WordPress-VIP-Go/ruleset.xml b/WordPress-VIP-Go/ruleset.xml index 9ea7f34f..58b0911f 100644 --- a/WordPress-VIP-Go/ruleset.xml +++ b/WordPress-VIP-Go/ruleset.xml @@ -188,7 +188,7 @@ 3 - + 3 diff --git a/WordPressVIPMinimum/ruleset-test.inc b/WordPressVIPMinimum/ruleset-test.inc index 03fd7e06..93b151e7 100644 --- a/WordPressVIPMinimum/ruleset-test.inc +++ b/WordPressVIPMinimum/ruleset-test.inc @@ -72,7 +72,7 @@ new WP_Query( array( // WordPress.WP.GlobalVariablesOverride $GLOBALS['wpdb'] = 'test'; // Error. -// WordPress.PHP.StrictComparisons +// Universal.Operators.StrictComparisons if ( true == $true ) { // Warning. } diff --git a/WordPressVIPMinimum/ruleset.xml b/WordPressVIPMinimum/ruleset.xml index 87394301..70527b17 100644 --- a/WordPressVIPMinimum/ruleset.xml +++ b/WordPressVIPMinimum/ruleset.xml @@ -36,7 +36,9 @@ - + + warning + diff --git a/composer.json b/composer.json index 47ff77c0..7f4db735 100644 --- a/composer.json +++ b/composer.json @@ -17,6 +17,7 @@ ], "require": { "php": ">=5.4", + "phpcsstandards/phpcsextra": "^1.1.0", "phpcsstandards/phpcsutils": "^1.0.8", "sirbrillig/phpcs-variable-analysis": "^2.11.17", "squizlabs/php_codesniffer": "^3.7.2", From 6d05fffb309bc5fee67a95ed1ab81ff93bde6e45 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 22 Aug 2023 11:14:58 +0200 Subject: [PATCH 08/11] VIPMinimum ruleset: replace assignment-in-condition sniff ... which has been split. --- WordPress-VIP-Go/ruleset-test.inc | 7 ++++++- WordPress-VIP-Go/ruleset-test.php | 1 + WordPress-VIP-Go/ruleset.xml | 5 ++++- WordPressVIPMinimum/ruleset-test.inc | 5 ++++- WordPressVIPMinimum/ruleset-test.php | 3 ++- WordPressVIPMinimum/ruleset.xml | 3 ++- 6 files changed, 19 insertions(+), 5 deletions(-) diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index 7503201a..7d69fbe7 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -153,7 +153,7 @@ url_to_postid( $url ); // Warning + Message. wpcom_vip_old_slug_redirect(); // Ok. wp_old_slug_redirect(); // Warning. -// WordPress.CodeAnalysis.AssignmentInCondition.Found +// Generic.CodeAnalysis.AssignmentInCondition.Found if ($a = 123) { // Warning. } @@ -572,3 +572,8 @@ $_SERVER["REMOTE_ADDR"]; // Error. <<<<<<< HEAD // Error. >>>>>>> // Error. + + 1, 550 => 1, 556 => 1, + 579 => 1, ], 'messages' => [ 7 => [ diff --git a/WordPress-VIP-Go/ruleset.xml b/WordPress-VIP-Go/ruleset.xml index 58b0911f..13129fe1 100644 --- a/WordPress-VIP-Go/ruleset.xml +++ b/WordPress-VIP-Go/ruleset.xml @@ -179,7 +179,10 @@ 1 - + + 1 + + 1 diff --git a/WordPressVIPMinimum/ruleset-test.inc b/WordPressVIPMinimum/ruleset-test.inc index 93b151e7..0cef3c4b 100644 --- a/WordPressVIPMinimum/ruleset-test.inc +++ b/WordPressVIPMinimum/ruleset-test.inc @@ -76,7 +76,7 @@ $GLOBALS['wpdb'] = 'test'; // Error. if ( true == $true ) { // Warning. } -// WordPress.CodeAnalysis.AssignmentInCondition +// Generic.CodeAnalysis.AssignmentInCondition if ( $test = get_post( $post ) ) { // Warning. } @@ -614,6 +614,9 @@ class MyClass { >>>>>>> // Error. diff --git a/WordPressVIPMinimum/ruleset-test.php b/WordPressVIPMinimum/ruleset-test.php index 881da76b..05076415 100644 --- a/WordPressVIPMinimum/ruleset-test.php +++ b/WordPressVIPMinimum/ruleset-test.php @@ -197,7 +197,7 @@ 597 => 1, 612 => 1, 614 => 1, - 618 => 1, + 621 => 1, ], 'warnings' => [ 32 => 1, @@ -290,6 +290,7 @@ 559 => 1, 565 => 1, 589 => 1, + 618 => 1, ], 'messages' => [ 130 => [ diff --git a/WordPressVIPMinimum/ruleset.xml b/WordPressVIPMinimum/ruleset.xml index 70527b17..04695707 100644 --- a/WordPressVIPMinimum/ruleset.xml +++ b/WordPressVIPMinimum/ruleset.xml @@ -39,7 +39,8 @@ warning - + + From 436b0fe5bf9fbf8995e57283d2979d53be1ee095 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 22 Aug 2023 11:16:42 +0200 Subject: [PATCH 09/11] VIPMinimum ruleset: update excludes for the WP/AlternativeFunctions sniff ... to prevent introducing new duplicate error messages. Co-authored-by: Gary Jones --- WordPress-VIP-Go/ruleset-test.inc | 2 +- WordPress-VIP-Go/ruleset.xml | 6 +++--- WordPressVIPMinimum/ruleset.xml | 28 +++++++++++++++++++++++++--- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index 7d69fbe7..a2149ba6 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -59,7 +59,7 @@ if ( isset( $_SERVER['HTTP_USER_AGENT'] ) && $_SERVER['HTTP_USER_AGENT'] === 'so // Make sure nonce verification is done in global scope to silence notices about use of superglobals without later on in the file. isset( $_GET['my_nonce'] ) && wp_verify_nonce( sanitize_text_field( $_GET['my_nonce'] ) ); -// WordPress.WP.AlternativeFunctions.file_system_read_fopen +// WordPress.WP.AlternativeFunctions.file_system_operations_fopen fopen( 'file.txt', 'r' ); // Warning + Message. // WordPressVIPMinimum.Performance.FetchingRemoteData.FileGetContentsUnknown diff --git a/WordPress-VIP-Go/ruleset.xml b/WordPress-VIP-Go/ruleset.xml index 13129fe1..190bebc8 100644 --- a/WordPress-VIP-Go/ruleset.xml +++ b/WordPress-VIP-Go/ruleset.xml @@ -104,7 +104,7 @@ This includes potential security holes as well as functions that may bring down sites for performance reasons. --> - + File system operations only work on the `/tmp/` and `wp-content/uploads/` directories. To avoid unexpected results, please use helper functions like `get_temp_dir()` or `wp_get_upload_dir()` to get the proper directory path when using functions such as %s(). For more details, please see: https://docs.wpvip.com/technical-references/vip-go-files-system/local-file-operations/ @@ -249,10 +249,10 @@ 0 - + 0 - + 0 diff --git a/WordPressVIPMinimum/ruleset.xml b/WordPressVIPMinimum/ruleset.xml index 04695707..13670326 100644 --- a/WordPressVIPMinimum/ruleset.xml +++ b/WordPressVIPMinimum/ruleset.xml @@ -135,10 +135,32 @@ - - - + + + + + + + + + + + + + + + + + + + + + + + + + From 4094eec61768dc81cd7f21a8417f1158a16e3e12 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 22 Aug 2023 15:42:44 +0200 Subject: [PATCH 10/11] Update ruleset tests: account for new errors from WPCS * The `WordPress.Security.ValidatedSanitizedInput` sniff will now also examine `$_SESSION` variables. As the test is about another sniff, let's just ignore the notices coming from `WordPress.Security.ValidatedSanitizedInput`. --- WordPress-VIP-Go/ruleset-test.inc | 2 +- WordPressVIPMinimum/ruleset-test.inc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPress-VIP-Go/ruleset-test.inc b/WordPress-VIP-Go/ruleset-test.inc index a2149ba6..773842b0 100644 --- a/WordPress-VIP-Go/ruleset-test.inc +++ b/WordPress-VIP-Go/ruleset-test.inc @@ -557,7 +557,7 @@ echo " 999, // Warning. ); _query_posts( 'posts_per_page=999' ); // Warning. @@ -45,7 +45,7 @@ $query_args['posts_per_page'] = 999; // Warning. date_default_timezone_set( 'FooBar' ); // Error. // WordPress.DB.PreparedSQL -$b = function () { +$b = function () { // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable global $wpdb; $listofthings = wp_cache_get( 'foo' ); if ( ! $listofthings ) { @@ -57,7 +57,7 @@ $b = function () { }; // WordPress.DB.DirectDatabaseQuery -$baz = $wpdb->get_results( $wpdb->prepare( 'SELECT X FROM Y ' ) ); // Warning x 2. +$baz = $wpdb->get_results( $wpdb->prepare( 'SELECT X FROM Y ' ) ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Warning x 2. // WordPress.DB.SlowDBQuery $test = [ @@ -139,9 +139,9 @@ serialize(); // Warning. unserialize(); // Warning. urlencode(); // Warning. passthru( 'cat myfile.zip', $err ); // Warning. -$process = proc_open( 'php', $descriptorspec, $pipes, $cwd, $env ); // Warning. -$last_line = system( 'ls', $retval ); // Warning. -$handle = popen( '/bin/ls', 'r' ); // Warning. +$process = proc_open( 'php', $descriptorspec, $pipes, $cwd, $env ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Warning. +$last_line = system( 'ls', $retval ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Warning. +$handle = popen( '/bin/ls', 'r' ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Warning. // WordPress.PHP.DiscouragedPHPFunctions.runtime_configuration_error_reporting error_reporting(); // Error. @@ -174,7 +174,7 @@ dl(); // Error. exec( 'whoami' ); // Error. // WordPress.PHP.DiscouragedPHPFunctions.system_calls_shell_exec -$output = shell_exec( 'ls -lart' ); // Error. +$output = shell_exec( 'ls -lart' ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Error. // WordPress.PHP.DevelopmentFunctions var_dump(); // Warning. @@ -243,7 +243,7 @@ curl_init(); // Warning + Message. curl_close( $ch ); // Warning + Message. CURL_getinfo(); // Warning + Message. parse_url( 'http://example.com/' ); // Warning. -$json = json_encode( $thing ); // Warning. +$json = json_encode( $thing ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Warning. readfile(); // Warning. fclose(); // Warning. fopen(); // Warning. @@ -402,7 +402,7 @@ wp_remote_get( $url ); // Warning. setcookie( 'cookie[three]', 'cookiethree' ); // Error. get_posts(); // Warning. wp_get_recent_posts(); // Warning. -$wp_random_testing = create_function( '$a, $b', 'return ( $b / $a ); '); // Warning. +$wp_random_testing = create_function( '$a, $b', 'return ( $b / $a ); '); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Warning. wpcom_vip_get_term_link(); // Warning. wpcom_vip_get_term_by(); // Warning. wpcom_vip_get_category_by_slug(); // Warning. @@ -447,11 +447,11 @@ add_filter( 'robots_txt', function() { // Warning. // WordPressVIPMinimum.Performance.CacheValueOverride -$bad_wp_users = wp_cache_get( md5( self::CACHE_KEY . '_wp_users'), self::CACHE_GROUP ); -$bad_wp_users = false; // Error. +$bad_wp_users = wp_cache_get( md5( self::CACHE_KEY . '_wp_users'), self::CACHE_GROUP ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable +$bad_wp_users = false; // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Error. // WordPressVIPMinimum.Performance.FetchingRemoteData -$external_resource = file_get_contents( 'https://example.com' ); // Warning. +$external_resource = file_get_contents( 'https://example.com' ); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Warning. // WordPressVIPMinimum.Performance.LowExpiryCacheTime wp_cache_set( 'test', $data, $group, 100 ); // Warning. @@ -459,13 +459,13 @@ wp_cache_add( 123, $data, null, 1.5 * MINUTE_IN_SECONDS ); // Warning. wp_cache_replace( 'test', $data, $group, 2*MINUTE_IN_SECONDS ); // Warning. // WordPressVIPMinimum.Performance.NoPaging -$args = array( +$args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable 'nopaging' => true, // Error. ); _query_posts( 'nopaging=true' ); // Error. // WordPressVIPMinimum.Performance.OrderByRand -$args = array( +$args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable "orderby" => "RAND", // Error. ); $query_args['orderby'] = 'rand'; // Error. @@ -585,8 +585,8 @@ echo ''; / users"; // Error. -$x = foo( sanitize_text_field( $_SERVER['HTTP_USER_AGENT'] ) ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated -- Warning. +$query = "SELECT * FROM $wpdb->users"; // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Error. +$x = foo( sanitize_text_field( $_SERVER['HTTP_USER_AGENT'] ) ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated,VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Warning. foo( $_SESSION['bar'] ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- Error. // WordPressVIPMinimum.Variables.ServerVariables