fix: [BUG][code-analyzer] 5.16.0 silently drops custom PMD rulesets that point at a sibling file by relative path

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}`);
 

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 <rule ... ref="..."/> and <rule-set ... ref="..."/> style elements.
+const RULE_REF_REGEX: RegExp = /<rule\b[^>]*?\bref\s*=\s*["']([^"']+)["']/gi;
+
+/**
+ * PMD resolves refs inside of a ruleset (like <rule ref="path/in/repo.xml/MyRule"/>) 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<string | undefined> {
+    const realBase: string = await realpathOrUndefined(baseFolder) ?? baseFolder;
+    const visited: Set<string> = 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<void> => {
+        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<string | undefined> {
+    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<string | undefined> {
+    try {
+        return await fs.promises.realpath(p);
+    } catch (_err) {
+        return undefined;
+    }
+}

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, `<?xml version="1.0"?>\n` +
+                `<ruleset name="Wrapper" xmlns="http://pmd.sourceforge.net/ruleset/2.0.0">\n` +
+                `  <description>Wrapper</description>\n` +
+                `  <rule ref="scanner/full.xml/fakerule7"/>\n` +
+                `</ruleset>\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,

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 =>
+        `<?xml version="1.0"?><ruleset name="x">${refs.map(r => `<rule ref="${r}"/>`).join('')}</ruleset>`;
+
+    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();
+    });
+});