Skip to content

Commit 5de9637

Browse files
nzakasmdjermanovic
nzakas
and
mdjermanovic
authored
fix: Ensure shared references in rule configs are separated (#17666)
* fix: Ensure shared references in rule configs are separated. In eslint.config.js files, it's possible for two rule configs to contain references to the same object. That object may be modified during validation, thus affecting all configs, and may create validation errors. This clones rule configs before validation to ensure that each rule is using unique references to manage its configs. Fixes #12592 * Update tests/lib/config/flat-config-array.js Co-authored-by: Milos Djermanovic <[email protected]> * Update tests/lib/config/flat-config-array.js Co-authored-by: Milos Djermanovic <[email protected]> * Update lib/config/flat-config-schema.js Co-authored-by: Milos Djermanovic <[email protected]> * Update lib/config/flat-config-schema.js Co-authored-by: Milos Djermanovic <[email protected]> * Upgrade config-array and update tests --------- Co-authored-by: Milos Djermanovic <[email protected]>
1 parent f30cefe commit 5de9637

3 files changed

Lines changed: 124 additions & 37 deletions

File tree

‎lib/config/flat-config-schema.js‎

Lines changed: 55 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@
55

66
"use strict";
77

8+
//-----------------------------------------------------------------------------
9+
// Requirements
10+
//-----------------------------------------------------------------------------
11+
12+
/*
13+
* Note: This can be removed in ESLint v9 because structuredClone is available globally
14+
* starting in Node.js v17.
15+
*/
16+
const structuredClone = require("@ungap/structured-clone").default;
17+
818
//-----------------------------------------------------------------------------
919
// Type Definitions
1020
//-----------------------------------------------------------------------------
@@ -119,7 +129,7 @@ function normalizeRuleOptions(ruleOptions) {
119129
: [ruleOptions];
120130

121131
finalOptions[0] = ruleSeverities.get(finalOptions[0]);
122-
return finalOptions;
132+
return structuredClone(finalOptions);
123133
}
124134

125135
//-----------------------------------------------------------------------------
@@ -378,48 +388,57 @@ const rulesSchema = {
378388
...second
379389
};
380390

381-
for (const ruleId of Object.keys(result)) {
382-
383-
// avoid hairy edge case
384-
if (ruleId === "__proto__") {
385-
386-
/* eslint-disable-next-line no-proto -- Though deprecated, may still be present */
387-
delete result.__proto__;
388-
continue;
389-
}
390-
391-
result[ruleId] = normalizeRuleOptions(result[ruleId]);
392-
393-
/*
394-
* If either rule config is missing, then the correct
395-
* config is already present and we just need to normalize
396-
* the severity.
397-
*/
398-
if (!(ruleId in first) || !(ruleId in second)) {
399-
continue;
400-
}
401391

402-
const firstRuleOptions = normalizeRuleOptions(first[ruleId]);
403-
const secondRuleOptions = normalizeRuleOptions(second[ruleId]);
392+
for (const ruleId of Object.keys(result)) {
404393

405-
/*
406-
* If the second rule config only has a severity (length of 1),
407-
* then use that severity and keep the rest of the options from
408-
* the first rule config.
409-
*/
410-
if (secondRuleOptions.length === 1) {
411-
result[ruleId] = [secondRuleOptions[0], ...firstRuleOptions.slice(1)];
412-
continue;
394+
try {
395+
396+
// avoid hairy edge case
397+
if (ruleId === "__proto__") {
398+
399+
/* eslint-disable-next-line no-proto -- Though deprecated, may still be present */
400+
delete result.__proto__;
401+
continue;
402+
}
403+
404+
result[ruleId] = normalizeRuleOptions(result[ruleId]);
405+
406+
/*
407+
* If either rule config is missing, then the correct
408+
* config is already present and we just need to normalize
409+
* the severity.
410+
*/
411+
if (!(ruleId in first) || !(ruleId in second)) {
412+
continue;
413+
}
414+
415+
const firstRuleOptions = normalizeRuleOptions(first[ruleId]);
416+
const secondRuleOptions = normalizeRuleOptions(second[ruleId]);
417+
418+
/*
419+
* If the second rule config only has a severity (length of 1),
420+
* then use that severity and keep the rest of the options from
421+
* the first rule config.
422+
*/
423+
if (secondRuleOptions.length === 1) {
424+
result[ruleId] = [secondRuleOptions[0], ...firstRuleOptions.slice(1)];
425+
continue;
426+
}
427+
428+
/*
429+
* In any other situation, then the second rule config takes
430+
* precedence. That means the value at `result[ruleId]` is
431+
* already correct and no further work is necessary.
432+
*/
433+
} catch (ex) {
434+
throw new Error(`Key "${ruleId}": ${ex.message}`, { cause: ex });
413435
}
414436

415-
/*
416-
* In any other situation, then the second rule config takes
417-
* precedence. That means the value at `result[ruleId]` is
418-
* already correct and no further work is necessary.
419-
*/
420437
}
421438

422439
return result;
440+
441+
423442
},
424443

425444
validate(value) {

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,10 @@
6464
"@eslint-community/regexpp": "^4.6.1",
6565
"@eslint/eslintrc": "^2.1.2",
6666
"@eslint/js": "8.51.0",
67-
"@humanwhocodes/config-array": "^0.11.11",
67+
"@humanwhocodes/config-array": "^0.11.13",
6868
"@humanwhocodes/module-importer": "^1.0.1",
6969
"@nodelib/fs.walk": "^1.2.8",
70+
"@ungap/structured-clone": "^1.2.0",
7071
"ajv": "^6.12.4",
7172
"chalk": "^4.0.0",
7273
"cross-spawn": "^7.0.2",

‎tests/lib/config/flat-config-array.js‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1987,4 +1987,71 @@ describe("FlatConfigArray", () => {
19871987
});
19881988

19891989
});
1990+
1991+
// https://github.com/eslint/eslint/issues/12592
1992+
describe("Shared references between rule configs", () => {
1993+
1994+
it("shared rule config should not cause a rule validation error", () => {
1995+
1996+
const ruleConfig = ["error", {}];
1997+
1998+
const configs = new FlatConfigArray([{
1999+
rules: {
2000+
camelcase: ruleConfig,
2001+
"default-case": ruleConfig
2002+
}
2003+
}]);
2004+
2005+
configs.normalizeSync();
2006+
2007+
const config = configs.getConfig("foo.js");
2008+
2009+
assert.deepStrictEqual(config.rules, {
2010+
camelcase: [2, {
2011+
ignoreDestructuring: false,
2012+
ignoreGlobals: false,
2013+
ignoreImports: false
2014+
}],
2015+
"default-case": [2, {}]
2016+
});
2017+
2018+
});
2019+
2020+
2021+
it("should throw rule validation error for camelcase", async () => {
2022+
2023+
const ruleConfig = ["error", {}];
2024+
2025+
const configs = new FlatConfigArray([
2026+
{
2027+
rules: {
2028+
camelcase: ruleConfig
2029+
}
2030+
},
2031+
{
2032+
rules: {
2033+
"default-case": ruleConfig,
2034+
2035+
2036+
camelcase: [
2037+
"error",
2038+
{
2039+
ignoreDestructuring: Date
2040+
}
2041+
2042+
]
2043+
}
2044+
}
2045+
]);
2046+
2047+
configs.normalizeSync();
2048+
2049+
// exact error may differ based on structuredClone implementation so just test prefix
2050+
assert.throws(() => {
2051+
configs.getConfig("foo.js");
2052+
}, /Key "rules": Key "camelcase":/u);
2053+
2054+
});
2055+
2056+
});
19902057
});

0 commit comments

Comments
 (0)