eslint-community / eslint-plugin-security

ESLint rules for Node Security
Apache License 2.0
2.22k stars 109 forks source link

Bug: Crashes ESLint due to circular reference in config #131

Closed maxmilton closed 11 months ago

maxmilton commented 11 months ago

What version of eslint-plugin-security are you using?

2.0.0

ESLint Environment

Node version: 21.4.0, Bun: 1.0.18 npm version: 9.8.1, pnpm: 8.12.1 Local ESLint version: 8.55.0 Global ESLint version: none Operating System: Arch Linux; Linux 6.6.6-arch1-1 x86_64 unknown

What parser are you using?

Default (Espree)

What did you do?

Configuration

package.json:

{
  "name": "repro-eslint-sec",
  "version": "0.0.0",
  "license": "MIT",
  "private": true,
  "scripts": {
    "lint": "eslint ."
  },
  "devDependencies": {
    "eslint": "8.55.0",
    "eslint-plugin-security": "2.0.0"
  },
  "eslintConfig": {
    "plugins": ["security"],
    "extends": ["plugin:security/recommended"]
  }
}
console.log()

What did you expect to happen?

ESLint config is loaded correctly and ESLint runs without error.

What actually happened?

❱ pnpm run lint

> repro-eslint-sec@0.0.0 lint /home/xxxx/Projects/repro-eslint-sec
> eslint .

Oops! Something went wrong! :(

ESLint: 8.55.0

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'security' -> object with constructor 'Object'
    |     property 'configs' -> object with constructor 'Object'
    |     property 'recommended' -> object with constructor 'Object'
    --- property 'plugins' closes the circle
Referenced from: /home/xxxx/Projects/repro-eslint-sec/package.json
    at JSON.stringify (<anonymous>)
    at /home/xxxx/Projects/repro-eslint-sec/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2156:45
    at Array.map (<anonymous>)
    at ConfigValidator.formatErrors (/home/xxxx/Projects/repro-eslint-sec/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2147:23)
    at ConfigValidator.validateConfigSchema (/home/xxxx/Projects/repro-eslint-sec/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2177:84)
    at ConfigArrayFactory._normalizeConfigData (/home/xxxx/Projects/repro-eslint-sec/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3019:19)
    at ConfigArrayFactory._loadExtendedPluginConfig (/home/xxxx/Projects/repro-eslint-sec/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3239:25)
    at ConfigArrayFactory._loadExtends (/home/xxxx/Projects/repro-eslint-sec/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3154:29)
    at ConfigArrayFactory._normalizeObjectConfigDataBody (/home/xxxx/Projects/repro-eslint-sec/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3095:25)
    at _normalizeObjectConfigDataBody.next (<anonymous>)
 ELIFECYCLE  Command failed with exit code 2.

Participation

Additional comments

A circular reference in the ESLint config was introduced in https://github.com/eslint-community/eslint-plugin-security/pull/118. This causes ESLint to error upon loading the config, when the user config is supplied as anything other than a "flat eslint config"; JS with require('eslint-plugin-security').configs.recommended.

The ESLint flat config is fine to support, however, it should not come at the expense of the regular config variants e.g., package.json#eslintConfig, .eslintrc.

Please support both flat config and regular configs.

aladdin-add commented 11 months ago

Yes, given the flat config has not been widely supported in the community, I'm 👍 to support both. will make a PR later.

Update: to use eslint-plugin-security v2 with eslintrc configs, just use recommended-legacy:

module.exports = {
  extends: ['plugin:security/recommended-legacy'],
};
StefanFl commented 11 months ago

The same problem appears to be there with release 2.1.0

This is my .eslintrc.js file:

module.exports = {
  env: {
    browser: true,
    es2021: true,
  },
  extends: [
    "eslint:recommended",
    "plugin:react/recommended",
    "plugin:@typescript-eslint/recommended",
    "plugin:security/recommended",
    "plugin:react-hooks/recommended",
  ],
  parser: "@typescript-eslint/parser",
  parserOptions: {
    ecmaFeatures: {
      jsx: true,
    },
    ecmaVersion: "latest",
    sourceType: "module",
  },
  plugins: ["react", "@typescript-eslint", "security"],
  rules: {},
  settings: {
    react: {
      pragma: "React", // Pragma to use, default to "React"
      version: "detect", // React version. "detect" automatically picks the version you have installed.
    },
  },
};

I still get

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'security' -> object with constructor 'Object'
    |     property 'configs' -> object with constructor 'Object'
    |     property 'recommended' -> object with constructor 'Object'
    --- property 'plugins' closes the circle

My configuration works with release 1.7.1

aladdin-add commented 11 months ago

@StefanFl the eslintrc config has renamed to recommended-legacy, see https://github.com/eslint-community/eslint-plugin-security?tab=readme-ov-file#eslintrc-config-deprecated.

StefanFl commented 11 months ago

@aladdin-add Thank you for the clarification, that works for the moment and now I start changing the configuration to the more modern way.