PHPStanのカスタムルールを導入しました

2024年2月21日

笹山雷雅

URLをコピーしました！

こんにちは。開発本部でインターンをしている笹山　雷雅です。

今回はPHPStanを用いて独自のLintRuleを導入したので紹介します。

PHPStanとは

PHPStanとはPHPのLinterの一つで、静的解析ツールです。

コードの書き方を分析し、潜在的なバグのある書き方を検知できます。

導入がしやすく、Zero Configでもかなり高度な解析ができるのが特徴です。

https://phpstan.org

なぜLintが必要なのか

エラーだけでは検出できない脆弱性の潜む「疑わしい」コードを検知するために、Lintが必要です。PHPのような書き方の自由度の高い言語での集団開発では、規約を徹底しても、人間によるミスは書く時チェックする時にどうしても発生してしまいます。

そこで、ミスを人間で指摘するのでなくリンターのようなシステムでカバーすることで、コーディング、レビュー両方の労力を減らすことができます。

（また、人に指摘されるより、ツールが指摘してくれる方が、心理的に安心できます）

PHPStanでのLintについて

PHPStanのLintは、PHPParserがコードを抽象構文木(Abstract Syntax Tree;AST)に変換し、そのASTから検査に必要な情報を取得して行います。

PHPParserはPHPコードをパースするためのライブラリです。

https://github.com/nikic/PHP-Parser

PHPStanからの引用ですが、このコードは、

return $this->foo && self::bar();

このようなASTに変換されます。

PhpParser\Node\Stmt\Return_
└─ expr: PhpParser\Node\Expr\BinaryOp\BooleanAnd
   ├─ left:  PhpParser\Node\Expr\PropertyFetch
   |         ├─ var:  PhpParser\Node\Expr\Variable
   |         └─ name: PhpParser\Node\Identifier
   └─ right: PhpParser\Node\Expr\StaticCall
             ├─ class: PhpParser\Node\Name
             ├─ name:  PhpParser\Node\Identifier
             └─ args:  array()

PHPParserの型を知っておくと、PHPStanを用いてLintRuleを作る際に非常に役立ちます。

ちなみに扱える型は、基本のNode型を継承したものが180個程存在しています。

コードの解析について

基本的な解析については、NodeとScopeを用いることで情報を取得できます。

Nodeはそのコード片が持つ情報を、Scopeはコード片の属するファイルやスコープの情報を持っています。

検査に必要なものを適宜NodeとScopeから取得し、ルールを作っていきます。

NodeやScopeが何を持っているかは、リファレンスを参照してください。

Nodeがどんな情報を返すかはNodeAbstractに書かれています。またどんな型があるかの一覧にもなっています。

https://apiref.phpstan.org/1.11.x/namespace-PhpParser.Node.html

https://apiref.phpstan.org/1.11.x/PHPStan.Analyser.Scope.html

https://apiref.phpstan.org/1.11.x/PhpParser.NodeAbstract.html

Lintを導入した背景

PR TIMESでは、SQL実行を行う関数に、クエリを引数として渡すことでDB操作を行なっており、SQLインジェクションの可能性を排除できないため、文字列結合・変数渡しは許可せず、文字列リテラルを直接利用するルールになっています。

しかし、そのチェックはレビュワーが担当しており、ヒューマンエラーが発生する可能性があります。

そこで、PHPStanは導入されていたため、システムでこのチェックをするために、カスタムルールを導入することになりました。

導入するルール

導入するルールは、以下のような条件で検査します。

引数の数が正しいこと
SQLインジェクションを防ぐためにSQL実行用の関数で、引数に渡すSQLクエリは必ず文字列リテラルであること
文字列リテラルはシングルクォートまたはNowDoc構文であること
- ダブルクォートとヒアドキュメント構文は値のパースの際に変数を展開するため、SQLインジェクションを起こす可能性があるため違反対象とします。

この条件を満たすことができれば、SQLインジェクションが起こる可能性をコーディング時点で減らすことができます。

LintRuleを実装するクラスは、必ずRuleインターフェースを実装する必要があり、このRuleインターフェースは、

getNodeType() ルールの対象となるNodeのタイプを取得
processNode() ルールを処理する

この二つのメソッドを実装します。

Ruleインターフェースのコードはこちらを参照してください。

https://github.com/phpstan/phpstan-src/blob/1f47c5364206096e01ecd977c9c5d35d2a34608b/src/Rules/Rule.php

getNodeType() で対象の型を返すように実装します。今回は静的メソッドを検査するので、StaticCall::class を返します。

processNode() には、クラスへのDocCommentに@implements Rule<対象の型> とすることで検査する型を絞ることができます。この型の持つクラス名は、getNodeType() の返すクラス名と同じです。

/**
 * @implements Rule<StaticCall>
 */
class SqlRule implements Rule {
    public function getNodeType(): string
    {
        return StaticCall::class;
    }
    public function processNode(Node $node, Scope $scope): array

これで、検査する準備ができたので、processNode() で検査を以下のように実装していきます。

文字列リテラルかどうかはScalar\\String_ で判定
ダブルクォーテーション("")の場合、変数埋め込みができSQLインジェクションの可能性が残る
- getAttribute() して文字列の種類からシングルクォート、NowDocのみ許可

/**
 * @implements Rule<StaticCall>
 */
class SqlRule implements Rule {
    public function getNodeType(): string
    {
        return StaticCall::class;
    }
    public function processNode(Node $node, Scope $scope): array
    {
        if (!$node->class instanceof Node\Name || (string)$node->class !== 'Sql' ||
            !$node->name instanceof Node\Identifier || (string)$node->name !== 'exec') {
            return [];
        }

        if(count($node->args) < 3) {
            return ['Sql::exec()の引数は3個必要です。'];
        }

        if (!$node->args[1]->value instanceof Node\Scalar\String_) {
            return ['Sql::exec()に代入するクエリは文字列リテラルである必要があります。'];
        }

        $attributes = $node->args[1]->value->getAttributes();
        if (!in_array($attributes['kind'], [String_::KIND_SINGLE_QUOTED, String_::KIND_NOWDOC])) {
            return ['Sql::exec()に代入するクエリはSQLインジェクション防止のため、シングルクォートで記述する必要があります。'];
        }

        return [];
    }

}

PHPStanのカスタムルールは解析する際に、composerが認識している必要があるので、以下のようにneonファイルに追加します。

rules:
	- CustomRules\SqlRule

注: これは独自にコンストラクタを実装しない場合など、シンプルな実装に限ります。詳しくは以下のドキュメントを参照してください。

Dependency Injection & Configuration

ルールをテストする

RuleTestCace を継承する(LintRule同様に、DocCommentでテストで使うルールを参照する)
Lintが失敗するコードを書き、検知される行数が一致するかをテストする

ルールの正確性を保証するために、テストを設定します。

PHPUnitで通常TestCaseクラスを継承するところをRuleTestCase を継承します。

RuleTestCaseのコードはこちらを参照してください。

https://github.com/phpstan/phpstan-src/blob/1f47c5364206096e01ecd977c9c5d35d2a34608b/src/Testing/RuleTestCase.php

こちらもLintRule同様にPHPDocで使用するルールを参照します。

テストの書き方は、対象のファイルで違反時の内容と、何行目で起きたかがあっているかを確認します。

実際のテストの実装は以下のとおりです。

<?php

declare(strict_types=1);

use PHPStan\Testing\RuleTestCase;
//snip

/**
 * @extends RuleTestCase<SqlRule>
 */
class SqlRuleTest extends RuleTestCase
{
    protected function getRule(): Rule
    {
        return new SqlRule();
    }

    public function testRule(): void
    {
        $this->analyse([__DIR__ . '/data/sql-exec-args.php'], [
            [
                'Sql::exec()の引数は3個必要です。', //違反時の内容
                9 //　何行目で起きたか
            ],
            [
                'Sql::exec()に代入するクエリは文字列リテラルである必要があります。',
                11
            ],
            [
                'Sql::exec()に代入するクエリは文字列リテラルである必要があります。',
                13
            ],
            [
                'Sql::exec()に代入するクエリは文字列リテラルである必要があります。',
                15
            ],
            [
                'Sql::exec()に代入するクエリはSQLインジェクション防止のため、シングルクォートで記述する必要があります。',
                17
            ]
        ]);
    }
}

ルール違反を発見するためのテスト対象のPHPのコードは以下のとおりです。

<?php

use \Sql;

$pdo = \PDOFactory::getNewPDOInstance();
$sql = 'SELECT * FROM hoge';
$attr = 'name';
//引数が不足している
Sql::exec($sql, []);
//変数を使用している
Sql::exec($pdo, $sql, []);
//文字列結合を使用している
Sql::exec($pdo, 'SELECT * FROM huga' . 'WHERE 1 = 1', []);
//変数埋め込みを使用している
Sql::exec($pdo, "SELECT $attr FROM hoge", []);
//2重引用符を使用している
Sql::exec($pdo, "SELECT * FROM hoge", []);

//クラッシュ回避目的
$class = Foo::class;
$class::exec();

$method = 'bar';
Sql::$method();

ベースラインについて

今回の改善の目的はLintRule自体を導入することなので、違反した箇所を改善するのは無視しておきたいです。

CI上にてPHPStanの実行は結果がキャッシュされるのですが、失敗時にはされないため、実行時間が長くなってしまう問題もあるためです。

なぜ実行時間が長いとよくないのかは以下の記事で説明されています。

そこで、ベースラインを設定します。ベースラインとは、現在起こっている違反を基準にして、一旦無視するための概念です。

ただし、特定のルール違反のみを無視して作成するわけではないので、生成した後にベースラインの差分を確認して他のエラーが入っていないかを確認することが必要です。

./vendor/bin/phpstan analyse --generate-baseline

このコマンドを実行することでphpstan-baseline.neon が作成されます。

大規模なプロジェクトの場合、ベースラインの作成にメモリが不足して生成できない場合があります。その際には--memory-limit オプションで必要なメモリを確保してください。

カスタムルールを導入するためのTips

カスタムルールを導入する上で、一番大変だったのは、対象となるコードがPHPParserではどの型なのかを調べることでした。今回は公式ドキュメントと同じ例だったのですんなりいきましたが、実際には183個のクラスから探す、なんてことになりかねません。

そこで、以下の二つの方法を見つけたので紹介したいと思います。

php-parser-nodes-docsを参照する

このリポジトリのREADME.mdには、PHPParserと実際のコードの対応が書かれています。PHPParserの型から具体例を知りたい場合におすすめです。

nikic/php-parserのdumpを使う

こちらはCLIでコードがどのようにASTにパースされるかを調べることが可能で、導入も簡単に行えます。

composer require nikic/php-parser

--var-dump をつけることで、簡単にASTを入手できます。ただかなり長い結果が出力されるので、型を知りたい！という場合は適宜grep object -A ４ などを併用することで、どのコード片がどの型を知ることができます。

例えば、このようなコードが、

<?php

namespace PrCustomRules;

use PRTIMES\PrSql;

以下のように出力されます。

sh-5.1# vendor/bin/php-parse --var-dump dev-script/PrCustomRules/test.php | grep -A 4 object
====> File dev-script/PrCustomRules/test.php:
==> var_dump():
  object(PhpParser\Node\Stmt\Namespace_)#602 (3) {
    ["name"]=>
    object(PhpParser\Node\Name)#601 (2) {
      ["name"]=>
      string(13) "PrCustomRules"
      ["attributes":protected]=>
      array(6) {
--
      object(PhpParser\Node\Stmt\Use_)#605 (3) {
        ["type"]=>
        int(1)
        ["uses"]=>
        array(1) {
--
          object(PhpParser\Node\UseItem)#604 (4) {
            ["type"]=>
            int(0)
            ["name"]=>
            object(PhpParser\Node\Name)#603 (2) {
              ["name"]=>
              string(13) "PRTIMES\PrSql"
              ["attributes":protected]=>
              array(6) {
--

objectの行でPHPParserでの型が、name などの値で実際のコード情報が得られます。