NEW @W-23906075@ SF CLI - Automatic ESLint config import leads to RCE during code analyzer process

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

@@ -144,6 +144,18 @@ export const LEGACY_ESLINT_CONFIG_FILES: string[] =
 
 export const LEGACY_ESLINT_IGNORE_FILE: string = '.eslintignore';
 
+// Extensions of ESLint configuration files whose contents are evaluated as JavaScript when loaded. This covers both
+// flat config files (eslint.config.{js,cjs,mjs}) and legacy config files (.eslintrc.{js,cjs}). Declarative config
+// files (.json/.yaml/.yml) are passive data and therefore excluded.
+export const EXECUTABLE_ESLINT_CONFIG_FILE_EXTS: string[] = ['.js', '.cjs', '.mjs'];
+
+// Returns true when the given config file is one whose top-level code executes when the file is loaded (i.e. it is a
+// JavaScript module rather than a declarative data file). This is used to keep auto-discovery from executing untrusted
+// config files that live inside the workspace being scanned.
+export function isExecutableConfigFile(filePath: string): boolean {
+    return EXECUTABLE_ESLINT_CONFIG_FILE_EXTS.includes(path.extname(filePath).toLowerCase());
+}
+
 
 export function validateAndNormalizeConfig(configValueExtractor: ConfigValueExtractor): ESLintEngineConfig {
     configValueExtractor.validateContainsOnlySpecifiedKeys(['eslint_config_file', 'eslint_ignore_file',

packages/code-analyzer-eslint-engine/src/engine.ts

@@ -196,7 +196,8 @@ export class ESLintEngine extends Engine {
     private getUserConfigInfo(workspace?: Workspace): UserConfigInfo {
         const cacheKey: string = workspace?.getWorkspaceId() ?? process.cwd();
         if (!this.userConfigInfoCache.has(cacheKey)) {
-            this.userConfigInfoCache.set(cacheKey, new UserConfigInfo(this.engineConfig, workspace));
+            this.userConfigInfoCache.set(cacheKey, new UserConfigInfo(this.engineConfig, workspace,
+                (logLevel: LogLevel, message: string) => this.emitLogEvent(logLevel, message)));
         }
         return this.userConfigInfoCache.get(cacheKey)!;
     }

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

@@ -107,6 +107,15 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
     ApplyingFlatConfigFile:
         `Applying the flat ESLint configuration file: %s`,
 
+    SkippedAutoDiscoveredExecutableConfigFile:
+        `The executable ESLint configuration file '%s' was automatically discovered in your workspace but was NOT applied.\n` +
+        `Code Analyzer does not execute automatically discovered configuration files because their top-level JavaScript would run during analysis.\n` +
+        `If you trust this file and want to apply it, set it explicitly as the eslint_config_file value in your Code Analyzer configuration.`,
+
+    ExplicitExecutableConfigFileWillExecute:
+        `The explicitly configured ESLint configuration file '%s' contains executable JavaScript whose top-level code will run during analysis.\n` +
+        `Only apply configuration files that you trust.`,
+
     UnableToCalculateBaseDirectory:
         `Couldn't calculate base directory for ESLint from the list of relevant targeted files to scan.\n` +
         `This can occur if you are attempting to target files from more than one drive (like C: and D: drives for example).\n` +

packages/code-analyzer-eslint-engine/src/user-config-info.ts

@@ -3,11 +3,19 @@ import fs from "node:fs";
 import {
     ESLintEngineConfig,
     DISCOVERABLE_FLAT_ESLINT_CONFIG_FILES,
+    isExecutableConfigFile,
     LEGACY_ESLINT_CONFIG_FILES,
     LEGACY_ESLINT_IGNORE_FILE
 } from "./config";
+import {getMessage} from "./messages";
 import {makeUnique} from "./utils";
-import {Workspace} from "@salesforce/code-analyzer-engine-api";
+import {LogLevel, Workspace} from "@salesforce/code-analyzer-engine-api";
+
+// Callback used to forward log events (such as security warnings) up to the owning engine so that they surface in the
+// main Code Analyzer log. Defaults to a no-op so that UserConfigInfo can still be constructed in isolation.
+export type EmitLogEventFunction = (logLevel: LogLevel, message: string) => void;
+
+const NO_OP_EMIT_LOG_EVENT: EmitLogEventFunction = () => {};
 
 export enum UserConfigState {
     NO_USER_CONFIG = "NO_USER_CONFIG",
@@ -18,15 +26,17 @@ export enum UserConfigState {
 export class UserConfigInfo {
     private readonly engineConfig: ESLintEngineConfig;
     private readonly workspace?: Workspace;
+    private readonly emitLogEvent: EmitLogEventFunction;
     private userConfigState?: UserConfigState;
     private userConfigFile?: string;
     private userIgnoreFile?: string;
     private discoveredConfigFile?: string;
     private discoveredIgnoreFile?: string;
 
-    constructor(config: ESLintEngineConfig, workspace?: Workspace) {
+    constructor(config: ESLintEngineConfig, workspace?: Workspace, emitLogEvent: EmitLogEventFunction = NO_OP_EMIT_LOG_EVENT) {
         this.engineConfig = config;
         this.workspace = workspace;
+        this.emitLogEvent = emitLogEvent;
     }
 
     getState(): UserConfigState {
@@ -64,12 +74,30 @@ export class UserConfigInfo {
         this.userConfigState = UserConfigState.NO_USER_CONFIG;
 
         if (this.engineConfig.eslint_config_file) {
+            // An explicitly configured config file is chosen by the operator (trusted), so we still honor it even when
+            // it is executable. We do, however, warn that its top-level code will run during analysis.
             this.userConfigFile = this.engineConfig.eslint_config_file;
+            if (isExecutableConfigFile(this.userConfigFile)) {
+                this.emitLogEvent(LogLevel.Warn,
+                    getMessage('ExplicitExecutableConfigFileWillExecute', this.userConfigFile));
+            }
         } else {
             this.discoveredConfigFile = this.discoverFile(
                 [...DISCOVERABLE_FLAT_ESLINT_CONFIG_FILES, ...LEGACY_ESLINT_CONFIG_FILES]);
-            this.userConfigFile = this.engineConfig.auto_discover_eslint_config ?
-                this.discoveredConfigFile : undefined;
+            if (this.engineConfig.auto_discover_eslint_config && this.discoveredConfigFile) {
+                // Auto-discovery pulls config files from the untrusted workspace being scanned. To eliminate arbitrary
+                // code execution (the RCE vector) we refuse to apply executable config files found this way and instead
+                // warn the operator to opt in explicitly. Declarative config files are passive data, so they still apply.
+                if (isExecutableConfigFile(this.discoveredConfigFile)) {
+                    this.emitLogEvent(LogLevel.Warn,
+                        getMessage('SkippedAutoDiscoveredExecutableConfigFile', this.discoveredConfigFile));
+                    this.userConfigFile = undefined;
+                } else {
+                    this.userConfigFile = this.discoveredConfigFile;
+                }
+            } else {
+                this.userConfigFile = undefined;
+            }
         }
         if (this.userConfigFile) {
             this.userConfigState = isLegacyConfigFile(this.userConfigFile) ? UserConfigState.LEGACY_USER_CONFIG : UserConfigState.FLAT_USER_CONFIG;

packages/code-analyzer-eslint-engine/test/end-to-end.test.ts

@@ -13,10 +13,13 @@ import {
     Violation,
     Workspace
 } from "@salesforce/code-analyzer-engine-api";
+import * as fs from "node:fs";
+import * as os from "node:os";
 import * as path from "node:path";
 import * as process from "node:process";
 import {ESLint8EnginePlugin} from "@salesforce/code-analyzer-eslint8-engine";
 import { createDescribeOptions, createRunOptions } from "./test-helpers";
+import {getMessage} from "../src/messages";
 
 jest.setTimeout(30_000);
 
@@ -137,4 +140,63 @@ describe('End to end test', () => {
             }
         });
     });
+
+    describe('Security regression: RCE via auto-discovered executable ESLint config', () => {
+        const maliciousWorkspace: string = path.resolve(__dirname, 'test-data', 'workspaceWithMaliciousFlatConfig');
+        const maliciousConfigFile: string = path.join(maliciousWorkspace, 'eslint.config.cjs');
+        let sentinelPath: string;
+
+        beforeEach(() => {
+            sentinelPath = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'rce-sentinel-')), 'sentinel.txt');
+            process.env.SENTINEL_PATH = sentinelPath;
+        });
+        afterEach(() => {
+            delete process.env.SENTINEL_PATH;
+            if (fs.existsSync(sentinelPath)) {
+                fs.rmSync(sentinelPath);
+            }
+        });
+
+        it('When auto_discover_eslint_config=true and workspace contains a malicious executable config, then it is NOT executed and a skip warning is emitted', async () => {
+            const plugin: EnginePluginV1 = new ESLintEnginePlugin();
+            const configValueExtractor: ConfigValueExtractor = new ConfigValueExtractor({
+                auto_discover_eslint_config: true
+            }, 'engines.eslint');
+            const engineConfig: ConfigObject = await plugin.createEngineConfig('eslint', configValueExtractor);
+            const engine: Engine = await plugin.createEngine('eslint', engineConfig);
+            const logEvents: LogEvent[] = [];
+            engine.onEvent(EventType.LogEvent, (e: LogEvent) => logEvents.push(e));
+
+            const workspace: Workspace = new Workspace('id', [maliciousWorkspace]);
+            const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions(workspace));
+            const recommendedRuleNames: string[] = ruleDescriptions.filter(rd => rd.tags.includes('Recommended')).map(rd => rd.name);
+            await engine.runRules(recommendedRuleNames, createRunOptions(workspace));
+
+            // The malicious top-level code must NEVER have run during auto-discovery.
+            expect(fs.existsSync(sentinelPath)).toEqual(false);
+
+            const warnMessages: string[] = logEvents.filter(e => e.logLevel === LogLevel.Warn).map(e => e.message);
+            expect(warnMessages).toContainEqual(getMessage('SkippedAutoDiscoveredExecutableConfigFile', maliciousConfigFile));
+        });
+
+        it('When the same config is explicitly opted into via eslint_config_file, then it executes and an execution warning is emitted', async () => {
+            const plugin: EnginePluginV1 = new ESLintEnginePlugin();
+            const configValueExtractor: ConfigValueExtractor = new ConfigValueExtractor({
+                eslint_config_file: maliciousConfigFile
+            }, 'engines.eslint');
+            const engineConfig: ConfigObject = await plugin.createEngineConfig('eslint', configValueExtractor);
+            const engine: Engine = await plugin.createEngine('eslint', engineConfig);
+            const logEvents: LogEvent[] = [];
+            engine.onEvent(EventType.LogEvent, (e: LogEvent) => logEvents.push(e));
+
+            const workspace: Workspace = new Workspace('id', [maliciousWorkspace]);
+            await engine.describeRules(createDescribeOptions(workspace));
+
+            // Explicit opt-in preserves the ability to apply (and therefore execute) a trusted config file.
+            expect(fs.existsSync(sentinelPath)).toEqual(true);
+
+            const warnMessages: string[] = logEvents.filter(e => e.logLevel === LogLevel.Warn).map(e => e.message);
+            expect(warnMessages).toContainEqual(getMessage('ExplicitExecutableConfigFileWillExecute', maliciousConfigFile));
+        });
+    });
 });