Skip to content

Commit

Permalink
Add check for setting sanitization
Browse files Browse the repository at this point in the history
  • Loading branch information
ernilambar committed Jan 10, 2025
1 parent c7348a9 commit e3d3fb1
Show file tree
Hide file tree
Showing 8 changed files with 364 additions and 0 deletions.
113 changes: 113 additions & 0 deletions includes/Checker/Checks/Plugin_Repo/Setting_Sanitization_Check.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
<?php
/**
* Class Setting_Sanitization_Check.
*
* @package plugin-check
*/

namespace WordPress\Plugin_Check\Checker\Checks\Plugin_Repo;

use WordPress\Plugin_Check\Checker\Check_Categories;
use WordPress\Plugin_Check\Checker\Check_Result;
use WordPress\Plugin_Check\Checker\Checks\Abstract_PHP_CodeSniffer_Check;
use WordPress\Plugin_Check\Traits\Amend_Check_Result;
use WordPress\Plugin_Check\Traits\Stable_Check;

/**
* Check to detect sanitization in register_setting().
*
* @since 1.4.0
*/
class Setting_Sanitization_Check extends Abstract_PHP_CodeSniffer_Check {

use Amend_Check_Result;
use Stable_Check;

/**
* Gets the categories for the check.
*
* Every check must have at least one category.
*
* @since 1.4.0
*
* @return array The categories for the check.
*/
public function get_categories() {
return array( Check_Categories::CATEGORY_PLUGIN_REPO );
}

/**
* Returns an associative array of arguments to pass to PHPCS.
*
* @since 1.4.0
*
* @param Check_Result $result The check result to amend, including the plugin context to check.
* @return array An associative array of PHPCS CLI arguments.
*/
protected function get_args( Check_Result $result ) {
return array(
'extensions' => 'php',
'standard' => 'PluginCheck',
'sniffs' => 'PluginCheck.CodeAnalysis.SettingSanitization',
);
}

/**
* Gets the description for the check.
*
* Every check must have a short description explaining what the check does.
*
* @since 1.4.0
*
* @return string Description.
*/
public function get_description(): string {
return __( 'Ensures sanitization in register_setting().', 'plugin-check' );

Check warning on line 65 in includes/Checker/Checks/Plugin_Repo/Setting_Sanitization_Check.php

View check run for this annotation

Codecov / codecov/patch

includes/Checker/Checks/Plugin_Repo/Setting_Sanitization_Check.php#L64-L65

Added lines #L64 - L65 were not covered by tests
}

/**
* Gets the documentation URL for the check.
*
* Every check must have a URL with further information about the check.
*
* @since 1.4.0
*
* @return string The documentation URL.
*/
public function get_documentation_url(): string {
return __( 'https://developer.wordpress.org/reference/functions/register_setting/', 'plugin-check' );

Check warning on line 78 in includes/Checker/Checks/Plugin_Repo/Setting_Sanitization_Check.php

View check run for this annotation

Codecov / codecov/patch

includes/Checker/Checks/Plugin_Repo/Setting_Sanitization_Check.php#L77-L78

Added lines #L77 - L78 were not covered by tests
}

/**
* Amends the given result with a message for the specified file, including error information.
*
* @since 1.4.0
*
* @param Check_Result $result The check result to amend, including the plugin context to check.
* @param bool $error Whether it is an error or notice.
* @param string $message Error message.
* @param string $code Error code.
* @param string $file Absolute path to the file where the issue was found.
* @param int $line The line on which the message occurred. Default is 0 (unknown line).
* @param int $column The column on which the message occurred. Default is 0 (unknown column).
* @param string $docs URL for further information about the message.
* @param int $severity Severity level. Default is 5.
*/
protected function add_result_message_for_file( Check_Result $result, $error, $message, $code, $file, $line = 0, $column = 0, string $docs = '', $severity = 5 ) {
// Update severity.
switch ( $code ) {
case 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingMissing':
case 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingInvalid':
$severity = 7;
break;

default:
break;
}

// Add docs link.
$docs = __( 'https://developer.wordpress.org/reference/functions/register_setting/', 'plugin-check' );

parent::add_result_message_for_file( $result, $error, $message, $code, $file, $line, $column, $docs, $severity );
}
}
1 change: 1 addition & 0 deletions includes/Checker/Default_Check_Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ private function register_default_checks() {
'non_blocking_scripts' => new Checks\Performance\Non_Blocking_Scripts_Check(),
'offloading_files' => new Checks\Plugin_Repo\Offloading_Files_Check(),
'image_functions' => new Checks\Performance\Image_Functions_Check(),
'setting_sanitization' => new Checks\Plugin_Repo\Setting_Sanitization_Check(),
)
);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?php
/**
* SettingSanitizationSniff
*
* Based on code from {@link https://github.com/WordPress/WordPress-Coding-Standards}
* which is licensed under {@link https://opensource.org/licenses/MIT}.
*
* @package PluginCheck
*/

namespace PluginCheckCS\PluginCheck\Sniffs\CodeAnalysis;

use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\MessageHelper;
use PHPCSUtils\Utils\PassedParameters;
use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\AbstractFunctionParameterSniff;

/**
* Detect sanitization in register_setting().
*
* @since 1.4.0
*/
final class SettingSanitizationSniff extends AbstractFunctionParameterSniff {

/**
* The group name for this group of functions.
*
* @since 1.4.0
*
* @var string
*/
protected $group_name = 'register_setting';

/**
* List of functions to examine.
*
* @since 1.4.0
*
* @var array<string, true> Key is function name, value irrelevant.
*/

protected $target_functions = array(
'register_setting' => true,
);

/**
* Process the parameters of a matched function.
*
* @since 1.4.0
*
* @param int $stackPtr The position of the current token in the stack.
* @param string $group_name The name of the group which was matched.
* @param string $matched_content The token content (function name) which was matched in lowercase.
* @param array $parameters Array with information about the parameters.
*
* @return void
*/
public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
$third_param = PassedParameters::getParameterFromStack( $parameters, 3, 'args' );
$error_code = MessageHelper::stringToErrorCode( $matched_content, true );

if ( false === $third_param ) {
$this->phpcsFile->addError(
'Sanitization missing for %s().',
$stackPtr,
$error_code . 'Missing',
array( $matched_content )
);

return;
}

$content = TextStrings::stripQuotes( $third_param['clean'] );

if ( is_numeric( $content ) || in_array( strtolower( $content ), array( 'true', 'false', 'null' ), true ) ) {
$this->phpcsFile->addError(
'Invalid sanitization provided for %s().',
$stackPtr,
$error_code . 'Invalid',
array( $matched_content )
);

return;
}

$next_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, $third_param['start'], ( $third_param['end'] + 1 ), true );

if ( false !== $next_token ) {
$next_type = $this->tokens[ $next_token ]['type'];

if ( in_array( $next_type, array( 'T_ARRAY', 'T_OPEN_SHORT_ARRAY', 'T_VARIABLE', 'T_CALLABLE' ), true ) ) {
$this->phpcsFile->addWarning(
'Dynamic argument passed in third parameter of %s(). Please ensure proper sanitization.',
$stackPtr,
$error_code . 'Dynamic',
array( $matched_content )
);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php
register_setting( 'my_options_group', 'my_option_name' ); // Error.
register_setting( 'my_options_group', 'my_option_name', 10 ); // Error.
register_setting( 'my_options_group', 'my_option_name', false ); // Error.
register_setting( 'my_options_group', 'my_option_name', 'absint' ); // Good.
register_setting('my_options_group','my_option_name', 'sanitize_text_field' ); // Good.
register_setting('my_options_group', 'my_option_name', [ 'sanitize_callback' => 'sanitize_text_field']); // Warning.
$args = array( 'sanitize_callback' => 'absint' );
register_setting( 'my_options_group', 'my_option_name', $args ); // Warning.

class TestClass {
public function register_setting() {
}
}

$obj = new TestClass();
$obj->register_setting(); // Good.

register_setting('my_options_group', 'my_option_name',array(&$this,'validate')); // Warning.

register_setting(
'my_options_group',
$setting_key,
array(
'sanitize_callback' => $setting['sanitize_callback']
)
); // Warning.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?php
/**
* Unit tests for SettingSanitizationSniff.
*
* @package PluginCheck
*/

namespace PluginCheckCS\PluginCheck\Tests\CodeAnalysis;

use PluginCheckCS\PluginCheck\Sniffs\CodeAnalysis\SettingSanitizationSniff;
use PluginCheckCS\PluginCheck\Tests\AbstractSniffUnitTest;
use PHP_CodeSniffer\Sniffs\Sniff;

/**
* Unit tests for SettingSanitizationSniff.
*/
final class SettingSanitizationUnitTest extends AbstractSniffUnitTest {

/**
* Returns the lines where errors should occur.
*
* @return array <int line number> => <int number of errors>
*/
public function getErrorList() {
return array(
2 => 1,
3 => 1,
4 => 1,
);
}

/**
* Returns the lines where warnings should occur.
*
* @return array <int line number> => <int number of warnings>
*/
public function getWarningList() {
return array(
7 => 1,
9 => 1,
19 => 1,
21 => 1,
);
}

/**
* Returns the fully qualified class name (FQCN) of the sniff.
*
* @return string The fully qualified class name of the sniff.
*/
protected function get_sniff_fqcn() {
return SettingSanitizationSniff::class;
}

/**
* Sets the parameters for the sniff.
*
* @throws \RuntimeException If unable to set the ruleset parameters required for the test.
*
* @param Sniff $sniff The sniff being tested.
*/
public function set_sniff_parameters( Sniff $sniff ) {
}
}
1 change: 1 addition & 0 deletions phpcs-sniffs/PluginCheck/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
<rule ref="PluginCheck.CodeAnalysis.Localhost" />
<rule ref="PluginCheck.CodeAnalysis.Offloading" />
<rule ref="PluginCheck.CodeAnalysis.RequiredFunctionParameters" />
<rule ref="PluginCheck.CodeAnalysis.SettingSanitization" />

</ruleset>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php
/**
* Plugin Name: Test Plugin Setting Sanitization check with errors
* Plugin URI: https://github.com/WordPress/plugin-check
* Description: Test plugin for the Setting Sanitization check.
* Requires at least: 6.0
* Requires PHP: 5.6
* 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-setting-sanitization-check-with-errors
*
* @package test-plugin-setting-sanitization-check-with-errors
*/

// Settings.
register_setting( 'my_options_group', 'option_1' ); // Error.
register_setting( 'my_options_group', 'option_2', false ); // Error.
register_setting( 'my_options_group', 'option_3', 'absint' ); // Good.
$args = array( 'sanitize_callback' => 'absint' );
register_setting( 'my_options_group', 'option_4', $args ); // Warning.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php
/**
* Tests for the Setting_Sanitization_Check class.
*
* @package plugin-check
*/

use WordPress\Plugin_Check\Checker\Check_Context;
use WordPress\Plugin_Check\Checker\Check_Result;
use WordPress\Plugin_Check\Checker\Checks\Plugin_Repo\Setting_Sanitization_Check;

class Setting_Sanitization_Check_Test extends WP_UnitTestCase {

public function test_run_with_errors() {
$check = new Setting_Sanitization_Check();
$check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-setting-sanitization-check-with-errors/load.php' );
$check_result = new Check_Result( $check_context );

$check->run( $check_result );

$errors = $check_result->get_errors();
$warnings = $check_result->get_warnings();

$this->assertNotEmpty( $errors );
$this->assertNotEmpty( $warnings );
$this->assertArrayHasKey( 'load.php', $errors );
$this->assertArrayHasKey( 'load.php', $warnings );

$this->assertSame( 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingMissing', $errors['load.php'][19][1][0]['code'] );
$this->assertSame( 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingInvalid', $errors['load.php'][20][1][0]['code'] );
$this->assertSame( 'PluginCheck.CodeAnalysis.SettingSanitization.register_settingDynamic', $warnings['load.php'][23][1][0]['code'] );
}
}

0 comments on commit e3d3fb1

Please sign in to comment.