Skip to content

Commit 75baabb

Browse files
authored
Merge pull request #59 from CakeDC/feature/disallow-debug-functions
Feature/disallow debug functions
2 parents 0f025af + 7f04343 commit 75baabb

File tree

8 files changed

+354
-0
lines changed

8 files changed

+354
-0
lines changed

README.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,21 @@ Table::hasMany, Table::belongsToMany, Table::hasOne and AssociationCollection::l
133133
### AddBehaviorExistsClassRule
134134
This rule check if the target behavior has a valid class when calling to Table::addBehavior and BehaviorRegistry::load.
135135

136+
### DisallowDebugFuncCallRule
137+
This rule disallow use of debug functions (`dd, debug, debug_print_backtrace, debug_zval_dump, pr, print_r, stacktrace, var_dump and var_export`).
138+
139+
The use of these functions in shipped code is discouraged because they can leak sensitive information or clutter output.
140+
141+
### DisallowDebugStaticCallRule
142+
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.
143+
144+
Methods covered:
145+
146+
- Cake\Error\Debugger::dump
147+
- Cake\Error\Debugger::printVar
148+
- DebugKit\DebugSql::sql
149+
- DebugKit\DebugSql::sqld
150+
136151
### DisallowEntityArrayAccessRule
137152
This rule disallow array access to entity in favor of object notation, is easier to detect a wrong property and to refactor code.
138153

rules.neon

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ parameters:
99
getMailerExistsClassRule: true
1010
loadComponentExistsClassRule: true
1111
controllerMethodMustBeUsedRule: true
12+
disallowDebugFuncCallRule: true
13+
disallowDebugStaticCallRule: true
1214
parametersSchema:
1315
cakeDC: structure([
1416
addAssociationExistsTableClassRule: anyOf(bool(), arrayOf(bool()))
@@ -20,6 +22,8 @@ parametersSchema:
2022
getMailerExistsClassRule: anyOf(bool(), arrayOf(bool()))
2123
loadComponentExistsClassRule: anyOf(bool(), arrayOf(bool()))
2224
controllerMethodMustBeUsedRule: anyOf(bool(), arrayOf(bool()))
25+
disallowDebugFuncCallRule: anyOf(bool(), arrayOf(bool()))
26+
disallowDebugStaticCallRule: anyOf(bool(), arrayOf(bool()))
2327
])
2428

2529
conditionalTags:
@@ -43,6 +47,10 @@ conditionalTags:
4347
phpstan.rules.rule: %cakeDC.tableGetMatchOptionsTypesRule%
4448
CakeDC\PHPStan\Rule\Model\OrmSelectQueryFindMatchOptionsTypesRule:
4549
phpstan.rules.rule: %cakeDC.ormSelectQueryFindMatchOptionsTypesRule%
50+
CakeDC\PHPStan\Rule\Debug\DisallowDebugFuncCallRule:
51+
phpstan.rules.rule: %cakeDC.disallowDebugFuncCallRule%
52+
CakeDC\PHPStan\Rule\Debug\DisallowDebugStaticCallRule:
53+
phpstan.rules.rule: %cakeDC.disallowDebugStaticCallRule%
4654

4755
services:
4856
-
@@ -65,3 +73,7 @@ services:
6573
class: CakeDC\PHPStan\Rule\Model\TableGetMatchOptionsTypesRule
6674
-
6775
class: CakeDC\PHPStan\Rule\Model\OrmSelectQueryFindMatchOptionsTypesRule
76+
-
77+
class: CakeDC\PHPStan\Rule\Debug\DisallowDebugFuncCallRule
78+
-
79+
class: CakeDC\PHPStan\Rule\Debug\DisallowDebugStaticCallRule
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace CakeDC\PHPStan\Rule\Debug;
5+
6+
use PhpParser\Node;
7+
use PhpParser\Node\Expr\ConstFetch;
8+
use PhpParser\Node\Expr\FuncCall;
9+
use PHPStan\Analyser\Scope;
10+
use PHPStan\Rules\Rule;
11+
use PHPStan\Rules\RuleErrorBuilder;
12+
13+
class DisallowDebugFuncCallRule implements Rule
14+
{
15+
protected const BASIC = 'basic';
16+
protected const RETURNABLE = 'returnable';
17+
18+
/**
19+
* @var array<string, string>
20+
*/
21+
private array $disallowedFunctions = [
22+
'dd' => self::BASIC,
23+
'debug' => self::BASIC,
24+
'debug_print_backtrace' => self::BASIC,
25+
'debug_zval_dump' => self::BASIC,
26+
'pr' => self::BASIC,
27+
'print_r' => self::RETURNABLE,
28+
'stacktrace' => self::BASIC,
29+
'var_dump' => self::BASIC,
30+
'var_export' => self::RETURNABLE,
31+
];
32+
33+
/**
34+
* @inheritDoc
35+
*/
36+
public function getNodeType(): string
37+
{
38+
return FuncCall::class;
39+
}
40+
41+
/**
42+
* @inheritDoc
43+
*/
44+
public function processNode(Node $node, Scope $scope): array
45+
{
46+
assert($node instanceof FuncCall);
47+
if (!$node->name instanceof Node\Name) {
48+
return [];
49+
}
50+
$usedName = $node->name->name;
51+
$name = strtolower($usedName);
52+
if (!isset($this->disallowedFunctions[$name])) {
53+
return [];
54+
}
55+
if ($this->disallowedFunctions[$name] === self::RETURNABLE) {
56+
$arg = $node->getArgs()[1]->value ?? null;
57+
if ($arg instanceof ConstFetch && $arg->name->name === 'true') {
58+
return [];
59+
}
60+
}
61+
62+
return [
63+
RuleErrorBuilder::message(sprintf(
64+
'Use of debug function "%s" is not allowed. %s',
65+
$usedName,
66+
'The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
67+
))
68+
->identifier('cake.debug.debugFuncCallUse')
69+
->build(),
70+
];
71+
}
72+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace CakeDC\PHPStan\Rule\Debug;
5+
6+
use Cake\Error\Debugger;
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\StaticCall;
9+
use PhpParser\Node\Identifier;
10+
use PhpParser\Node\Name;
11+
use PHPStan\Analyser\Scope;
12+
use PHPStan\Rules\Rule;
13+
use PHPStan\Rules\RuleErrorBuilder;
14+
15+
class DisallowDebugStaticCallRule implements Rule
16+
{
17+
/**
18+
* @var array<string, array<int, string>>
19+
*/
20+
private array $disallowed = [
21+
//Methods must be lowercased
22+
Debugger::class => ['dump', 'printvar'],
23+
'DebugKit\DebugSql' => ['sql', 'sqld'],
24+
];
25+
26+
/**
27+
* @inheritDoc
28+
*/
29+
public function getNodeType(): string
30+
{
31+
return StaticCall::class;
32+
}
33+
34+
/**
35+
* @inheritDoc
36+
*/
37+
public function processNode(Node $node, Scope $scope): array
38+
{
39+
assert($node instanceof StaticCall);
40+
if (!$node->class instanceof Name || !$node->name instanceof Identifier) {
41+
return [];
42+
}
43+
44+
$className = (string)$node->class;
45+
if (!isset($this->disallowed[$className])) {
46+
return [];
47+
}
48+
$methodUsed = (string)$node->name;
49+
$method = strtolower($methodUsed);
50+
if (!in_array($method, $this->disallowed[$className], true)) {
51+
return [];
52+
}
53+
54+
return [
55+
RuleErrorBuilder::message(sprintf(
56+
'Use of debug method "%s::%s" is not allowed. %s',
57+
$className,
58+
$methodUsed,
59+
'The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
60+
))
61+
->identifier('cake.debug.debugStaticCallUse')
62+
->build(),
63+
];
64+
}
65+
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace CakeDC\PHPStan\Test\TestCase\Rule\Debug;
5+
6+
use CakeDC\PHPStan\Rule\Debug\DisallowDebugFuncCallRule;
7+
use PHPStan\Rules\Rule;
8+
use PHPStan\Testing\RuleTestCase;
9+
10+
class DisallowDebugFuncCallRuleTest extends RuleTestCase
11+
{
12+
/**
13+
* @return \PHPStan\Rules\Rule
14+
*/
15+
protected function getRule(): Rule
16+
{
17+
// getRule() method needs to return an instance of the tested rule
18+
return new DisallowDebugFuncCallRule();
19+
}
20+
21+
/**
22+
* @return void
23+
*/
24+
public function testRule(): void
25+
{
26+
// first argument: path to the example file that contains some errors that should be reported by MyRule
27+
// second argument: an array of expected errors,
28+
// each error consists of the asserted error message, and the asserted error file line
29+
$this->analyse([__DIR__ . '/Fake/FailingDebugUseLogic.php'], [
30+
[
31+
'Use of debug function "debug" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
32+
14, // asserted error line
33+
],
34+
[
35+
'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.',
36+
15, // asserted error line
37+
],
38+
[
39+
'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.',
40+
16, // asserted error line
41+
],
42+
[
43+
'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.',
44+
19, // asserted error line
45+
],
46+
[
47+
'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.',
48+
20, // asserted error line
49+
],
50+
[
51+
'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.',
52+
21, // asserted error line
53+
],
54+
[
55+
'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.',
56+
23, // asserted error line
57+
],
58+
[
59+
'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.',
60+
24, // asserted error line
61+
],
62+
[
63+
'Use of debug function "stackTrace" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
64+
25, // asserted error line
65+
],
66+
[
67+
'Use of debug function "pr" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
68+
26, // asserted error line
69+
],
70+
[
71+
'Use of debug function "dd" is not allowed. The use in shipped code is discouraged because they can leak sensitive information or clutter output.',
72+
28, // asserted error line
73+
],
74+
]);
75+
}
76+
77+
/**
78+
* @inheritDoc
79+
*/
80+
public static function getAdditionalConfigFiles(): array
81+
{
82+
return [__DIR__ . '/../../../../extension.neon'];
83+
}
84+
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace CakeDC\PHPStan\Test\TestCase\Rule\Debug;
5+
6+
use CakeDC\PHPStan\Rule\Debug\DisallowDebugStaticCallRule;
7+
use PHPStan\Rules\Rule;
8+
use PHPStan\Testing\RuleTestCase;
9+
10+
class DisallowDebugStaticCallRuleTest extends RuleTestCase
11+
{
12+
/**
13+
* @return \PHPStan\Rules\Rule
14+
*/
15+
protected function getRule(): Rule
16+
{
17+
return new DisallowDebugStaticCallRule();
18+
}
19+
20+
/**
21+
* @return void
22+
*/
23+
public function testRule(): void
24+
{
25+
// first argument: path to the example file that contains some errors that should be reported by MyRule
26+
// second argument: an array of expected errors,
27+
// each error consists of the asserted error message, and the asserted error file line
28+
$this->analyse([__DIR__ . '/Fake/FailingDebugStaticUseLogic.php'], [
29+
[
30+
'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.',
31+
16, // asserted error line
32+
],
33+
[
34+
'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.',
35+
17, // asserted error line
36+
],
37+
[
38+
'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.',
39+
18, // asserted error line
40+
],
41+
[
42+
'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.',
43+
19, // asserted error line
44+
],
45+
]);
46+
}
47+
48+
/**
49+
* @inheritDoc
50+
*/
51+
public static function getAdditionalConfigFiles(): array
52+
{
53+
return [__DIR__ . '/../../../../extension.neon'];
54+
}
55+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace CakeDC\PHPStan\Test\TestCase\Rule\Debug\Fake;
5+
6+
use Cake\Error\Debugger;
7+
use DebugKit\DebugSql;
8+
9+
class FailingDebugStaticUseLogic
10+
{
11+
/**
12+
* @return void
13+
*/
14+
public function execute(): void
15+
{
16+
Debugger::dump(['a' => 1, 'b' => 2, 'c' => 3]);
17+
Debugger::printVar(['y' => 10, 'x' => 20, 'z' => 30]);
18+
DebugSql::sql();
19+
DebugSql::sqld();
20+
}
21+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace CakeDC\PHPStan\Test\TestCase\Rule\Debug\Fake;
5+
6+
class FailingDebugUseLogic
7+
{
8+
/**
9+
* @return void
10+
*/
11+
public function execute(): void
12+
{
13+
$list = [1, 2, 3, 4];
14+
debug($list);//Error
15+
debug_print_backtrace();//Error
16+
debug_zval_dump('Hello World');//Error
17+
ksort($list);//Not a debug should not fail
18+
print_r(['Hello World!'], true);//No error, text is returned
19+
print_r(['Hello World!']);//Error
20+
print_r(['Hello World!'], false);//Error
21+
var_dump(['a' => 1, 'b' => 2, 'c' => 3]);//Error
22+
var_export($list, true);//No error, text is returned
23+
var_export($list);//Error
24+
var_export($list, false);//Error
25+
stackTrace();
26+
pr(['b' => 2, 'c' => 3]);
27+
//Last
28+
dd(['a' => 1, 'b' => 2, 'c' => 3]);
29+
}
30+
}

0 commit comments

Comments
 (0)