diff --git a/docs/allow-in-instance-of.md b/docs/allow-in-instance-of.md index d905b28..9fe6fb2 100644 --- a/docs/allow-in-instance-of.md +++ b/docs/allow-in-instance-of.md @@ -40,6 +40,44 @@ parameters: - 'MyInterface' ``` +### Combining with parameter conditions + +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. + +For example, to allow `dispatch()` in classes implementing `HandlerInterface` but only when the first argument is of type `SafeEvent`: + +```neon +parameters: + disallowedFunctionCalls: + - + function: 'dispatch()' + allowInInstanceOf: + - 'App\Handlers\HandlerInterface' + allowParamsInAllowed: + - + position: 1 + name: 'event' + typeString: 'App\Events\SafeEvent' +``` + +To disallow `dispatch()` in `HandlerInterface` classes only when the first argument is of type `DangerousEvent`, and allow it with any other argument: + +```neon +parameters: + disallowedFunctionCalls: + - + function: 'dispatch()' + disallowInInstanceOf: + - 'App\Handlers\HandlerInterface' + allowExceptParamsInAllowed: + - + position: 1 + name: 'event' + typeString: 'App\Events\DangerousEvent' +``` + +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). + ### Allow in `use` imports The `allowInInstanceOf` configuration above will also report an error on the line with the import, if present: ```php diff --git a/docs/allow-with-parameters.md b/docs/allow-with-parameters.md index a4d7f6a..a866fed 100644 --- a/docs/allow-with-parameters.md +++ b/docs/allow-with-parameters.md @@ -33,7 +33,7 @@ parameters: value: true ``` -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`. +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`. 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: - `log(..., true)` (or `log(..., alert: true)`) anywhere - `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: ``` This configuration will disallow calls like `waldo('foo', 'bar')` or `waldo('*', '*')`, but `waldo('foo')` or `waldo()` will be still allowed. -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. +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. Take this example configuration: diff --git a/src/Allowed/Allowed.php b/src/Allowed/Allowed.php index cb25241..aff2691 100644 --- a/src/Allowed/Allowed.php +++ b/src/Allowed/Allowed.php @@ -98,10 +98,16 @@ public function isAllowed(?Node $node, Scope $scope, ?array $args, Disallowed $d } } if ($disallowed->getAllowInInstanceOf()) { - return $this->isInstanceOf($scope, $disallowed->getAllowInInstanceOf()); + if (!$this->isInstanceOf($scope, $disallowed->getAllowInInstanceOf())) { + return false; + } + return !$hasParams || $this->hasAllowedParamsInAllowed($scope, $args, $disallowed); } if ($disallowed->getAllowExceptInInstanceOf()) { - return !$this->isInstanceOf($scope, $disallowed->getAllowExceptInInstanceOf()); + if (!$this->isInstanceOf($scope, $disallowed->getAllowExceptInInstanceOf())) { + return true; + } + return $hasParams && $this->hasAllowedParamsInAllowed($scope, $args, $disallowed, false); } if ($hasParams && $disallowed->getAllowExceptParams()) { return $this->hasAllowedParams($scope, $args, $disallowed->getAllowExceptParams(), false); @@ -223,9 +229,10 @@ private function hasAllowedParams(Scope $scope, ?array $args, array $allowConfig * @param Scope $scope * @param array|null $args * @param DisallowedWithParams $disallowed + * @param bool $defaultResult * @return bool */ - private function hasAllowedParamsInAllowed(Scope $scope, ?array $args, DisallowedWithParams $disallowed): bool + private function hasAllowedParamsInAllowed(Scope $scope, ?array $args, DisallowedWithParams $disallowed, bool $defaultResult = true): bool { if ($disallowed->getAllowExceptParamsInAllowed()) { return $this->hasAllowedParams($scope, $args, $disallowed->getAllowExceptParamsInAllowed(), false); @@ -233,7 +240,7 @@ private function hasAllowedParamsInAllowed(Scope $scope, ?array $args, Disallowe if ($disallowed->getAllowParamsInAllowed()) { return $this->hasAllowedParams($scope, $args, $disallowed->getAllowParamsInAllowed(), true); } - return true; + return $defaultResult; } diff --git a/tests/Calls/FunctionCallsTest.php b/tests/Calls/FunctionCallsTest.php index 5ae303f..49b111e 100644 --- a/tests/Calls/FunctionCallsTest.php +++ b/tests/Calls/FunctionCallsTest.php @@ -225,6 +225,46 @@ protected function getRule(): Rule Stringable::class, ], ], + // test allowInInstanceOf + allowExceptParamsInAllowed: allowed in hierarchy except when param is 'forbidden' + [ + 'function' => 'str_starts_with()', + 'allowInInstanceOf' => [ + 'Waldo\Foo\BarBase', + ], + 'allowExceptParamsInAllowed' => [ + 2 => 'forbidden', + ], + ], + // test disallowInInstanceOf + allowExceptParamsInAllowed: disallowed in hierarchy only when param is 'forbidden' + [ + 'function' => 'str_ends_with()', + 'disallowInInstanceOf' => [ + 'Waldo\Foo\BarBase', + ], + 'allowExceptParamsInAllowed' => [ + 2 => 'forbidden', + ], + ], + // test disallowInInstanceOf + allowParamsInAllowed: disallowed in hierarchy unless param is 'allowed_param' + [ + 'function' => 'str_contains()', + 'disallowInInstanceOf' => [ + 'Waldo\Foo\BarBase', + ], + 'allowParamsInAllowed' => [ + 2 => 'allowed_param', + ], + ], + // test allowInInstanceOf + allowParamsInAllowed: allowed in hierarchy only when param is 'allowed_chars' + [ + 'function' => 'ltrim()', + 'allowInInstanceOf' => [ + 'Waldo\Foo\BarBase', + ], + 'allowParamsInAllowed' => [ + 2 => 'allowed_chars', + ], + ], // test allowed instances with wildcards, intentionally wrong case to test FNM_CASEFOLD [ 'function' => 'str_pad()', @@ -467,6 +507,61 @@ public function testAllowInInstanceOfWildcard(): void } + public function testInstanceOfWithParams(): void + { + $this->analyse([__DIR__ . '/../src/BarInstanceOfWithParams.php'], [ + [ + 'Calling str_starts_with() is forbidden.', + 11, + ], + [ + 'Calling str_ends_with() is forbidden.', + 13, + ], + [ + 'Calling str_contains() is forbidden.', + 16, + ], + [ + 'Calling str_starts_with() is forbidden.', + 26, + ], + [ + 'Calling str_ends_with() is forbidden.', + 28, + ], + [ + 'Calling str_contains() is forbidden.', + 31, + ], + [ + 'Calling str_starts_with() is forbidden.', + 42, + ], + [ + 'Calling str_starts_with() is forbidden.', + 43, + ], + [ + 'Calling ltrim() is forbidden.', + 59, + ], + [ + 'Calling ltrim() is forbidden.', + 70, + ], + [ + 'Calling ltrim() is forbidden.', + 71, + ], + [ + 'Calling ltrim() is forbidden.', + 82, + ], + ]); + } + + public static function getAdditionalConfigFiles(): array { return [ diff --git a/tests/src/BarInstanceOfWithParams.php b/tests/src/BarInstanceOfWithParams.php new file mode 100644 index 0000000..88b2766 --- /dev/null +++ b/tests/src/BarInstanceOfWithParams.php @@ -0,0 +1,85 @@ +