TEST ONLY: combined #512 #514 #515 #516 #517

.github/workflows/publish-package-to-npm.yml

@@ -172,6 +172,10 @@ jobs:
   build-and-test-and-publish:
     runs-on: ubuntu-latest
     needs: [validate-packages-as-releasable, prepare-release-branch]
+    permissions:
+      # Required for npm Trusted Publishing (OIDC).
+      id-token: write
+      contents: read
     steps:
       - name: Support long paths
         run: git config --global core.longpaths true
@@ -180,7 +184,21 @@ jobs:
           ref: ${{ needs.prepare-release-branch.outputs.branch-name }}
       - uses: actions/setup-node@v4
         with:
-          node-version: 20
+          # Trusted Publishing requires Node >= 22.14.0.
+          node-version: 22
+      # Trusted Publishing requires npm >= 11.5.1, and managing dist-tags via OIDC requires npm >= 11.21.0.
+      # Pinned to the npm 11 line to stay consistent with the code-analyzer repo (npm 12 removed shrinkwrap support).
+      - name: Upgrade npm for Trusted Publishing
+        run: |
+          npm install -g npm@11
+          # Print the resolved version, and fail fast if the moving 'npm@11' tag ever resolves below what
+          # dist-tag management via OIDC needs, rather than failing later in the middle of a release.
+          NPM_VERSION="$(npm --version)"
+          echo "Using npm ${NPM_VERSION}"
+          if [ "$(printf '%s\n' "11.21.0" "${NPM_VERSION}" | sort -V | head -n1)" != "11.21.0" ]; then
+            echo "::error::npm ${NPM_VERSION} is older than the required 11.21.0"
+            exit 1
+          fi
       - name: Build and test and publish packages
         run: |
           PACKAGES_TO_PUBLISH=(${{ needs.validate-packages-as-releasable.outputs.packages-to-release }})
@@ -191,8 +209,8 @@ jobs:
             npm install
             npm run build
             npm run test
-            # We need the NPM token to publish.
-            echo "//registry.npmjs.org/:_authToken=${{ secrets.NPM_TOKEN }}" > ~/.npmrc
+            # No NPM token is needed. npm authenticates via Trusted Publishing (OIDC). Each package must have this
+            # workflow configured as a trusted publisher on npmjs.com, with "Allow npm dist-tag" enabled.
             PUBLISHED_VERSION=$(jq -r ".version" package.json)
             if [[ "${{ inputs.dryrun }}" == "true" ]]; then
               npm publish --tag latest-beta --access public --verbose --dry-run

packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py

@@ -763,7 +763,14 @@ def get_run_mode(self) -> RunMode:
         if len(elems) == 0:
             return RunMode.DefaultMode
         else:
-            return RunMode[elems[0].text]
+            declared = (elems[0].text or '').strip()
+            try:
+                return RunMode[declared]
+            except KeyError:
+                # A runInMode value we don't know about (for example, one added in a newer API version) must not crash
+                # the whole scan. Fall back to the default (user context) behavior and say so in the log.
+                logger.warning(f"Unrecognized runInMode '{declared}' in {self.flow_path}, treating it as DefaultMode")
+                return RunMode.DefaultMode
 
     def get_api_version(self) -> str:
         """Get the API version of the flow.
@@ -1085,6 +1092,10 @@ def build_vartype_from_elem(elem: El) -> VariableType | None:
         elif tag == 'decisions':
             return VariableType(tag=tag, reference=ReferenceType.NodeReference)
 
+        elif tag == 'ends':
+            # API v68+ top-level End element. Terminal node, holds no value.
+            return VariableType(tag=tag, reference=ReferenceType.NodeReference)
+
         elif tag == 'dynamicChoiceSets':
             # These are effectively record lookups
             # TODO: handle this better, right now we just have a stub

packages/code-analyzer-flow-engine/FlowScanner/public/enums.py

@@ -89,6 +89,7 @@ class RunMode(Enum):
     SystemModeWithoutSharing = 0
     SystemModeWithSharing = 1
     DefaultMode = 2
+    UserMode = 3  # API v68+: runInMode=UserMode. Runs in user context, so it behaves like DefaultMode.
 
 
 class DataType(Enum):

packages/code-analyzer-flow-engine/FlowScanner/public/parse_utils.py

@@ -80,6 +80,7 @@
                   "collectionProcessors",
                   "customErrors",
                   "decisions",
+                  "ends",
                   "loops",
                   "orchestratedStages",
                   "recordLookups",

packages/code-analyzer-flow-engine/FlowScanner/queries/optional_query.py

@@ -24,7 +24,7 @@
 DEFAULT_HELP_URL = ("https://developer.salesforce.com/docs/atlas.en-us.secure_coding_guide.meta"
                     "/secure_coding_guide/secure_coding_considerations_flow_design.htm")
 
-id_pattern = re.compile(r"^(?=.*[0-9][0-9])[a-zA-Z0-9]+[a-zA-Z0-9]{15}(?:[0-5][A-Z0-5]{3})?$")
+id_pattern = re.compile(r"^[a-zA-Z0-9]{5}0[a-zA-Z0-9]{9}(?:[A-Z0-5]{3})?$")
 copy_name_pattern = re.compile(r"^Copy_\d+_of_[a-zA-Z]+")
 copy_label_pattern = re.compile(r"^Copy \d+ of [a-zA-Z]+")
 

packages/code-analyzer-flow-engine/FlowScanner/tests/test_parse_utils.py

@@ -71,5 +71,18 @@ def test_missing_file_returns_none(self):
         self.assertIsNone(pu.quick_validate(self.flow_path + '.does-not-exist'))
 
 
+class EndsElementTest(unittest.TestCase):
+    """Regression tests for issue #2098: API v68 top-level <ends> elements."""
+
+    def test_ends_is_control_flow_element(self):
+        self.assertIn('ends', pu.CTRL_FLOW_ELEM)
+
+    def test_ends_has_variable_type(self):
+        import xml.etree.ElementTree as ET
+        from flow_parser import parse
+        elem = ET.fromstring('<ends xmlns="http://soap.sforce.com/2006/04/metadata"><name>End1</name></ends>')
+        self.assertIsNotNone(parse.build_vartype_from_elem(elem))
+
+
 if __name__ == '__main__':
     unittest.main()

packages/code-analyzer-flow-engine/FlowScanner/tests/test_run_mode.py

@@ -0,0 +1,64 @@
+"""Tests for how the parser handles the flow's runInMode element.
+
+Run from the FlowScanner directory with::
+
+    python3 -m unittest tests.test_run_mode
+"""
+import os
+import tempfile
+import unittest
+
+from flow_parser.parse import Parser
+from public.enums import RunMode
+from flow_scanner import util
+
+FLOW_TEMPLATE = (
+    '<?xml version="1.0" encoding="UTF-8"?>\n'
+    '<Flow xmlns="http://soap.sforce.com/2006/04/metadata">'
+    '<apiVersion>68.0</apiVersion>'
+    '<label>RunModeTest</label>'
+    '<processType>AutoLaunchedFlow</processType>'
+    '{run_in_mode}'
+    '<status>Active</status>'
+    '<start><locationX>0</locationX><locationY>0</locationY></start>'
+    '</Flow>'
+)
+
+
+def parse_flow_with_run_in_mode(run_in_mode_value):
+    """Writes a minimal flow to disk and returns its parser's declared run mode."""
+    element = '' if run_in_mode_value is None else f'<runInMode>{run_in_mode_value}</runInMode>'
+    with tempfile.TemporaryDirectory() as tmp:
+        path = os.path.join(tmp, 'RunModeTest.flow-meta.xml')
+        with open(path, 'w', encoding='utf-8') as f:
+            f.write(FLOW_TEMPLATE.format(run_in_mode=element))
+        return Parser.from_file(path).get_declared_run_mode()
+
+
+class RunInModeTest(unittest.TestCase):
+    """Regression tests for API v68's runInMode=UserMode, which used to raise KeyError and crash the scan."""
+
+    def test_user_mode_is_recognized(self):
+        self.assertIs(parse_flow_with_run_in_mode('UserMode'), RunMode.UserMode)
+
+    def test_existing_modes_are_unchanged(self):
+        self.assertIs(parse_flow_with_run_in_mode('SystemModeWithoutSharing'), RunMode.SystemModeWithoutSharing)
+        self.assertIs(parse_flow_with_run_in_mode('SystemModeWithSharing'), RunMode.SystemModeWithSharing)
+        self.assertIs(parse_flow_with_run_in_mode('DefaultMode'), RunMode.DefaultMode)
+
+    def test_missing_run_in_mode_is_default_mode(self):
+        self.assertIs(parse_flow_with_run_in_mode(None), RunMode.DefaultMode)
+
+    def test_unknown_run_in_mode_falls_back_to_default_mode_instead_of_crashing(self):
+        with self.assertLogs('flow_parser.parse', level='WARNING'):
+            self.assertIs(parse_flow_with_run_in_mode('SomeFutureMode'), RunMode.DefaultMode)
+
+    def test_user_mode_subflow_inherits_the_parent_mode(self):
+        # Like DefaultMode, UserMode must not override a system mode that a parent flow is already running in
+        self.assertIs(util.get_effective_run_mode(RunMode.SystemModeWithoutSharing, RunMode.UserMode),
+                      RunMode.SystemModeWithoutSharing)
+        self.assertIs(util.get_effective_run_mode(None, RunMode.UserMode), RunMode.UserMode)
+
+
+if __name__ == '__main__':
+    unittest.main()

packages/code-analyzer-flow-engine/FlowScanner/tests/test_salesforce_id.py

@@ -0,0 +1,32 @@
+"""Tests for queries.optional_query.is_valid_salesforce_id.
+
+Run from the FlowScanner directory with::
+
+    python3 -m unittest tests.test_salesforce_id
+"""
+import unittest
+
+from queries.optional_query import is_valid_salesforce_id, to18
+
+
+class IsValidSalesforceIdTest(unittest.TestCase):
+    def test_valid_18_char_id(self):
+        self.assertTrue(is_valid_salesforce_id("001cU000009Tt2MQAS"))
+
+    def test_valid_15_char_id(self):
+        self.assertTrue(is_valid_salesforce_id("001cU000009Tt2M"))
+
+    def test_valid_18_char_id_without_adjacent_digits(self):
+        base = "aBcDe0FgHiJkLmN"
+        self.assertTrue(is_valid_salesforce_id(to18(base)))
+
+    def test_invalid_18_char_checksum(self):
+        self.assertFalse(is_valid_salesforce_id("001cU000009Tt2MAAA"))
+
+    def test_rejects_other_lengths(self):
+        for word in ["abcdefghij11klmn", "abcdefghij11klmno", "abcdefghijk11lmnopq", "short11"]:
+            self.assertFalse(is_valid_salesforce_id(word), word)
+
+
+if __name__ == "__main__":
+    unittest.main()

packages/code-analyzer-flow-engine/src/messages.ts

@@ -20,6 +20,12 @@ const MESSAGE_CATALOG: {[key: string]: string} = {
     ResultsFileNotValidJson:
         `Unexpected error: Results file contents are not a valid JSON: %s`,
 
+    ResultsFileNotProduced:
+        `Unexpected error: The Flow Scanner did not produce a results file, which usually means it failed to parse one or more flows. See the log file for details: %s`,
+
+    ResultsFileNotReadable:
+        `Unexpected error: The Flow Scanner results file '%s' could not be read: %s`,
+
     CouldNotParseExecutionResults:
         `Unexpected error: Could not parse results from %s`,
 

packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts

@@ -101,7 +101,18 @@ export class RunTimeFlowScannerCommandWrapper implements FlowScannerCommandWrapp
 
         await this.pythonCommandExecutor.exec(pythonArgs, processStdout);
 
-        const outputFileContents: string = await fs.promises.readFile(flowScannerResultsFile, 'utf-8');
+        let outputFileContents: string;
+        try {
+            outputFileContents = await fs.promises.readFile(flowScannerResultsFile, 'utf-8');
+        } catch (err) {
+            // Only a missing file means the scanner failed to produce results. Any other failure (permissions, the
+            // path being a directory, etc.) is a different problem, so we keep the original error as the cause.
+            if ((err as NodeJS.ErrnoException)?.code === 'ENOENT') {
+                throw new Error(getMessage('ResultsFileNotProduced', absLogFilePath));
+            }
+            throw new Error(getMessage('ResultsFileNotReadable', flowScannerResultsFile, (err as Error)?.message ?? String(err)),
+                {cause: err});
+        }
 
         let parsedResults: object;
         try {

packages/code-analyzer-flow-engine/test/python/FlowScannerCommandWrapper.test.ts

@@ -184,6 +184,25 @@ describe('FlowScannerCommandWrapper implementations', () => {
                         .rejects
                         .toThrow(expectedMessage);
                 });
+
+                it.each([
+                    {code: 'ENOENT', expectedMessage: 'did not produce a results file'},
+                    {code: 'EACCES', expectedMessage: 'could not be read'},
+                    {code: 'EISDIR', expectedMessage: 'could not be read'}
+                ])('When reading the results file fails with $code, the error message reflects the real problem', async ({code, expectedMessage}) => {
+                    jest.spyOn(PythonCommandExecutor.prototype, 'exec').mockImplementation(async (_args, _processStdout) => {
+                        return Promise.resolve();
+                    });
+                    const fsError: NodeJS.ErrnoException = Object.assign(new Error(`${code}: simulated failure`), {code});
+                    jest.spyOn(fs.promises, 'readFile').mockRejectedValue(fsError);
+
+                    const wrapper: RunTimeFlowScannerCommandWrapper = new RunTimeFlowScannerCommandWrapper(PYTHON_COMMAND);
+                    const promise = wrapper.runFlowScannerRules(workingFolder, [PATH_TO_EXAMPLE1, PATH_TO_EXAMPLE2], [PATH_TO_EXAMPLE1, PATH_TO_EXAMPLE2], tempLogFile, [], (_num: number) => {});
+                    await expect(promise).rejects.toThrow(expectedMessage);
+                    if (code !== 'ENOENT') {
+                        await expect(promise).rejects.toMatchObject({cause: fsError});
+                    }
+                });
             });
         });
     });

packages/code-analyzer-pmd-engine/src/config.ts

@@ -42,7 +42,7 @@ export type PmdEngineConfig = {
     // List of xml ruleset files containing custom PMD rules to be made available for rule selection.
     // Each ruleset must be an xml file that is either::
     //   - on disk (provided as an absolute path or a relative path to 'config_root')
-    //   - or a relative resource found on the Java classpath.
+    //   - or a relative resource found on the Java classpath (or, for a ruleset that references another ruleset file by a path relative to the folder you run Code Analyzer from, that referenced file).
     // Not all custom rules can be fully defined within an xml ruleset file. For example, Java based rules may be defined in jar files.
     // In these cases, you will need to also add your additional files to the Java classpath using the 'java_classpath_entries' field.
     // See https://pmd.github.io/pmd/pmd_userdocs_making_rulesets.html

packages/code-analyzer-pmd-engine/src/messages.ts

@@ -27,7 +27,7 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
         `List of xml ruleset files containing custom PMD rules to be made available for rule selection.\n` +
         `Each ruleset must be an xml file that is either:\n` +
         `  - on disk (provided as an absolute path or a relative path to 'config_root')\n` +
-        `  - or a relative resource found on the Java classpath.\n` +
+        `  - or a relative resource found on the Java classpath (or, for a ruleset that references another ruleset file by a path relative to the folder you run Code Analyzer from, that referenced file).\n` +
         `Not all custom rules can be fully defined within an xml ruleset file. For example, Java based rules may be defined in jar files.\n` +
         `In these cases, you will need to also add your additional files to the Java classpath using the 'java_classpath_entries' field.\n` +
         `See https://pmd.github.io/pmd/pmd_userdocs_making_rulesets.html to learn more about PMD rulesets.`,

packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts

@@ -1,6 +1,7 @@
 import {JavaCommandExecutor} from "@salesforce/code-analyzer-engine-api/utils";
 import path from "node:path";
 import fs from "node:fs";
+import {stageReferencedRulesets} from "./ruleset-staging";
 import {getMessageFromCatalog, LogLevel, SHARED_MESSAGE_CATALOG} from "@salesforce/code-analyzer-engine-api";
 
 const PMD_WRAPPER_JAVA_CLASS: string = "com.salesforce.sfca.pmdwrapper.PmdWrapper";
@@ -78,6 +79,26 @@ export class PmdWrapperInvoker {
         this.emitLogEvent = emitLogEvent;
     }
 
+    /**
+     * Creates the Java classpath. Note that the Java process is pinned to the engine's install folder (instead of the
+     * inherited, possibly untrusted, folder being scanned) to avoid CWE-427. But PMD resolves refs inside of custom
+     * rulesets (like <rule ref="path/in/repo.xml/Rule"/>) relative to the cwd or the classpath. To keep those refs
+     * resolving without exposing the entire repository to the JVM as classes/resources, we stage copies of only the
+     * referenced xml ruleset files into a dedicated folder that we place last on the classpath.
+     */
+    private async createJavaClassPaths(rulesets: string[], workingFolder: string): Promise<string[]> {
+        const classPaths: string[] = [
+            path.join(PMD_WRAPPER_LIB_FOLDER, '*'),
+            ...this.userProvidedJavaClasspathEntries.map(toJavaClasspathEntry)
+        ];
+        const stagingFolder: string | undefined = await stageReferencedRulesets(
+            rulesets, process.cwd(), path.join(workingFolder, 'stagedRulesets'));
+        if (stagingFolder) {
+            classPaths.push(stagingFolder + path.sep); // Last so that it has the lowest precedence
+        }
+        return classPaths;
+    }
+
     async invokeDescribeCommand(customRulesets: string[], pmdRuleLanguageIds: string[],
         workingFolder: string, emitProgress: (percComplete: number) => void): Promise<PmdRuleInfo[]> {
 
@@ -87,10 +108,7 @@ export class PmdWrapperInvoker {
         emitProgress(10);
 
         const javaCmdArgs: string[] = [PMD_WRAPPER_JAVA_CLASS, 'describe', pmdRulesOutputFile, customRulesetsListFile, pmdRuleLanguageIds.join(',')];
-        const javaClassPaths: string[] = [
-            path.join(PMD_WRAPPER_LIB_FOLDER, '*'),
-            ... this.userProvidedJavaClasspathEntries.map(toJavaClasspathEntry)
-        ];
+        const javaClassPaths: string[] = await this.createJavaClassPaths(customRulesets, workingFolder);
         await this.javaCommandExecutor.exec(javaCmdArgs, javaClassPaths, (stdOutMsg) => {
             if (stdOutMsg.startsWith(STDOUT_ERROR_MARKER)) {
                 const errorMessage: string = stdOutMsg.slice(STDOUT_ERROR_MARKER.length).replaceAll('{NEWLINE}','\n');
@@ -138,10 +156,8 @@ export class PmdWrapperInvoker {
 
         const resultsOutputFile: string = path.join(workingFolder, 'resultsFile.json');
         const javaCmdArgs: string[] = [PMD_WRAPPER_JAVA_CLASS, 'run', inputFile, resultsOutputFile];
-        const javaClassPaths: string[] = [
-            path.join(PMD_WRAPPER_LIB_FOLDER, '*'),
-            ... this.userProvidedJavaClasspathEntries.map(toJavaClasspathEntry)
-        ];
+        const javaClassPaths: string[] = await this.createJavaClassPaths(
+            [...new Set(pmdRuleInfoList.map(ruleInfo => ruleInfo.ruleSetFile))], workingFolder);
         this.emitLogEvent(LogLevel.Fine, `Calling JAVA command with class path containing ${JSON.stringify(javaClassPaths)} and arguments: ${JSON.stringify(javaCmdArgs)}`);
         await this.javaCommandExecutor.exec(javaCmdArgs, javaClassPaths, (stdOutMsg: string) => {
             if (stdOutMsg.startsWith(STDOUT_PROGRESS_MARKER)) {
@@ -189,10 +205,7 @@ export class PmdWrapperInvoker {
 
         const resultsOutputFile: string = path.join(workingFolder, 'astDumpResults.json');
         const javaCmdArgs: string[] = [PMD_WRAPPER_JAVA_CLASS, 'ast-dump', inputFile, resultsOutputFile];
-        const javaClassPaths: string[] = [
-            path.join(PMD_WRAPPER_LIB_FOLDER, '*'),
-            ...this.userProvidedJavaClasspathEntries.map(toJavaClasspathEntry)
-        ];
+        const javaClassPaths: string[] = await this.createJavaClassPaths([], workingFolder);
 
         this.emitLogEvent(LogLevel.Fine, `Calling AST dump for file: ${fileToDump}`);