Skip to content

Commit 18f006c

Browse files
spazespaze
committed
Support param conditions in allowExceptIn and allowExceptInMethods
`allowParamsInAllowed` and `allowExceptParamsInAllowed` were silently ignored when combined with `allowExceptIn` or `allowExceptInMethods`. The config looked valid, produced no schema error, and had no effect. `allowExceptInInstanceOf` already supported param conditions; this extends the same behaviour to the path-based (`allowExceptIn`) and call-based (`allowExceptInMethods`, `allowExceptInFunctions`) directives. In the matched location, `allowParamsInAllowed` can allow a call when params match, and `allowExceptParamsInAllowed` can allow a call when params don't match the forbidden values.
1 parent 79ddf33 commit 18f006c

6 files changed

Lines changed: 168 additions & 4 deletions

File tree

‎docs/allow-with-parameters.md‎

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

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`.
36+
When using `allowParamsInAllowed`, the parameter condition is applied to the location matched by the accompanying allow directive — `allowIn`, `allowExceptIn`, `allowInInstanceOf`, `allowExceptInInstanceOf`, `allowInMethods`/`allowInFunctions`, or `allowExceptInMethods`/`allowExceptInFunctions`. For `allowIn`-style directives the call must be in a matching location *and* have the right params to be allowed. For `allowExceptIn`-style directives the call is normally disallowed in the matched location, but the right params can allow it there.
37+
38+
For example, to allow `redirect()` everywhere except in `handleRequest()`, where it remains disallowed unless the first parameter is of type `App\SafeUrl`:
39+
40+
```neon
41+
parameters:
42+
disallowedMethodCalls:
43+
-
44+
method: 'Controller::redirect()'
45+
message: 'use a safe redirect instead'
46+
allowExceptInMethods:
47+
- Controller::handleRequest()
48+
allowParamsInAllowed:
49+
-
50+
position: 1
51+
name: 'url'
52+
typeString: App\SafeUrl
53+
```
54+
3755
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:
3856
- `log(..., true)` (or `log(..., alert: true)`) anywhere
3957
- `log('foo', true)` (or `log(message: 'foo', alert: true)`) in `another/file.php` or `optional/path/to/log.tests.php`
@@ -117,7 +135,7 @@ parameters:
117135
```
118136
This configuration will disallow calls like `waldo('foo', 'bar')` or `waldo('*', '*')`, but `waldo('foo')` or `waldo()` will be still allowed.
119137

120-
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.
138+
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. The same applies in reverse for the `allowExceptIn`-style directives: a call that would otherwise be disallowed in the matched location is allowed unless the parameters match. This is done using the `allowExceptParamsInAllowed` config option.
121139

122140
Take this example configuration:
123141

‎src/Allowed/Allowed.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function isAllowed(?Node $node, Scope $scope, ?array $args, Disallowed $d
6767
if ($disallowed->getAllowExceptInCalls()) {
6868
foreach ($disallowed->getAllowExceptInCalls() as $call) {
6969
if ($this->callMatches($scope, $call)) {
70-
return false;
70+
return $hasParams && $this->hasAllowedParamsInAllowed($scope, $args, $disallowed, false);
7171
}
7272
}
7373
return true;
@@ -80,7 +80,7 @@ public function isAllowed(?Node $node, Scope $scope, ?array $args, Disallowed $d
8080
if ($disallowed->getAllowExceptIn()) {
8181
foreach ($disallowed->getAllowExceptIn() as $allowedExceptPath) {
8282
if ($this->allowedPath->matches($scope, $allowedExceptPath)) {
83-
return false;
83+
return $hasParams && $this->hasAllowedParamsInAllowed($scope, $args, $disallowed, false);
8484
}
8585
}
8686
return true;

‎tests/Calls/FunctionCallsAllowExceptInMethodsWithParamsTest.php‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
namespace Spaze\PHPStan\Rules\Disallowed\Calls;
5+
6+
use PHPStan\Rules\Rule;
7+
use PHPStan\ShouldNotHappenException;
8+
use PHPStan\Testing\RuleTestCase;
9+
use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory;
10+
use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedCallableParameterRuleErrors;
11+
use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedFunctionRuleErrors;
12+
13+
/**
14+
* @extends RuleTestCase<FunctionCalls>
15+
*/
16+
class FunctionCallsAllowExceptInMethodsWithParamsTest extends RuleTestCase
17+
{
18+
19+
/**
20+
* @throws ShouldNotHappenException
21+
*/
22+
protected function getRule(): Rule
23+
{
24+
$container = self::getContainer();
25+
return new FunctionCalls(
26+
$container->getByType(DisallowedFunctionRuleErrors::class),
27+
$container->getByType(DisallowedCallableParameterRuleErrors::class),
28+
$container->getByType(DisallowedCallFactory::class),
29+
[
30+
[
31+
'function' => 'crc32()',
32+
'allowExceptInMethods' => [
33+
'Fiction\Pulp\RoyaleExceptWithParams::methodA()',
34+
],
35+
'allowParamsInAllowed' => [
36+
1 => 'a',
37+
],
38+
],
39+
[
40+
'function' => 'strtolower()',
41+
'allowExceptInMethods' => [
42+
'Fiction\Pulp\RoyaleExceptWithParams::methodA()',
43+
],
44+
'allowExceptParamsInAllowed' => [
45+
1 => 'a',
46+
],
47+
],
48+
]
49+
);
50+
}
51+
52+
53+
public function testRule(): void
54+
{
55+
$this->analyse([__DIR__ . '/../src/RoyaleExceptWithParams.php'], [
56+
[
57+
'Calling crc32() is forbidden.',
58+
12,
59+
],
60+
[
61+
'Calling strtolower() is forbidden.',
62+
13,
63+
],
64+
]);
65+
}
66+
67+
68+
public static function getAdditionalConfigFiles(): array
69+
{
70+
return [
71+
__DIR__ . '/../../extension.neon',
72+
];
73+
}
74+
75+
}

‎tests/Calls/FunctionCallsTest.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,26 @@ protected function getRule(): Rule
278278
'Waldo\Foo\Wild*',
279279
],
280280
],
281+
// test allowExceptIn + allowParamsInAllowed
282+
[
283+
'function' => 'pow()',
284+
'allowExceptIn' => [
285+
__DIR__ . '/../src/disallowed-allow/*.php',
286+
],
287+
'allowParamsInAllowed' => [
288+
1 => 2,
289+
],
290+
],
291+
// test allowExceptIn + allowExceptParamsInAllowed
292+
[
293+
'function' => 'intdiv()',
294+
'allowExceptIn' => [
295+
__DIR__ . '/../src/disallowed-allow/*.php',
296+
],
297+
'allowExceptParamsInAllowed' => [
298+
1 => 2,
299+
],
300+
],
281301
]
282302
);
283303
}
@@ -507,6 +527,21 @@ public function testAllowInInstanceOfWildcard(): void
507527
}
508528

509529

530+
public function testAllowExceptInWithParams(): void
531+
{
532+
$this->analyse([__DIR__ . '/../src/disallowed-allow/functionCallsExceptWithParams.php'], [
533+
[
534+
'Calling pow() is forbidden.',
535+
6,
536+
],
537+
[
538+
'Calling intdiv() is forbidden.',
539+
10,
540+
],
541+
]);
542+
}
543+
544+
510545
public function testInstanceOfWithParams(): void
511546
{
512547
$this->analyse([__DIR__ . '/../src/BarInstanceOfWithParams.php'], [

‎tests/src/RoyaleExceptWithParams.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
namespace Fiction\Pulp;
5+
6+
class RoyaleExceptWithParams
7+
{
8+
9+
public function methodA(): void
10+
{
11+
crc32('a'); // allowed: in except zone, param matches allowParamsInAllowed
12+
crc32('b'); // disallowed: in except zone, param doesn't match allowParamsInAllowed
13+
strtolower('a'); // disallowed: in except zone, param is forbidden by allowExceptParamsInAllowed
14+
strtolower('b'); // allowed: in except zone, param not forbidden
15+
}
16+
17+
18+
public function methodB(): void
19+
{
20+
crc32('a'); // allowed: not in except zone
21+
crc32('b'); // allowed: not in except zone
22+
strtolower('a'); // allowed: not in except zone
23+
strtolower('b'); // allowed: not in except zone
24+
}
25+
26+
}

‎tests/src/disallowed-allow/functionCallsExceptWithParams.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
declare(strict_types = 1);
3+
4+
// in the except zone: allowed only when first param matches allowParamsInAllowed
5+
pow(2, 3);
6+
pow(3, 2);
7+
8+
// in the except zone: allowed only when first param doesn't match allowExceptParamsInAllowed
9+
intdiv(3, 2);
10+
intdiv(2, 3);

0 commit comments

Comments
 (0)