feat(eslint-plugin-devtools): add rule forbidding non-null prototypes for Records (#604)

`Record` types in TS are typically used to indicate that an object
may have arbitrary keys. This means that using `{}` or similar object
literals to initialize them is a poor choice -- if user code can
cause keys like `hasOwnProperty` or `__proto__` to be used as
keys on those objects, they will not behave as intended.

This has been the cause of many security issues in the JS
ecosystem, so let's add a lint rule to limit impact
of these issues and force ourselves to stick to best practices.

Author: addaleax

configs/eslint-config-devtools/common.js

@@ -21,6 +21,7 @@ const tsRules = {
     'error',
     { prefer: 'type-imports' },
   ],
+  '@mongodb-js/devtools/no-plain-object-records': 'error',
 };
 
 const tsxRules = {

configs/eslint-plugin-devtools/index.js

@@ -2,5 +2,6 @@
 module.exports = {
   rules: {
     'no-expect-method-without-call': require('./rules/no-expect-method-without-call'),
+    'no-plain-object-records': require('./rules/no-plain-object-records'),
   },
 };

configs/eslint-plugin-devtools/package.json

@@ -28,8 +28,9 @@
   "devDependencies": {
     "@mongodb-js/mocha-config-devtools": "^1.0.5",
     "@mongodb-js/prettier-config-devtools": "^1.0.1",
+    "@typescript-eslint/rule-tester": "^8.49.0",
     "depcheck": "^1.4.7",
-    "eslint": "^7.25.0",
+    "eslint": "^7.25.0 || ^8.0.0",
     "mocha": "^8.4.0",
     "nyc": "^15.1.0"
   }

configs/eslint-plugin-devtools/rules/no-plain-object-records.js

@@ -0,0 +1,46 @@
+'use strict';
+
+/** @type {import('eslint').Rule.RuleModule} */
+module.exports = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description:
+        'Using {} to initialize records means that keys present on Object.prototype will not be handled properly',
+    },
+    fixable: 'code',
+    schema: [],
+  },
+  create(context) {
+    return {
+      VariableDeclarator(node) {
+        if (
+          node.id.type === 'Identifier' &&
+          node.id.typeAnnotation &&
+          node.id.typeAnnotation.typeAnnotation.type === 'TSTypeReference' &&
+          node.id.typeAnnotation.typeAnnotation.typeName.name === 'Record' &&
+          node.init &&
+          node.init.type === 'ObjectExpression' &&
+          !node.init.properties.some(
+            (prop) =>
+              prop.key.type === 'Identifier' && prop.key.name === '__proto__',
+          )
+        ) {
+          context.report({
+            node,
+            message:
+              '{} is not a good initializer for records. Use Object.create(null) instead.',
+            fix:
+              node.init.properties.length === 0
+                ? (fixer) => fixer.replaceText(node.init, 'Object.create(null)')
+                : (fixer) =>
+                    fixer.insertTextBefore(
+                      node.init.properties[0],
+                      '__proto__: null, ',
+                    ),
+          });
+        }
+      },
+    };
+  },
+};

configs/eslint-plugin-devtools/rules/no-plain-object-records.test.js

@@ -0,0 +1,56 @@
+'use strict';
+const { RuleTester } = require('@typescript-eslint/rule-tester');
+const rule = require('./no-plain-object-records');
+
+RuleTester.afterAll = after;
+const ruleTester = new RuleTester();
+
+ruleTester.run('no-plain-object-records', rule, {
+  valid: [
+    {
+      code: 'const record: Record<string, number> = Object.create(null);',
+    },
+    {
+      code: 'const record: Record<string, number> = {__proto__: null, a: 1, b: 2};',
+    },
+    {
+      code: 'const record: Record<string, number> = {__proto__: 42, a: 1, b: 2};',
+    },
+    {
+      code: 'const record: Record<string, number> = {a, b, __proto__: 42};',
+    },
+  ],
+
+  invalid: [
+    {
+      code: 'const record: Record<string, number> = {a, b};',
+      output: 'const record: Record<string, number> = {__proto__: null, a, b};',
+      errors: [
+        {
+          message:
+            '{} is not a good initializer for records. Use Object.create(null) instead.',
+        },
+      ],
+    },
+    {
+      code: 'const record: Record<string, unknown> = {};',
+      output: 'const record: Record<string, unknown> = Object.create(null);',
+      errors: [
+        {
+          message:
+            '{} is not a good initializer for records. Use Object.create(null) instead.',
+        },
+      ],
+    },
+    {
+      code: 'let foo, record: Record<string, unknown> = {};',
+      output: 'let foo, record: Record<string, unknown> = Object.create(null);',
+      errors: [
+        {
+          message:
+            '{} is not a good initializer for records. Use Object.create(null) instead.',
+        },
+      ],
+    },
+  ],
+});