Fix: clone config before validating (fixes #12592) (#13034)

* Fix: clone config data before validation (fixes #12592)

* Chore: added test  and cloning before validation

* Chore: test fix

* Chore: rules tests fixes

* Update: cloning config using json parse and stringify

* Update: changed test description

* Chore: removed unnecessary tests

* Update: changed clone logic

* Update: using lodash.cloneDeepWith for cloning

* Update: handling circular ref cases

* Update: using JSON method for cloning logic

* Update: moved clone logic to normalieObjectConfigData

* Chore: added comments for cloning reason

lib/cli-engine/config-array-factory.js

@@ -697,6 +697,33 @@ class ConfigArrayFactory {
             ctx.matchBasePath
         );
 
+        /**
+         * Cloning the rule's config as we are setting `useDefaults` to true`
+         * which mutates the config with its default value if present. And when we
+         * refer to a same variable for config for different rules, that referred variable will
+         * be mutated and it will be used for both.
+         *
+         * Example:
+         *
+         * const commonRuleConfig  = ['error', {}];
+         *
+         * Now if we use this variable as a config for rules like this
+         *
+         * {
+         *      rules: {
+         *          "a" : commonRuleConfig,
+         *          "b" : commonRuleConfig,
+         *      }
+         * }
+         *
+         * And if these rules have default values in their schema, their
+         * config will be mutated with default values, the mutated `commonRuleConfig` will be used for `b` as well and it probably
+         * throw schema voilation errors.
+         *
+         * Refer https://github.com/eslint/eslint/issues/12592
+         */
+        const clonedRulesConfig = rules && JSON.parse(JSON.stringify((rules)));
+
         // Flatten `extends`.
         for (const extendName of extendList.filter(Boolean)) {
             yield* this._loadExtends(extendName, ctx);
@@ -731,7 +758,7 @@ class ConfigArrayFactory {
             processor,
             reportUnusedDisableDirectives,
             root,
-            rules,
+            rules: clonedRulesConfig,
             settings
         };
 

tests/fixtures/config-file/cloned-config/circularRefEslintConfig.js

@@ -0,0 +1,15 @@
+let errorConfig = ["error", {}];
+
+class CircularRef {
+  constructor() {
+    this.self = this;
+  }
+}
+
+module.exports = {
+  settings: {
+    sharedData: new CircularRef()
+  },
+
+  rules: { camelcase: errorConfig, "default-case": errorConfig }
+};

tests/fixtures/config-file/cloned-config/eslintConfig.js

@@ -0,0 +1,4 @@
+let errorConfig = ["error", {}];
+module.exports = {
+    rules: { camelcase: errorConfig, "default-case": errorConfig }
+};

tests/fixtures/config-file/cloned-config/eslintConfigFail.js

@@ -0,0 +1,13 @@
+let errorConfig = ["error", {}];
+module.exports = {
+    rules: {
+        camelcase: errorConfig,
+        "default-case": errorConfig ,
+        "camelcase" : [
+            'error',
+            {
+                "ignoreDestructuring": new Date().getUTCFullYear // returns a function
+            }
+        ]
+    }
+};

tests/fixtures/config-file/cloned-config/index.js

@@ -0,0 +1 @@
+var someValue  = "bar";

tests/fixtures/config-file/cloned-config/inlineText.js

@@ -0,0 +1,3 @@
+/*eslint default-case: ["error", {}]*/
+/*eslint camelcase: ["error", {}]*/
+var someValue  = "bar";

tests/lib/cli.js

@@ -187,6 +187,7 @@ describe("cli", () => {
         });
     });
 
+
     describe("when given a config with environment set to Node.js", () => {
         it("should execute without any errors", async () => {
             const configPath = getFixturePath("configurations", "env-node.json");
@@ -1160,4 +1161,58 @@ describe("cli", () => {
         });
     });
 
+    describe("testing the cloned config", () => {
+        describe("config file and input file", () => {
+            it("should not modify original configuration object", async () => {
+                const configPath = getFixturePath("config-file", "cloned-config", "eslintConfig.js");
+                const filePath = getFixturePath("config-file", "cloned-config", "index.js");
+                const args = `--config ${configPath} ${filePath}`;
+
+                const exit = await cli.execute(args);
+
+                assert.strictEqual(exit, 0);
+            });
+        });
+
+        describe("config file and input file", () => {
+            it("should exit with 1 as camelcase has wrong property type", async () => {
+                const configPath = getFixturePath("config-file", "cloned-config", "eslintConfigFail.js");
+                const filePath = getFixturePath("config-file", "cloned-config", "index.js");
+                const args = `--config ${configPath} ${filePath}`;
+
+                try {
+                    await cli.execute(args);
+                } catch (error) {
+                    assert.strictEqual(/Configuration for rule "camelcase" is invalid:/u.test(error), true);
+                }
+
+            });
+        });
+
+        describe("inline config and input file", () => {
+            it("should not modify original configuration object", async () => {
+                const filePath = getFixturePath("config-file", "cloned-config", "inlineText.js");
+                const args = `${filePath}`;
+
+                const exit = await cli.execute(args);
+
+                assert.strictEqual(exit, 0);
+            });
+        });
+
+    });
+
+    describe("handling circular reference while cloning", () => {
+        it("should handle circular ref", async () => {
+            const configPath = getFixturePath("config-file", "cloned-config", "circularRefEslintConfig.js");
+            const filePath = getFixturePath("config-file", "cloned-config", "index.js");
+            const args = `--config ${configPath} ${filePath}`;
+
+            try {
+                await cli.execute(args);
+            } catch (error) {
+                assert.instanceOf(error, Error);
+            }
+        });
+    });
 });