diff --git a/README.md b/README.md index 7404ced..42a83c5 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,21 @@ Table::hasMany, Table::belongsToMany, Table::hasOne and AssociationCollection::l ### AddBehaviorExistsClassRule This rule check if the target behavior has a valid class when calling to Table::addBehavior and BehaviorRegistry::load. +### DisallowDebugFuncCallRule +This rule disallow use of debug functions (`dd, debug, debug_print_backtrace, debug_zval_dump, pr, print_r, stacktrace, var_dump and var_export`). + +The use of these functions in shipped code is discouraged because they can leak sensitive information or clutter output. + +### DisallowDebugStaticCallRule +This rule disallow use of debug methods. The use of these methods in shipped code is discouraged because they can leak sensitive information or clutter output. + +Methods covered: + + - Cake\Error\Debugger::dump + - Cake\Error\Debugger::printVar + - DebugKit\DebugSql::sql + - DebugKit\DebugSql::sqld + ### DisallowEntityArrayAccessRule This rule disallow array access to entity in favor of object notation, is easier to detect a wrong property and to refactor code. diff --git a/rules.neon b/rules.neon index 4b637f4..f570bd3 100644 --- a/rules.neon +++ b/rules.neon @@ -9,6 +9,8 @@ parameters: getMailerExistsClassRule: true loadComponentExistsClassRule: true controllerMethodMustBeUsedRule: true + disallowDebugFuncCallRule: true + disallowDebugStaticCallRule: true parametersSchema: cakeDC: structure([ addAssociationExistsTableClassRule: anyOf(bool(), arrayOf(bool())) @@ -20,6 +22,8 @@ parametersSchema: getMailerExistsClassRule: anyOf(bool(), arrayOf(bool())) loadComponentExistsClassRule: anyOf(bool(), arrayOf(bool())) controllerMethodMustBeUsedRule: anyOf(bool(), arrayOf(bool())) + disallowDebugFuncCallRule: anyOf(bool(), arrayOf(bool())) + disallowDebugStaticCallRule: anyOf(bool(), arrayOf(bool())) ]) conditionalTags: @@ -43,6 +47,10 @@ conditionalTags: phpstan.rules.rule: %cakeDC.tableGetMatchOptionsTypesRule% CakeDC\PHPStan\Rule\Model\OrmSelectQueryFindMatchOptionsTypesRule: phpstan.rules.rule: %cakeDC.ormSelectQueryFindMatchOptionsTypesRule% + CakeDC\PHPStan\Rule\Debug\DisallowDebugFuncCallRule: + phpstan.rules.rule: %cakeDC.disallowDebugFuncCallRule% + CakeDC\PHPStan\Rule\Debug\DisallowDebugStaticCallRule: + phpstan.rules.rule: %cakeDC.disallowDebugStaticCallRule% services: - @@ -65,3 +73,7 @@ services: class: CakeDC\PHPStan\Rule\Model\TableGetMatchOptionsTypesRule - class: CakeDC\PHPStan\Rule\Model\OrmSelectQueryFindMatchOptionsTypesRule + - + class: CakeDC\PHPStan\Rule\Debug\DisallowDebugFuncCallRule + - + class: CakeDC\PHPStan\Rule\Debug\DisallowDebugStaticCallRule diff --git a/src/Rule/Debug/DisallowDebugFuncCallRule.php b/src/Rule/Debug/DisallowDebugFuncCallRule.php new file mode 100644 index 0000000..9cf4ebb --- /dev/null +++ b/src/Rule/Debug/DisallowDebugFuncCallRule.php @@ -0,0 +1,72 @@ + + */ + private array $disallowedFunctions = [ + 'dd' => self::BASIC, + 'debug' => self::BASIC, + 'debug_print_backtrace' => self::BASIC, + 'debug_zval_dump' => self::BASIC, + 'pr' => self::BASIC, + 'print_r' => self::RETURNABLE, + 'stacktrace' => self::BASIC, + 'var_dump' => self::BASIC, + 'var_export' => self::RETURNABLE, + ]; + + /** + * @inheritDoc + */ + public function getNodeType(): string + { + return FuncCall::class; + } + + /** + * @inheritDoc + */ + public function processNode(Node $node, Scope $scope): array + { + assert($node instanceof FuncCall); + if (!$node->name instanceof Node\Name) { + return []; + } + $usedName = $node->name->name; + $name = strtolower($usedName); + if (!isset($this->disallowedFunctions[$name])) { + return []; + } + if ($this->disallowedFunctions[$name] === self::RETURNABLE) { + $arg = $node->getArgs()[1]->value ?? null; + if ($arg instanceof ConstFetch && $arg->name->name === 'true') { + return []; + } + } + + return [ + RuleErrorBuilder::message(sprintf( + 'Use of debug function "%s" is not allowed. %s', + $usedName, + 'The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + )) + ->identifier('cake.debug.debugFuncCallUse') + ->build(), + ]; + } +} diff --git a/src/Rule/Debug/DisallowDebugStaticCallRule.php b/src/Rule/Debug/DisallowDebugStaticCallRule.php new file mode 100644 index 0000000..8b86867 --- /dev/null +++ b/src/Rule/Debug/DisallowDebugStaticCallRule.php @@ -0,0 +1,65 @@ +> + */ + private array $disallowed = [ + //Methods must be lowercased + Debugger::class => ['dump', 'printvar'], + 'DebugKit\DebugSql' => ['sql', 'sqld'], + ]; + + /** + * @inheritDoc + */ + public function getNodeType(): string + { + return StaticCall::class; + } + + /** + * @inheritDoc + */ + public function processNode(Node $node, Scope $scope): array + { + assert($node instanceof StaticCall); + if (!$node->class instanceof Name || !$node->name instanceof Identifier) { + return []; + } + + $className = (string)$node->class; + if (!isset($this->disallowed[$className])) { + return []; + } + $methodUsed = (string)$node->name; + $method = strtolower($methodUsed); + if (!in_array($method, $this->disallowed[$className], true)) { + return []; + } + + return [ + RuleErrorBuilder::message(sprintf( + 'Use of debug method "%s::%s" is not allowed. %s', + $className, + $methodUsed, + 'The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + )) + ->identifier('cake.debug.debugStaticCallUse') + ->build(), + ]; + } +} diff --git a/tests/TestCase/Rule/Debug/DisallowDebugFuncCallRuleTest.php b/tests/TestCase/Rule/Debug/DisallowDebugFuncCallRuleTest.php new file mode 100644 index 0000000..392f36c --- /dev/null +++ b/tests/TestCase/Rule/Debug/DisallowDebugFuncCallRuleTest.php @@ -0,0 +1,84 @@ +analyse([__DIR__ . '/Fake/FailingDebugUseLogic.php'], [ + [ + 'Use of debug function "debug" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 14, // asserted error line + ], + [ + 'Use of debug function "debug_print_backtrace" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 15, // asserted error line + ], + [ + 'Use of debug function "debug_zval_dump" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 16, // asserted error line + ], + [ + 'Use of debug function "print_r" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 19, // asserted error line + ], + [ + 'Use of debug function "print_r" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 20, // asserted error line + ], + [ + 'Use of debug function "var_dump" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 21, // asserted error line + ], + [ + 'Use of debug function "var_export" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 23, // asserted error line + ], + [ + 'Use of debug function "var_export" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 24, // asserted error line + ], + [ + 'Use of debug function "stackTrace" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 25, // asserted error line + ], + [ + 'Use of debug function "pr" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 26, // asserted error line + ], + [ + 'Use of debug function "dd" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 28, // asserted error line + ], + ]); + } + + /** + * @inheritDoc + */ + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/../../../../extension.neon']; + } +} diff --git a/tests/TestCase/Rule/Debug/DisallowDebugStaticCallRuleTest.php b/tests/TestCase/Rule/Debug/DisallowDebugStaticCallRuleTest.php new file mode 100644 index 0000000..b50551b --- /dev/null +++ b/tests/TestCase/Rule/Debug/DisallowDebugStaticCallRuleTest.php @@ -0,0 +1,55 @@ +analyse([__DIR__ . '/Fake/FailingDebugStaticUseLogic.php'], [ + [ + 'Use of debug method "Cake\Error\Debugger::dump" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 16, // asserted error line + ], + [ + 'Use of debug method "Cake\Error\Debugger::printVar" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 17, // asserted error line + ], + [ + 'Use of debug method "DebugKit\DebugSql::sql" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 18, // asserted error line + ], + [ + 'Use of debug method "DebugKit\DebugSql::sqld" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.', + 19, // asserted error line + ], + ]); + } + + /** + * @inheritDoc + */ + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/../../../../extension.neon']; + } +} diff --git a/tests/TestCase/Rule/Debug/Fake/FailingDebugStaticUseLogic.php b/tests/TestCase/Rule/Debug/Fake/FailingDebugStaticUseLogic.php new file mode 100644 index 0000000..d4da46e --- /dev/null +++ b/tests/TestCase/Rule/Debug/Fake/FailingDebugStaticUseLogic.php @@ -0,0 +1,21 @@ + 1, 'b' => 2, 'c' => 3]); + Debugger::printVar(['y' => 10, 'x' => 20, 'z' => 30]); + DebugSql::sql(); + DebugSql::sqld(); + } +} diff --git a/tests/TestCase/Rule/Debug/Fake/FailingDebugUseLogic.php b/tests/TestCase/Rule/Debug/Fake/FailingDebugUseLogic.php new file mode 100644 index 0000000..750b096 --- /dev/null +++ b/tests/TestCase/Rule/Debug/Fake/FailingDebugUseLogic.php @@ -0,0 +1,30 @@ + 1, 'b' => 2, 'c' => 3]);//Error + var_export($list, true);//No error, text is returned + var_export($list);//Error + var_export($list, false);//Error + stackTrace(); + pr(['b' => 2, 'c' => 3]); + //Last + dd(['a' => 1, 'b' => 2, 'c' => 3]); + } +}