Skip to content

Commit cd8929a

Browse files
spazespaze
committed
Allow param conditions with allowInInstanceOf and disallowInInstanceOf
Combining instanceof-based directives with `allowParamsInAllowed` or `allowExceptParamsInAllowed` makes it possible to narrow which calls within a class hierarchy are reported, without introducing new config keys.
1 parent 725b0fa commit cd8929a

5 files changed

Lines changed: 231 additions & 6 deletions

File tree

‎docs/allow-in-instance-of.md‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,44 @@ parameters:
4040
- 'MyInterface'
4141
```
4242

43+
### Combining with parameter conditions
44+
45+
Both `allowInInstanceOf` and `disallowInInstanceOf` can be combined with `allowParamsInAllowed` and `allowExceptParamsInAllowed` to add parameter-based conditions within the class hierarchy scope. See [allow with parameters](allow-with-parameters.md) for details on parameter configuration.
46+
47+
For example, to allow `dispatch()` in classes implementing `HandlerInterface` but only when the first argument is of type `SafeEvent`:
48+
49+
```neon
50+
parameters:
51+
disallowedFunctionCalls:
52+
-
53+
function: 'dispatch()'
54+
allowInInstanceOf:
55+
- 'App\Handlers\HandlerInterface'
56+
allowParamsInAllowed:
57+
-
58+
position: 1
59+
name: 'event'
60+
typeString: 'App\Events\SafeEvent'
61+
```
62+
63+
To disallow `dispatch()` in `HandlerInterface` classes only when the first argument is of type `DangerousEvent`, and allow it with any other argument:
64+
65+
```neon
66+
parameters:
67+
disallowedFunctionCalls:
68+
-
69+
function: 'dispatch()'
70+
disallowInInstanceOf:
71+
- 'App\Handlers\HandlerInterface'
72+
allowExceptParamsInAllowed:
73+
-
74+
position: 1
75+
name: 'event'
76+
typeString: 'App\Events\DangerousEvent'
77+
```
78+
79+
The `allowExceptParamsInAllowed` counterpart works with `allowInInstanceOf` too (allowed in hierarchy except when the parameter matches), and `allowParamsInAllowed` works with `disallowInInstanceOf` (disallowed in hierarchy unless the parameter matches).
80+
4381
### Allow in `use` imports
4482
The `allowInInstanceOf` configuration above will also report an error on the line with the import, if present:
4583
```php

‎docs/allow-with-parameters.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ parameters:
3333
value: true
3434
```
3535

36-
When using `allowParamsInAllowed`, calls will be allowed only when they are in one of the `allowIn` paths, and are called with all parameters listed in `allowParamsInAllowed`.
36+
When using `allowParamsInAllowed`, calls will be allowed only when they are in one of the `allowIn` paths (or in a class hierarchy matched by `allowInInstanceOf`), and are called with all parameters listed in `allowParamsInAllowed`.
3737
With `allowParamsAnywhere`, calls are allowed when called with all parameters listed no matter in which file. In the example above, the `log()` method will be disallowed unless called as:
3838
- `log(..., true)` (or `log(..., alert: true)`) anywhere
3939
- `log('foo', true)` (or `log(message: 'foo', alert: true)`) in `another/file.php` or `optional/path/to/log.tests.php`
@@ -115,7 +115,7 @@ parameters:
115115
```
116116
This configuration will disallow calls like `waldo('foo', 'bar')` or `waldo('*', '*')`, but `waldo('foo')` or `waldo()` will be still allowed.
117117

118-
It's also possible to disallow functions and methods previously allowed by path (using `allowIn`) or by function/method name (`allowInMethods`) when they're called with specified parameters, and allow when called with any other parameter. This is done using the `allowExceptParamsInAllowed` config option.
118+
It's also possible to disallow functions and methods previously allowed by path (using `allowIn`), by function/method name (`allowInMethods`), or by class hierarchy (`allowInInstanceOf`) when they're called with specified parameters, and allow when called with any other parameter. This is done using the `allowExceptParamsInAllowed` config option.
119119

120120
Take this example configuration:
121121

‎src/Allowed/Allowed.php‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,16 @@ public function isAllowed(?Node $node, Scope $scope, ?array $args, Disallowed $d
9898
}
9999
}
100100
if ($disallowed->getAllowInInstanceOf()) {
101-
return $this->isInstanceOf($scope, $disallowed->getAllowInInstanceOf());
101+
if (!$this->isInstanceOf($scope, $disallowed->getAllowInInstanceOf())) {
102+
return false;
103+
}
104+
return !$hasParams || $this->hasAllowedParamsInAllowed($scope, $args, $disallowed);
102105
}
103106
if ($disallowed->getAllowExceptInInstanceOf()) {
104-
return !$this->isInstanceOf($scope, $disallowed->getAllowExceptInInstanceOf());
107+
if (!$this->isInstanceOf($scope, $disallowed->getAllowExceptInInstanceOf())) {
108+
return true;
109+
}
110+
return $hasParams && $this->hasAllowedParamsInAllowed($scope, $args, $disallowed, false);
105111
}
106112
if ($hasParams && $disallowed->getAllowExceptParams()) {
107113
return $this->hasAllowedParams($scope, $args, $disallowed->getAllowExceptParams(), false);
@@ -223,17 +229,18 @@ private function hasAllowedParams(Scope $scope, ?array $args, array $allowConfig
223229
* @param Scope $scope
224230
* @param array<Arg>|null $args
225231
* @param DisallowedWithParams $disallowed
232+
* @param bool $defaultResult
226233
* @return bool
227234
*/
228-
private function hasAllowedParamsInAllowed(Scope $scope, ?array $args, DisallowedWithParams $disallowed): bool
235+
private function hasAllowedParamsInAllowed(Scope $scope, ?array $args, DisallowedWithParams $disallowed, bool $defaultResult = true): bool
229236
{
230237
if ($disallowed->getAllowExceptParamsInAllowed()) {
231238
return $this->hasAllowedParams($scope, $args, $disallowed->getAllowExceptParamsInAllowed(), false);
232239
}
233240
if ($disallowed->getAllowParamsInAllowed()) {
234241
return $this->hasAllowedParams($scope, $args, $disallowed->getAllowParamsInAllowed(), true);
235242
}
236-
return true;
243+
return $defaultResult;
237244
}
238245

239246

‎tests/Calls/FunctionCallsTest.php‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,46 @@ protected function getRule(): Rule
225225
Stringable::class,
226226
],
227227
],
228+
// test allowInInstanceOf + allowExceptParamsInAllowed: allowed in hierarchy except when param is 'forbidden'
229+
[
230+
'function' => 'str_starts_with()',
231+
'allowInInstanceOf' => [
232+
'Waldo\Foo\BarBase',
233+
],
234+
'allowExceptParamsInAllowed' => [
235+
2 => 'forbidden',
236+
],
237+
],
238+
// test disallowInInstanceOf + allowExceptParamsInAllowed: disallowed in hierarchy only when param is 'forbidden'
239+
[
240+
'function' => 'str_ends_with()',
241+
'disallowInInstanceOf' => [
242+
'Waldo\Foo\BarBase',
243+
],
244+
'allowExceptParamsInAllowed' => [
245+
2 => 'forbidden',
246+
],
247+
],
248+
// test disallowInInstanceOf + allowParamsInAllowed: disallowed in hierarchy unless param is 'allowed_param'
249+
[
250+
'function' => 'str_contains()',
251+
'disallowInInstanceOf' => [
252+
'Waldo\Foo\BarBase',
253+
],
254+
'allowParamsInAllowed' => [
255+
2 => 'allowed_param',
256+
],
257+
],
258+
// test allowInInstanceOf + allowParamsInAllowed: allowed in hierarchy only when param is 'allowed_chars'
259+
[
260+
'function' => 'ltrim()',
261+
'allowInInstanceOf' => [
262+
'Waldo\Foo\BarBase',
263+
],
264+
'allowParamsInAllowed' => [
265+
2 => 'allowed_chars',
266+
],
267+
],
228268
// test allowed instances with wildcards, intentionally wrong case to test FNM_CASEFOLD
229269
[
230270
'function' => 'str_pad()',
@@ -467,6 +507,61 @@ public function testAllowInInstanceOfWildcard(): void
467507
}
468508

469509

510+
public function testInstanceOfWithParams(): void
511+
{
512+
$this->analyse([__DIR__ . '/../src/BarInstanceOfWithParams.php'], [
513+
[
514+
'Calling str_starts_with() is forbidden.',
515+
11,
516+
],
517+
[
518+
'Calling str_ends_with() is forbidden.',
519+
13,
520+
],
521+
[
522+
'Calling str_contains() is forbidden.',
523+
16,
524+
],
525+
[
526+
'Calling str_starts_with() is forbidden.',
527+
26,
528+
],
529+
[
530+
'Calling str_ends_with() is forbidden.',
531+
28,
532+
],
533+
[
534+
'Calling str_contains() is forbidden.',
535+
31,
536+
],
537+
[
538+
'Calling str_starts_with() is forbidden.',
539+
42,
540+
],
541+
[
542+
'Calling str_starts_with() is forbidden.',
543+
43,
544+
],
545+
[
546+
'Calling ltrim() is forbidden.',
547+
59,
548+
],
549+
[
550+
'Calling ltrim() is forbidden.',
551+
70,
552+
],
553+
[
554+
'Calling ltrim() is forbidden.',
555+
71,
556+
],
557+
[
558+
'Calling ltrim() is forbidden.',
559+
82,
560+
],
561+
]);
562+
}
563+
564+
470565
public static function getAdditionalConfigFiles(): array
471566
{
472567
return [

‎tests/src/BarInstanceOfWithParams.php‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
namespace Waldo\Foo;
5+
6+
class BarBase
7+
{
8+
9+
public function inHierarchy(): void
10+
{
11+
str_starts_with('foo', 'forbidden');
12+
str_starts_with('foo', 'allowed');
13+
str_ends_with('foo', 'forbidden');
14+
str_ends_with('foo', 'allowed');
15+
str_contains('foo', 'allowed_param');
16+
str_contains('foo', 'other');
17+
}
18+
19+
}
20+
21+
class BarBaseChild extends BarBase
22+
{
23+
24+
public function inHierarchy(): void
25+
{
26+
str_starts_with('foo', 'forbidden');
27+
str_starts_with('foo', 'allowed');
28+
str_ends_with('foo', 'forbidden');
29+
str_ends_with('foo', 'allowed');
30+
str_contains('foo', 'allowed_param');
31+
str_contains('foo', 'other');
32+
}
33+
34+
}
35+
36+
// outside the hierarchy: str_ends_with and str_contains calls are allowed regardless of params; str_starts_with is still disallowed (allowInInstanceOf)
37+
class BarOutside
38+
{
39+
40+
public function outsideHierarchy(): void
41+
{
42+
str_starts_with('foo', 'forbidden');
43+
str_starts_with('foo', 'allowed');
44+
str_ends_with('foo', 'forbidden');
45+
str_ends_with('foo', 'allowed');
46+
str_contains('foo', 'allowed_param');
47+
str_contains('foo', 'other');
48+
}
49+
50+
}
51+
52+
// test allowInInstanceOf + allowParamsInAllowed: allowed in hierarchy only when param is 'allowed_chars'
53+
class BarBaseForAllowParams extends BarBase
54+
{
55+
56+
public function inHierarchy(): void
57+
{
58+
ltrim('foo', 'allowed_chars');
59+
ltrim('foo', 'other');
60+
}
61+
62+
}
63+
64+
// outside the hierarchy: all ltrim calls are disallowed (allowInInstanceOf)
65+
class BarOutsideForAllowParams
66+
{
67+
68+
public function outsideHierarchy(): void
69+
{
70+
ltrim('foo', 'allowed_chars');
71+
ltrim('foo', 'other');
72+
}
73+
74+
}
75+
76+
class BarBaseChildForAllowParams extends BarBaseForAllowParams
77+
{
78+
79+
public function inHierarchy(): void
80+
{
81+
ltrim('foo', 'allowed_chars');
82+
ltrim('foo', 'other');
83+
}
84+
85+
}

0 commit comments

Comments
 (0)