Add custom ESLint rule: safe-typeof-object (#5993)

Ensures that you are not allowed to check if something is an object
simply by using `typeof x === 'object'` without also checking if `x !==
null`.

Author: watson

.github/workflows/eslint-rules.yml

@@ -0,0 +1,23 @@
+name: ESLint Rules
+
+on:
+  pull_request:
+    paths:
+      - 'eslint-rules/**'
+  push:
+    branches: [master]
+    paths:
+      - 'eslint-rules/**'
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref || github.run_id }}
+  cancel-in-progress: true
+
+jobs:
+  eslint-rules:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+      - uses: ./.github/actions/node/active-lts
+      - uses: ./.github/actions/install
+      - run: yarn test:eslint-rules

eslint-rules/eslint-safe-typeof-object.mjs

@@ -0,0 +1,128 @@
+// The content of this file is inspired from the following rule:
+// https://github.com/liferay/liferay-frontend-projects/blob/master/projects/eslint-plugin/rules/general/lib/rules/no-typeof-object.js
+
+export default {
+  meta: {
+    type: 'problem',
+    docs: {
+      description: "Ensure that `typeof x === 'object'` checks are guarded against not-null values",
+      recommended: true
+    },
+    fixable: 'code',
+    schema: [],
+  },
+
+  create (context) {
+    return {
+      BinaryExpression (node) {
+        if (
+          node.left?.type === 'UnaryExpression' &&
+          node.left.operator === 'typeof' &&
+          node.operator === '===' &&
+          node.right?.value === 'object'
+        ) {
+          // Get the expression being checked (x or x.y)
+          const targetExpression = getSourceText(context, node.left.argument)
+
+          let hasNullCheck = false
+
+          // First check if there's a null guard at the immediate AND level
+          let current = node.parent
+          if (current && current.type === 'LogicalExpression' && current.operator === '&&') {
+            const conditions = collectAndConditions(current)
+            hasNullCheck = conditions.some(condition =>
+              condition !== node && isNullCheck(condition, targetExpression, context)
+            )
+          }
+
+          // If not found at immediate level, check broader logical expression
+          if (!hasNullCheck) {
+            current = node.parent
+            let rootLogicalExpression = null
+
+            // Find the root of any logical expression chain
+            while (current && current.type === 'LogicalExpression') {
+              rootLogicalExpression = current
+              current = current.parent
+            }
+
+            if (rootLogicalExpression) {
+              // Collect all conditions in the AND chain
+              const conditions = collectAndConditions(rootLogicalExpression)
+
+              // Check if any condition is a null check for our variable
+              hasNullCheck = conditions.some(condition =>
+                condition !== node && isNullCheck(condition, targetExpression, context)
+              )
+            }
+          }
+
+          if (!hasNullCheck) {
+            context.report({
+              fix (fixer) {
+                return fixer.insertTextBefore(node, `${targetExpression} !== null && `)
+              },
+              message: `"typeof ${targetExpression} === 'object'" missing not-null guard`,
+              node
+            })
+          }
+        }
+      }
+    }
+  }
+}
+
+// Helper function to get source text of a node
+function getSourceText (context, node) {
+  return context.getSourceCode().getText(node)
+}
+
+// Helper function to collect all conditions from a logical AND chain
+function collectAndConditions (node) {
+  const conditions = []
+
+  function traverse (node) {
+    if (node.type === 'LogicalExpression' && node.operator === '&&') {
+      traverse(node.left)
+      traverse(node.right)
+    } else {
+      // Don't traverse into OR expressions, just add them as a whole condition
+      conditions.push(node)
+    }
+  }
+
+  traverse(node)
+
+  return conditions
+}
+
+// Helper function to check if a condition is a null check for the given variable
+function isNullCheck (condition, targetExpression, context) {
+  // Check for x !== null or x.y !== null
+  if (
+    condition.type === 'BinaryExpression' &&
+    condition.operator === '!==' &&
+    condition.right?.value === null &&
+    getSourceText(context, condition.left) === targetExpression
+  ) {
+    return true
+  }
+
+  // Check for x (truthy check) or x.y (truthy check)
+  if (getSourceText(context, condition) === targetExpression) {
+    return true
+  }
+
+  // Check for !!x (boolean conversion check) or !!x.y
+  if (
+    condition.type === 'UnaryExpression' &&
+    condition.operator === '!' &&
+    condition.argument?.type === 'UnaryExpression' &&
+    condition.argument.operator === '!' &&
+    getSourceText(context, condition.argument.argument) === targetExpression
+  ) {
+    return true
+  }
+
+  return false
+}

eslint-rules/eslint-safe-typeof-object.test.mjs

@@ -0,0 +1,69 @@
+import { RuleTester } from 'eslint'
+import rule from './eslint-safe-typeof-object.mjs'
+
+const ruleTester = new RuleTester({
+  languageOptions: { ecmaVersion: 2015 }
+})
+
+ruleTester.run('no-typeof-object', rule, {
+  valid: [
+    "x !== null && typeof x === 'object'",
+    "typeof x === 'object' && x !== null",
+
+    "x.y !== null && typeof x.y === 'object'",
+
+    "true && x !== null && typeof x === 'object'",
+    "someCondition && x !== null && typeof x === 'object'",
+    "someCheck() && x !== null && typeof x === 'object'",
+    "a && b && x !== null && typeof x === 'object'",
+
+    "x && typeof x === 'object'",
+    "!!x && typeof x === 'object'",
+
+    "x !== null && (typeof x === 'object' || typeof x === 'function')",
+    "someCondition && x && (typeof x === 'object' || typeof x === 'function')",
+    "(x !== null) && (typeof x === 'object' || typeof x === 'function')",
+    "!!x && (typeof x === 'object' || typeof x === 'function')",
+
+    "(x !== null && typeof x === 'object') || typeof x === 'number'",
+
+    "typeof x !== 'object'"
+  ],
+  invalid: [
+    {
+      code: "typeof x === 'object'",
+      output: "x !== null && typeof x === 'object'",
+      errors: 1
+    },
+    {
+      code: "typeof x.y === 'object'",
+      output: "x.y !== null && typeof x.y === 'object'",
+      errors: 1
+    },
+    {
+      code: "!x && typeof x === 'object'",
+      output: "!x && x !== null && typeof x === 'object'",
+      errors: 1
+    },
+    {
+      code: "true && typeof x === 'object'",
+      output: "true && x !== null && typeof x === 'object'",
+      errors: 1
+    },
+    {
+      code: "someCondition && someOtherCondition && typeof x === 'object'",
+      output: "someCondition && someOtherCondition && x !== null && typeof x === 'object'",
+      errors: 1
+    },
+    {
+      code: "a && b && c && typeof x === 'object'",
+      output: "a && b && c && x !== null && typeof x === 'object'",
+      errors: 1
+    },
+    {
+      code: "(typeof x === 'object' || typeof x === 'function')",
+      output: "(x !== null && typeof x === 'object' || typeof x === 'function')",
+      errors: 1
+    }
+  ]
+})

eslint.config.mjs

@@ -11,6 +11,7 @@ import globals from 'globals'
 
 import eslintProcessEnv from './eslint-rules/eslint-process-env.mjs'
 import eslintEnvAliases from './eslint-rules/eslint-env-aliases.mjs'
+import eslintSafeTypeOfObject from './eslint-rules/eslint-safe-typeof-object.mjs'
 
 const __filename = fileURLToPath(import.meta.url)
 const __dirname = path.dirname(__filename)
@@ -117,13 +118,15 @@ export default [
       'eslint-rules': {
         rules: {
           'eslint-process-env': eslintProcessEnv,
-          'eslint-env-aliases': eslintEnvAliases
+          'eslint-env-aliases': eslintEnvAliases,
+          'eslint-safe-typeof-object': eslintSafeTypeOfObject
         }
       }
     },
     rules: {
       'eslint-rules/eslint-process-env': 'error',
       'eslint-rules/eslint-env-aliases': 'error',
+      'eslint-rules/eslint-safe-typeof-object': 'error',
       'n/no-restricted-require': ['error', [
         {
           name: 'diagnostics_channel',

package.json

@@ -24,6 +24,7 @@
     "test:appsec:plugins:ci": "yarn services && nyc --no-clean --include \"packages/dd-trace/src/appsec/**/*.js\" -- npm run test:appsec:plugins",
     "test:debugger": "mocha -r 'packages/dd-trace/test/setup/mocha.js' 'packages/dd-trace/test/debugger/**/*.spec.js'",
     "test:debugger:ci": "nyc --no-clean --include 'packages/dd-trace/src/debugger/**/*.js' -- npm run test:debugger",
+    "test:eslint-rules": "node eslint-rules/*.test.mjs",
     "test:trace:core": "tap packages/dd-trace/test/*.spec.js \"packages/dd-trace/test/{ci-visibility,datastreams,encode,exporters,opentelemetry,opentracing,plugins,service-naming,standalone,telemetry}/**/*.spec.js\"",
     "test:trace:core:ci": "npm run test:trace:core -- --coverage --nyc-arg=--include=\"packages/dd-trace/src/**/*.js\"",
     "test:instrumentations": "mocha -r 'packages/dd-trace/test/setup/mocha.js' \"packages/datadog-instrumentations/test/@($(echo $PLUGINS)).spec.js\"",

packages/datadog-instrumentations/src/azure-functions.js

@@ -25,7 +25,7 @@ addHook({ name: '@azure/functions', versions: ['>=4'] }, azureFunction => {
 // The arguments are either an object with a handler property or the handler function itself
 function wrapHandler (method) {
   return function (name, arg) {
-    if (typeof arg === 'object' && arg.hasOwnProperty('handler')) {
+    if (arg !== null && typeof arg === 'object' && arg.hasOwnProperty('handler')) {
       const options = arg
       shimmer.wrap(options, 'handler', handler => traceHandler(handler, name, method.name))
     } else if (typeof arg === 'function') {

packages/datadog-instrumentations/src/helpers/register.js

@@ -72,7 +72,7 @@ for (const packageName of names) {
 
   let hook = hooks[packageName]
 
-  if (typeof hook === 'object') {
+  if (hook !== null && typeof hook === 'object') {
     if (hook.serverless === false && isInServerlessEnvironment()) continue
 
     // some integrations are disabled by default, but can be enabled by setting

packages/datadog-instrumentations/src/mysql2.js

@@ -35,7 +35,7 @@ function wrapConnection (Connection, version) {
   shimmer.wrap(Connection.prototype, 'query', query => function (sql, values, cb) {
     if (!startOuterQueryCh.hasSubscribers) return query.apply(this, arguments)
 
-    if (typeof sql === 'object') sql = sql?.sql
+    if (sql !== null && typeof sql === 'object') sql = sql.sql
 
     if (!sql) return query.apply(this, arguments)
 
@@ -76,7 +76,7 @@ function wrapConnection (Connection, version) {
   shimmer.wrap(Connection.prototype, 'execute', execute => function (sql, values, cb) {
     if (!startOuterQueryCh.hasSubscribers) return execute.apply(this, arguments)
 
-    if (typeof sql === 'object') sql = sql?.sql
+    if (sql !== null && typeof sql === 'object') sql = sql.sql
 
     if (!sql) return execute.apply(this, arguments)
 
@@ -167,7 +167,7 @@ function wrapPool (Pool, version) {
   shimmer.wrap(Pool.prototype, 'query', query => function (sql, values, cb) {
     if (!startOuterQueryCh.hasSubscribers) return query.apply(this, arguments)
 
-    if (typeof sql === 'object') sql = sql?.sql
+    if (sql !== null && typeof sql === 'object') sql = sql.sql
 
     if (!sql) return query.apply(this, arguments)
 
@@ -206,7 +206,7 @@ function wrapPool (Pool, version) {
   shimmer.wrap(Pool.prototype, 'execute', execute => function (sql, values, cb) {
     if (!startOuterQueryCh.hasSubscribers) return execute.apply(this, arguments)
 
-    if (typeof sql === 'object') sql = sql?.sql
+    if (sql !== null && typeof sql === 'object') sql = sql.sql
 
     if (!sql) return execute.apply(this, arguments)
 
@@ -239,7 +239,7 @@ function wrapPoolCluster (PoolCluster) {
 
     if (startOuterQueryCh.hasSubscribers && !wrappedPoolNamespaces.has(poolNamespace)) {
       shimmer.wrap(poolNamespace, 'query', query => function (sql, values, cb) {
-        if (typeof sql === 'object') sql = sql?.sql
+        if (sql !== null && typeof sql === 'object') sql = sql.sql
 
         if (!sql) return query.apply(this, arguments)
 
@@ -274,7 +274,7 @@ function wrapPoolCluster (PoolCluster) {
       })
 
       shimmer.wrap(poolNamespace, 'execute', execute => function (sql, values, cb) {
-        if (typeof sql === 'object') sql = sql?.sql
+        if (sql !== null && typeof sql === 'object') sql = sql.sql
 
         if (!sql) return execute.apply(this, arguments)
 

packages/datadog-instrumentations/src/next.js

@@ -51,7 +51,9 @@ function wrapHandleApiRequest (handleApiRequest) {
 function wrapHandleApiRequestWithMatch (handleApiRequest) {
   return function (req, res, query, match) {
     return instrument(req, res, () => {
-      const page = (match !== null && typeof match === 'object' && typeof match.definition === 'object')
+      const page = (
+        match !== null && typeof match === 'object' && match.definition !== null && typeof match.definition === 'object'
+      )
         ? match.definition.pathname
         : undefined
 

packages/datadog-plugin-child_process/src/scrub-cmd-params.js

@@ -63,7 +63,7 @@ function scrubChildProcessCmd (expression) {
 
       if (token === null) {
         continue
-      } else if (typeof token === 'object') {
+      } else if (typeof token === 'object') { // eslint-disable-line eslint-rules/eslint-safe-typeof-object
         if (token.pattern) {
           result.push(token.pattern)
         } else if (token.op) {