microsoft / rushstack

Monorepo for tools developed by the Rush Stack community
https://rushstack.io/
Other
5.92k stars 595 forks source link

[eslint-config] Enable security rules for Node.js services #2197

Open octogonz opened 4 years ago

octogonz commented 4 years ago

Is this a feature or a bug?

Please describe the actual behavior.

The ESLint ruleset imports eslint-plugin-security but does not enable any rules. This seems to be an oversight.

What is the expected behavior?

Many of those security rules are specific to Node.js web services. For example, detect-object-injection and detect-child-process forbid practices that are a security risk for a Node.js service (which must consider malicious HTTP requests) but safe and common practice for a Node.js tool (which must inherently trust whoever invoked the CLI tool).

To distinguish these cases, it seems that our ruleset should provide separate configs for:

I'm thinking we would distinguish them using import paths. We would deprecate the old index.js entry point and replace it with 3 main options:

Alongside this, we would preserve the @rushstack/eslint-config/react entry point, which gets mixed in for apps using React. (It was separated because the rule requires the project to configure its React version.) But to avoid confusion between "main" entry points versus mixins, we could move it into a folder:

Thus for example, a web service might do this:

my-web-app/.eslintrc.js

require('@rushstack/eslint-config/patch/modern-module-resolution');
module.exports = {
  extends: [
    '@rushstack/eslint-config/web-app',
    '@rushstack/eslint-config/mixins/react',
  ],
  parserOptions: { tsconfigRootDir: __dirname }
};

Whereas a Node.js service might do this: my-web-app/.eslintrc.js

require('@rushstack/eslint-config/patch/modern-module-resolution');
module.exports = {
  extends: ['@rushstack/eslint-config/node-service'],
  parserOptions: { tsconfigRootDir: __dirname }
};

@jjnkr

iclanton commented 4 years ago

This sounds like a good idea to me.