NEW: @W-17310939@: Add in our first AppExchange security rule to serv…

…e as a template for adding in more
forcedotcom · Dec 16, 2024 · 5abdae6 · 5abdae6
1 parent f18af25
commit 5abdae6
Show file tree

Hide file tree

Showing 9 changed files with 144 additions and 16 deletions.
diff --git a/...s/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml b/...s/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset name="AppExchange"
+         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>AppExchange Security Rules</description>
+
+    <rule name="AvoidInsecureHttpRemoteSiteSetting"
+          language="xml"
+          class="net.sourceforge.pmd.lang.rule.xpath.XPathRule"
+          message="Avoid using insecure http urls in Remote Site Settings.">
+          <!-- TODO: NEED TO ADD IN externalInfoUrl ONCE WE HAVE A PERMANENT LOCATION FOR THE DOC PAGE -->
+        <description>Detects instances of a Remote Site Settings that use HTTP.Use HTTPS instead.</description>
+        <priority>3</priority>
+        <properties>
+            <property name="xpath">
+                <value>
+<![CDATA[
+/document/RemoteSiteSetting/url/text[starts-with(lower-case(@Text),"http://")]
+]]>
+                </value>
+            </property>
+        </properties>
+    </rule>
+
+
+</ruleset>
diff --git a/...s/src/test/java/sfca/rulesets/appexchange_xml/AvoidInsecureHttpRemoteSiteSettingTest.java b/...s/src/test/java/sfca/rulesets/appexchange_xml/AvoidInsecureHttpRemoteSiteSettingTest.java
@@ -0,0 +1,16 @@
+package sfca.rulesets.appexchange_xml;
+
+import net.sourceforge.pmd.test.SimpleAggregatorTst;
+
+public class AvoidInsecureHttpRemoteSiteSettingTest extends SimpleAggregatorTst {
+    @Override
+    protected void setUp() {
+        // The test data xml file for this rule's test will always be in the resources directory using a naming
+        // convention based off the package for this test and the rule being tested:
+        //     "resources/<TestPackageName>/xml/<RuleName>.xml".
+        // In this case "sfca.rulesets.appexchange_xml" is the package name of this test file. Thus, the associated test
+        // data xml file for this rule must be found at:
+        //      "resource/sfca/rulesets/appexchange_xml/xml/AvoidInsecureHttpRemoteSiteSetting.xml"
+        addRule("sfca/rulesets/AppExchange_xml.xml", "AvoidInsecureHttpRemoteSiteSetting");
+    }
+}
diff --git a/...c/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidInsecureHttpRemoteSiteSetting.xml b/...c/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidInsecureHttpRemoteSiteSetting.xml
@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test-data
+        xmlns="http://pmd.sourceforge.net/rule-tests"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests https://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
+
+    <test-code>
+        <description>When url contains http then violation should be reported</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>6</expected-linenumbers>
+        <expected-messages>
+            <message>Avoid using insecure http urls in Remote Site Settings.</message>
+        </expected-messages>
+        <code><![CDATA[
+<?xml version="1.0" encoding="UTF-8"?>
+<RemoteSiteSetting xmlns="http://soap.sforce.com/2006/04/metadata">
+    <description>Used for Apex callout to mapping web service</description>
+    <disableProtocolSecurity>false</disableProtocolSecurity>
+    <isActive>true</isActive>
+    <url>http://www.maptestsite.net/mapping1</url>
+</RemoteSiteSetting>
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>When url contains https then violation should not be reported</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+<?xml version="1.0" encoding="UTF-8"?>
+<RemoteSiteSetting xmlns="http://soap.sforce.com/2006/04/metadata">
+    <description>Used for Apex callout to mapping web service</description>
+    <disableProtocolSecurity>false</disableProtocolSecurity>
+    <isActive>true</isActive>
+    <url>https://www.maptestsite.net/mapping1</url>
+</RemoteSiteSetting>
+        ]]></code>
+    </test-code>
+
+</test-data>
diff --git a/packages/code-analyzer-pmd-engine/src/config.ts b/packages/code-analyzer-pmd-engine/src/config.ts
@@ -312,14 +312,14 @@ abstract class SharedConfigValueExtractor {
         const extToLangMap: Map<string, Language> = new Map(); // To keep track if file extension shows up with more than one language
         const fileExtensionsMap: Record<Language, string[]> = {... DEFAULT_FILE_EXTENSIONS}; // Start with copy
         for (const language of Object.keys(fileExtensionsMap) as Language[]) {
-            const fileExts: string[] = makeUnique(fileExtensionsExtractor.extractArray(language,
+            const fileExts: string[] = makeUniqueCaseInsensitive(fileExtensionsExtractor.extractArray(language,
                 (element, elementFieldPath) => ValueValidator.validateString(element,
                     elementFieldPath, FILE_EXT_PATTERN),
                 DEFAULT_FILE_EXTENSIONS[language]
-            )!).map(fileExt => fileExt.toLowerCase());
+            )!);
 
             // Validate that none of the file extensions already exist in another language
-            for (const fileExt of fileExts) {
+            for (const fileExt of fileExts.map(fileExt => fileExt.toLowerCase())) {
                 if (extToLangMap.has(fileExt) && extToLangMap.get(fileExt) !== language) {
                     throw new Error(getMessage('InvalidFileExtensionDueToItBeingListedTwice',
                         fileExtensionsExtractor.getFieldPath(), fileExt,
@@ -427,6 +427,14 @@ function toAvailableLanguagesText(languages: string[]): string {
         .join(', ').replace(`'javascript'`, `'javascript' (or 'ecmascript')`);
 }
 
-export function makeUnique(values: string[]): string[] {
-    return [...new Set(values)];
+export function makeUniqueCaseInsensitive(arr: string[]): string[] {
+    const seen = new Set<string>();
+    return arr.filter((str) => {
+        const lowerStr = str.toLowerCase();
+        if (seen.has(lowerStr)) {
+            return false;
+        }
+        seen.add(lowerStr);
+        return true;
+    });
 }
diff --git a/packages/code-analyzer-pmd-engine/src/constants.ts b/packages/code-analyzer-pmd-engine/src/constants.ts
@@ -58,13 +58,25 @@ export const DEFAULT_FILE_EXTENSIONS: Record<Language, string[]> = {
 
     [Language.XML]: [
         // FROM PMD's XmlLanguageModule:
-        '.xml'
+        '.xml',
 
         // Salesforce metadata file extensions to associate to XML language, specifically for the AppExchange rules:
-        // TODO: COMING SOON
+        //   Note: The metadata api pages over at
+        //           https://developer.salesforce.com/docs/atlas.en-us.api_meta.meta/api_meta/meta_types_list.htm
+        //         helps to list the file extensions for each metadata type. For example, the RemoteSiteSettings page
+        //           https://developer.salesforce.com/docs/atlas.en-us.api_meta.meta/api_meta/meta_remotesitesetting.htm
+        //         specifies that .remoteSite is the file extension for remote site settings files.
+        '.remoteSite'
     ]
 }
 
+// List of our own rulesets inside our sfca-pmd-rules jar file that we want to make available for rule selection without
+// the user needing to add it to their custom_rulesets configuration list. See "pmd-rules/src/main/resources" to see
+// which rulesets we have bundled inside our sfca-pmd-rules jar file.
+export const SFCA_RULESETS_TO_MAKE_AVAILABLE: string[] = [
+    "sfca/rulesets/AppExchange_xml.xml"
+];
+
 // This object lists all the PMD rule names that are shared across languages which helps us map back and forth to unique names
 export const SHARED_RULE_NAMES: Record<string, Language[]> = {
     ForLoopsMustUseBraces: [Language.APEX, Language.JAVASCRIPT],

diff --git a/packages/code-analyzer-pmd-engine/src/pmd-engine.ts b/packages/code-analyzer-pmd-engine/src/pmd-engine.ts
@@ -13,7 +13,7 @@ import {
 import {indent, JavaCommandExecutor, toExtensionsToLanguageMap, WorkspaceLiaison} from "./utils";
 import path from "node:path";
 import * as fs from 'node:fs/promises';
-import {Language, PMD_ENGINE_NAME, SHARED_RULE_NAMES} from "./constants";
+import {Language, PMD_ENGINE_NAME, SFCA_RULESETS_TO_MAKE_AVAILABLE, SHARED_RULE_NAMES} from "./constants";
 import {
     LanguageSpecificPmdRunData,
     PmdResults,
@@ -118,8 +118,12 @@ export class PmdEngine extends Engine {
         if (!this.pmdRuleInfoListCache.has(cacheKey)) {
             const relevantLanguages: Language[] = await workspaceLiaison.getRelevantLanguages();
             const pmdRuleLanguageIds: string[] = relevantLanguages.map(toPmdLanguageId);
+            const allCustomRulesets: string[] = [
+                ... SFCA_RULESETS_TO_MAKE_AVAILABLE, // Our custom rulesets
+                ... this.config.custom_rulesets // The user's custom rulesets
+            ]
             const ruleInfoList: PmdRuleInfo[] = relevantLanguages.length === 0 ? [] :
-                await this.pmdWrapperInvoker.invokeDescribeCommand(this.config.custom_rulesets, pmdRuleLanguageIds, emitProgress);
+                await this.pmdWrapperInvoker.invokeDescribeCommand(allCustomRulesets, pmdRuleLanguageIds, emitProgress);
             this.pmdRuleInfoListCache.set(cacheKey, ruleInfoList);
         }
         return this.pmdRuleInfoListCache.get(cacheKey)!;

diff --git a/packages/code-analyzer-pmd-engine/src/pmd-rule-mappings.ts b/packages/code-analyzer-pmd-engine/src/pmd-rule-mappings.ts
@@ -1,5 +1,7 @@
 import {COMMON_TAGS, SeverityLevel} from "@salesforce/code-analyzer-engine-api";
 
+const APP_EXCHANGE_TAG: string = "AppExchange";
+
 /**
  * The following is a list of the base PMD rules that we have reviewed where we have designated the rule tags and
  * severity (most important to determine if the "Recommended" tag is applied or not). This also helps fixed these values
@@ -12,7 +14,7 @@ import {COMMON_TAGS, SeverityLevel} from "@salesforce/code-analyzer-engine-api";
 export const RULE_MAPPINGS: Record<string, {severity: SeverityLevel, tags: string[]}> = {
 
     // =================================================================================================================
-    //   APEX RULES
+    //   PMD-APEX RULES
     // =================================================================================================================
     "ApexAssertionsShouldIncludeMessage": {
         severity: SeverityLevel.Moderate,
@@ -269,7 +271,7 @@ export const RULE_MAPPINGS: Record<string, {severity: SeverityLevel, tags: strin
 
 
     // =================================================================================================================
-    //   HTML RULES
+    //   PMD-HTML RULES
     // =================================================================================================================
     "AvoidInlineStyles": {
         severity: SeverityLevel.Low,
@@ -286,7 +288,7 @@ export const RULE_MAPPINGS: Record<string, {severity: SeverityLevel, tags: strin
 
 
     // =================================================================================================================
-    //   JAVASCRIPT RULES
+    //   PMD-JAVASCRIPT RULES
     // =================================================================================================================
     "AssignmentInOperand": {
         severity: SeverityLevel.Moderate,
@@ -363,7 +365,7 @@ export const RULE_MAPPINGS: Record<string, {severity: SeverityLevel, tags: strin
 
 
     // =================================================================================================================
-    //   VISUALFORCE RULES
+    //   PMD-VISUALFORCE RULES
     // =================================================================================================================
     "VfCsrf": {
         severity: SeverityLevel.High,
@@ -380,7 +382,7 @@ export const RULE_MAPPINGS: Record<string, {severity: SeverityLevel, tags: strin
 
 
     // =================================================================================================================
-    //   XML RULES
+    //   PMD-XML RULES
     // =================================================================================================================
     "MissingEncoding": {
         severity: SeverityLevel.Moderate,
@@ -389,6 +391,15 @@ export const RULE_MAPPINGS: Record<string, {severity: SeverityLevel, tags: strin
 
     "MistypedCDATASection": {
         severity: SeverityLevel.Moderate,
-        tags: [/* NOT RECOMMENDED */    COMMON_TAGS.CATEGORIES.ERROR_PRONE, COMMON_TAGS.LANGUAGES.XML]
+        tags: [/* NOT RECOMMENDED */    COMMON_TAGS.CATEGORIES.ERROR_PRONE,    COMMON_TAGS.LANGUAGES.XML]
+    },
+
+
+    // =================================================================================================================
+    //   SFCA-PMD-RULES - APPEXCHANGE XML RULES
+    // =================================================================================================================
+    "AvoidInsecureHttpRemoteSiteSetting": {
+        severity: SeverityLevel.Moderate,
+        tags: [/* NOT RECOMMENDED */  APP_EXCHANGE_TAG, COMMON_TAGS.CATEGORIES.SECURITY, COMMON_TAGS.LANGUAGES.XML]
     }
 }
diff --git a/packages/code-analyzer-pmd-engine/test/plugin.test.ts b/packages/code-analyzer-pmd-engine/test/plugin.test.ts
@@ -209,7 +209,7 @@ describe('Tests for the PmdCpdEnginesPlugin', () => {
         const resolvedConfig: ConfigObject = await plugin.createEngineConfig(engineName, configValueExtractor);
         expect(resolvedConfig['file_extensions']).toEqual({
             ...DEFAULT_FILE_EXTENSIONS,
-            javascript: ['.js', '.jsx']}); // Also checks that duplicates are removed and that we convert to lowercase
+            javascript: ['.js', '.jsX']}); // Also checks that duplicates are removed
     });
 
     it.each(['cpd','pmd'])(`When createEngineConfig for '%s' is given an invalid file extension value type, then error`, async (engineName) => {

diff --git a/...ges/code-analyzer-pmd-engine/test/test-data/pmdGoldfiles/rules_allLanguages.goldfile.json b/...ges/code-analyzer-pmd-engine/test/test-data/pmdGoldfiles/rules_allLanguages.goldfile.json
@@ -320,6 +320,17 @@
       "https://docs.pmd-code.org/pmd-doc-{{PMD_VERSION}}/pmd_rules_html_bestpractices.html#avoidinlinestyles"
     ]
   },
+  {
+    "name": "AvoidInsecureHttpRemoteSiteSetting",
+    "severityLevel": 3,
+    "tags": [
+      "AppExchange",
+      "Security",
+      "Xml"
+    ],
+    "description": "Detects instances of a Remote Site Settings that use HTTP.Use HTTPS instead.",
+    "resourceUrls": []
+  },
   {
     "name": "AvoidLogicInTrigger",
     "severityLevel": 3,