Issue 1157/Proposal 2: rename sniffs

WordPress-Core/ruleset.xml

@@ -259,7 +259,7 @@
 
 	<!-- Covers rule: Escaping should be done as close to the time of the query as possible,
 		 preferably by using $wpdb->prepare() -->
-	<rule ref="WordPress.WP.PreparedSQL"/>
+	<rule ref="WordPress.DB.PreparedSQL"/>
 
 
 	<!--
@@ -400,7 +400,7 @@
 	Ref: https://make.wordpress.org/core/handbook/coding-standards/php/#dont-extract
 	#############################################################################
 	-->
-	<rule ref="WordPress.Functions.DontExtract"/>
+	<rule ref="WordPress.PHP.DontExtract"/>
 
 
 	<!--

WordPress-Extra/ruleset.xml

@@ -101,11 +101,11 @@
 		<severity>5</severity>
 	</rule>
 
-	<rule ref="WordPress.XSS.EscapeOutput"/>
+	<rule ref="WordPress.Security.EscapeOutput"/>
 
 	<!-- Verify that a nonce check is done before using values in superglobals.
 		 https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/73 -->
-	<rule ref="WordPress.CSRF.NonceVerification"/>
+	<rule ref="WordPress.Security.NonceVerification"/>
 
 	<rule ref="WordPress.PHP.DevelopmentFunctions"/>
 	<rule ref="WordPress.PHP.DiscouragedPHPFunctions"/>
@@ -122,7 +122,7 @@
 
 	<!-- Warn against overriding WP global variables.
 		 https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/26 -->
-	<rule ref="WordPress.Variables.GlobalVariables"/>
+	<rule ref="WordPress.WP.GlobalVariablesOverride"/>
 
 	<!-- Encourage the use of strict ( === and !== ) comparisons.
 		 https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/242 -->

WordPress-VIP/ruleset.xml

@@ -8,25 +8,23 @@
 
 	<!-- Covers:
 		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#removing-the-admin-bar
-		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#cron-schedules-less-than-15-minutes-or-expensive-events
-		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#direct-database-queries
 		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#filesystem-writes
 		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#order-by-rand
-		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#using-__file__-for-page-registration
 		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#uncached-functions
 		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#flush_rewrite_rules
 		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#session_start-and-other-session-related-functions
 		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#functions-that-use-joins-taxonomy-relation-queries-cat-tax-queries-subselects-or-api-calls
 		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#querying-on-meta_value
-		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#manipulating-the-timezone-server-side
-		 https://vip.wordpress.com/documentation/code-review-what-we-look-for/#validation-sanitization-and-escaping
 	-->
 	<rule ref="WordPress.VIP"/>
 
 	<!-- https://vip.wordpress.com/documentation/code-review-what-we-look-for/#validation-sanitization-and-escaping -->
 	<!-- https://vip.wordpress.com/documentation/best-practices/security/validating-sanitizing-escaping/ -->
-	<rule ref="WordPress.XSS.EscapeOutput"/>
-	<rule ref="WordPress.CSRF.NonceVerification"/>
+	<rule ref="WordPress.Security.EscapeOutput"/>
+	<rule ref="WordPress.Security.NonceVerification"/>
+
+	<!-- https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/issues/69 -->
+	<rule ref="WordPress.Security.ValidatedSanitizedInput"/>
 
 	<!-- https://vip.wordpress.com/documentation/code-review-what-we-look-for/#using-instead-of -->
 	<rule ref="WordPress.PHP.StrictComparisons"/>
@@ -82,4 +80,36 @@
 	<!-- https://vip.wordpress.com/documentation/code-review-what-we-look-for/#using-theme-constants -->
 	<rule ref="WordPress.WP.DiscouragedConstants"/>
 
+	<!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#direct-database-queries -->
+	<!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#database-alteration -->
+	<rule ref="WordPress.DB.DirectDatabaseQuery"/>
+	<rule ref="WordPress.DB.DirectDatabaseQuery.SchemaChange">
+		<type>error</type>
+		<message>Attempting a database schema change is highly discouraged.</message>
+	</rule>
+	<rule ref="WordPress.DB.DirectDatabaseQuery.NoCaching">
+		<type>error</type>
+		<message>Usage of a direct database call without caching is prohibited on the WP VIP platform. Use wp_cache_get / wp_cache_set or wp_cache_delete.</message>
+	</rule>
+
+	<!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#uncached-pageload -->
+	<rule ref="WordPress.DB.SlowDBQuery"/>
+
+	<!-- https://vip.wordpress.com/documentation/vip/code-review-what-we-look-for/#using-__file__-for-page-registration -->
+	<rule ref="WordPress.Security.PluginMenuSlug"/>
+	<rule ref="WordPress.Security.PluginMenuSlug.Using__FILE__">
+		<type>error</type>
+	</rule>
+
+	<!-- https://vip.wordpress.com/documentation/code-review-what-we-look-for/#cron-schedules-less-than-15-minutes-or-expensive-events -->
+	<rule ref="WordPress.WP.CronInterval"/>
+	<rule ref="WordPress.WP.CronInterval.CronSchedulesInterval">
+		<type>error</type>
+		<message>Scheduling crons at %s sec ( less than %s minutes ) is prohibited.</message>
+	</rule>
+
+	<!-- https://vip.wordpress.com/documentation/code-review-what-we-look-for/#manipulating-the-timezone-server-side -->
+	<!-- https://vip.wordpress.com/documentation/use-current_time-not-date_default_timezone_set/ -->
+	<rule ref="WordPress.WP.TimezoneChange"/>
+
 </ruleset>

WordPress/Sniff.php

@@ -86,7 +86,7 @@ abstract class Sniff implements PHPCS_Sniff {
 	 * This property allows end-users to add to the $test_class_whitelist via their ruleset.
 	 * This property will need to be set for each sniff which uses the
 	 * `is_test_class()` method.
-	 * Currently the method is used by the `WordPress.Variables.GlobalVariables`,
+	 * Currently the method is used by the `WordPress.WP.GlobalVariablesOverride`,
 	 * `WordPress.NamingConventions.PrefixAllGlobals` and the `WordPress.Files.Filename` sniffs.
 	 *
 	 * Example usage:

WordPress/Sniffs/CSRF/NonceVerificationSniff.php

@@ -20,184 +20,60 @@
  *
  * @since   0.5.0
  * @since   0.13.0 Class name changed: this class is now namespaced.
+ *
+ * @deprecated 0.15.0 This sniff has been moved to the `Security` category.
+ *                    This file remains for now to prevent BC breaks.
  */
-class NonceVerificationSniff extends Sniff {
+class NonceVerificationSniff extends \WordPress\Sniffs\Security\NonceVerificationSniff {
 
 	/**
-	 * Superglobals to notify about when not accompanied by an nonce check.
-	 *
-	 * A value of `true` results in an error. A value of `false` in a warning.
+	 * Keep track of whether the warnings have been thrown to prevent
+	 * the messages being thrown for every token triggering the sniff.
 	 *
-	 * @since 0.12.0
+	 * @since 0.15.0
 	 *
 	 * @var array
 	 */
-	protected $superglobals = array(
-		'$_POST'    => true,
-		'$_FILE'    => true,
-		'$_GET'     => false,
-		'$_REQUEST' => false,
+	private $thrown = array(
+		'DeprecatedSniff'                 => false,
+		'FoundPropertyForDeprecatedSniff' => false,
 	);
 
 	/**
-	 * Superglobals to give an error for when not accompanied by an nonce check.
-	 *
-	 * @since 0.5.0
-	 * @since 0.11.0 Changed visibility from public to protected.
-	 *
-	 * @deprecated 0.12.0 Replaced by $superglobals property.
-	 *
-	 * @var array
-	 */
-	protected $errorForSuperGlobals = array();
-
-	/**
-	 * Superglobals to give a warning for when not accompanied by an nonce check.
-	 *
-	 * If the variable is also in the error list, that takes precedence.
-	 *
-	 * @since 0.5.0
-	 * @since 0.11.0 Changed visibility from public to protected.
-	 *
-	 * @deprecated 0.12.0 Replaced by $superglobals property.
-	 *
-	 * @var array
-	 */
-	protected $warnForSuperGlobals = array();
-
-	/**
-	 * Custom list of functions which verify nonces.
+	 * Don't use.
 	 *
-	 * @since 0.5.0
-	 *
-	 * @var string|string[]
-	 */
-	public $customNonceVerificationFunctions = array();
-
-	/**
-	 * Custom list of functions that sanitize the values passed to them.
-	 *
-	 * @since 0.11.0
-	 *
-	 * @var string|string[]
-	 */
-	public $customSanitizingFunctions = array();
-
-	/**
-	 * Custom sanitizing functions that implicitly unslash the values passed to them.
-	 *
-	 * @since 0.11.0
-	 *
-	 * @var string|string[]
-	 */
-	public $customUnslashingSanitizingFunctions = array();
-
-	/**
-	 * Cache of previously added custom functions.
-	 *
-	 * Prevents having to do the same merges over and over again.
-	 *
-	 * @since 0.5.0
-	 * @since 0.11.0 - Changed from public static to protected non-static.
-	 *               - Changed the format from simple bool to array.
-	 *
-	 * @var array
-	 */
-	protected $addedCustomFunctions = array(
-		'nonce'           => null,
-		'sanitize'        => null,
-		'unslashsanitize' => null,
-	);
-
-	/**
-	 * Returns an array of tokens this test wants to listen for.
-	 *
-	 * @return array
-	 */
-	public function register() {
-
-		return array(
-			T_VARIABLE,
-		);
-	}
-
-	/**
-	 * Processes this test, when one of its tokens is encountered.
+	 * @deprecated 0.15.0
 	 *
 	 * @param int $stackPtr The position of the current token in the stack.
 	 *
-	 * @return void
+	 * @return void|int
 	 */
 	public function process_token( $stackPtr ) {
-
-		$instance = $this->tokens[ $stackPtr ];
-
-		if ( ! isset( $this->superglobals[ $instance['content'] ] ) ) {
-			return;
-		}
-
-		if ( $this->has_whitelist_comment( 'CSRF', $stackPtr ) ) {
-			return;
-		}
-
-		if ( $this->is_assignment( $stackPtr ) ) {
-			return;
-		}
-
-		$this->mergeFunctionLists();
-
-		if ( $this->is_only_sanitized( $stackPtr ) ) {
-			return;
-		}
-
-		if ( $this->has_nonce_check( $stackPtr ) ) {
-			return;
-		}
-
-		// If we're still here, no nonce-verification function was found.
-		$this->addMessage(
-			'Processing form data without nonce verification.',
-			$stackPtr,
-			$this->superglobals[ $instance['content'] ],
-			'NoNonceVerification'
-		);
-
-	} // End process_token().
-
-	/**
-	 * Merge custom functions provided via a custom ruleset with the defaults, if we haven't already.
-	 *
-	 * @since 0.11.0 Split out from the `process()` method.
-	 *
-	 * @return void
-	 */
-	protected function mergeFunctionLists() {
-		if ( $this->customNonceVerificationFunctions !== $this->addedCustomFunctions['nonce'] ) {
-			$this->nonceVerificationFunctions = $this->merge_custom_array(
-				$this->customNonceVerificationFunctions,
-				$this->nonceVerificationFunctions
+		if ( false === $this->thrown['DeprecatedSniff'] ) {
+			$this->phpcsFile->addWarning(
+				'The "WordPress.CSRF.NonceVerification" sniff has been renamed to "WordPress.Security.NonceVerification". Please update your custom ruleset.',
+				0,
+				'DeprecatedSniff'
 			);
 
-			$this->addedCustomFunctions['nonce'] = $this->customNonceVerificationFunctions;
+			$this->thrown['DeprecatedSniff'] = true;
 		}
 
-		if ( $this->customSanitizingFunctions !== $this->addedCustomFunctions['sanitize'] ) {
-			$this->sanitizingFunctions = $this->merge_custom_array(
-				$this->customSanitizingFunctions,
-				$this->sanitizingFunctions
+		if ( ( $this->customNonceVerificationFunctions !== $this->addedCustomFunctions['nonce']
+			|| $this->customSanitizingFunctions !== $this->addedCustomFunctions['sanitize']
+			|| $this->customUnslashingSanitizingFunctions !== $this->addedCustomFunctions['unslashsanitize'] )
+			&& false === $this->thrown['FoundPropertyForDeprecatedSniff']
+		) {
+			$this->phpcsFile->addWarning(
+				'The "WordPress.CSRF.NonceVerification" sniff has been renamed to "WordPress.Security.NonceVerification". Please update your custom ruleset.',
+				0,
+				'FoundPropertyForDeprecatedSniff'
 			);
 
-			$this->addedCustomFunctions['sanitize'] = $this->customSanitizingFunctions;
+			$this->thrown['FoundPropertyForDeprecatedSniff'] = true;
 		}
 
-		if ( $this->customUnslashingSanitizingFunctions !== $this->addedCustomFunctions['unslashsanitize'] ) {
-			$this->unslashingSanitizingFunctions = $this->merge_custom_array(
-				$this->customUnslashingSanitizingFunctions,
-				$this->unslashingSanitizingFunctions
-			);
-
-			$this->addedCustomFunctions['unslashsanitize'] = $this->customUnslashingSanitizingFunctions;
-		}
+		return parent::process_token( $stackPtr );
 	}
 
 } // End class.