見出し画像

ゆるっとSalesforce #12 Code Analyzer 〜 v3で追加された機能を検証

株式会社co-meeting

こんにちは遠藤です。

ゆるっとSalesforce #12で取り上げたSalesforce Code Analyzer（旧 Salesforce CLI Scanner)について、イベント向けに検証した内容をまとめてみました。

この記事は Salesforceのカレンダー | Advent Calendar 2022 - Qiita 第13日目の投稿でもあります。


Salesforce Code Analyzerとは

Code Analyzerは、sfdxコマンドのプラグインsfdx-scannerとして提供されています。v3でSalesforce CLI ScannerからSalesforce Code Analyzerに名前が変更になりましたが、プラグイン名はscannerなので実態がちょっとわかりにくいです。

Code Anallyzer自体の説明は、v2の説明ですが以下の公式開発ブログの記事がわかり易いです。

この記事からの抜粋ですが、Code Analyzerが解決する課題は以下になります

Many powerful static analysis tools are available. Most of them specialize in one language or set of languages. Many applications (including typical Salesforce packages), however, contain an assortment of components created using different languages. A single static analyzer is insufficient to address all aspects of such applications. And managing multiple static analyzer tools can quickly become a pain.

つまり、PMDやESLintなど言語ごとの静的解析ツールをそれぞれ構成したり、結果のレポートをサマリーしたりするのは大変です。Code Analyzerは、それを解決する統合インタフェース（コマンド）を提供します。

ということらしいです。

v2ではPMD、ESlintのみでしたが、v3では以下のエンジンがサポートされています

  • PMD: ApexとVisualforce用の静的解析ツール

  • ESLInt: Javascriptの性的解析ツール、主にLightning Web Components用

  • RetireJS: Node.jsパッケージ向け、脆弱性が発見された依存ライブラリを検出

  • Copy-Paste Detector (CPD): 重複コードの検出 PMDの1ルール

  • Salesforce Graph Engine (sfge): ApexFlsViolationRuleはsfgeから提供

Salesforce Graph Engine

Code Analyzerつまりsfdx-scanner v3の目玉は、Salesforce Graph Engineです。
Salesforce Graph Engineについては以下の公式開発ブログ記事に詳しく解説されています。

ドキュメントでは以下のページ

v3のWhat's Newには、AppExchangeのSecurity reviewプロセスにかなり近づいたよと記載されており、これはGraph Engineによるところのようです。

Capture more complex violations. We’re much closer to bridging the gap between violations found locally versus what can be detected only during the AppExchange Security Review process. Read on to find out more.

Graph Engineについてざっくり解説すると、静的解析のアルゴリズムにDFAアプローチを採用することによりASTアプローチより偽陽性をへらすことに成功したらしいです。

現在提供されるルールは、項目レベルセキュリティの漏れをチェックするApexFlsViolationRuleのみになります。

Code Analyzerの使い方

ここからCode Analyzerを実際に利用する手順について解説します。

前提条件

前提条件は以下になります。

  • Java (バージョン 8以上)

  • Salesforce CLI

sfdxで開発しているのであればすでにどちらもセットアップ済みかと思います。

インストール手順

概要で説明したとおり、Code Analyzerはsfdxプラグインなので以下のコマンドでsfdx-scannerをインストールします。

$ sfdx plugins:install @salesforce/sfdx-scanner

インストールできたら利用可能なルールセットを確認してみましょう

$ sfdx scanner:rule:list
WARNING: We're continually improving Salesforce Code Analyzer. Tell us what you think! Give feedback at https://research.net/r/SalesforceCA
NAME                                                    LANGUAGES    CATEGORIES             RULESETS [DEP]                                    ENGINE
──────────────────────────────────────────────────────  ───────────  ─────────────────────  ────────────────────────────────────────────────  ─────────────────
VfCsrf                                                  visualforce  Security               Basic VF                                          pmd
VfHtmlStyleTagXss                                       visualforce  Security                                                                 pmd
VfUnescapeEl                                            visualforce  Security               Basic VF                                          pmd
ApexAssertionsShouldIncludeMessage                      apex         Best Practices                                                           pmd
ApexUnitTestClassShouldHaveAsserts                      apex         Best Practices         Default ruleset...,quickstart,ApexUnit            pmd
ApexUnitTestMethodShouldHaveIsTestAnnotation            apex         Best Practices                                                           pmd
ApexUnitTestShouldNotUseSeeAllDataTrue                  apex         Best Practices         Default ruleset...,quickstart,ApexUnit            pmd

...

insecure-bundled-dependencies                           javascript   Insecure Dependencies                                                    retire-js
ApexFlsViolationRule                                    apex         Security                                                                 sfge

たくさんありますね。最後にエンジンがretire-jsとsfgeのルールが表示されていることでもv3がインストールされていることを確認できます。

スキャナの実行方法

sfdx-scannerを試すにあたって公式ブログ記事で紹介されているサンプルリポジトリを利用しています。

クローンしてサンプルプロジェクトにディレクトリに移動します。

git clone https://github.com/forcedotcom/sfdx-scanner.git
cd sfdx-scanner/test/code-fixtures/projects/sfge-working-app/

従来のscannerでの実行

PMDを利用してセキュリティスキャンをsfdx-scannerから実行してみます。
対象のApexクラスは、AuraEnabledFls.clsです。
sfdx scanner:runコマンドに、必要最小限のパラメータ、--targetにクラス名、--categoryに「Security」を指定して実行してみます。

$ sfdx scanner:run --target './force-app/main/default/classes/AuraEnabledFls.cls' --category 'Security'
LOCATION                                              DESCRIPTION                                           CATEGORY  U R L
────────────────────────────────────────────────────  ────────────────────────────────────────────────────  ────────  ───────────────────────────────────────────────────────────────────────────────
force-app/main/default/classes/AuraEnabledFls.cls:16    Validate CRUD permission before SOQL/DML operation  Security  https://pmd.github.io/pmd-6.50.0/pmd_rules_apex_security.html#apexcrudviolation
force-app/main/default/classes/AuraEnabledFls.cls:40    Validate CRUD permission before SOQL/DML operation  Security  https://pmd.github.io/pmd-6.50.0/pmd_rules_apex_security.html#apexcrudviolation
force-app/main/default/classes/AuraEnabledFls.cls:46    Validate CRUD permission before SOQL/DML operation  Security  https://pmd.github.io/pmd-6.50.0/pmd_rules_apex_security.html#apexcrudviolation
force-app/main/default/classes/AuraEnabledFls.cls:55    Validate CRUD permission before SOQL/DML operation  Security  https://pmd.github.io/pmd-6.50.0/pmd_rules_apex_security.html#apexcrudviolation
force-app/main/default/classes/AuraEnabledFls.cls:71    Validate CRUD permission before SOQL/DML operation  Security  https://pmd.github.io/pmd-6.50.0/pmd_rules_apex_security.html#apexcrudviolation
force-app/main/default/classes/AuraEnabledFls.cls:87    Validate CRUD permission before SOQL/DML operation  Security  https://pmd.github.io/pmd-6.50.0/pmd_rules_apex_security.html#apexcrudviolation
force-app/main/default/classes/AuraEnabledFls.cls:95    Validate CRUD permission before SOQL/DML operation  Security  https://pmd.github.io/pmd-6.50.0/pmd_rules_apex_security.html#apexcrudviolation
Executed pmd, found 7 violation(s) across 1 file(s).
Executed retire-js, found 0 violation(s) across 0 file(s).
Rule violations were logged to the console.

クラス内のすべてのinsert文でApexCRUDViolationルール（オブジェクト権限のチェック）でエラーとなっています。

Salesforce Graph Engineを使用して実行

次にSalesforce Graph Engineを利用したスキャンも実行してみます。
sfgeはscanner:run:dfaコマンドを実行します。
必要最小限のパラメータ、--targetと--projectdirを指定して実行してみます。

$ sfdx scanner:run:dfa --target './force-app/main/default/classes/AuraEnabledFls.cls' --projectdir './force-app/main/default'
Analyzing with SFGE. See /Users/hrendoh/.sfdx-scanner/sfge.log for details.... Done
SOURCE LOCATION                                       SINK LOCATION                                         DESCRIPTION                                          CATEGORY  U R L
────────────────────────────────────────────────────  ────────────────────────────────────────────────────  ───────────────────────────────────────────────────  ────────  ──────────────────────────────────────────────────────────────────────────────────────────────────────
force-app/main/default/classes/AuraEnabledFls.cls:52  force-app/main/default/classes/AuraEnabledFls.cls:55    FLS validation is missing for [INSERT] operation   Security  https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#ApexFlsViolationRule
                                                                                                              on [Account] with field(s) [Name,Phone].
force-app/main/default/classes/AuraEnabledFls.cls:4   force-app/main/default/classes/AuraEnabledFls.cls:16    FLS validation is missing for [INSERT] operation   Security  https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#ApexFlsViolationRule
                                                                                                              on [Account] with field(s) [Name,Phone].
force-app/main/default/classes/AuraEnabledFls.cls:21  force-app/main/default/classes/AuraEnabledFls.cls:46    FLS validation is missing for [INSERT] operation   Security  https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#ApexFlsViolationRule
                                                                                                              on [Contact] with field(s) [Account,LastName].
force-app/main/default/classes/AuraEnabledFls.cls:60  force-app/main/default/classes/AuraEnabledFls.cls:71    FLS validation is missing for [INSERT] operation   Security  https://forcedotcom.github.io/sfdx-scanner/en/v3.x/salesforce-graph-engine/rules/#ApexFlsViolationRule
                                                                                                              on [Account] with field(s) [Name,Phone].
Executed sfge, found 4 violation(s) across 1 file(s).
Rule violations were logged to the console.

dfaコマンドを使った場合は、脆弱性が4に減りました。
21行目のflsHelperMultipleInstancesメソッドはメソッド内で2回insertしていますが、dfs場合46行目のinsertのみが報告されています。

76行目のflsDoneCorrectlyメソッドは名前の通り正しくFLSチェックをしているメソッドです。こちらはPMDを利用した実行では偽陽性でしたが、sfgeを利用するdfaコマンドでは陰性となっています。

@AuraEnabled
public static Account flsDoneCorrectly(String name, String phone) {
	FlsHelperClass helper = new FlsHelperClass('Account');
	String[] fieldsToCheck = new String[]{'Name', 'Phone'};

	for (String fieldToCheck : fieldsToCheck) {
		helper.verifyCreateable(fieldToCheck);
	}

	Account a = new Account(Name = 'Acme Inc.', Phone = '312-555-0123');
	// Because we performed CRUD/FLS checks against all of the necessary fields, this line will succeed and no
	// violation will be thrown.
	insert a;
	return a;
}

たしかに、PMDを利用したチェックよりも品質が向上していることがサンプルコードから確認できました。

さらに使いこなすために

ここまで基本的な使い方を解説してきましたが、細かく動作を指定する方法について解説していきます。

Graph Engineのエントリポイント

Salesforce Graph Engineがチェックする対象は、エンドユーザが画面から操作した際に実行されるApexコードに限定されているようです。

sfgeが対象とするのは以下のエントリポイントから呼び出されるコードになります。

  • @AuraEnabledアノテーションが付与されたメソッド

  • @InvocableMethodアノテーションが付与されたメソッド

  • @NamespaceAccessibleアノテーションが付与されたメソッド

  • @RemoteActionアノテーションが付与されたメソッド

  • PageReferenceを返すメソッド

  • Visualforce Controllersのpublicメソッド

  • すべてのglobalメソッド

  • Messaging.InboundEmailHandlerを実装したクラスのMessaging.InboundEmailResult handleInboundEmail()メソッド

  • 上記から呼び出されるメソッド

ApexトリガーやBatchable、Schedulableなどシステム用途のクラスは含まれていません。sfgeを使用する場合は、PMDと違ってApexバッチクラスにSuppressWarningsをはじめから付与する必要が無いということになりそうです。

このことから実際のセキュリティレビューにおいても、チェック対象のApexコードは同様の考え方が適用できるのではと考えられます。この辺りは最後のまとめでも触れています。

Graph Engineが現在対応しているサニタイザーについて

Graph Enginが現在対応しているサニタイザーは、以下になります。

  • Access check performed using Schema.DescribeSObjectResult

  • Acceptable only for operations that require CRUD-level checks such as DELETE, UNDELETE, and MERGE.

  • Access check performed using Schema.DescribeFieldResult

  • Acceptable for operations that require FLS-level checks. Includes READ, INSERT, UPDATE, UPSERT for Standard data objects and Custom Objects

  • SOQL queries that use WITH SECURITY_ENFORCED

  • Lists filtered by Security.stripInaccessible

Winter '23のユーザモードはベータだからなのか現在未対応です。

Graph Engineのチェックをスキップするコメント

チェックをスキップするコメントはドキュメントの「Add Engine Directives」にまとまっています。

行ごとに無効化する場合は /* sfge-disable-next-line <rule_name> */ を使用します。以下の例はinsertのApexFlsViolationRuleのチェックを無効にします。

/* sfge-disable-next-line ApexFlsViolationRule */
insert a;

メソッド単位は /* sfge-disable-stack <rule_name> */ を使用します。

@AuraEnabled
/* sfge-disable-stack ApexFlsViolationRule */
public static boolean someMethodName() {

メソッドの直ぐ上の行にコメントを入れる必要があり、アノテーションの上だと無効化されないので注意です。

同様にクラス全体を無効化する /* sfge-disable <rule_name> */ も用意されています。

/* sfge-disable ApexFlsViolationRule */
public class MyClass {

Graph Engineの実行に時間がかかる問題の対処

既存のプロジェクトに対して scanner:run:dfa コマンドを実行してみたところタイムアウトが頻出してしまっていました。

ドキュメントのTroubleshootingのページを参照するとタイムアウトしてしまう場合は、以下のいずれかの方法でタイムアウトを延ばすようにと記載があります。

  • 環境変数SFCA_RULE_THREAD_TIMEOUTを指定

  • --rule-thread-timeoutパラメータを指定

いずれもミリ秒単位で指定します。またデフォルトのタイムアウト時間は15分とのことです。

しかし、実行完了できたとしてもローカルで実行するにはかなり厳しいため、大きめのリソースを割り当てたCIサーバーを用意しつつ、更新があったファイルのみチェックするようにスクリプトを組むなど工夫が必要そうです。

また、メモリーの消費も激しく、ドキュメントには「java.lang.OutOfMemoryError: Java heap space.」が発生する場合には、以下の方法でヒープメモリーサイズを指定して実行するようにと記載されています。

  • --sfgejvmargs にヒープサイズを指定

  • 環境変数SFGE_JVM_ARGSにヒープサイズを指定

ヒープサイズを大きく取っておけば、GC回数は減るので実行時間も短くなる可能性があります。

PMDのSuppressing warnings

こちらは従来からあるscanner:runコマンドでPMDによるチェックを実行した場合に、対象外であることを知らせるための仕組みです。
アノテーションとコメントを利用できます。

公式な解説はPMDのドキュメントの以下のページにあります。

@SuppressWarnings でメソッド全体を除外

メソッド単位に除外するには、以下のようにアノテーションを指定します。

@SuppressWarnings('PMD')
public static Account flsInIfBranchOnly(Boolean enterIfBranch) {

PMDの一部のルールを除外したい場合は、「PMD.<ルール名>」をカンマ区切りで指定します。

@SuppressWarnings('PMD.ApexCRUDViolation, PMD.ApexSOQLInjection')
public static Account flsInIfBranchOnly(Boolean enterIfBranch) {

コメント「//NOPMD」で行単位に指定

行単位で除外するには、「//NOPMD」コメントを付与します。ApexCRUDViolationを除外したいケースは、コメントの方が使いやすそうです。

以下のコードでは、insertの行がチェックされなくなります。

@AuraEnabled
public static Account flsInIfBranchOnly(Boolean enterIfBranch) {
    FlsHelperClass helper = new FlsHelperClass('Account');
    String[] fieldsToCheck = new String[]{'Name', 'Phone'};
    if (enterIfBranch) {
        for (String fieldToCheck : fieldsToCheck) {
            helper.verifyCreateable(fieldToCheck);
        }
    }

    Account a = new Account(Name = 'Acme Inc.', Phone = '312-555-0123');
    // Because we performed CRUD/FLS checks in only one arm of the if-else branch, this line will throw a violation.
    insert a;//NOPMD
    return a;
}

各エンジンの構成ファイルを指定

v3では、オプション--rulesetsが非推奨となりました。非推奨で削除はされていないので動いて欲しいところですが、試したところパラメータエラーにはなりませんが動作しませんでした。

そのため、scanner:runコマンドで特定のルールのみを実行するためには、エンジンごとの構成ファイルを作成してパラメータ--pmdconfigや--tsconfigに指定する必要があります。

ここではPMDのルールセットを作成してコマンドに指定する方法について解説します。

公式の解説はこちらです。

こちらの記事の先頭に記載されているテンプレートに、Code StyleカテゴリーからEmptyCatchBlockを追加してpmdconfig.xmlという名前で保存します。

<?xml version="1.0"?>

<ruleset name="Custom Rules"
    xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">

    <description>
        My custom rules
    </description>


    <rule ref="category/apex/errorprone.xml/EmptyCatchBlock" />

</ruleset>

flsInIfBranchOnlyメソッドのinsert文をtry〜catchで囲みcatchでは何もしない状態にします。

@AuraEnabled
public static Account flsInIfBranchOnly(Boolean enterIfBranch) {
    FlsHelperClass helper = new FlsHelperClass('Account');
    String[] fieldsToCheck = new String[]{'Name', 'Phone'};
    if (enterIfBranch) {
        for (String fieldToCheck : fieldsToCheck) {
            helper.verifyCreateable(fieldToCheck);
        }
    }
    Account a = new Account(Name = 'Acme Inc.', Phone = '312-555-0123');
    try {
        // Because we performed CRUD/FLS checks in only one arm of the if-else branch, this line will throw a violation.
        insert a;
    } catch(Exception e) {
        // do nothing
    }

    return a;
}

--pmdconfigパラメータに作成した構成ファイル「./pmdconfig.xml」を指定して実行すると以下の結果となります。ApexCRUDViolationは出力されずEmptyCatchBlockのみ出力されています。

$ sfdx scanner:run --target './force-app/main/default/classes/AuraEnabledFls.cls' --pmdconfig=./pmdconfig.xml 
LOCATION                                              DESCRIPTION                 CATEGORY     U R L
────────────────────────────────────────────────────  ──────────────────────────  ───────────  ───────────────────────────────────────────────────────────────────────────────
force-app/main/default/classes/AuraEnabledFls.cls:73    Avoid empty catch blocks  Error Prone  https://pmd.github.io/pmd-6.50.0/pmd_rules_apex_errorprone.html#emptycatchblock
Executed pmd-custom, found 1 violation(s) across 1 file(s).
Executed retire-js, found 0 violation(s) across 0 file(s).
Rule violations were logged to the console.

Checkmaxとの棲み分け？

AppExchangeベンダーとしては、セキュリティポータルのスキャナーCheckmaxはどうなるのか？というところですが、公式ブログ記事には、以下のように、セキュリティレビューを置き換えるものではなく、あくまで開発効率を上げるものだと記載されています。
将来的にCode Analyzerのレポートを提出すれば良くなるということは無さそうです。

Using the plug-in doesn’t take the place of a formal security review, but it can identify a variety of security, quality, and performance issues early in development.

AppExchange セキュリティレビュー観点でのまとめ

たしかに、Code Analyzer つまりsfdx-scanner v3では、sfgeによりFLSチェックの精度が高くなっているようです。ただ、実行負荷が高く小さめのプロジェクトでも全体を対象として実行するとタイムアウトが多発してしまっていました。
文中にも書いていますが、実際に運用するには手元で実行するものではなく、時間がかかっても許容できるCIサーバーでのみのチェックすることになりそうです。しかし、CIサーバーで実行するにもJVMのチューニングがそれなりに必要となり環境構築の難易度は高くなりそうなのも気になるところです。

ところで、実際のAppExchangeアプリのセキュリティレビューでは、パッケージ内のオブジェクトに対するApexからのアクセスの多くはFLSのチェックが不要です。
逆に必要なケースは、登録者組織のオブジェクトへアクセスするケースか、意図的にユーザがFLSを設定して利用することを想定したオブジェクトのみになります。
それ以外は、チェック無効のコメントを指定することになり、うまくこの辺りのコメントを利用してチェック対象を絞り込んだりする工夫はできそうな気もします。
余談ですが、無効な箇所にコメントを入れてマークするので、ついでに偽陽性ドキュメントを自動で出力できるよなスクリプトもあったらなあとイベント中に話が上がりました。これは、セキュリティレビュー申請の際にとても役に立ちそうです。

以上ですが、たしかにうまく活用すると忘れた頃にやってくるセキュリティーレビュー依頼に慌てることなく、少ない工数でセキュリティレビューを乗り越えることが期待できると感じました。利用するには工夫が必要ですが、改めてプロダクトの運用に載せられるかは別途検証してみたいと思います。