From cd5399b5c2d1b42b331687830332f19ed088dbfa Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Mon, 16 Dec 2024 11:49:40 -0500 Subject: [PATCH] NEW: @W-17310939@: Add in our first AppExchange security rule to serve as a template for adding in more --- .../code-analyzer-pmd-engine/package.json | 2 +- .../sfca/rulesets/AppExchange_xml.xml | 27 +++++++++++++ ...voidInsecureHttpRemoteSiteSettingTest.java | 16 ++++++++ .../AvoidInsecureHttpRemoteSiteSetting.xml | 39 +++++++++++++++++++ .../code-analyzer-pmd-engine/src/config.ts | 18 ++++++--- .../code-analyzer-pmd-engine/src/constants.ts | 16 +++++++- .../src/pmd-engine.ts | 8 +++- .../src/pmd-rule-mappings.ts | 23 ++++++++--- .../test/plugin.test.ts | 4 +- .../rules_allLanguages.goldfile.json | 11 ++++++ 10 files changed, 146 insertions(+), 18 deletions(-) create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidInsecureHttpRemoteSiteSettingTest.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidInsecureHttpRemoteSiteSetting.xml diff --git a/packages/code-analyzer-pmd-engine/package.json b/packages/code-analyzer-pmd-engine/package.json index 759a72c8..34da3144 100644 --- a/packages/code-analyzer-pmd-engine/package.json +++ b/packages/code-analyzer-pmd-engine/package.json @@ -1,7 +1,7 @@ { "name": "@salesforce/code-analyzer-pmd-engine", "description": "Plugin package that adds 'pmd' and 'cpd' as engines into Salesforce Code Analyzer", - "version": "0.16.1", + "version": "0.16.2-SNAPSHOT", "author": "The Salesforce Code Analyzer Team", "license": "BSD-3-Clause", "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview", diff --git a/packages/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml b/packages/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml new file mode 100644 index 00000000..9dbca504 --- /dev/null +++ b/packages/code-analyzer-pmd-engine/pmd-rules/src/main/resources/sfca/rulesets/AppExchange_xml.xml @@ -0,0 +1,27 @@ + + + AppExchange Security Rules + + + + Detects instances of a Remote Site Settings that use HTTP.Use HTTPS instead. + 3 + + + + + + + + + + + \ No newline at end of file diff --git a/packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidInsecureHttpRemoteSiteSettingTest.java b/packages/code-analyzer-pmd-engine/pmd-rules/src/test/java/sfca/rulesets/appexchange_xml/AvoidInsecureHttpRemoteSiteSettingTest.java new file mode 100644 index 00000000..e2af13d6 --- /dev/null +++ b/packages/code-analyzer-pmd-engine/pmd-rules/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//xml/.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/packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidInsecureHttpRemoteSiteSetting.xml b/packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidInsecureHttpRemoteSiteSetting.xml new file mode 100644 index 00000000..96213e75 --- /dev/null +++ b/packages/code-analyzer-pmd-engine/pmd-rules/src/test/resources/sfca/rulesets/appexchange_xml/xml/AvoidInsecureHttpRemoteSiteSetting.xml @@ -0,0 +1,39 @@ + + + + + When url contains http then violation should be reported + 1 + 6 + + Avoid using insecure http urls in Remote Site Settings. + + + + Used for Apex callout to mapping web service + false + true + http://www.maptestsite.net/mapping1 + + ]]> + + + + When url contains https then violation should not be reported + 0 + + + Used for Apex callout to mapping web service + false + true + https://www.maptestsite.net/mapping1 + + ]]> + + + \ No newline at end of file diff --git a/packages/code-analyzer-pmd-engine/src/config.ts b/packages/code-analyzer-pmd-engine/src/config.ts index a34ae8b2..1afa6b2c 100644 --- 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 = new Map(); // To keep track if file extension shows up with more than one language const fileExtensionsMap: Record = {... 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(); + return arr.filter((str) => { + const lowerStr = str.toLowerCase(); + if (seen.has(lowerStr)) { + return false; + } + seen.add(lowerStr); + return true; + }); } \ No newline at end of file diff --git a/packages/code-analyzer-pmd-engine/src/constants.ts b/packages/code-analyzer-pmd-engine/src/constants.ts index fadb0a40..4bb3ef15 100644 --- 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.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 = { 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 index 1507c0ce..ad9d5afd 100644 --- 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 index c1a8e4c8..75ab4e5d 100644 --- 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 = { // ================================================================================================================= - // APEX RULES + // PMD-APEX RULES // ================================================================================================================= "ApexAssertionsShouldIncludeMessage": { severity: SeverityLevel.Moderate, @@ -269,7 +271,7 @@ export const RULE_MAPPINGS: Record { it.each(['cpd','pmd'])(`When createEngineConfig for '%s' is given a valid file_extensions value is passed to createEngineConfig, then it is set on the config`, async (engineName) => { const rawConfig: ConfigObject = { file_extensions: { - javaScriPT: ['.js', '.jsX', '.js'] + javaScriPT: ['.js', '.jsX', '.JS'] // And check we do uniqueness with case insensitivity } }; const configValueExtractor: ConfigValueExtractor = new ConfigValueExtractor(rawConfig, `engines.${engineName}`); 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/packages/code-analyzer-pmd-engine/test/test-data/pmdGoldfiles/rules_allLanguages.goldfile.json b/packages/code-analyzer-pmd-engine/test/test-data/pmdGoldfiles/rules_allLanguages.goldfile.json index a0d5ae6a..77e41a0f 100644 --- a/packages/code-analyzer-pmd-engine/test/test-data/pmdGoldfiles/rules_allLanguages.goldfile.json +++ b/packages/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,