From ad74c3afa5b84868750e144617b65be7b8921039 Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Tue, 27 Oct 2026 11:35:21 +0530 Subject: [PATCH 01/13] FIX Remediate SFGE transitive dependency vulnerabilities Removes all known-vulnerable libraries from the SFGE Java runtime classpath (osv-scanner on dist/java-lib: 84 advisories across 24 artifacts -> 0; 97 JARs -> 35). - Remove unused gremlin-driver (and the netty-all constraint). It was the sole source of Netty and AWS SDK JARs. - Move Gremlin from 4.0.0-beta.1 to stable 3.7.7. Every 4.0 beta bundles Jackson 2.15.2 inside gremlin-shaded. (3.8.2 changes traversal behavior and breaks tests.) - Exclude gremlin-shaded (only used for GraphSON/Gryo/GraphML IO, which SFGE does not use; its bundled Jackson cannot be patched via constraints). Add a minimal JsonProcessingException stand-in because TinkerGraph references the type. - Exclude commons-collections 3.x (no patched release; SFGE uses collections4). - Upgrade log4j to 2.25.5; force patched commons-configuration2, commons-beanutils and snakeyaml; declare commons-lang3 and commons-text explicitly. - Add verifyNoBannedDependencies build check and NoVulnerableBundledLibrariesTest to prevent regressions. Co-Authored-By: Claude Sonnet 5.5 --- .../gradle/libs.versions.toml | 21 ++++--- .../sfge/build.gradle.kts | 61 +++++++++++++++++-- .../jackson/core/JsonProcessingException.java | 23 +++++++ .../ops/NoVulnerableBundledLibrariesTest.java | 43 +++++++++++++ 4 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 packages/code-analyzer-sfge-engine/sfge/src/main/java/org/apache/tinkerpop/shaded/jackson/core/JsonProcessingException.java create mode 100644 packages/code-analyzer-sfge-engine/sfge/src/test/java/com/salesforce/graph/ops/NoVulnerableBundledLibrariesTest.java diff --git a/packages/code-analyzer-sfge-engine/gradle/libs.versions.toml b/packages/code-analyzer-sfge-engine/gradle/libs.versions.toml index ab26b93f..423a8c57 100644 --- a/packages/code-analyzer-sfge-engine/gradle/libs.versions.toml +++ b/packages/code-analyzer-sfge-engine/gradle/libs.versions.toml @@ -10,11 +10,10 @@ hamcrest = "3.0" junit-jupiter = "5.13.4" commons-cli = "1.10.0" commons-collections4 = "4.5.0" -tinkergraph-gremlin = "4.0.0-beta.1" -gremlin-driver = "4.0.0-beta.1" +tinkergraph-gremlin = "3.7.7" antlr-runtime = "3.5.3" -log4j-api = "2.25.1" -log4j-core = "2.25.1" +log4j-api = "2.25.5" +log4j-core = "2.25.5" gson = "2.13.2" guava = "33.4.8-jre" jsr305 = "3.0.2" @@ -23,7 +22,12 @@ reflections = "0.10.2" asm = "9.8" mockito-core = "5.19.0" mockito-junit-jupiter = "5.19.0" -netty-all = "4.1.127.Final" +# Versions below force patched releases of dependencies that arrive transitively (via gremlin-core). See sfge/build.gradle.kts. +commons-configuration2 = "2.15.1" +commons-beanutils = "1.11.0" +commons-lang3 = "3.21.0" +commons-text = "1.15.0" +snakeyaml = "2.7" [libraries] hamcrest = { module = "org.hamcrest:hamcrest", version.ref = "hamcrest" } @@ -31,7 +35,6 @@ junit-jupiter = { module = "org.junit.jupiter:junit-jupiter", version.ref = "jun commons-cli = { module = "commons-cli:commons-cli", version.ref = "commons-cli" } commons-collections4 = { module = "org.apache.commons:commons-collections4", version.ref = "commons-collections4" } tinkergraph-gremlin = { module = "org.apache.tinkerpop:tinkergraph-gremlin", version.ref = "tinkergraph-gremlin" } -gremlin-driver = { module = "org.apache.tinkerpop:gremlin-driver", version.ref = "gremlin-driver" } antlr-runtime = { module = "org.antlr:antlr-runtime", version.ref = "antlr-runtime" } log4j-api = { module = "org.apache.logging.log4j:log4j-api", version.ref = "log4j-api" } log4j-core = { module = "org.apache.logging.log4j:log4j-core", version.ref = "log4j-core" } @@ -44,4 +47,8 @@ asm = { module = "org.ow2.asm:asm", version.ref = "asm" } junit-platform-launcher = { module = "org.junit.platform:junit-platform-launcher" } mockito-core = { module = "org.mockito:mockito-core", version.ref = "mockito-core" } mockito-junit-jupiter = { module = "org.mockito:mockito-junit-jupiter", version.ref = "mockito-junit-jupiter" } -netty-all = { module = "io.netty:netty-all", version.ref = "netty-all" } +commons-configuration2 = { module = "org.apache.commons:commons-configuration2", version.ref = "commons-configuration2" } +commons-beanutils = { module = "commons-beanutils:commons-beanutils", version.ref = "commons-beanutils" } +commons-lang3 = { module = "org.apache.commons:commons-lang3", version.ref = "commons-lang3" } +commons-text = { module = "org.apache.commons:commons-text", version.ref = "commons-text" } +snakeyaml = { module = "org.yaml:snakeyaml", version.ref = "snakeyaml" } diff --git a/packages/code-analyzer-sfge-engine/sfge/build.gradle.kts b/packages/code-analyzer-sfge-engine/sfge/build.gradle.kts index f5d29853..a1619f18 100644 --- a/packages/code-analyzer-sfge-engine/sfge/build.gradle.kts +++ b/packages/code-analyzer-sfge-engine/sfge/build.gradle.kts @@ -22,8 +22,18 @@ repositories { dependencies { implementation(libs.commons.cli) implementation(libs.commons.collections4) - implementation(libs.tinkergraph.gremlin) - implementation(libs.gremlin.driver) + implementation(libs.tinkergraph.gremlin) { + // commons-collections 3.x (pulled in by commons-beanutils) has known vulnerabilities and no patched release. + // SFGE and the Gremlin graph layer do not use it, so it is removed from the classpath entirely. + exclude(group = "commons-collections", module = "commons-collections") + // gremlin-shaded bundles (relocated) copies of Jackson and Kryo that are only used for graph import/export + // (GraphSON/Gryo/GraphML). SFGE only builds and traverses an in-memory graph, and the bundled Jackson version + // contains known vulnerabilities that cannot be patched via dependency constraints, so we do not ship it. + exclude(group = "org.apache.tinkerpop", module = "gremlin-shaded") + } + // These are used directly by SFGE source (they previously arrived only transitively), so declare them explicitly. + implementation(libs.commons.lang3) + implementation(libs.commons.text) implementation(libs.antlr.runtime) implementation(libs.log4j.api) implementation(libs.log4j.core) @@ -35,9 +45,16 @@ dependencies { } implementation(libs.reflections) implementation(libs.asm) + // Force patched versions of transitive dependencies of gremlin-core (embedded in-memory graph). constraints { - implementation(libs.netty.all) { - because("Force specific version of transitive dependency") + implementation(libs.commons.configuration2) { + because("Patched version of a transitive dependency of gremlin-core") + } + implementation(libs.commons.beanutils) { + because("Patched version of a transitive dependency of gremlin-core") + } + implementation(libs.snakeyaml) { + because("Patched version of a transitive dependency of gremlin-core") } } implementation(files("lib/apex-jorje-lsp-sfge.jar")) @@ -76,6 +93,42 @@ tasks.build { dependsOn("copyDependencies") // Ensure dependencies are copied during the build process } +// Fail the build if a library that is known to carry unpatchable vulnerabilities (or that SFGE has no need for) sneaks +// back onto the runtime classpath, e.g., through a Gremlin upgrade. Everything on the runtime classpath gets shipped. +val bannedRuntimeDependencies = mapOf( + "io.netty" to "Only needed by gremlin-driver (remote Gremlin client), which SFGE does not use", + "software.amazon.awssdk" to "Only needed by gremlin-driver (remote Gremlin client), which SFGE does not use", + "org.apache.tinkerpop:gremlin-driver" to "SFGE only uses the embedded in-memory graph", + "org.apache.tinkerpop:gremlin-util" to "SFGE only uses the embedded in-memory graph", + "org.apache.tinkerpop:gremlin-shaded" to "Bundles Jackson/Kryo with known vulnerabilities that cannot be patched via constraints", + "commons-collections:commons-collections" to "3.x has known vulnerabilities and no patched release (SFGE uses commons-collections4)" +) +tasks.register("verifyNoBannedDependencies") { + group = "verification" + description = "Fails if a banned dependency is on the runtime classpath (and therefore would be shipped)." + val resolvedModules = configurations.runtimeClasspath.map { config -> + config.incoming.resolutionResult.allComponents + .mapNotNull { it.moduleVersion } + .map { "${it.group}:${it.name}:${it.version}" } + } + doLast { + val violations = resolvedModules.get().flatMap { coordinates -> + val (group, name) = coordinates.split(":") + bannedRuntimeDependencies.filter { (banned, _) -> banned == group || banned == "$group:$name" } + .map { (_, reason) -> "$coordinates -> $reason" } + } + if (violations.isNotEmpty()) { + throw GradleException("Banned dependencies found on the runtime classpath:\n " + violations.joinToString("\n ")) + } + } +} +tasks.check { + dependsOn("verifyNoBannedDependencies") +} +tasks.build { + dependsOn("verifyNoBannedDependencies") +} + // ======== TEST RELATED TASKS ========================================================================================= jacoco { diff --git a/packages/code-analyzer-sfge-engine/sfge/src/main/java/org/apache/tinkerpop/shaded/jackson/core/JsonProcessingException.java b/packages/code-analyzer-sfge-engine/sfge/src/main/java/org/apache/tinkerpop/shaded/jackson/core/JsonProcessingException.java new file mode 100644 index 00000000..300ba775 --- /dev/null +++ b/packages/code-analyzer-sfge-engine/sfge/src/main/java/org/apache/tinkerpop/shaded/jackson/core/JsonProcessingException.java @@ -0,0 +1,23 @@ +package org.apache.tinkerpop.shaded.jackson.core; + +import java.io.IOException; + +/** + * Minimal stand-in for the relocated Jackson exception type that {@code TinkerGraph} references in a {@code catch} + * clause. + * + *

Why this exists: {@code gremlin-shaded} bundles (relocated) copies of Jackson and Kryo. The Jackson version it + * bundles contains known vulnerabilities that cannot be fixed with dependency constraints, and SFGE does not use it + * since SFGE only builds and traverses an in-memory graph (no GraphSON/Gryo/GraphML import or export). So we exclude + * {@code gremlin-shaded} from the runtime classpath (see {@code build.gradle.kts}). However, the JVM must still be able + * to load this one exception class when it verifies {@code TinkerGraph}, even though no Jackson code ever runs. + * + *

This class intentionally contains no Jackson code. Remove it (and re-evaluate the {@code gremlin-shaded} exclusion) + * when upgrading to a Gremlin release whose bundled Jackson is patched. {@code NoVulnerableBundledLibrariesTest} + * guards this behavior. + */ +public class JsonProcessingException extends IOException { + public JsonProcessingException(String message) { + super(message); + } +} diff --git a/packages/code-analyzer-sfge-engine/sfge/src/test/java/com/salesforce/graph/ops/NoVulnerableBundledLibrariesTest.java b/packages/code-analyzer-sfge-engine/sfge/src/test/java/com/salesforce/graph/ops/NoVulnerableBundledLibrariesTest.java new file mode 100644 index 00000000..679f3532 --- /dev/null +++ b/packages/code-analyzer-sfge-engine/sfge/src/test/java/com/salesforce/graph/ops/NoVulnerableBundledLibrariesTest.java @@ -0,0 +1,43 @@ +package com.salesforce.graph.ops; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +/** + * Guards the dependency-hygiene decisions documented in {@code build.gradle.kts}: SFGE only needs an in-memory + * TinkerGraph, and must not ship libraries that carry known vulnerabilities. + */ +public class NoVulnerableBundledLibrariesTest { + @Test + public void inMemoryGraphWorksWithoutGremlinShaded() { + // Fails with NoClassDefFoundError if TinkerGraph needs anything from gremlin-shaded beyond what we provide. + assertNotNull(GraphUtil.getGraph()); + } + + @Test + public void gremlinShadedJacksonImplementationIsNotOnClasspath() { + assertThrows( + ClassNotFoundException.class, + () -> Class.forName("org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper")); + assertThrows( + ClassNotFoundException.class, + () -> Class.forName("org.apache.tinkerpop.shaded.jackson.core.JsonFactory")); + } + + @Test + public void commonsCollections3IsNotOnClasspath() { + assertThrows( + ClassNotFoundException.class, + () -> Class.forName("org.apache.commons.collections.FastHashMap")); + } + + @Test + public void nettyAndGremlinDriverAreNotOnClasspath() { + assertThrows(ClassNotFoundException.class, () -> Class.forName("io.netty.channel.Channel")); + assertThrows( + ClassNotFoundException.class, + () -> Class.forName("org.apache.tinkerpop.gremlin.driver.Cluster")); + } +} From 6629d6b9865f60144521dd2ed200fe2120fab006 Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Tue, 27 Oct 2026 12:27:14 +0530 Subject: [PATCH 02/13] Bump SFGE engine version to 0.26.0-SNAPSHOT Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/code-analyzer-sfge-engine/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/code-analyzer-sfge-engine/package.json b/packages/code-analyzer-sfge-engine/package.json index b9583ecc..2826a8d3 100644 --- a/packages/code-analyzer-sfge-engine/package.json +++ b/packages/code-analyzer-sfge-engine/package.json @@ -1,7 +1,7 @@ { "name": "@salesforce/code-analyzer-sfge-engine", "description": "Plugin package that adds 'Salesforce Graph Engine' as an engine into Salesforce Code Analyzer", - "version": "0.25.0", + "version": "0.26.0-SNAPSHOT", "author": "The Salesforce Code Analyzer Team", "license": "BSD-3-Clause", "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview", From f36bfcfafd783fe382ae5c0d16bbab8f1941057a Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Thu, 29 Oct 2026 12:34:39 +0530 Subject: [PATCH 03/13] CHANGE @W-00000000@ Migrate npm publishing to Trusted Publishing (OIDC) Replace the long-lived NPM_TOKEN with npm Trusted Publishing, ahead of npm's bypass-2fa publish deprecation. - Grant id-token: write on the publish job - Move the publish job to Node 22 and upgrade npm (Trusted Publishing needs Node >= 22.14.0 and npm >= 11.5.1; OIDC dist-tag needs npm >= 11.21.0) - Drop the NPM_TOKEN .npmrc write Requires each published package to have this workflow configured as a trusted publisher on npmjs.com (with "Allow npm dist-tag" enabled) before merging. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/publish-package-to-npm.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/publish-package-to-npm.yml b/.github/workflows/publish-package-to-npm.yml index 40698451..2e2be834 100644 --- a/.github/workflows/publish-package-to-npm.yml +++ b/.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,11 @@ 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. + - name: Upgrade npm for Trusted Publishing + run: npm install -g npm@latest - name: Build and test and publish packages run: | PACKAGES_TO_PUBLISH=(${{ needs.validate-packages-as-releasable.outputs.packages-to-release }}) @@ -191,8 +199,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 From 67a5e569d5b2e1322df12d5095d2848f38f257f1 Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Thu, 29 Oct 2026 13:14:11 +0530 Subject: [PATCH 04/13] CHANGE @W-00000000@ Pin npm to 11 for Trusted Publishing Install npm@11 (>= 11.21.0 has OIDC dist-tag support) instead of latest, to stay consistent with the code-analyzer repo, which must stay on npm 11 because npm 12 removed shrinkwrap support. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/publish-package-to-npm.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/publish-package-to-npm.yml b/.github/workflows/publish-package-to-npm.yml index 2e2be834..f1a1072e 100644 --- a/.github/workflows/publish-package-to-npm.yml +++ b/.github/workflows/publish-package-to-npm.yml @@ -187,8 +187,9 @@ jobs: # 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@latest + run: npm install -g npm@11 - name: Build and test and publish packages run: | PACKAGES_TO_PUBLISH=(${{ needs.validate-packages-as-releasable.outputs.packages-to-release }}) From 2926c973761224a9e3e3d4f9d1f84b849ea24685 Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Thu, 29 Oct 2026 14:02:38 +0530 Subject: [PATCH 05/13] CHANGE @W-24708955@ Retrigger PR checks with work item Co-Authored-By: Claude Opus 5.5 (1M context) From bcbe2d368186f8b0b925c0ca880687e029492ec4 Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Mon, 2 Nov 2026 16:38:09 +0530 Subject: [PATCH 06/13] fix: support API v68 ends elements in flow scanner Co-Authored-By: Claude Sonnet 5.5 --- .../FlowScanner/flow_parser/parse.py | 4 ++++ .../FlowScanner/public/parse_utils.py | 1 + .../FlowScanner/tests/test_parse_utils.py | 13 +++++++++++++ packages/code-analyzer-flow-engine/src/messages.ts | 3 +++ .../src/python/FlowScannerCommandWrapper.ts | 7 ++++++- 5 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py b/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py index b14dd479..a8e723a0 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py +++ b/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py @@ -1085,6 +1085,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 diff --git a/packages/code-analyzer-flow-engine/FlowScanner/public/parse_utils.py b/packages/code-analyzer-flow-engine/FlowScanner/public/parse_utils.py index 7c06d28c..ea9b8cab 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/public/parse_utils.py +++ b/packages/code-analyzer-flow-engine/FlowScanner/public/parse_utils.py @@ -80,6 +80,7 @@ "collectionProcessors", "customErrors", "decisions", + "ends", "loops", "orchestratedStages", "recordLookups", diff --git a/packages/code-analyzer-flow-engine/FlowScanner/tests/test_parse_utils.py b/packages/code-analyzer-flow-engine/FlowScanner/tests/test_parse_utils.py index eb7fc58e..3fea3341 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/tests/test_parse_utils.py +++ b/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 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('End1') + self.assertIsNotNone(parse.build_vartype_from_elem(elem)) + + if __name__ == '__main__': unittest.main() diff --git a/packages/code-analyzer-flow-engine/src/messages.ts b/packages/code-analyzer-flow-engine/src/messages.ts index 0dba4231..7b8ce95a 100644 --- a/packages/code-analyzer-flow-engine/src/messages.ts +++ b/packages/code-analyzer-flow-engine/src/messages.ts @@ -20,6 +20,9 @@ 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`, + CouldNotParseExecutionResults: `Unexpected error: Could not parse results from %s`, diff --git a/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts b/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts index 257de506..5fe6d0c7 100644 --- a/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts +++ b/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts @@ -101,7 +101,12 @@ 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) { + throw new Error(getMessage('ResultsFileNotProduced', absLogFilePath)); + } let parsedResults: object; try { From 46ddc1f6dc786853bea68c136eced24eab987254 Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Mon, 2 Nov 2026 17:21:38 +0530 Subject: [PATCH 07/13] fix: add scan folder to PMD classpath so repo-relative rule refs in custom rulesets resolve Co-Authored-By: Claude Sonnet 5.5 --- .../code-analyzer-pmd-engine/src/config.ts | 2 +- .../code-analyzer-pmd-engine/src/messages.ts | 2 +- .../src/pmd-wrapper.ts | 26 ++++++++++--------- .../test/pmd-engine.test.ts | 26 +++++++++++++++++++ 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/packages/code-analyzer-pmd-engine/src/config.ts b/packages/code-analyzer-pmd-engine/src/config.ts index d7a79210..cc0660b8 100644 --- a/packages/code-analyzer-pmd-engine/src/config.ts +++ b/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 (which implicitly includes the folder you run Code Analyzer from). // 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 diff --git a/packages/code-analyzer-pmd-engine/src/messages.ts b/packages/code-analyzer-pmd-engine/src/messages.ts index 6884c9dd..03298cee 100644 --- a/packages/code-analyzer-pmd-engine/src/messages.ts +++ b/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 (which implicitly includes the folder you run Code Analyzer from).\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.`, diff --git a/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts b/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts index ed26a98f..c6815701 100644 --- a/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts +++ b/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts @@ -78,6 +78,17 @@ export class PmdWrapperInvoker { this.emitLogEvent = emitLogEvent; } + private createJavaClassPaths(): string[] { + return [ + path.join(PMD_WRAPPER_LIB_FOLDER, '*'), + ...this.userProvidedJavaClasspathEntries.map(toJavaClasspathEntry), + // The java process is pinned to the engine's install folder, but PMD resolves refs inside custom rulesets + // (like ) relative to the cwd or the classpath. So we add the user's + // cwd (last, so it has the lowest precedence) so that repo-relative refs continue to resolve. + process.cwd() + ]; + } + async invokeDescribeCommand(customRulesets: string[], pmdRuleLanguageIds: string[], workingFolder: string, emitProgress: (percComplete: number) => void): Promise { @@ -87,10 +98,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[] = this.createJavaClassPaths(); 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 +146,7 @@ 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[] = this.createJavaClassPaths(); 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 +194,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[] = this.createJavaClassPaths(); this.emitLogEvent(LogLevel.Fine, `Calling AST dump for file: ${fileToDump}`); diff --git a/packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts b/packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts index 198de645..bdcf9ed6 100644 --- a/packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts +++ b/packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts @@ -16,6 +16,7 @@ import { import {PmdEngine} from "../src/pmd-engine"; import fs from "node:fs"; import * as fsPromises from "node:fs/promises"; +import os from "node:os"; import path from "node:path"; import {Language, PMD_VERSION} from "../src/constants"; import {DEFAULT_PMD_ENGINE_CONFIG, PMD_AVAILABLE_LANGUAGES, PmdEngineConfig} from "../src/config"; @@ -321,6 +322,31 @@ describe('Tests for the describeRules method of PmdEngine', () => { } }); + it('When a custom ruleset references a rule using a path relative to the folder Code Analyzer is run from, then the rule is resolved', async () => { + const repoFolder: string = fs.mkdtempSync(path.join(os.tmpdir(), 'pmd-cwd-ref-test-')); + const originalCwd: string = process.cwd(); + try { + await fsPromises.mkdir(path.join(repoFolder, 'scanner')); + await fsPromises.copyFile(path.join(TEST_DATA_FOLDER, 'custom rules', 'somecat3.xml'), path.join(repoFolder, 'scanner', 'full.xml')); + const wrapperRuleset: string = path.join(repoFolder, 'wrapper.xml'); + await fsPromises.writeFile(wrapperRuleset, `\n` + + `\n` + + ` Wrapper\n` + + ` \n` + + `\n`, 'utf-8'); + process.chdir(repoFolder); + const engine: PmdEngine = new PmdEngine({ + ...DEFAULT_PMD_ENGINE_CONFIG, + custom_rulesets: [wrapperRuleset] + }); + const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions()); + expectContainsRuleWithName(ruleDescriptions, 'fakerule7'); + } finally { + process.chdir(originalCwd); + fs.rmSync(repoFolder, {recursive: true, force: true}); + } + }); + it('When specifying a custom_ruleset that cannot be found, then error with a good error message', async () => { const engine: PmdEngine = new PmdEngine({ ...DEFAULT_PMD_ENGINE_CONFIG, From 9d3c3e24f8533bcd3222e86d1bcfd60a34ebeda2 Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Tue, 3 Nov 2026 09:22:51 +0530 Subject: [PATCH 08/13] fix: correct Salesforce ID regex in flow engine hardcoded id check Co-Authored-By: Claude Sonnet 5.5 --- .../FlowScanner/queries/optional_query.py | 2 +- .../FlowScanner/tests/test_salesforce_id.py | 32 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 packages/code-analyzer-flow-engine/FlowScanner/tests/test_salesforce_id.py diff --git a/packages/code-analyzer-flow-engine/FlowScanner/queries/optional_query.py b/packages/code-analyzer-flow-engine/FlowScanner/queries/optional_query.py index 61b49cc1..abb91562 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/queries/optional_query.py +++ b/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]+") diff --git a/packages/code-analyzer-flow-engine/FlowScanner/tests/test_salesforce_id.py b/packages/code-analyzer-flow-engine/FlowScanner/tests/test_salesforce_id.py new file mode 100644 index 00000000..97923681 --- /dev/null +++ b/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() From ef8a7f5892d4cd265144280780153470efbc9378 Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Tue, 3 Nov 2026 11:44:19 +0530 Subject: [PATCH 09/13] fix: only report a missing Flow results file for ENOENT Previously every readFile failure on the results file was reported as "the Flow Scanner did not produce a results file". Now only ENOENT gets that message; any other failure (permissions, directory, etc.) is reported with a new ResultsFileNotReadable message and keeps the original error as the cause. Co-Authored-By: Claude Sonnet 5.5 --- .../code-analyzer-flow-engine/src/messages.ts | 3 +++ .../src/python/FlowScannerCommandWrapper.ts | 10 ++++++++-- .../python/FlowScannerCommandWrapper.test.ts | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/code-analyzer-flow-engine/src/messages.ts b/packages/code-analyzer-flow-engine/src/messages.ts index 7b8ce95a..549c5b8f 100644 --- a/packages/code-analyzer-flow-engine/src/messages.ts +++ b/packages/code-analyzer-flow-engine/src/messages.ts @@ -23,6 +23,9 @@ const MESSAGE_CATALOG: {[key: string]: string} = { 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`, diff --git a/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts b/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts index 5fe6d0c7..548501a4 100644 --- a/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts +++ b/packages/code-analyzer-flow-engine/src/python/FlowScannerCommandWrapper.ts @@ -104,8 +104,14 @@ export class RunTimeFlowScannerCommandWrapper implements FlowScannerCommandWrapp let outputFileContents: string; try { outputFileContents = await fs.promises.readFile(flowScannerResultsFile, 'utf-8'); - } catch (_err) { - throw new Error(getMessage('ResultsFileNotProduced', absLogFilePath)); + } 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; diff --git a/packages/code-analyzer-flow-engine/test/python/FlowScannerCommandWrapper.test.ts b/packages/code-analyzer-flow-engine/test/python/FlowScannerCommandWrapper.test.ts index 48bda38f..e8c24712 100644 --- a/packages/code-analyzer-flow-engine/test/python/FlowScannerCommandWrapper.test.ts +++ b/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}); + } + }); }); }); }); From 41605b91642fd6be2a6c93986511c0bf54bd4f94 Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Tue, 3 Nov 2026 11:44:23 +0530 Subject: [PATCH 10/13] FIX Remove stale dependency jars before copying SFGE runtime dependencies copyDependencies only added files to dist/java-lib, so an incremental build after switching branches could keep jars that were removed from the runtime classpath (Netty, AWS, shaded Jackson, etc.) and still pass the banned dependency check. Delete everything except sfge-*.jar first so the packed directory always matches a clean build. Co-Authored-By: Claude Sonnet 5.5 --- packages/code-analyzer-sfge-engine/sfge/build.gradle.kts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/code-analyzer-sfge-engine/sfge/build.gradle.kts b/packages/code-analyzer-sfge-engine/sfge/build.gradle.kts index a1619f18..d4076831 100644 --- a/packages/code-analyzer-sfge-engine/sfge/build.gradle.kts +++ b/packages/code-analyzer-sfge-engine/sfge/build.gradle.kts @@ -88,6 +88,12 @@ tasks.jar { tasks.register("copyDependencies") { from(configurations.runtimeClasspath) // This includes all runtime dependencies into(javaLibDir) // Target directory for dependencies + // The directory is also the output of the jar task (sfge-*.jar), so we can't use Sync. Instead, remove any stale + // dependency jars left over from earlier builds (e.g. ones since removed from the runtime classpath) so that an + // incremental build packs exactly the same dependency set as a clean build. + doFirst { + delete(fileTree(javaLibDir) { exclude("sfge-*.jar") }) + } } tasks.build { dependsOn("copyDependencies") // Ensure dependencies are copied during the build process From e697718b7514a2f915ceac7f488c9d6fff46416d Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Tue, 3 Nov 2026 11:44:26 +0530 Subject: [PATCH 11/13] CHANGE Print and enforce minimum npm version in publish workflow The workflow installs the moving npm@11 tag. Print the resolved version and fail fast if it is older than 11.21.0 (needed for dist-tag management via OIDC) instead of failing mid-release. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/publish-package-to-npm.yml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/.github/workflows/publish-package-to-npm.yml b/.github/workflows/publish-package-to-npm.yml index f1a1072e..fdf5cd16 100644 --- a/.github/workflows/publish-package-to-npm.yml +++ b/.github/workflows/publish-package-to-npm.yml @@ -189,7 +189,16 @@ jobs: # 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 + 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 }}) From 7fd77a8a3133268644e2c27b94685ef59b7ff36e Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Tue, 3 Nov 2026 11:44:29 +0530 Subject: [PATCH 12/13] fix: stage only referenced rulesets instead of putting cwd on the Java classpath Adding process.cwd() to the PMD classpath re-exposed the scanned repository to the JVM as a source of classes and resources (CWE-427, see #503). Instead, copy only the xml ruleset files that custom rulesets reference (transitively, and never from outside the base folder, including via symlinks) into a staging folder in the working folder and put just that folder last on the classpath. Repo-relative rule refs keep resolving (#2087). Co-Authored-By: Claude Sonnet 5.5 --- .../code-analyzer-pmd-engine/src/config.ts | 2 +- .../code-analyzer-pmd-engine/src/messages.ts | 2 +- .../src/pmd-wrapper.ts | 31 +++-- .../src/ruleset-staging.ts | 112 ++++++++++++++++++ .../test/ruleset-staging.test.ts | 92 ++++++++++++++ 5 files changed, 227 insertions(+), 12 deletions(-) create mode 100644 packages/code-analyzer-pmd-engine/src/ruleset-staging.ts create mode 100644 packages/code-analyzer-pmd-engine/test/ruleset-staging.test.ts diff --git a/packages/code-analyzer-pmd-engine/src/config.ts b/packages/code-analyzer-pmd-engine/src/config.ts index cc0660b8..49955a4e 100644 --- a/packages/code-analyzer-pmd-engine/src/config.ts +++ b/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 (which implicitly includes the folder you run Code Analyzer from). + // - 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 diff --git a/packages/code-analyzer-pmd-engine/src/messages.ts b/packages/code-analyzer-pmd-engine/src/messages.ts index 03298cee..9339e3bb 100644 --- a/packages/code-analyzer-pmd-engine/src/messages.ts +++ b/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 (which implicitly includes the folder you run Code Analyzer from).\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.`, diff --git a/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts b/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts index c6815701..8c4efae3 100644 --- a/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts +++ b/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,15 +79,24 @@ export class PmdWrapperInvoker { this.emitLogEvent = emitLogEvent; } - private createJavaClassPaths(): string[] { - return [ + /** + * 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 ) 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 { + const classPaths: string[] = [ path.join(PMD_WRAPPER_LIB_FOLDER, '*'), - ...this.userProvidedJavaClasspathEntries.map(toJavaClasspathEntry), - // The java process is pinned to the engine's install folder, but PMD resolves refs inside custom rulesets - // (like ) relative to the cwd or the classpath. So we add the user's - // cwd (last, so it has the lowest precedence) so that repo-relative refs continue to resolve. - process.cwd() + ...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[], @@ -98,7 +108,7 @@ export class PmdWrapperInvoker { emitProgress(10); const javaCmdArgs: string[] = [PMD_WRAPPER_JAVA_CLASS, 'describe', pmdRulesOutputFile, customRulesetsListFile, pmdRuleLanguageIds.join(',')]; - const javaClassPaths: string[] = this.createJavaClassPaths(); + 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'); @@ -146,7 +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[] = this.createJavaClassPaths(); + 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)) { @@ -194,7 +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[] = this.createJavaClassPaths(); + const javaClassPaths: string[] = await this.createJavaClassPaths([], workingFolder); this.emitLogEvent(LogLevel.Fine, `Calling AST dump for file: ${fileToDump}`); diff --git a/packages/code-analyzer-pmd-engine/src/ruleset-staging.ts b/packages/code-analyzer-pmd-engine/src/ruleset-staging.ts new file mode 100644 index 00000000..14572b00 --- /dev/null +++ b/packages/code-analyzer-pmd-engine/src/ruleset-staging.ts @@ -0,0 +1,112 @@ +import fs from "node:fs"; +import path from "node:path"; + +// Matches the "ref" attribute of and style elements. +const RULE_REF_REGEX: RegExp = /]*?\bref\s*=\s*["']([^"']+)["']/gi; + +/** + * PMD resolves refs inside of a ruleset (like ) relative to the folder it was run + * from or from the Java classpath. Since we pin the Java process away from the (untrusted) folder being scanned, those + * refs would no longer resolve. Instead of putting the whole folder on the Java classpath (which would let a scanned + * repository supply arbitrary classes and resources to the JVM), we copy only the xml ruleset files that the custom + * rulesets actually reference (transitively) into a dedicated staging folder, and put just that folder on the classpath. + * + * Only regular ".xml" files that live underneath the base folder are ever staged. + * + * @param rulesetFiles the rulesets to inspect (absolute paths on disk, or paths relative to the base folder). Entries that are not xml files on disk are ignored. + * @param baseFolder the folder that relative refs are resolved against + * @param stagingFolder the folder to copy the referenced rulesets into + * @returns the staging folder if at least one file was staged, otherwise undefined + */ +export async function stageReferencedRulesets(rulesetFiles: string[], baseFolder: string, stagingFolder: string): Promise { + const realBase: string = await realpathOrUndefined(baseFolder) ?? baseFolder; + const visited: Set = new Set(); + const queue: string[] = []; + let stagedSomething: boolean = false; + + // Stages the file given by a (relative) ref and queues it up so that its own refs get processed too + const stageRef = async (ref: string): Promise => { + const relativeRulesetPath: string | undefined = extractRelativeXmlPath(ref); + if (relativeRulesetPath === undefined) { + return; + } + const source: string | undefined = await resolveStageableFile(relativeRulesetPath, realBase); + if (source === undefined) { + return; + } + const destination: string = path.join(stagingFolder, relativeRulesetPath); + await fs.promises.mkdir(path.dirname(destination), {recursive: true}); + await fs.promises.copyFile(source, destination); + stagedSomething = true; + queue.push(source); + }; + + for (const rulesetFile of rulesetFiles) { + if (path.isAbsolute(rulesetFile)) { + queue.push(rulesetFile); + } else { + await stageRef(rulesetFile); + } + } + + while (queue.length > 0) { + const currentFile: string = queue.shift()!; + if (visited.has(currentFile)) { + continue; + } + visited.add(currentFile); + + let contents: string; + try { + contents = await fs.promises.readFile(currentFile, 'utf-8'); + } catch (_err) { + continue; // Not a readable file on disk (for example, a ruleset inside of a jar file) + } + + for (const match of contents.matchAll(RULE_REF_REGEX)) { + await stageRef(match[1]); + } + } + return stagedSomething ? stagingFolder : undefined; +} + +/** + * Returns the relative path to the xml file portion of a rule ref (stripping off any trailing "/RuleName"), or + * undefined if the ref is not a relative path to an xml file (for example, "category/apex/design.xml" is fine to stage + * if it exists in the base folder, but absolute paths and non-xml refs are ignored). + */ +function extractRelativeXmlPath(ref: string): string | undefined { + const xmlMatch: RegExpMatchArray | null = ref.match(/^(.*?\.xml)(\/.*)?$/i); + if (!xmlMatch) { + return undefined; + } + const candidate: string = xmlMatch[1]; + if (path.isAbsolute(candidate) || path.win32.isAbsolute(candidate)) { + return undefined; + } + return path.normalize(candidate); +} + +async function resolveStageableFile(relativePath: string, realBase: string): Promise { + const real: string | undefined = await realpathOrUndefined(path.join(realBase, relativePath)); + if (real === undefined) { + return undefined; + } + const relativeToBase: string = path.relative(realBase, real); + if (relativeToBase === '' || relativeToBase.startsWith('..') || path.isAbsolute(relativeToBase)) { + return undefined; // Never stage anything that resolves (even through symlinks) outside of the base folder + } + try { + return (await fs.promises.stat(real)).isFile() ? real : undefined; + } catch (_err) /* istanbul ignore next */ { + return undefined; + } +} + +async function realpathOrUndefined(p: string): Promise { + try { + return await fs.promises.realpath(p); + } catch (_err) { + return undefined; + } +} diff --git a/packages/code-analyzer-pmd-engine/test/ruleset-staging.test.ts b/packages/code-analyzer-pmd-engine/test/ruleset-staging.test.ts new file mode 100644 index 00000000..21a3c54b --- /dev/null +++ b/packages/code-analyzer-pmd-engine/test/ruleset-staging.test.ts @@ -0,0 +1,92 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import {stageReferencedRulesets} from "../src/ruleset-staging"; + +describe('Tests for stageReferencedRulesets', () => { + let baseFolder: string; + let stagingFolder: string; + + beforeEach(() => { + const tempRoot: string = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'staging-test-'))); + baseFolder = path.join(tempRoot, 'repo'); + stagingFolder = path.join(tempRoot, 'staged'); + fs.mkdirSync(path.join(baseFolder, 'scanner'), {recursive: true}); + }); + + afterEach(() => { + fs.rmSync(path.dirname(baseFolder), {recursive: true, force: true}); + }); + + const rulesetWithRefs = (...refs: string[]): string => + `${refs.map(r => ``).join('')}`; + + it('When a ruleset references a sibling xml file, then only that xml file is staged', async () => { + fs.writeFileSync(path.join(baseFolder, 'scanner', 'full.xml'), rulesetWithRefs()); + fs.writeFileSync(path.join(baseFolder, 'scanner', 'unrelated.xml'), rulesetWithRefs()); + fs.writeFileSync(path.join(baseFolder, 'Evil.class'), 'not really a class'); + const wrapper: string = path.join(baseFolder, 'wrapper.xml'); + fs.writeFileSync(wrapper, rulesetWithRefs('scanner/full.xml/MyRule')); + + const result: string | undefined = await stageReferencedRulesets([wrapper], baseFolder, stagingFolder); + + expect(result).toEqual(stagingFolder); + expect(fs.readdirSync(stagingFolder, {recursive: true}).sort()).toEqual(['scanner', path.join('scanner', 'full.xml')]); + }); + + it('When referenced rulesets reference other rulesets, then they are staged transitively (and cycles are tolerated)', async () => { + fs.writeFileSync(path.join(baseFolder, 'a.xml'), rulesetWithRefs('b.xml/R')); + fs.writeFileSync(path.join(baseFolder, 'b.xml'), rulesetWithRefs('a.xml/R', 'scanner/c.xml')); + fs.writeFileSync(path.join(baseFolder, 'scanner', 'c.xml'), rulesetWithRefs()); + const wrapper: string = path.join(baseFolder, 'wrapper.xml'); + fs.writeFileSync(wrapper, rulesetWithRefs('a.xml/R')); + + await stageReferencedRulesets([wrapper], baseFolder, stagingFolder); + + expect(fs.existsSync(path.join(stagingFolder, 'a.xml'))).toBe(true); + expect(fs.existsSync(path.join(stagingFolder, 'b.xml'))).toBe(true); + expect(fs.existsSync(path.join(stagingFolder, 'scanner', 'c.xml'))).toBe(true); + }); + + it('When a relative ruleset is given directly, then it is resolved against the base folder and staged', async () => { + fs.writeFileSync(path.join(baseFolder, 'scanner', 'full.xml'), rulesetWithRefs()); + + const result: string | undefined = await stageReferencedRulesets(['scanner/full.xml'], baseFolder, stagingFolder); + + expect(result).toEqual(stagingFolder); + expect(fs.existsSync(path.join(stagingFolder, 'scanner', 'full.xml'))).toBe(true); + }); + + it('When refs try to escape the base folder, are absolute, or are not xml, then nothing is staged', async () => { + const outsideFile: string = path.join(path.dirname(baseFolder), 'outside.xml'); + fs.writeFileSync(outsideFile, rulesetWithRefs()); + fs.writeFileSync(path.join(baseFolder, 'Evil.class'), 'x'); + const wrapper: string = path.join(baseFolder, 'wrapper.xml'); + fs.writeFileSync(wrapper, rulesetWithRefs('../outside.xml/R', outsideFile + '/R', 'Evil.class', 'does/not/exist.xml/R')); + + const result: string | undefined = await stageReferencedRulesets([wrapper], baseFolder, stagingFolder); + + expect(result).toBeUndefined(); + expect(fs.existsSync(stagingFolder)).toBe(false); + }); + + it('When a ref is a symlink pointing outside of the base folder, then it is not staged', async () => { + const outsideFile: string = path.join(path.dirname(baseFolder), 'secret.xml'); + fs.writeFileSync(outsideFile, rulesetWithRefs()); + try { + fs.symlinkSync(outsideFile, path.join(baseFolder, 'link.xml')); + } catch (_err) { + return; // Symlink creation isn't permitted in this environment (e.g. Windows without privileges) + } + const wrapper: string = path.join(baseFolder, 'wrapper.xml'); + fs.writeFileSync(wrapper, rulesetWithRefs('link.xml/R')); + + expect(await stageReferencedRulesets([wrapper], baseFolder, stagingFolder)).toBeUndefined(); + }); + + it('When rulesets are not readable files on disk (like jar based rulesets), then they are ignored', async () => { + const result: string | undefined = await stageReferencedRulesets( + [path.join(baseFolder, 'missing.xml'), 'category/apex/design.xml'], baseFolder, stagingFolder); + expect(result).toBeUndefined(); + }); +}); From 9acbd97e4361af8b6952cec31e44d7bbab5635fb Mon Sep 17 00:00:00 2001 From: nikhil-mittal-165 Date: Tue, 3 Nov 2026 18:57:05 +0530 Subject: [PATCH 13/13] fix: support API v68 runInMode=UserMode in the Flow engine API v68 flows can declare UserMode, which was missing from the RunMode enum, so RunMode[...] raised KeyError and the scan crashed (even for flows with no element). Add UserMode (treated like DefaultMode, i.e. user context), and make unrecognized runInMode values log a warning and fall back to DefaultMode instead of crashing the whole scan. Co-Authored-By: Claude Sonnet 5.5 --- .../FlowScanner/flow_parser/parse.py | 9 ++- .../FlowScanner/public/enums.py | 1 + .../FlowScanner/tests/test_run_mode.py | 64 +++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 packages/code-analyzer-flow-engine/FlowScanner/tests/test_run_mode.py diff --git a/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py b/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py index a8e723a0..2eb617b5 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/flow_parser/parse.py +++ b/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. diff --git a/packages/code-analyzer-flow-engine/FlowScanner/public/enums.py b/packages/code-analyzer-flow-engine/FlowScanner/public/enums.py index 1e0505f9..3e3812f2 100644 --- a/packages/code-analyzer-flow-engine/FlowScanner/public/enums.py +++ b/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): diff --git a/packages/code-analyzer-flow-engine/FlowScanner/tests/test_run_mode.py b/packages/code-analyzer-flow-engine/FlowScanner/tests/test_run_mode.py new file mode 100644 index 00000000..af3109ec --- /dev/null +++ b/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 = ( + '\n' + '' + '68.0' + '' + 'AutoLaunchedFlow' + '{run_in_mode}' + 'Active' + '00' + '' +) + + +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'{run_in_mode_value}' + 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()