From a48f5a81ce0675e5095f40b1371db21da5e60560 Mon Sep 17 00:00:00 2001 From: Nikhil Mittal Date: Tue, 18 Aug 2026 21:57:26 +0530 Subject: [PATCH 1/3] FIX [Bug Bounty/H1] eslint engine: restrict auto-discovery to declarative config to prevent RCE Auto-discovering an executable ESLint config (eslint.config.{js,cjs,mjs} or legacy .eslintrc.{js,cjs}) caused its top-level JavaScript to execute during analysis, an arbitrary code execution vector reachable from an untrusted workspace when auto_discover_eslint_config is enabled. Auto-discovery now applies only declarative config (.json/.yaml/.yml). An executable config found by auto-discovery is skipped with a Warn. An explicitly configured eslint_config_file may still be executable (trusted operator opt-in) but emits a Warn that its top-level code will run. Additive only: no exported symbols were removed or renamed. --- .../code-analyzer-eslint-engine/src/config.ts | 12 ++ .../code-analyzer-eslint-engine/src/engine.ts | 3 +- .../src/messages.ts | 9 ++ .../src/user-config-info.ts | 36 ++++- .../test/end-to-end.test.ts | 62 +++++++ .../test/engine.test.ts | 54 +++++-- .../dummy1.js | 4 + .../eslint.config.cjs | 17 ++ .../test/user-config-info.test.ts | 151 ++++++++++++++---- .../test/utils.test.ts | 50 ++++++ 10 files changed, 348 insertions(+), 50 deletions(-) create mode 100644 packages/code-analyzer-eslint-engine/test/test-data/workspaceWithMaliciousFlatConfig/dummy1.js create mode 100644 packages/code-analyzer-eslint-engine/test/test-data/workspaceWithMaliciousFlatConfig/eslint.config.cjs diff --git a/packages/code-analyzer-eslint-engine/src/config.ts b/packages/code-analyzer-eslint-engine/src/config.ts index ad64612a..f65b8411 100644 --- a/packages/code-analyzer-eslint-engine/src/config.ts +++ b/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', diff --git a/packages/code-analyzer-eslint-engine/src/engine.ts b/packages/code-analyzer-eslint-engine/src/engine.ts index ae6b894b..284f4085 100644 --- a/packages/code-analyzer-eslint-engine/src/engine.ts +++ b/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)!; } diff --git a/packages/code-analyzer-eslint-engine/src/messages.ts b/packages/code-analyzer-eslint-engine/src/messages.ts index 0778a35d..20aca659 100644 --- a/packages/code-analyzer-eslint-engine/src/messages.ts +++ b/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` + diff --git a/packages/code-analyzer-eslint-engine/src/user-config-info.ts b/packages/code-analyzer-eslint-engine/src/user-config-info.ts index cc1b0d30..fee49ff5 100644 --- a/packages/code-analyzer-eslint-engine/src/user-config-info.ts +++ b/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; diff --git a/packages/code-analyzer-eslint-engine/test/end-to-end.test.ts b/packages/code-analyzer-eslint-engine/test/end-to-end.test.ts index f8b3ba49..e7e85345 100644 --- a/packages/code-analyzer-eslint-engine/test/end-to-end.test.ts +++ b/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)); + }); + }); }); \ No newline at end of file diff --git a/packages/code-analyzer-eslint-engine/test/engine.test.ts b/packages/code-analyzer-eslint-engine/test/engine.test.ts index 49e11569..74c6e57f 100644 --- a/packages/code-analyzer-eslint-engine/test/engine.test.ts +++ b/packages/code-analyzer-eslint-engine/test/engine.test.ts @@ -79,7 +79,10 @@ describe('Tests for the describeRules method of ESLintEngine', () => { // React rules now apply to all JS files, so all tests with JS files get React rules - type TEST_SCENARIO = {description: string, folder: string, expectationRuleDescriptions: RuleDescription[]}; + // The 'configFile' field, when supplied, is an executable flat config that is now only applied when set explicitly + // via eslint_config_file (the trusted opt-in path); auto-discovery no longer executes such files. When it is + // undefined the scenario relies purely on auto-discovery, which finds nothing executable to apply. + type TEST_SCENARIO = {description: string, folder: string, configFile?: string, expectationRuleDescriptions: RuleDescription[]}; const testScenarios: TEST_SCENARIO[] = [ { description: 'with no customizations', @@ -89,6 +92,7 @@ describe('Tests for the describeRules method of ESLintEngine', () => { { description: 'with config that modifies existing rules', folder: workspaceThatHasCustomConfigModifyingExistingRules, + configFile: path.join(workspaceThatHasCustomConfigModifyingExistingRules, 'eslint.config.cjs'), // The config modifies some rule properties. This does not impact the rule descriptions. If error is // modified to warn then it doesn't impact the severity since we still grab it from our rule mappings. // But if a rule is turned off, then it is indeed removed, this no-useless-escape is removed. @@ -97,17 +101,19 @@ describe('Tests for the describeRules method of ESLintEngine', () => { { description: 'with config that adds a new plugin and rules', folder: workspaceThatHasCustomConfigWithNewRules, + configFile: path.join(workspaceThatHasCustomConfigWithNewRules, 'eslint.config.js'), expectationRuleDescriptions: makeUniqueAndSorted([...DEFAULT_RULES, ...CUSTOM_RULES]) } ] - it.each(testScenarios)('When describing rules while cwd is folder $description and auto_discover_eslint_config=true, then return expected', async (caseObj: TEST_SCENARIO) => { + it.each(testScenarios)('When describing rules while cwd is folder $description with the config applied explicitly, then return expected', async (caseObj: TEST_SCENARIO) => { const origWorkingDir: string = process.cwd(); process.chdir(caseObj.folder); try { const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, config_root: __dirname, auto_discover_eslint_config: true, + eslint_config_file: caseObj.configFile }); const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions()); expect(ruleDescriptions).toEqual(caseObj.expectationRuleDescriptions); @@ -116,23 +122,40 @@ describe('Tests for the describeRules method of ESLintEngine', () => { } }); - it.each(testScenarios)('When describing rules while from a workspace $description and auto_discover_eslint_config=true, then return expected', async (caseObj: TEST_SCENARIO) => { + it.each(testScenarios)('When describing rules while from a workspace $description with the config applied explicitly, then return expected', async (caseObj: TEST_SCENARIO) => { const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, - auto_discover_eslint_config: true + auto_discover_eslint_config: true, + eslint_config_file: caseObj.configFile }); const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions(new Workspace('id', [caseObj.folder]))); expect(ruleDescriptions).toEqual(caseObj.expectationRuleDescriptions); }); - it.each(testScenarios)('When describing rules while config_root is folder $description and auto_discover_eslint_config=true, then return expected', async (caseObj: TEST_SCENARIO) => { + it.each(testScenarios)('When describing rules while config_root is folder $description with the config applied explicitly, then return expected', async (caseObj: TEST_SCENARIO) => { const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, auto_discover_eslint_config: true, - config_root: caseObj.folder + config_root: caseObj.folder, + eslint_config_file: caseObj.configFile }); const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions()); expect(ruleDescriptions).toEqual(caseObj.expectationRuleDescriptions); }); + it('When an executable flat config is auto-discovered (not set explicitly), then it is skipped with a warning and only base rules are returned', async () => { + const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, + auto_discover_eslint_config: true, + config_root: workspaceThatHasCustomConfigModifyingExistingRules + }); + const logEvents: LogEvent[] = []; + engine.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); + const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions()); + // The custom config is NOT applied, so no-useless-escape is not removed and we get the full default rule set. + expect(ruleDescriptions).toEqual(DEFAULT_RULES); + const warnLogs: LogEvent[] = logEvents.filter(e => e.logLevel === LogLevel.Warn); + expect(warnLogs.map(e => e.message)).toContainEqual(getMessage('SkippedAutoDiscoveredExecutableConfigFile', + path.join(workspaceThatHasCustomConfigModifyingExistingRules, 'eslint.config.cjs'))); + }); + it('When describing rules from a workspace targeting no javascript files, then no javascript rules should return', async () => { const engine: Engine = await createEngineFromPlugin(DEFAULT_CONFIG_FOR_TESTING); const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions(new Workspace('id', @@ -435,10 +458,10 @@ describe('Tests for the describeRules method of ESLintEngine', () => { } }); - it('When all base configs are off and custom config with new rules exists, when auto_discover_eslint_config=true then only custom config is applied', async() => { + it('When all base configs are off and custom config with new rules is explicitly set, then only custom config is applied', async() => { const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, config_root: workspaceThatHasCustomConfigWithNewRules, - auto_discover_eslint_config: true, // Sanity test that we can auto discover in config root + eslint_config_file: path.join(workspaceThatHasCustomConfigWithNewRules, 'eslint.config.js'), disable_javascript_base_config: true, disable_typescript_base_config: true, disable_lwc_base_config: true, @@ -452,9 +475,9 @@ describe('Tests for the describeRules method of ESLintEngine', () => { expect(ruleDescriptions).toEqual(CUSTOM_RULES); }); - it('When all base configs are off and custom config that modifies eslint rules exists and auto_discover_eslint_config=true, then only custom config is applied', async() => { + it('When all base configs are off and custom config that modifies eslint rules is explicitly set, then only custom config is applied', async() => { const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, - auto_discover_eslint_config: true, + eslint_config_file: path.join(testDataFolder, 'workspaceWithFlatConfigJs', 'eslint.config.js'), disable_javascript_base_config: true, disable_typescript_base_config: true, disable_lwc_base_config: true, @@ -488,9 +511,12 @@ describe('Tests for the describeRules method of ESLintEngine', () => { expect(ruleDescriptions).toEqual(EXPECTED_RULES_FOR_BASE_PLUS_CONFIG_THAT_MODIFIES_EXISTING_RULES); const warnLogs: LogEvent[] = logEvents.filter(e => e.logLevel === LogLevel.Warn); - expect(warnLogs).toHaveLength(1); - expect(warnLogs[0].message).toEqual(getMessage('IgnoringLegacyIgnoreFile', + const warnMessages: string[] = warnLogs.map(e => e.message); + expect(warnMessages).toContainEqual(getMessage('IgnoringLegacyIgnoreFile', path.join(testDataFolder, 'workspaceWithLegacyIgnoreFile', '.eslintignore'))); + // Since the config file is explicitly set to an executable config, we also warn that it will execute. + expect(warnMessages).toContainEqual(getMessage('ExplicitExecutableConfigFileWillExecute', + path.join(workspaceThatHasCustomConfigModifyingExistingRules, 'eslint.config.cjs'))); }); it('When custom rules only apply to other file extensions, then without specifying file extensions in custom config, they are not picked up', async () => { @@ -662,7 +688,7 @@ describe('Typical tests for the runRules method of ESLintEngine', () => { it('When using custom plugin rules, then violations from custom rules are returned', async () => { const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, - auto_discover_eslint_config: true + eslint_config_file: path.join(workspaceThatHasCustomConfigWithNewRules, 'eslint.config.js') }); const runOptions: RunOptions = createRunOptions(new Workspace('id', [path.join(workspaceThatHasCustomConfigWithNewRules, 'dummy1.js')])); const results: EngineRunResults = await engine.runRules(['dummy/my-rule-1', 'dummy/my-rule-2'], runOptions); @@ -767,7 +793,7 @@ describe('Typical tests for the runRules method of ESLintEngine', () => { const workspaceFolder: string = path.join(tempFolder, 'workspaceWithConflictingConfig'); const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, - auto_discover_eslint_config: true, + eslint_config_file: path.join(workspaceFolder, 'eslint.config.mjs'), config_root: workspaceFolder }); const logEvents: LogEvent[] = []; diff --git a/packages/code-analyzer-eslint-engine/test/test-data/workspaceWithMaliciousFlatConfig/dummy1.js b/packages/code-analyzer-eslint-engine/test/test-data/workspaceWithMaliciousFlatConfig/dummy1.js new file mode 100644 index 00000000..bb2ad607 --- /dev/null +++ b/packages/code-analyzer-eslint-engine/test/test-data/workspaceWithMaliciousFlatConfig/dummy1.js @@ -0,0 +1,4 @@ +function foo() { + var unused = 1; + return 2; +} diff --git a/packages/code-analyzer-eslint-engine/test/test-data/workspaceWithMaliciousFlatConfig/eslint.config.cjs b/packages/code-analyzer-eslint-engine/test/test-data/workspaceWithMaliciousFlatConfig/eslint.config.cjs new file mode 100644 index 00000000..67daa074 --- /dev/null +++ b/packages/code-analyzer-eslint-engine/test/test-data/workspaceWithMaliciousFlatConfig/eslint.config.cjs @@ -0,0 +1,17 @@ +// This is a deliberately "malicious" flat ESLint config used only by the RCE regression test. Its top-level code +// simulates arbitrary code execution (the reported bug popped a calculator) by writing a sentinel file to the path +// given in the SENTINEL_PATH environment variable. The test asserts this side effect NEVER happens during +// auto-discovery, and DOES happen only when the operator explicitly opts in via eslint_config_file. +const fs = require('node:fs'); + +if (process.env.SENTINEL_PATH) { + fs.writeFileSync(process.env.SENTINEL_PATH, 'code-was-executed'); +} + +module.exports = [ + { + rules: { + 'no-unused-vars': ['error'] + } + } +]; diff --git a/packages/code-analyzer-eslint-engine/test/user-config-info.test.ts b/packages/code-analyzer-eslint-engine/test/user-config-info.test.ts index 1aa1ee20..f466053d 100644 --- a/packages/code-analyzer-eslint-engine/test/user-config-info.test.ts +++ b/packages/code-analyzer-eslint-engine/test/user-config-info.test.ts @@ -1,9 +1,20 @@ import * as path from "node:path"; import * as process from "node:process"; -import {Workspace} from "@salesforce/code-analyzer-engine-api"; +import {LogLevel, Workspace} from "@salesforce/code-analyzer-engine-api"; import {DEFAULT_CONFIG, ESLintEngineConfig} from "../src/config"; +import {getMessage} from "../src/messages"; import {UserConfigInfo, UserConfigState} from "../src/user-config-info"; +type CollectedLogEvent = { logLevel: LogLevel, message: string }; + +function createLogEventCollector(): { events: CollectedLogEvent[], emit: (logLevel: LogLevel, message: string) => void } { + const events: CollectedLogEvent[] = []; + return { + events, + emit: (logLevel: LogLevel, message: string) => events.push({logLevel, message}) + }; +} + const TEST_DATA_FOLDER: string = path.join(__dirname, 'test-data'); const WORKSPACE_WITH_LEGACY_CONFIG_JSON: string = path.join(TEST_DATA_FOLDER, 'workspaceWithLegacyConfigJson'); const WORKSPACE_WITH_LEGACY_CONFIG_YML: string = path.join(TEST_DATA_FOLDER, 'workspaceWithLegacyConfigYml'); @@ -319,30 +330,45 @@ describe('Tests for the UserConfigInfo class', () => { expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(path.join(WORKSPACE_WITH_LEGACY_IGNORE, '.eslintignore')); }); - it('...and workspace root contains a flat config file, then return FLAT_USER_CONFIG state', () => { + it('...and workspace root contains an executable flat config file, then skip it with a warning and return NO_USER_CONFIG state', () => { const workspace: Workspace = new Workspace('id', [WORKSPACE_WITH_FLAT_CONFIG_JS]); engineConfig.config_root = WORKSPACE_WITH_LEGACY_CONFIG_YML; // Confirming that workspace wins so this is ignored - const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , workspace); - expect(userConfigInfo.getState()).toEqual(UserConfigState.FLAT_USER_CONFIG); - expect(userConfigInfo.getChosenUserConfigFile()).toEqual(path.join(WORKSPACE_WITH_FLAT_CONFIG_JS, 'eslint.config.js')); + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , workspace, logCollector.emit); + expect(userConfigInfo.getState()).toEqual(UserConfigState.NO_USER_CONFIG); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(undefined); expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(undefined); + expect(logCollector.events).toContainEqual({ + logLevel: LogLevel.Warn, + message: getMessage('SkippedAutoDiscoveredExecutableConfigFile', path.join(WORKSPACE_WITH_FLAT_CONFIG_JS, 'eslint.config.js')) + }); }); - it('...and config root contains a legacy config and ignore file, then return LEGACY_USER_CONFIG state', () => { + it('...and config root contains an executable legacy config and ignore file, then skip the config with a warning but keep the ignore file', () => { engineConfig.config_root = WORKSPACE_WITH_LEGACY_CONFIG_CJS process.chdir(WORKSPACE_WITH_LEGACY_CONFIG_YML); // Also confirm that config root wins so this should be ignored - const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined); + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined, logCollector.emit); expect(userConfigInfo.getState()).toEqual(UserConfigState.LEGACY_USER_CONFIG); - expect(userConfigInfo.getChosenUserConfigFile()).toEqual(path.join(WORKSPACE_WITH_LEGACY_CONFIG_CJS, '.eslintrc.cjs')); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(undefined); expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(path.join(WORKSPACE_WITH_LEGACY_CONFIG_CJS, '.eslintignore')); + expect(logCollector.events).toContainEqual({ + logLevel: LogLevel.Warn, + message: getMessage('SkippedAutoDiscoveredExecutableConfigFile', path.join(WORKSPACE_WITH_LEGACY_CONFIG_CJS, '.eslintrc.cjs')) + }); }); - it('...and config root contains a flat config file and legacy ignore file, then return FLAT_USER_CONFIG state', () => { + it('...and config root contains an executable flat config file and legacy ignore file, then skip the config with a warning but keep the ignore file', () => { engineConfig.config_root = WORKSPACE_WITH_FLAT_CONFIG_MJS; - const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined); - expect(userConfigInfo.getState()).toEqual(UserConfigState.FLAT_USER_CONFIG); - expect(userConfigInfo.getChosenUserConfigFile()).toEqual(path.join(WORKSPACE_WITH_FLAT_CONFIG_MJS, 'eslint.config.mjs')); + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined, logCollector.emit); + expect(userConfigInfo.getState()).toEqual(UserConfigState.LEGACY_USER_CONFIG); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(undefined); expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(path.join(WORKSPACE_WITH_FLAT_CONFIG_MJS, '.eslintignore')); + expect(logCollector.events).toContainEqual({ + logLevel: LogLevel.Warn, + message: getMessage('SkippedAutoDiscoveredExecutableConfigFile', path.join(WORKSPACE_WITH_FLAT_CONFIG_MJS, 'eslint.config.mjs')) + }); }); @@ -354,12 +380,26 @@ describe('Tests for the UserConfigInfo class', () => { expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(path.join(WORKSPACE_WITH_LEGACY_IGNORE, '.eslintignore')); }); - it('...and pwd contains a flat config file, then return FLAT_USER_CONFIG state', () => { + it('...and pwd contains an executable flat config file, then skip it with a warning and return NO_USER_CONFIG state', () => { process.chdir(WORKSPACE_WITH_FLAT_CONFIG_CJS); - const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined); - expect(userConfigInfo.getState()).toEqual(UserConfigState.FLAT_USER_CONFIG); - expect(userConfigInfo.getChosenUserConfigFile()).toEqual(path.join(WORKSPACE_WITH_FLAT_CONFIG_CJS, 'eslint.config.cjs')); + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined, logCollector.emit); + expect(userConfigInfo.getState()).toEqual(UserConfigState.NO_USER_CONFIG); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(undefined); expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(undefined); + expect(logCollector.events).toContainEqual({ + logLevel: LogLevel.Warn, + message: getMessage('SkippedAutoDiscoveredExecutableConfigFile', path.join(WORKSPACE_WITH_FLAT_CONFIG_CJS, 'eslint.config.cjs')) + }); + }); + + it('...and workspace root contains a declarative legacy config file, then still apply it with no warning', () => { + const workspace: Workspace = new Workspace('id', [WORKSPACE_WITH_LEGACY_CONFIG_YML]); + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , workspace, logCollector.emit); + expect(userConfigInfo.getState()).toEqual(UserConfigState.LEGACY_USER_CONFIG); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(path.join(WORKSPACE_WITH_LEGACY_CONFIG_YML, '.eslintrc.yml')); + expect(logCollector.events).toHaveLength(0); }); it('... and workspace contains legacy config and pwd contains flat config, then return LEGACY_USER_CONFIG state', () => { @@ -385,30 +425,45 @@ describe('Tests for the UserConfigInfo class', () => { expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(engineConfig.eslint_ignore_file); }); - it('...and workspace root contains a legacy config file and another legacy ignore file, then return LEGACY_USER_CONFIG state', () => { + it('...and workspace root contains an executable legacy config file and another legacy ignore file, then skip the config with a warning and return LEGACY_USER_CONFIG state', () => { const workspace: Workspace = new Workspace('id', [WORKSPACE_WITH_LEGACY_CONFIG_CJS]); - const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , workspace); + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , workspace, logCollector.emit); expect(userConfigInfo.getState()).toEqual(UserConfigState.LEGACY_USER_CONFIG); - expect(userConfigInfo.getChosenUserConfigFile()).toEqual(path.join(WORKSPACE_WITH_LEGACY_CONFIG_CJS, '.eslintrc.cjs')); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(undefined); expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(engineConfig.eslint_ignore_file); + expect(logCollector.events).toContainEqual({ + logLevel: LogLevel.Warn, + message: getMessage('SkippedAutoDiscoveredExecutableConfigFile', path.join(WORKSPACE_WITH_LEGACY_CONFIG_CJS, '.eslintrc.cjs')) + }); }); - it('...and workspace root contains a flat config file, then return FLAT_USER_CONFIG state', () => { + it('...and workspace root contains an executable flat config file, then skip the config with a warning and return LEGACY_USER_CONFIG state', () => { const workspace: Workspace = new Workspace('id', [WORKSPACE_WITH_FLAT_CONFIG_JS]); engineConfig.config_root = WORKSPACE_WITH_LEGACY_CONFIG_YML; // Confirming that workspace wins so this is ignored - const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , workspace); - expect(userConfigInfo.getState()).toEqual(UserConfigState.FLAT_USER_CONFIG); - expect(userConfigInfo.getChosenUserConfigFile()).toEqual(path.join(WORKSPACE_WITH_FLAT_CONFIG_JS, 'eslint.config.js')); + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , workspace, logCollector.emit); + expect(userConfigInfo.getState()).toEqual(UserConfigState.LEGACY_USER_CONFIG); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(undefined); // Yes we still set the ignore file so that the engine can detect it and issue a warning if it wants to expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(engineConfig.eslint_ignore_file); + expect(logCollector.events).toContainEqual({ + logLevel: LogLevel.Warn, + message: getMessage('SkippedAutoDiscoveredExecutableConfigFile', path.join(WORKSPACE_WITH_FLAT_CONFIG_JS, 'eslint.config.js')) + }); }); - it('...and config root contains a flat config file and another legacy ignore file, then return FLAT_USER_CONFIG state', () => { + it('...and config root contains an executable flat config file and another legacy ignore file, then skip the config with a warning and return LEGACY_USER_CONFIG state', () => { engineConfig.config_root = WORKSPACE_WITH_FLAT_CONFIG_MJS; - const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined); - expect(userConfigInfo.getState()).toEqual(UserConfigState.FLAT_USER_CONFIG); - expect(userConfigInfo.getChosenUserConfigFile()).toEqual(path.join(WORKSPACE_WITH_FLAT_CONFIG_MJS, 'eslint.config.mjs')); + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined, logCollector.emit); + expect(userConfigInfo.getState()).toEqual(UserConfigState.LEGACY_USER_CONFIG); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(undefined); expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(engineConfig.eslint_ignore_file); + expect(logCollector.events).toContainEqual({ + logLevel: LogLevel.Warn, + message: getMessage('SkippedAutoDiscoveredExecutableConfigFile', path.join(WORKSPACE_WITH_FLAT_CONFIG_MJS, 'eslint.config.mjs')) + }); }); it('...and pwd contains an ignore file, then return LEGACY_USER_CONFIG state', () => { @@ -419,12 +474,17 @@ describe('Tests for the UserConfigInfo class', () => { expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(engineConfig.eslint_ignore_file); }); - it('...and pwd contains a flat config file, then return FLAT_USER_CONFIG state', () => { + it('...and pwd contains an executable flat config file, then skip the config with a warning and return LEGACY_USER_CONFIG state', () => { process.chdir(WORKSPACE_WITH_FLAT_CONFIG_CJS); - const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined); - expect(userConfigInfo.getState()).toEqual(UserConfigState.FLAT_USER_CONFIG); - expect(userConfigInfo.getChosenUserConfigFile()).toEqual(path.join(WORKSPACE_WITH_FLAT_CONFIG_CJS, 'eslint.config.cjs')); + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig , undefined, logCollector.emit); + expect(userConfigInfo.getState()).toEqual(UserConfigState.LEGACY_USER_CONFIG); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(undefined); expect(userConfigInfo.getChosenUserIgnoreFile()).toEqual(engineConfig.eslint_ignore_file); + expect(logCollector.events).toContainEqual({ + logLevel: LogLevel.Warn, + message: getMessage('SkippedAutoDiscoveredExecutableConfigFile', path.join(WORKSPACE_WITH_FLAT_CONFIG_CJS, 'eslint.config.cjs')) + }); }); it('... and workspace contains legacy config and pwd contains flat config, then return LEGACY_USER_CONFIG state', () => { @@ -604,4 +664,33 @@ describe('Tests for the UserConfigInfo class', () => { }); }); }); + + describe('When eslint_config_file is explicitly set to an executable config file...', () => { + it.each([ + path.join(WORKSPACE_WITH_FLAT_CONFIG_JS, 'eslint.config.js'), + path.join(WORKSPACE_WITH_FLAT_CONFIG_CJS, 'eslint.config.cjs'), + path.join(WORKSPACE_WITH_FLAT_CONFIG_MJS, 'eslint.config.mjs'), + path.join(WORKSPACE_WITH_LEGACY_CONFIG_CJS, '.eslintrc.cjs') + ])('...then it is still applied but a warning that it will execute is emitted (%s)', (executableConfigFile: string) => { + engineConfig.eslint_config_file = executableConfigFile; + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig, undefined, logCollector.emit); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(executableConfigFile); + expect(logCollector.events).toContainEqual({ + logLevel: LogLevel.Warn, + message: getMessage('ExplicitExecutableConfigFileWillExecute', executableConfigFile) + }); + }); + + it.each([ + path.join(WORKSPACE_WITH_LEGACY_CONFIG_JSON, '.eslintrc.json'), + path.join(WORKSPACE_WITH_LEGACY_CONFIG_YML, '.eslintrc.yml') + ])('...but a declarative config file is applied with no execution warning (%s)', (declarativeConfigFile: string) => { + engineConfig.eslint_config_file = declarativeConfigFile; + const logCollector = createLogEventCollector(); + const userConfigInfo: UserConfigInfo = new UserConfigInfo(engineConfig, undefined, logCollector.emit); + expect(userConfigInfo.getChosenUserConfigFile()).toEqual(declarativeConfigFile); + expect(logCollector.events.some(e => e.message === getMessage('ExplicitExecutableConfigFileWillExecute', declarativeConfigFile))).toEqual(false); + }); + }); }); diff --git a/packages/code-analyzer-eslint-engine/test/utils.test.ts b/packages/code-analyzer-eslint-engine/test/utils.test.ts index 6745146e..cdb63a4a 100644 --- a/packages/code-analyzer-eslint-engine/test/utils.test.ts +++ b/packages/code-analyzer-eslint-engine/test/utils.test.ts @@ -1,4 +1,54 @@ import {makeStringifiable, makeUnique} from "../src/utils"; +import {isExecutableConfigFile} from "../src/config"; +import {getMessage} from "../src/messages"; + +describe('Tests for the isExecutableConfigFile classifier', () => { + it.each([ + 'eslint.config.js', + 'eslint.config.cjs', + 'eslint.config.mjs', + '.eslintrc.js', + '.eslintrc.cjs' + ])('When given an executable config file (%s), then return true', (fileName: string) => { + expect(isExecutableConfigFile(fileName)).toEqual(true); + }); + + it.each([ + '.eslintrc.json', + '.eslintrc.yaml', + '.eslintrc.yml', + '.eslintignore' + ])('When given a declarative config file (%s), then return false', (fileName: string) => { + expect(isExecutableConfigFile(fileName)).toEqual(false); + }); + + it.each([ + 'ESLINT.CONFIG.JS', + 'ESLINT.CONFIG.CJS', + 'ESLINT.CONFIG.MJS' + ])('When given an executable config file with an upper-case extension (%s), then return true', (fileName: string) => { + expect(isExecutableConfigFile(fileName)).toEqual(true); + }); + + it('When given a path to an executable config file, then still classify it based on its extension', () => { + expect(isExecutableConfigFile('/some/abs/path/eslint.config.cjs')).toEqual(true); + expect(isExecutableConfigFile('/some/abs/path/.eslintrc.json')).toEqual(false); + }); +}); + +describe('Tests for the newly added ESLint config warning messages', () => { + it('SkippedAutoDiscoveredExecutableConfigFile message resolves and contains the file argument', () => { + const msg: string = getMessage('SkippedAutoDiscoveredExecutableConfigFile', 'eslint.config.cjs'); + expect(msg).toContain('eslint.config.cjs'); + expect(msg.length).toBeGreaterThan(0); + }); + + it('ExplicitExecutableConfigFileWillExecute message resolves and contains the file argument', () => { + const msg: string = getMessage('ExplicitExecutableConfigFileWillExecute', 'eslint.config.cjs'); + expect(msg).toContain('eslint.config.cjs'); + expect(msg.length).toBeGreaterThan(0); + }); +}); describe('Tests for the makeUnique utility function', () => { it('When an empty array is given, then return it', () => { From 23fbb342baee129b5082b59cd793d4434f2d252e Mon Sep 17 00:00:00 2001 From: Nikhil Mittal Date: Tue, 18 Aug 2026 21:57:52 +0530 Subject: [PATCH 2/3] FIX [Bug Bounty/H1] eslint8 engine: restrict auto-discovery to declarative config to prevent RCE Mirrors the eslint (flat) engine fix for the legacy ESLint v8 engine. Auto- discovering an executable legacy config (.eslintrc.{js,cjs}) executed its top- level JavaScript during analysis via ESLint's own .eslintrc tree-walk (useEslintrc) and via overrideConfigFile. Auto-discovery now applies only declarative legacy config (.json/.yaml/.yml): an auto-discovered executable config is skipped, useEslintrc is disabled so ESLint will not walk to it either, and a Warn is emitted. An explicitly configured executable eslint_config_file is still applied (trusted operator opt-in) but emits a Warn that its top-level code will run. Additive only: no exported symbols were removed or renamed. --- .../src/config.ts | 10 +++ .../src/messages.ts | 11 +++- .../src/strategy.ts | 29 ++++++++- .../src/workspace.ts | 25 ++++++- .../test/end-to-end.test.ts | 65 +++++++++++++++++++ .../test/engine.test.ts | 37 +++++++++++ .../.eslintrc.js | 15 +++++ .../dummy1.js | 4 ++ .../test/utils.test.ts | 44 +++++++++++++ 9 files changed, 234 insertions(+), 6 deletions(-) create mode 100644 packages/code-analyzer-eslint8-engine/test/test-data/workspaceWithMaliciousLegacyConfig/.eslintrc.js create mode 100644 packages/code-analyzer-eslint8-engine/test/test-data/workspaceWithMaliciousLegacyConfig/dummy1.js diff --git a/packages/code-analyzer-eslint8-engine/src/config.ts b/packages/code-analyzer-eslint8-engine/src/config.ts index 469a0169..ae904903 100644 --- a/packages/code-analyzer-eslint8-engine/src/config.ts +++ b/packages/code-analyzer-eslint8-engine/src/config.ts @@ -113,6 +113,16 @@ export const LEGACY_ESLINT_CONFIG_FILES: string[] = export const LEGACY_ESLINT_IGNORE_FILE: string = '.eslintignore'; +// Legacy ESLint config files whose top-level code executes when the file is loaded (i.e. they are JavaScript modules +// rather than declarative data files). Note that legacy config only supports '.eslintrc.js' and '.eslintrc.cjs' as +// executable variants (there is no '.eslintrc.mjs'). This is used to keep auto-discovery from executing untrusted +// config files that live inside the workspace being scanned. +export const EXECUTABLE_ESLINT_CONFIG_FILE_EXTS: string[] = ['.js', '.cjs']; + +export function isExecutableConfigFile(filePath: string): boolean { + return EXECUTABLE_ESLINT_CONFIG_FILE_EXTS.includes(path.extname(filePath).toLowerCase()); +} + export function validateAndNormalizeConfig(configValueExtractor: ConfigValueExtractor): ESLint8EngineConfig { configValueExtractor.validateContainsOnlySpecifiedKeys(['eslint_config_file', 'eslint_ignore_file', diff --git a/packages/code-analyzer-eslint8-engine/src/messages.ts b/packages/code-analyzer-eslint8-engine/src/messages.ts index 1a844754..a0f0d1b2 100644 --- a/packages/code-analyzer-eslint8-engine/src/messages.ts +++ b/packages/code-analyzer-eslint8-engine/src/messages.ts @@ -105,7 +105,16 @@ const MESSAGE_CATALOG : { [key: string]: string } = { ` engines:\n` + ` eslint:\n` + ` eslint_ignore_file: "%s"\n` + - `Alternatively, to have Code Analyzer automatically discover and apply any ESLint configuration and ignore files found in your workspace, set the auto_discover_eslint_config value to true.` + `Alternatively, to have Code Analyzer automatically discover and apply any ESLint configuration and ignore files found in your workspace, set the auto_discover_eslint_config value to true.`, + + 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.` } /** diff --git a/packages/code-analyzer-eslint8-engine/src/strategy.ts b/packages/code-analyzer-eslint8-engine/src/strategy.ts index a7fa15c1..7eb2b72c 100644 --- a/packages/code-analyzer-eslint8-engine/src/strategy.ts +++ b/packages/code-analyzer-eslint8-engine/src/strategy.ts @@ -3,7 +3,7 @@ import {ESLint, Linter, Rule} from "eslint"; import {AsyncFilterFnc, ESLintWorkspace, UserConfigInfo} from "./workspace"; import path from "node:path"; import {BaseRuleset, LegacyBaseConfigFactory} from "./base-config"; -import {ESLint8EngineConfig} from "./config"; +import {ESLint8EngineConfig, isExecutableConfigFile} from "./config"; import {LogLevel} from "@salesforce/code-analyzer-engine-api"; import {getMessage} from "./messages"; import {Worker} from "node:worker_threads"; @@ -64,6 +64,8 @@ export class LegacyESLintStrategy implements ESLintStrategy { const userConfigInfo: UserConfigInfo = this.workspace.getUserConfigInfo(); this.emitInfoMessageIfDiscoveredEslintConfigFileIsNotBeingUsed(userConfigInfo); this.emitInfoMessageIfDiscoveredEslintIgnoreFileIsNotBeingUsed(userConfigInfo); + this.emitWarningIfAutoDiscoveredExecutableConfigFileIsSkipped(userConfigInfo); + this.emitWarningIfExplicitConfigFileIsExecutable(); const eslintOptions: ESLint.Options = this.createESLintOptions(BaseRuleset.RECOMMENDED); const eslint: ESLint = new LegacyESLintWrapper(eslintOptions); @@ -177,7 +179,7 @@ export class LegacyESLintStrategy implements ESLintStrategy { errorOnUnmatchedPattern: false, reportUnusedDisableDirectives: 'off', baseConfig: this.baseConfigFactory.createBaseConfig(baseRuleset) as Linter.Config, // This is applied first (on bottom). - useEslintrc: this.config.auto_discover_eslint_config, // This is applied second. + useEslintrc: this.shouldUseEslintrc(userConfigInfo), // This is applied second. overrideConfigFile: userConfigInfo.getUserConfigFile(), // This is applied third. overrideConfig: overrideConfig as Linter.Config, // This is applied fourth (on top). ignorePath: userConfigInfo.getUserIgnoreFile() @@ -202,6 +204,29 @@ export class LegacyESLintStrategy implements ESLintStrategy { return baseRulesThatAreOn; } + // Determines whether to let ESLint perform its own ".eslintrc.*" tree-walk (which requires/executes any + // ".eslintrc.js"/".eslintrc.cjs" it encounters). We must turn this off whenever auto-discovery surfaced an + // executable config file, otherwise ESLint would execute the very file we are refusing to apply. + private shouldUseEslintrc(userConfigInfo: UserConfigInfo): boolean { + if (userConfigInfo.getSkippedExecutableConfigFile() !== undefined) { + return false; + } + return this.config.auto_discover_eslint_config; + } + + private emitWarningIfAutoDiscoveredExecutableConfigFileIsSkipped(userConfigInfo: UserConfigInfo): void { + const skippedConfigFile: string | undefined = userConfigInfo.getSkippedExecutableConfigFile(); + if (skippedConfigFile) { + this.emitLogEvent(LogLevel.Warn, getMessage('SkippedAutoDiscoveredExecutableConfigFile', skippedConfigFile)); + } + } + + private emitWarningIfExplicitConfigFileIsExecutable(): void { + if (this.config.eslint_config_file && isExecutableConfigFile(this.config.eslint_config_file)) { + this.emitLogEvent(LogLevel.Warn, getMessage('ExplicitExecutableConfigFileWillExecute', this.config.eslint_config_file)); + } + } + private emitInfoMessageIfDiscoveredEslintConfigFileIsNotBeingUsed(userConfigInfo: UserConfigInfo): void { const configFileFound: string | undefined = userConfigInfo.getAutoDiscoveredConfigFile(); if (this.config.eslint_config_file === undefined && !this.config.auto_discover_eslint_config && configFileFound) { diff --git a/packages/code-analyzer-eslint8-engine/src/workspace.ts b/packages/code-analyzer-eslint8-engine/src/workspace.ts index 5a4c8805..22a11cee 100644 --- a/packages/code-analyzer-eslint8-engine/src/workspace.ts +++ b/packages/code-analyzer-eslint8-engine/src/workspace.ts @@ -1,7 +1,7 @@ import {Workspace} from "@salesforce/code-analyzer-engine-api"; import fs from "node:fs"; import path from "node:path"; -import {ESLint8EngineConfig, LEGACY_ESLINT_CONFIG_FILES, LEGACY_ESLINT_IGNORE_FILE} from "./config"; +import {ESLint8EngineConfig, isExecutableConfigFile, LEGACY_ESLINT_CONFIG_FILES, LEGACY_ESLINT_IGNORE_FILE} from "./config"; import {makeUnique} from "./utils"; export type AsyncFilterFnc = (value: T) => Promise; @@ -25,8 +25,27 @@ export class UserConfigInfo { } getUserConfigFile(): string | undefined { - return this.engineConfig.eslint_config_file || - (this.engineConfig.auto_discover_eslint_config ? this.autoDiscoveredConfigFile : undefined); + if (this.engineConfig.eslint_config_file) { + return this.engineConfig.eslint_config_file; + } + // When auto-discovering config, we deliberately refuse to apply an executable config file because doing so + // would run its top-level JavaScript during analysis (an arbitrary code execution vector). Only declarative + // auto-discovered config files (e.g. .eslintrc.json/.yaml/.yml) are applied automatically. + if (this.engineConfig.auto_discover_eslint_config && this.autoDiscoveredConfigFile + && !isExecutableConfigFile(this.autoDiscoveredConfigFile)) { + return this.autoDiscoveredConfigFile; + } + return undefined; + } + + // Returns the auto-discovered config file that was NOT applied because it is executable (and thus would run + // arbitrary top-level code when loaded). Returns undefined when no such file was skipped. + getSkippedExecutableConfigFile(): string | undefined { + if (!this.engineConfig.eslint_config_file && this.engineConfig.auto_discover_eslint_config + && this.autoDiscoveredConfigFile && isExecutableConfigFile(this.autoDiscoveredConfigFile)) { + return this.autoDiscoveredConfigFile; + } + return undefined; } userConfigIsEnabled(): boolean { diff --git a/packages/code-analyzer-eslint8-engine/test/end-to-end.test.ts b/packages/code-analyzer-eslint8-engine/test/end-to-end.test.ts index 49f6d2f6..dec6570b 100644 --- a/packages/code-analyzer-eslint8-engine/test/end-to-end.test.ts +++ b/packages/code-analyzer-eslint8-engine/test/end-to-end.test.ts @@ -5,12 +5,18 @@ import { Engine, EnginePluginV1, EngineRunResults, + EventType, + LogEvent, + LogLevel, RuleDescription, 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 {changeWorkingDirectoryToPackageRoot, createDescribeOptions, createRunOptions} from "./test-helpers"; +import {getMessage} from "../src/messages"; changeWorkingDirectoryToPackageRoot(); @@ -50,4 +56,63 @@ describe('End to end test', () => { 'no-invalid-regexp' ])); }); + + describe('Security regression: RCE via auto-discovered executable ESLint config', () => { + const maliciousWorkspace: string = path.resolve('test', 'test-data', 'workspaceWithMaliciousLegacyConfig'); + const maliciousConfigFile: string = path.join(maliciousWorkspace, '.eslintrc.js'); + 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 ESLint8EnginePlugin(); + 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 ESLint8EnginePlugin(); + 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)); + }); + }); }); \ No newline at end of file diff --git a/packages/code-analyzer-eslint8-engine/test/engine.test.ts b/packages/code-analyzer-eslint8-engine/test/engine.test.ts index fd3ddbb0..acbab96b 100644 --- a/packages/code-analyzer-eslint8-engine/test/engine.test.ts +++ b/packages/code-analyzer-eslint8-engine/test/engine.test.ts @@ -34,6 +34,8 @@ const workspaceThatIgnoresFilesByConfig: string = path.join(legacyConfigCasesFolder, 'workspace_HasFilesIgnoredByConfig'); const workspaceThatHasEslintIgnoreFile: string = path.join(legacyConfigCasesFolder, 'workspace_HasEslintIgnoreFile'); +const workspaceWithMaliciousLegacyConfig: string = + path.join(__dirname, 'test-data', 'workspaceWithMaliciousLegacyConfig'); describe('Tests for the getName method of ESLint8Engine', () => { it('When getName is called, then eslint is returned', () => { @@ -277,6 +279,41 @@ describe('Tests for the describeRules method of ESLint8Engine', () => { expect(ruleDescriptions).toEqual(loadRuleDescriptions('rules_OnlyCustomConfigModifyingExistingRules.goldfile.json')); }); + it('When auto_discover_eslint_config=true and workspace has an executable .eslintrc.js, then it is skipped (only base rules apply) with a warning', async () => { + const engine: ESLint8Engine = new ESLint8Engine({...DEFAULT_CONFIG, + config_root: workspaceWithMaliciousLegacyConfig, + auto_discover_eslint_config: true + }); + const logEvents: LogEvent[] = []; + engine.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); + + const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions( + new Workspace('id', [workspaceWithMaliciousLegacyConfig]))); + + // The executable config is refused, so only the base rules (no custom config) should be applied. The workspace + // only contains a JavaScript file, so only the LWC and JavaScript base rules are expected. + expect(ruleDescriptions).toEqual(makeUniqueAndSorted([...LWC_CONFIG_RULES, ...JS_CONFIG_RULES])); + + const warnMessages: string[] = logEvents.filter(e => e.logLevel === LogLevel.Warn).map(e => e.message); + expect(warnMessages).toContainEqual(getMessage('SkippedAutoDiscoveredExecutableConfigFile', + path.join(workspaceWithMaliciousLegacyConfig, '.eslintrc.js'))); + }); + + it('When eslint_config_file is explicitly set to an executable .eslintrc.js, then it is applied with an execution warning', async () => { + const engine: ESLint8Engine = new ESLint8Engine({...DEFAULT_CONFIG, + config_root: workspaceWithMaliciousLegacyConfig, + eslint_config_file: path.join(workspaceWithMaliciousLegacyConfig, '.eslintrc.js') + }); + const logEvents: LogEvent[] = []; + engine.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event)); + + await engine.describeRules(createDescribeOptions(new Workspace('id', [workspaceWithMaliciousLegacyConfig]))); + + const warnMessages: string[] = logEvents.filter(e => e.logLevel === LogLevel.Warn).map(e => e.message); + expect(warnMessages).toContainEqual(getMessage('ExplicitExecutableConfigFileWillExecute', + path.join(workspaceWithMaliciousLegacyConfig, '.eslintrc.js'))); + }); + it('When file_extensions.javascript is empty, then javascript rules do not get picked up', async () => { const engine: ESLint8Engine = new ESLint8Engine({...DEFAULT_CONFIG, file_extensions: { diff --git a/packages/code-analyzer-eslint8-engine/test/test-data/workspaceWithMaliciousLegacyConfig/.eslintrc.js b/packages/code-analyzer-eslint8-engine/test/test-data/workspaceWithMaliciousLegacyConfig/.eslintrc.js new file mode 100644 index 00000000..981b0100 --- /dev/null +++ b/packages/code-analyzer-eslint8-engine/test/test-data/workspaceWithMaliciousLegacyConfig/.eslintrc.js @@ -0,0 +1,15 @@ +// This is a deliberately "malicious" legacy ESLint config used only by the RCE regression tests. Its top-level code +// simulates arbitrary code execution (the reported bug popped a calculator) by writing a sentinel file to the path +// given in the SENTINEL_PATH environment variable. The tests assert this side effect NEVER happens during +// auto-discovery, and DOES happen only when the operator explicitly opts in via eslint_config_file. +const fs = require('node:fs'); + +if (process.env.SENTINEL_PATH) { + fs.writeFileSync(process.env.SENTINEL_PATH, 'code-was-executed'); +} + +module.exports = { + rules: { + 'no-unused-vars': ['error'] + } +}; diff --git a/packages/code-analyzer-eslint8-engine/test/test-data/workspaceWithMaliciousLegacyConfig/dummy1.js b/packages/code-analyzer-eslint8-engine/test/test-data/workspaceWithMaliciousLegacyConfig/dummy1.js new file mode 100644 index 00000000..bb2ad607 --- /dev/null +++ b/packages/code-analyzer-eslint8-engine/test/test-data/workspaceWithMaliciousLegacyConfig/dummy1.js @@ -0,0 +1,4 @@ +function foo() { + var unused = 1; + return 2; +} diff --git a/packages/code-analyzer-eslint8-engine/test/utils.test.ts b/packages/code-analyzer-eslint8-engine/test/utils.test.ts index 195f7b36..a3e2fc2e 100644 --- a/packages/code-analyzer-eslint8-engine/test/utils.test.ts +++ b/packages/code-analyzer-eslint8-engine/test/utils.test.ts @@ -1,4 +1,6 @@ import {makeUnique} from "../src/utils"; +import {isExecutableConfigFile} from "../src/config"; +import {getMessage} from "../src/messages"; describe('Tests for the makeUnique utility function', () => { it('When an empty array is given, then return it', () => { @@ -13,3 +15,45 @@ describe('Tests for the makeUnique utility function', () => { expect(makeUnique(['hello','1','z','z','hello','world'])).toEqual(['hello','1','z','world']); }); }); + +describe('Tests for the isExecutableConfigFile utility function', () => { + it.each([ + '.eslintrc.js', + '.eslintrc.cjs', + '/some/absolute/path/.eslintrc.js', + 'relative/path/.eslintrc.cjs' + ])('When given an executable legacy config file (%s), then return true', (filePath: string) => { + expect(isExecutableConfigFile(filePath)).toEqual(true); + }); + + it.each([ + '.eslintrc.json', + '.eslintrc.yaml', + '.eslintrc.yml', + '/some/absolute/path/.eslintrc.json', + '.eslintignore' + ])('When given a declarative (non-executable) legacy config file (%s), then return false', (filePath: string) => { + expect(isExecutableConfigFile(filePath)).toEqual(false); + }); + + it('When the file extension is uppercase, then it is matched case-insensitively', () => { + expect(isExecutableConfigFile('.eslintrc.JS')).toEqual(true); + expect(isExecutableConfigFile('.eslintrc.CJS')).toEqual(true); + expect(isExecutableConfigFile('.eslintrc.JSON')).toEqual(false); + }); +}); + +describe('Tests for the new security-related messages', () => { + it('When getting the SkippedAutoDiscoveredExecutableConfigFile message, then it resolves with the file path filled in', () => { + const msg: string = getMessage('SkippedAutoDiscoveredExecutableConfigFile', '/some/.eslintrc.js'); + expect(msg).toContain(`'/some/.eslintrc.js'`); + expect(msg).toContain('was NOT applied'); + expect(msg).toContain('eslint_config_file'); + }); + + it('When getting the ExplicitExecutableConfigFileWillExecute message, then it resolves with the file path filled in', () => { + const msg: string = getMessage('ExplicitExecutableConfigFileWillExecute', '/some/.eslintrc.js'); + expect(msg).toContain(`'/some/.eslintrc.js'`); + expect(msg).toContain('will run during analysis'); + }); +}); From bbe074ba936b7e53117eecc3ebf5af1b68f1566a Mon Sep 17 00:00:00 2001 From: Nikhil Mittal Date: Tue, 18 Aug 2026 22:05:27 +0530 Subject: [PATCH 3/3] fix(eslint-engine): guard optional eslint_config_file in test scenarios to satisfy ConfigValue type --- packages/code-analyzer-eslint-engine/test/engine.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/code-analyzer-eslint-engine/test/engine.test.ts b/packages/code-analyzer-eslint-engine/test/engine.test.ts index 74c6e57f..a06ec50a 100644 --- a/packages/code-analyzer-eslint-engine/test/engine.test.ts +++ b/packages/code-analyzer-eslint-engine/test/engine.test.ts @@ -113,7 +113,7 @@ describe('Tests for the describeRules method of ESLintEngine', () => { const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, config_root: __dirname, auto_discover_eslint_config: true, - eslint_config_file: caseObj.configFile + ...(caseObj.configFile ? {eslint_config_file: caseObj.configFile} : {}) }); const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions()); expect(ruleDescriptions).toEqual(caseObj.expectationRuleDescriptions); @@ -125,7 +125,7 @@ describe('Tests for the describeRules method of ESLintEngine', () => { it.each(testScenarios)('When describing rules while from a workspace $description with the config applied explicitly, then return expected', async (caseObj: TEST_SCENARIO) => { const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, auto_discover_eslint_config: true, - eslint_config_file: caseObj.configFile + ...(caseObj.configFile ? {eslint_config_file: caseObj.configFile} : {}) }); const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions(new Workspace('id', [caseObj.folder]))); expect(ruleDescriptions).toEqual(caseObj.expectationRuleDescriptions); @@ -135,7 +135,7 @@ describe('Tests for the describeRules method of ESLintEngine', () => { const engine: Engine = await createEngineFromPlugin({...DEFAULT_CONFIG_FOR_TESTING, auto_discover_eslint_config: true, config_root: caseObj.folder, - eslint_config_file: caseObj.configFile + ...(caseObj.configFile ? {eslint_config_file: caseObj.configFile} : {}) }); const ruleDescriptions: RuleDescription[] = await engine.describeRules(createDescribeOptions()); expect(ruleDescriptions).toEqual(caseObj.expectationRuleDescriptions);