NEW @W-23766007@ Feature/h1 java command relative path fix

packages/ENGINE-TEMPLATE/package.json

@@ -14,7 +14,7 @@
   "types": "dist/index.d.ts",
   "dependencies": {
     "@types/node": "^20.0.0",
-    "@salesforce/code-analyzer-engine-api": "0.41.0"
+    "@salesforce/code-analyzer-engine-api": "0.42.0-SNAPSHOT"
   },
   "devDependencies": {
     "@eslint/js": "^9.39.5",

packages/code-analyzer-apexguru-engine/package.json

@@ -13,7 +13,7 @@
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
   "dependencies": {
-    "@salesforce/code-analyzer-engine-api": "0.41.0",
+    "@salesforce/code-analyzer-engine-api": "0.42.0-SNAPSHOT",
     "@salesforce/core": "^8.31.2",
     "archiver": "^7.0.1",
     "form-data": "^4.0.6"

packages/code-analyzer-core/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@salesforce/code-analyzer-core",
   "description": "Core Package for the Salesforce Code Analyzer",
-  "version": "0.52.0",
+  "version": "0.53.0-SNAPSHOT",
   "author": "The Salesforce Code Analyzer Team",
   "license": "BSD-3-Clause",
   "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",
@@ -16,7 +16,7 @@
   },
   "types": "dist/index.d.ts",
   "dependencies": {
-    "@salesforce/code-analyzer-engine-api": "0.41.0",
+    "@salesforce/code-analyzer-engine-api": "0.42.0-SNAPSHOT",
     "@types/node": "^20.0.0",
     "csv-stringify": "^6.8.1",
     "isbinaryfile": "^5.0.7",

packages/code-analyzer-core/test/java-command-security.test.ts

@@ -0,0 +1,209 @@
+import {CodeAnalyzer, CodeAnalyzerConfig, EventType, LogEvent, RuleSelection} from "../src";
+import {changeWorkingDirectoryToPackageRoot, FakeFileSystem, FixedUniqueIdGenerator} from "./test-helpers";
+import * as engApi from "@salesforce/code-analyzer-engine-api";
+
+/**
+ * Regression test for the H1 bug bounty finding:
+ *   An auto-discovered code-analyzer.yml that sets engines.pmd.java_command to a repository-relative executable path
+ *   (e.g. './scripts/afv-pmd-java') used to be spawned during the eager per-engine config extraction that happens on
+ *   addEnginePlugin, even when the user only requested the eslint engine via 'sf code-analyzer rules --rule-selector
+ *   eslint'. This resulted in arbitrary repo-controlled code execution.
+ *
+ * These tests lock in the two layers of the fix:
+ *   1. A relative java_command must be rejected during config validation WITHOUT ever spawning the binary (asserted by
+ *      a JavaVersionIdentifier spy that fails the test if it is ever asked to identify the java version).
+ *   2. Core's per-engine try/catch isolation must let an eslint-only run continue to select the eslint rules even when
+ *      the pmd engine failed to instantiate because of the malicious config (defense-in-depth ordering).
+ */
+describe("Security regression: relative java_command in an auto-discovered config is never spawned", () => {
+    changeWorkingDirectoryToPackageRoot();
+
+    let codeAnalyzer: CodeAnalyzer;
+    let logEvents: LogEvent[];
+
+    // The malicious auto-discovered config, expressed as engine overrides just like an auto-discovered
+    // code-analyzer.yml would be parsed into.
+    const MALICIOUS_CONFIG: object = {
+        engines: {
+            stubPmd: {
+                java_command: './scripts/afv-pmd-java'
+            }
+        }
+    };
+
+    beforeEach(() => {
+        codeAnalyzer = new CodeAnalyzer(CodeAnalyzerConfig.fromObject(MALICIOUS_CONFIG), new FakeFileSystem());
+        codeAnalyzer._setUniqueIdGenerator(new FixedUniqueIdGenerator());
+        logEvents = [];
+        codeAnalyzer.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event));
+    });
+
+    it("An eslint-only run with a malicious relative java_command never spawns the binary and still selects eslint rules", async () => {
+        const javaVersionSpy: ThrowIfCalledJavaVersionIdentifier = new ThrowIfCalledJavaVersionIdentifier();
+        const pmdPlugin: StubPmdEnginePlugin = new StubPmdEnginePlugin(javaVersionSpy);
+        const eslintPlugin: StubEslintEnginePlugin = new StubEslintEnginePlugin();
+
+        // Adding the malicious PMD-like plugin must not throw even though its config extraction fails.
+        await expect(codeAnalyzer.addEnginePlugin(pmdPlugin)).resolves.not.toThrow();
+        await expect(codeAnalyzer.addEnginePlugin(eslintPlugin)).resolves.not.toThrow();
+
+        // Acceptance criterion #2 & repro: the relative-path binary was never spawned.
+        expect(javaVersionSpy.wasCalled).toEqual(false);
+
+        // Acceptance criterion #1: the pmd engine failed to instantiate (bad config) and was NOT added.
+        expect(codeAnalyzer.getEngineNames()).not.toContain('stubPmd');
+        expect(() => codeAnalyzer.getEngineConfig('stubPmd')).toThrow();
+
+        // Per-engine failure isolation: the eslint engine was still added successfully.
+        expect(codeAnalyzer.getEngineNames()).toContain('stubEslint');
+
+        // Acceptance criterion #4: an eslint-only rule selection still succeeds and returns the eslint rules.
+        const selection: RuleSelection = await codeAnalyzer.selectRules(['stubEslint']);
+        expect(selection.getEngineNames()).toEqual(['stubEslint']);
+        expect(selection.getRulesFor('stubEslint').map(r => r.getName())).toEqual(['eslintRule']);
+
+        // The pmd engine failure should have been surfaced as a log event rather than silently swallowed.
+        const errorLogMessages: string[] = logEvents.map(e => e.message);
+        expect(errorLogMessages.some(m => m.includes('stubPmd'))).toEqual(true);
+    });
+
+    it("A malicious relative java_command is rejected with a clear validation error mentioning the field and reason", async () => {
+        const javaVersionSpy: ThrowIfCalledJavaVersionIdentifier = new ThrowIfCalledJavaVersionIdentifier();
+        await codeAnalyzer.addEnginePlugin(new StubPmdEnginePlugin(javaVersionSpy));
+
+        expect(javaVersionSpy.wasCalled).toEqual(false);
+        const combinedLogText: string = logEvents.map(e => e.message).join('\n');
+        expect(combinedLogText).toContain(`The 'engines.stubPmd.java_command' configuration value is invalid.`);
+        expect(combinedLogText).toContain('relative file paths are not allowed');
+    });
+});
+
+/**
+ * A JavaVersionIdentifier spy that stands in for the real one that spawns 'java -version'. It fails the test if it is
+ * ever invoked, which is exactly what would happen if a relative java_command were passed through to be spawned.
+ */
+class ThrowIfCalledJavaVersionIdentifier {
+    wasCalled: boolean = false;
+
+    identifyJavaVersion(_javaCommand: string): Promise<string | null> {
+        this.wasCalled = true;
+        return Promise.reject(new Error('spawn must not be called for a relative java_command'));
+    }
+}
+
+/**
+ * A minimal PMD-like engine plugin that mirrors the real pmd-engine's extractJavaCommand ordering: it validates the
+ * java_command value using the shared ValueValidator.validateJavaCommand helper (rejecting relative paths) BEFORE it
+ * ever asks the JavaVersionIdentifier to spawn the command.
+ */
+class StubPmdEnginePlugin extends engApi.EnginePluginV1 {
+    private readonly javaVersionIdentifier: ThrowIfCalledJavaVersionIdentifier;
+
+    constructor(javaVersionIdentifier: ThrowIfCalledJavaVersionIdentifier) {
+        super();
+        this.javaVersionIdentifier = javaVersionIdentifier;
+    }
+
+    getAvailableEngineNames(): string[] {
+        return ['stubPmd'];
+    }
+
+    describeEngineConfig(_engineName: string): engApi.ConfigDescription {
+        return {};
+    }
+
+    async createEngineConfig(engineName: string, configValueExtractor: engApi.ConfigValueExtractor): Promise<engApi.ConfigObject> {
+        const javaCommand: string | undefined = configValueExtractor.extractString('java_command');
+        if (javaCommand) {
+            try {
+                // Reject relative paths BEFORE spawning, exactly like the real SharedConfigValueExtractor now does.
+                engApi.ValueValidator.validateJavaCommand(javaCommand, configValueExtractor.getFieldPath('java_command'));
+                await this.javaVersionIdentifier.identifyJavaVersion(javaCommand);
+            } catch (err) {
+                throw new Error(`The '${configValueExtractor.getFieldPath('java_command')}' configuration value is invalid. ${(err as Error).message}`, {cause: err});
+            }
+        }
+        return {...configValueExtractor.getObject(), engine_name: engineName};
+    }
+
+    async createEngine(_engineName: string, config: engApi.ConfigObject): Promise<engApi.Engine> {
+        return new StubPmdEngine(config);
+    }
+}
+
+class StubPmdEngine extends engApi.Engine {
+    constructor(_config: engApi.ConfigObject) {
+        super();
+    }
+
+    getName(): string {
+        return 'stubPmd';
+    }
+
+    getEngineVersion(): Promise<string> {
+        return Promise.resolve('0.0.1');
+    }
+
+    async describeRules(_describeOptions: engApi.DescribeOptions): Promise<engApi.RuleDescription[]> {
+        return [{
+            name: 'pmdRule',
+            severityLevel: engApi.SeverityLevel.Moderate,
+            tags: ['Recommended'],
+            description: 'Some pmd rule',
+            resourceUrls: []
+        }];
+    }
+
+    async runRules(_ruleNames: string[], _runOptions: engApi.RunOptions): Promise<engApi.EngineRunResults> {
+        return {violations: []};
+    }
+}
+
+/**
+ * A minimal ESLint-like engine plugin whose config extraction has no java_command and thus always succeeds.
+ */
+class StubEslintEnginePlugin extends engApi.EnginePluginV1 {
+    getAvailableEngineNames(): string[] {
+        return ['stubEslint'];
+    }
+
+    describeEngineConfig(_engineName: string): engApi.ConfigDescription {
+        return {};
+    }
+
+    async createEngineConfig(engineName: string, configValueExtractor: engApi.ConfigValueExtractor): Promise<engApi.ConfigObject> {
+        return {...configValueExtractor.getObject(), engine_name: engineName};
+    }
+
+    async createEngine(_engineName: string, config: engApi.ConfigObject): Promise<engApi.Engine> {
+        return new StubEslintEngine(config);
+    }
+}
+
+class StubEslintEngine extends engApi.Engine {
+    constructor(_config: engApi.ConfigObject) {
+        super();
+    }
+
+    getName(): string {
+        return 'stubEslint';
+    }
+
+    getEngineVersion(): Promise<string> {
+        return Promise.resolve('0.0.1');
+    }
+
+    async describeRules(_describeOptions: engApi.DescribeOptions): Promise<engApi.RuleDescription[]> {
+        return [{
+            name: 'eslintRule',
+            severityLevel: engApi.SeverityLevel.Moderate,
+            tags: ['Recommended'],
+            description: 'Some eslint rule',
+            resourceUrls: []
+        }];
+    }
+
+    async runRules(_ruleNames: string[], _runOptions: engApi.RunOptions): Promise<engApi.EngineRunResults> {
+        return {violations: []};
+    }
+}

packages/code-analyzer-engine-api/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@salesforce/code-analyzer-engine-api",
   "description": "Engine API Package for the Salesforce Code Analyzer",
-  "version": "0.41.0",
+  "version": "0.42.0-SNAPSHOT",
   "author": "The Salesforce Code Analyzer Team",
   "license": "BSD-3-Clause",
   "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",

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

@@ -392,6 +392,31 @@ export class ValueValidator {
         return strValue;
     }
 
+    /**
+     * Validates that the provided value is a valid java command specifier: either the bare name of a command that is
+     *   resolvable via the PATH (containing no file path separators) or an absolute file path. Relative file paths are
+     *   rejected because they would resolve against the current working directory and could point to a
+     *   repository-controlled executable.
+     *
+     * NOTE: This check mitigates relative-path execution but does not fully eliminate the threat of repo-controlled
+     * absolute paths in auto-discovered configs. Consider: should auto-discovered configs be allowed to override
+     * java_command at all for engines the user did not explicitly select?
+     * @param value the value that you wish to validate
+     * @param fieldPath the field path of the value as you want it to appear in validation error messages
+     */
+    static validateJavaCommand(value: unknown, fieldPath: string): string {
+        const strValue: string = ValueValidator.validateString(value, fieldPath);
+        if (path.isAbsolute(strValue)) {
+            return strValue;
+        }
+        // A bare command name (no path separators) is resolved via the PATH environment variable and is allowed.
+        // Reject drive-relative paths (Windows-specific: 'C:foo' where path.isAbsolute('C:foo') == false).
+        if (!strValue.includes('/') && !strValue.includes('\\') && !strValue.includes(':')) {
+            return strValue;
+        }
+        throw new Error(getMessage('ConfigValueMustBeCommandNameOrAbsolutePath', fieldPath));
+    }
+
     /**
      * Validates that the provided value is a {@link SeverityLevel}.
      * @param rawValue the value that you wish to validate

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

@@ -43,6 +43,10 @@ export const SHARED_MESSAGE_CATALOG: MessageCatalog = {
     ConfigValueMustMatchRegExp:
         `The '%s' configuration value is invalid. The string did not match the regular expression pattern: %s`,
 
+    ConfigValueMustBeCommandNameOrAbsolutePath:
+        `The '%s' configuration value is invalid. The value must either be the name of a command that exists on ` +
+        `the path (containing no file path separators) or an absolute file path; relative file paths are not allowed.`,
+
     ConfigValueNotAValidSeverityLevel:
         `The '%s' configuration value must be one of the following: %s. Instead received: %s`,
 

packages/code-analyzer-engine-api/test/config.test.ts

@@ -60,6 +60,28 @@ describe("Tests for ValueValidator", () => {
             getMessage('ConfigValueMustMatchRegExp', 'someFieldName', '/^he.*/i'));
     });
 
+    it("When a bare command name is given to validateJavaCommand, then the value is returned", () => {
+        expect(ValueValidator.validateJavaCommand('java', 'engines.pmd.java_command')).toEqual('java');
+    });
+
+    it("When an absolute path is given to validateJavaCommand, then the value is returned unchanged", () => {
+        const absPath: string = path.resolve('/usr/bin/java');
+        expect(ValueValidator.validateJavaCommand(absPath, 'engines.pmd.java_command')).toEqual(absPath);
+        // A raw absolute path should also be returned unchanged regardless of platform
+        expect(ValueValidator.validateJavaCommand('/usr/bin/java', 'engines.pmd.java_command')).toEqual('/usr/bin/java');
+    });
+
+    it.each(['./scripts/afv-pmd-java', 'scripts/foo', '../x/java', 'a/b'])(
+        "When a relative path '%s' is given to validateJavaCommand, then error", (relPath) => {
+        expect(() => ValueValidator.validateJavaCommand(relPath, 'engines.pmd.java_command')).toThrow(
+            getMessage('ConfigValueMustBeCommandNameOrAbsolutePath', 'engines.pmd.java_command'));
+    });
+
+    it("When a non-string value is given to validateJavaCommand, then the validateString type error is thrown", () => {
+        expect(() => ValueValidator.validateJavaCommand(3, 'engines.pmd.java_command')).toThrow(
+            getMessage('ConfigValueMustBeOfType', 'engines.pmd.java_command', 'string', 'number'));
+    });
+
     it("When an object value is given to validateObject, then the value is returned", () => {
         expect(ValueValidator.validateObject({a:1}, 'someFieldName')).toEqual({a:1});
     });

packages/code-analyzer-eslint-engine/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@salesforce/code-analyzer-eslint-engine",
   "description": "Plugin package that adds 'eslint' as an engine into Salesforce Code Analyzer",
-  "version": "0.46.0",
+  "version": "0.47.0-SNAPSHOT",
   "author": "The Salesforce Code Analyzer Team",
   "license": "BSD-3-Clause",
   "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",
@@ -18,8 +18,8 @@
     "@lwc/eslint-plugin-lwc": "^3.5.0",
     "@lwc/eslint-plugin-lwc-platform": "^6.3.0",
     "@salesforce-ux/eslint-plugin-slds": "^1.2.1",
-    "@salesforce/code-analyzer-engine-api": "0.41.0",
-    "@salesforce/code-analyzer-eslint8-engine": "0.18.0",
+    "@salesforce/code-analyzer-engine-api": "0.42.0-SNAPSHOT",
+    "@salesforce/code-analyzer-eslint8-engine": "0.19.0-SNAPSHOT",
     "@salesforce/eslint-config-lwc": "^4.1.2",
     "@salesforce/eslint-plugin-lightning": "^2.0.0",
     "@types/node": "^20.0.0",

packages/code-analyzer-eslint8-engine/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@salesforce/code-analyzer-eslint8-engine",
   "description": "Plugin package that adds 'eslint' (version 8) as an engine into Salesforce Code Analyzer",
-  "version": "0.18.0",
+  "version": "0.19.0-SNAPSHOT",
   "author": "The Salesforce Code Analyzer Team",
   "license": "BSD-3-Clause",
   "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",
@@ -18,7 +18,7 @@
     "@eslint/js": "8.57.1",
     "@lwc/eslint-plugin-lwc": "2.2.0",
     "@lwc/eslint-plugin-lwc-platform": "5.2.0",
-    "@salesforce/code-analyzer-engine-api": "0.41.0",
+    "@salesforce/code-analyzer-engine-api": "0.42.0-SNAPSHOT",
     "@salesforce/eslint-config-lwc": "3.7.2",
     "@salesforce/eslint-plugin-lightning": "1.0.1",
     "@types/node": "^20.0.0",

packages/code-analyzer-flow-engine/package.json

@@ -13,7 +13,7 @@
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
   "dependencies": {
-    "@salesforce/code-analyzer-engine-api": "0.41.0",
+    "@salesforce/code-analyzer-engine-api": "0.42.0-SNAPSHOT",
     "@types/node": "^20.0.0",
     "@types/semver": "^7.7.1",
     "semver": "^7.8.5"

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.45.0",
+  "version": "0.46.0-SNAPSHOT",
   "author": "The Salesforce Code Analyzer Team",
   "license": "BSD-3-Clause",
   "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",
@@ -13,7 +13,7 @@
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
   "dependencies": {
-    "@salesforce/code-analyzer-engine-api": "0.41.0",
+    "@salesforce/code-analyzer-engine-api": "0.42.0-SNAPSHOT",
     "@types/node": "^20.0.0",
     "@types/semver": "^7.7.1",
     "semver": "^7.8.5"