Skip to content

Commit

Permalink
Add definedIn?:string|list<string> config option (#198)
Browse files Browse the repository at this point in the history
To further specify/limit files where the function or method should be defined to be disallowed.

Close #194
  • Loading branch information
spaze authored May 28, 2023
2 parents e9a2fdc + 7c755c4 commit ee07fd9
Show file tree
Hide file tree
Showing 56 changed files with 569 additions and 105 deletions.
28 changes: 25 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,26 @@ parameters:
This config would disallow all `pcntl` functions except (an imaginary) `pcntl_foobar()`.
Please note `exclude` also accepts [`fnmatch`](https://www.php.net/function.fnmatch) patterns so please be careful to not create a contradicting config.

Another option how to limit the set of functions or methods selected by the `function` or `method` directive is a file path in which these are defined which mostly makes sense when a [`fnmatch`](https://www.php.net/function.fnmatch) pattern is used in those directives.
Imagine a use case in which you want to disallow any function or method defined in any namespace, or none at all, by this legacy package:
```neon
parameters:
disallowedFunctionCalls:
-
function: '*'
definedIn:
- 'vendor/foo/bar'
disallowedMethodCalls:
-
method: '*'
definedIn:
- 'vendor/foo/bar'
filesRootDir: %rootDir%/../../..
```
Relative paths in `definedIn` are resolved based on the current working directory. When running PHPStan from a directory or subdirectory which is not your "root" directory, the paths will probably not work.
Use `filesRootDir` in that case to specify an absolute root directory, you can use [`%rootDir%`](https://phpstan.org/config-reference#expanding-paths) to start with PHPStan's root directory (usually `/something/something/vendor/phpstan/phpstan`) and then `..` from there to your "root" directory.
`filesRootDir` is also used to configure all `allowIn` directives, see below.

You can treat some language constructs as functions and disallow it in `disallowedFunctionCalls`. Currently detected language constructs are:
- `die()`
- `echo()`
Expand Down Expand Up @@ -269,13 +289,13 @@ parameters:
Paths in `allowIn` support [fnmatch()](https://www.php.net/function.fnmatch) patterns.

Relative paths in `allowIn` are resolved based on the current working directory. When running PHPStan from a directory or subdirectory which is not your "root" directory, the paths will probably not work.
Use `allowInRootDir` in that case to specify an absolute root directory for all `allowIn` paths. Absolute paths might change between machines (for example your local development machine and a continuous integration machine) but you
Use `filesRootDir` in that case to specify an absolute root directory for all `allowIn` paths. Absolute paths might change between machines (for example your local development machine and a continuous integration machine) but you
can use [`%rootDir%`](https://phpstan.org/config-reference#expanding-paths) to start with PHPStan's root directory (usually `/something/something/vendor/phpstan/phpstan`) and then `..` from there to your "root" directory.

For example when PHPStan is installed in `/home/foo/vendor/phpstan/phpstan` and you're using a configuration like this:
```neon
parameters:
allowInRootDir: %rootDir%/../../..
filesRootDir: %rootDir%/../../..
disallowedMethodCalls:
-
method: 'PotentiallyDangerous\Logger::log()'
Expand All @@ -287,7 +307,7 @@ then `Logger::log()` will be allowed in `/home/foo/path/to/some/file-bar.php`.
If you need to disallow a methods or a function call, a constant, a namespace, a class, a superglobal, or an attribute usage only in certain paths, as an inverse of `allowIn`, you can use `allowExceptIn` (or the `disallowIn` alias):
```neon
parameters:
allowInRootDir: %rootDir%/../../..
filesRootDir: %rootDir%/../../..
disallowedMethodCalls:
-
method: 'PotentiallyDangerous\Logger::log()'
Expand All @@ -296,6 +316,8 @@ parameters:
```
This will disallow `PotentiallyDangerous\Logger::log()` calls in `%rootDir%/../../../path/to/some/dir/*.php`.

Please note that before version 2.15, `filesRootDir` was called `allowInRootDir` which is still supported, but deprecated.

To allow a previously disallowed method or function only when called from a different method or function in any file, use `allowInFunctions` (or `allowInMethods` alias):

```neon
Expand Down
5 changes: 4 additions & 1 deletion extension.neon
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
parameters:
allowInRootDir: null
filesRootDir: %allowInRootDir%
disallowedNamespaces: []
disallowedClasses: []
disallowedMethodCalls: []
Expand All @@ -11,6 +12,7 @@ parameters:

parametersSchema:
allowInRootDir: schema(string(), nullable())
filesRootDir: schema(string(), nullable())
# These should be defined using `structure` with listed keys but it seems to me that PHPStan requires
# all keys to be present in a structure but `message` & `allow*`/`disallow*` are optional.
disallowedNamespaces: listOf(
Expand Down Expand Up @@ -92,12 +94,13 @@ parametersSchema:

services:
- Spaze\PHPStan\Rules\Disallowed\Allowed\Allowed
- Spaze\PHPStan\Rules\Disallowed\Allowed\AllowedPath(allowInRootDir: %allowInRootDir%)
- Spaze\PHPStan\Rules\Disallowed\Allowed\AllowedPath
- Spaze\PHPStan\Rules\Disallowed\DisallowedAttributeFactory
- Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory
- Spaze\PHPStan\Rules\Disallowed\DisallowedConstantFactory
- Spaze\PHPStan\Rules\Disallowed\DisallowedNamespaceFactory
- Spaze\PHPStan\Rules\Disallowed\DisallowedSuperglobalFactory
- Spaze\PHPStan\Rules\Disallowed\File\FilePath(rootDir: %filesRootDir%)
- Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter
- Spaze\PHPStan\Rules\Disallowed\Identifier\Identifier
- Spaze\PHPStan\Rules\Disallowed\Normalizer\Normalizer
Expand Down
2 changes: 1 addition & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ parameters:
CallParamAnyValueConfig: 'array<int|string, int|array{position:int, value?:int|bool|string, name?:string}>'
CallParamFlagAnyValueConfig: 'array<int|string, int|array{position:int, value?:int, name?:string}>'
AllowDirectives: 'allowIn?:string[], allowExceptIn?:string[], disallowIn?:string[], allowInFunctions?:string[], allowInMethods?:string[], allowExceptInFunctions?:string[], allowExceptInMethods?:string[], disallowInFunctions?:string[], disallowInMethods?:string[], allowParamsInAllowed?:CallParamConfig, allowParamsInAllowedAnyValue?:CallParamAnyValueConfig, allowParamFlagsInAllowed?:CallParamFlagAnyValueConfig, allowParamsAnywhere?:CallParamConfig, allowParamsAnywhereAnyValue?:CallParamAnyValueConfig, allowParamFlagsAnywhere?:CallParamFlagAnyValueConfig, allowExceptParamsInAllowed?:CallParamConfig, allowExceptParamFlagsInAllowed?:CallParamFlagAnyValueConfig, disallowParamFlagsInAllowed?:CallParamFlagAnyValueConfig, disallowParamsInAllowed?:CallParamConfig, allowExceptParams?:CallParamConfig, disallowParams?:CallParamConfig, allowExceptParamFlags?:CallParamFlagAnyValueConfig, disallowParamFlags?:CallParamFlagAnyValueConfig, allowExceptCaseInsensitiveParams?:CallParamConfig, disallowCaseInsensitiveParams?:CallParamConfig'
ForbiddenCallsConfig: 'array<array{function?:string|list<string>, method?:string|list<string>, exclude?:list<string>, message?:string, %typeAliases.AllowDirectives%, errorIdentifier?:string, errorTip?:string}>'
ForbiddenCallsConfig: 'array<array{function?:string|list<string>, method?:string|list<string>, exclude?:list<string>, definedIn?:string|list<string>, message?:string, %typeAliases.AllowDirectives%, errorIdentifier?:string, errorTip?:string}>'
DisallowedAttributesConfig: 'array<array{attribute:string|list<string>, exclude?:list<string>, message?:string, %typeAliases.AllowDirectives%, errorIdentifier?:string, errorTip?:string}>'
AllowDirectivesConfig: 'array{%typeAliases.AllowDirectives%}'

Expand Down
36 changes: 6 additions & 30 deletions src/Allowed/AllowedPath.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,50 +4,26 @@
namespace Spaze\PHPStan\Rules\Disallowed\Allowed;

use PHPStan\Analyser\Scope;
use PHPStan\File\FileHelper;
use Spaze\PHPStan\Rules\Disallowed\Disallowed;
use Spaze\PHPStan\Rules\Disallowed\File\FilePath;

class AllowedPath
{

/** @var FileHelper */
private $fileHelper;
/** @var FilePath */
private $filePath;

/** @var string|null */
private $allowInRootDir;


public function __construct(FileHelper $fileHelper, ?string $allowInRootDir = null)
{
$this->fileHelper = $fileHelper;
$this->allowInRootDir = $allowInRootDir !== null ? $this->fileHelper->normalizePath($fileHelper->absolutizePath($allowInRootDir)) : null;
}


/**
* Make path absolute unless it starts with a wildcard, then return as is.
*
* @param string $path
* @param string|null $allowInRootDir
* @return string
*/
private function absolutizePath(string $path, ?string $allowInRootDir): string
public function __construct(FilePath $filePath)
{
if (strpos($path, '*') === 0) {
return $path;
}

if ($allowInRootDir !== null) {
$path = rtrim($allowInRootDir, '/') . '/' . ltrim($path, '/');
}
return $this->fileHelper->normalizePath($this->fileHelper->absolutizePath($path));
$this->filePath = $filePath;
}


public function matches(Scope $scope, string $allowedPath): bool
{
$file = $scope->getTraitReflection() ? $scope->getTraitReflection()->getFileName() : $scope->getFile();
return $file !== null && fnmatch($this->absolutizePath($allowedPath, $this->allowInRootDir), $this->absolutizePath($file, null), FNM_NOESCAPE);
return $file !== null && $this->filePath->fnMatch($allowedPath, $file);
}


Expand Down
2 changes: 1 addition & 1 deletion src/Calls/EchoCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function getNodeType(): string
*/
public function processNode(Node $node, Scope $scope): array
{
return $this->disallowedCallsRuleErrors->get(null, $scope, 'echo', 'echo', $this->disallowedCalls);
return $this->disallowedCallsRuleErrors->get(null, $scope, 'echo', 'echo', null, $this->disallowedCalls);
}

}
2 changes: 1 addition & 1 deletion src/Calls/EmptyCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function getNodeType(): string
*/
public function processNode(Node $node, Scope $scope): array
{
return $this->disallowedCallsRuleErrors->get(null, $scope, 'empty', 'empty', $this->disallowedCalls);
return $this->disallowedCallsRuleErrors->get(null, $scope, 'empty', 'empty', null, $this->disallowedCalls);
}

}
2 changes: 1 addition & 1 deletion src/Calls/EvalCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function getNodeType(): string
*/
public function processNode(Node $node, Scope $scope): array
{
return $this->disallowedCallsRuleErrors->get(null, $scope, 'eval', 'eval', $this->disallowedCalls);
return $this->disallowedCallsRuleErrors->get(null, $scope, 'eval', 'eval', null, $this->disallowedCalls);
}

}
2 changes: 1 addition & 1 deletion src/Calls/ExitDieCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function getNodeType(): string
public function processNode(Node $node, Scope $scope): array
{
$kind = $node->getAttribute('kind', Exit_::KIND_DIE) === Exit_::KIND_EXIT ? 'exit' : 'die';
return $this->disallowedCallsRuleErrors->get(null, $scope, $kind, $kind, $this->disallowedCalls);
return $this->disallowedCallsRuleErrors->get(null, $scope, $kind, $kind, null, $this->disallowedCalls);
}

}
16 changes: 14 additions & 2 deletions src/Calls/FunctionCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Name;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\ShouldNotHappenException;
Expand All @@ -29,19 +30,24 @@ class FunctionCalls implements Rule
/** @var DisallowedCall[] */
private $disallowedCalls;

/** @var ReflectionProvider */
private $reflectionProvider;


/**
* @param DisallowedCallsRuleErrors $disallowedCallsRuleErrors
* @param DisallowedCallFactory $disallowedCallFactory
* @param ReflectionProvider $reflectionProvider
* @param array $forbiddenCalls
* @phpstan-param ForbiddenCallsConfig $forbiddenCalls
* @noinspection PhpUndefinedClassInspection ForbiddenCallsConfig is a type alias defined in PHPStan config
* @throws ShouldNotHappenException
*/
public function __construct(DisallowedCallsRuleErrors $disallowedCallsRuleErrors, DisallowedCallFactory $disallowedCallFactory, array $forbiddenCalls)
public function __construct(DisallowedCallsRuleErrors $disallowedCallsRuleErrors, DisallowedCallFactory $disallowedCallFactory, ReflectionProvider $reflectionProvider, array $forbiddenCalls)
{
$this->disallowedCallsRuleErrors = $disallowedCallsRuleErrors;
$this->disallowedCalls = $disallowedCallFactory->createFromConfig($forbiddenCalls);
$this->reflectionProvider = $reflectionProvider;
}


Expand Down Expand Up @@ -71,7 +77,13 @@ public function processNode(Node $node, Scope $scope): array
throw new ShouldNotHappenException();
}
foreach ([$namespacedName, $node->name] as $name) {
$message = $this->disallowedCallsRuleErrors->get($node, $scope, (string)$name, (string)($displayName ?? $node->name), $this->disallowedCalls);
if ($name && $this->reflectionProvider->hasFunction($name, $scope)) {
$functionReflection = $this->reflectionProvider->getFunction($name, $scope);
$definedIn = $functionReflection->isBuiltin() ? null : $functionReflection->getFileName();
} else {
$definedIn = null;
}
$message = $this->disallowedCallsRuleErrors->get($node, $scope, (string)$name, (string)($displayName ?? $node->name), $definedIn, $this->disallowedCalls);
if ($message) {
return $message;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Calls/NewCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public function processNode(Node $node, Scope $scope): array
$name .= self::CONSTRUCT;
$errors = array_merge(
$errors,
$this->disallowedCallsRuleErrors->get($node, $scope, $name, $type->getClassName() . self::CONSTRUCT, $this->disallowedCalls)
$this->disallowedCallsRuleErrors->get($node, $scope, $name, $type->getClassName() . self::CONSTRUCT, null, $this->disallowedCalls)
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Calls/PrintCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public function getNodeType(): string
*/
public function processNode(Node $node, Scope $scope): array
{
return $this->disallowedCallsRuleErrors->get(null, $scope, 'print', 'print', $this->disallowedCalls);
return $this->disallowedCallsRuleErrors->get(null, $scope, 'print', 'print', null, $this->disallowedCalls);
}

}
1 change: 1 addition & 0 deletions src/Calls/ShellExecCalls.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public function processNode(Node $node, Scope $scope): array
$scope,
'shell_exec',
null,
null,
$this->disallowedCalls,
'Using the backtick operator (`...`) is forbidden because shell_exec() is forbidden, %2$s%3$s'
);
Expand Down
15 changes: 15 additions & 0 deletions src/DisallowedCall.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ class DisallowedCall implements DisallowedWithParams
/** @var list<string> */
private $excludes;

/** @var list<string> */
private $definedIn;

/** @var string|null */
private $message;

Expand All @@ -30,6 +33,7 @@ class DisallowedCall implements DisallowedWithParams
/**
* @param string $call
* @param list<string> $excludes
* @param list<string> $definedIn
* @param string|null $message
* @param AllowedConfig $allowedConfig
* @param string|null $errorIdentifier
Expand All @@ -38,13 +42,15 @@ class DisallowedCall implements DisallowedWithParams
public function __construct(
string $call,
array $excludes,
array $definedIn,
?string $message,
AllowedConfig $allowedConfig,
?string $errorIdentifier,
?string $errorTip
) {
$this->call = $call;
$this->excludes = $excludes;
$this->definedIn = $definedIn;
$this->message = $message;
$this->allowedConfig = $allowedConfig;
$this->errorIdentifier = $errorIdentifier;
Expand All @@ -67,6 +73,15 @@ public function getExcludes(): array
}


/**
* @return list<string>
*/
public function getDefinedIn(): array
{
return $this->definedIn;
}


public function getMessage(): string
{
return $this->message ?? 'because reasons';
Expand Down
1 change: 1 addition & 0 deletions src/DisallowedCallFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public function createFromConfig(array $config): array
$disallowedCall = new DisallowedCall(
$this->normalizer->normalizeCall($call),
$excludes,
(array)($disallowed['definedIn'] ?? []),
$disallowed['message'] ?? null,
$this->allowed->getConfig($disallowed),
$disallowed['errorIdentifier'] ?? null,
Expand Down
50 changes: 50 additions & 0 deletions src/File/FilePath.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php
declare(strict_types = 1);

namespace Spaze\PHPStan\Rules\Disallowed\File;

use PHPStan\File\FileHelper;

class FilePath
{

/** @var FileHelper */
private $fileHelper;

/** @var string|null */
private $rootDir;


public function __construct(FileHelper $fileHelper, ?string $rootDir = null)
{
$this->fileHelper = $fileHelper;
$this->rootDir = $rootDir !== null ? $this->fileHelper->normalizePath($fileHelper->absolutizePath($rootDir)) : null;
}


public function fnMatch(string $path, string $file): bool
{
return fnmatch($this->absolutizePath($path, $this->rootDir), $this->absolutizePath($file, null), FNM_NOESCAPE);
}


/**
* Make path absolute unless it starts with a wildcard, then return as is.
*
* @param string $path
* @param string|null $rootDir
* @return string
*/
private function absolutizePath(string $path, ?string $rootDir): string
{
if (strpos($path, '*') === 0) {
return $path;
}

if ($rootDir !== null) {
$path = rtrim($rootDir, '/') . '/' . ltrim($path, '/');
}
return $this->fileHelper->normalizePath($this->fileHelper->absolutizePath($path));
}

}
Loading

0 comments on commit ee07fd9

Please sign in to comment.