Repository navigation
FIX Remediate SFGE transitive dependency vulnerabilities - #512
nikhil-mittal-165 wants to merge 3 commits into
Conversation
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 <noreply@anthropic.com>
|
Git2Gus App is installed but the |
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Self-review using the refreshed Code Analyzer regression-review skill. Finding[P2] Make the copied Java dependency directory converge on the runtime classpath
Manual validation before mergeRequired:
Recommended:
Evidence checked: dependency graph/build changes, package contents path, new guard/tests, CI matrix reported by GitHub, approval, and the NPSP comparison supplied in the PR description. I did not independently reproduce the long NPSP run. |
…cies 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 <noreply@anthropic.com>
|
Addressed the P2 finding in 41605b9: 🤖 Generated with Claude Code |
Summary
Removes every known-vulnerable library from the Java runtime that
@salesforce/code-analyzer-sfge-engineships (dist/java-lib), so that dependency firewalls (Sonatype Repository Firewall flagged 27 CVEs) stop blocking the engine.dist/java-lib)dev)Scanner:
osv-scanner scan source --no-ignore --x-plugins=java/archive dist/java-lib. This inspects JAR contents, including shaded/relocated classes. The 84 vs Sonatype's 27 is mostly alias double counting plus advisories published since the Sonatype scan.Background
Exploitability: not exploitable in SFGE
These are dependency-hygiene findings, not vulnerabilities reachable in SFGE. SFGE parses Apex from disk into an embedded, in-memory
TinkerGraph, analyses it, and writes JSON.gremlin-driver(remote Gremlin client). SFGE opens no ports and makes no remote connections; 0 references togremlin.driver,Cluster,DriverRemoteConnection,withRemote.gremlin-shaded)commons-collections3.xcommons-collections4.commons-configuration2,commons-beanutilsTinkerGraph.open()with no arguments (GraphUtil.getGraph()).log4j2.xmluses onlyPatternLayout(Console, RollingFile, two custom appenders). No Syslog/Socket/RFC5424/XML layouts, no JNDI. The only lookup is${sys:sfge_log_name}, which the TS wrapper sets.We remove them anyway, because "not exploitable" does not get the engine through other companies' firewalls.
Root cause
gremlin-driver. It has been there since the SFGE Java layer was copied in (4e77be1, Mar 2025) and is the only source of Netty and the AWS SDK (about 60 JARs).gremlin-core:commons-configuration22.9.0,commons-beanutils1.9.4,commons-collections3.2.2,commons-lang33.12.0.4.0.0-beta.1. It came in withfb51918(Oct 2025, titled "Updated node dependencies"). Every 4.0 beta bundles Jackson 2.15.2 insidegremlin-shaded, and a dependency constraint cannot patch that.gradle/libs.versions.tomlhad not changed since Oct 21, 2025; the monthly release only refreshes npm.Changes
gradle/libs.versions.tomlgremlin-drivernetty-all(constraint)tinkergraph-gremlinlog4j-api,log4j-corecommons-configuration2commons-beanutilssnakeyamlcommons-lang3commons-textsfge/build.gradle.ktsimplementation(libs.gremlin.driver)and thenetty-allconstraint.tinkergraph-gremlin:commons-collections:commons-collections(3.x has no patched release).org.apache.tinkerpop:gremlin-shaded(bundles vulnerable Jackson and Kryo that cannot be patched by constraint).commons-configuration2,commons-beanutilsandsnakeyaml.commons-lang3andcommons-textare now declared explicitly. SFGE imports both (ApexStringValueusescommons-text), but they previously arrived only transitively.verifyNoBannedDependenciestask, wired intobuildandcheck. It fails if Netty, the AWS SDK,gremlin-driver,gremlin-util,gremlin-shadedorcommons-collections3.x reappear on the runtime classpath, for example after a Gremlin upgrade.New
sfge/src/main/java/org/apache/tinkerpop/shaded/jackson/core/JsonProcessingException.javaA minimal stand-in class (extends
IOException, contains no Jackson code).TinkerGraphnames this type in acatchclause, so the JVM must be able to load it, even though no Jackson code ever runs. Without it, every test fails withNoClassDefFoundErroratTinkerGraph.<init>. The Javadoc explains when to remove it.New
sfge/src/test/java/com/salesforce/graph/ops/NoVulnerableBundledLibrariesTest.javaChecks four things: the in-memory graph still opens; the real shaded Jackson implementation (
ObjectMapper,JsonFactory) is absent;commons-collections3.x is absent; Netty andgremlin-driverare absent.package.json0.25.0→0.26.0-SNAPSHOT, as required byvalidate_packages.Why Gremlin 3.7.7 rather than a newer version
gremlin-shaded, and are pre-release.TraversalUtilTest×6,InboundEmailHandlerSupplierTest), so traversal behaviour differs. The same tests pass on 4.0.0-beta.1, 3.7.7 and 3.7.4. Moving to 3.8 needs its own investigation.Even 3.7.7 bundles Jackson 2.18.9, which has 7 advisories fixed only in 2.18.11. That is why
gremlin-shadedis excluded regardless of the Gremlin version.Testing
Unit and integration tests
./gradlew test jacocoTestReport jacocoTestCoverageVerification: full Java suite passes (~4,600 tests), 80% coverage gate passes.npm run buildandnpm run test-typescript: 4 suites, 100 tests pass.engine.test.tslaunches the real JVM on sample workspaces and compares results with gold files.run_testspasses on ubuntu, macOS and windows.gremlin-drivermakesverifyNoBannedDependenciesfail and listgremlin-driver,gremlin-util,netty-*andcommons-collections:3.2.2.Real-world comparison on NPSP (Nonprofit Success Pack)
Both engine builds were run on SalesforceFoundation/NPSP
main(1e9e619). Scope:force-app, 1,061 Apex files (~319K lines), all 7 SFGE rules. Each build ranjava com.salesforce.Sfge executewith the same input file and the same defaults as the TS wrapper (8 threads, 30 s per-path timeout), one at a time on the same machine (16 cores, 64 GB).Full run, default settings
dev(Gremlin 4.0.0-beta.1)Per rule:
devdevand 16 only in the PR. Every one comes from one of 7 entry points that hit the 30 s timeout in one run and finished in the other. For example,EP_ManageEPTemplate_CTRL.addTasktimed out ondev, so its 10 violations are missing there;BGE_BatchGiftEntryTab_CTRL.getTableModeltimed out on the PR, so its 2 violations are missing there.Rerun of those 7 entry points with a 10-minute timeout (nothing can time out)
devThe only remaining difference is one
StackOverflowErrorengine warning. It is at the same location, rule and message on both engines; only the top frames of the stack trace differ, because the overflow happens at a slightly different depth.Conclusion: on NPSP the two Gremlin versions find the same violations. The full-run differences come entirely from entry points that sit close to the 30 s timeout.
Run-to-run noise check (
devrun twice)devrun 1devrun 2devbuild differs from itself: 5 extra violations in run 2 (4 fromHH_Container_LCTRL.addContactAddresses, one of the same borderline entry points, and 1 fromRD2_ElevateInformation_CTRL.getAppConfigurationData), and 8 different timeouts.devruns, so the +4% seen in the first comparison is ordinary run-to-run variation. Wall time and memory are effectively the same.Overall: no functional or performance regression was observed on NPSP.
Reproduce: the runner (
run_sfge.py) and comparison (compare.py) scripts are available on request. They build the samesfgeInput.jsonthe wrapper writes and diff every field of every result.Not covered / follow-ups
npm auditstill flags monorepo dev tooling (eslint, jest and similar). Nothing in this package's runtime npm dependencies (engine-api,semver) is flagged.gradle/libs.versions.tomlandsfge/build.gradle.ktsto the monthly dependency-update checklist; consider an osv-scanner CI gate ondist/java-lib.JsonProcessingExceptionstand-in and thegremlin-shadedexclusion once a Gremlin release bundles a patched Jackson (≥ 2.18.11).🤖 Generated with Claude Code