chore(eslint): Add eslint rule to flag new RegExp() usage (#10009)

Add a new eslint rule that flags the usage of `new RegExp()`
constructor calls. The purpose of this rule is to make us aware of the
potential danger of creating a regular expression from (end) user input.
This has led to security incidents in the past.

To be clear, it is perfectly okay to ignore this rule in cases where
we're sure that there's no danger or where input is already escaped.

Author: Lms24

packages/astro/src/server/meta.ts

@@ -73,6 +73,7 @@ export function isValidBaggageString(baggage?: string): boolean {
   const keyRegex = "[-!#$%&'*+.^_`|~A-Za-z0-9]+";
   const valueRegex = '[!#-+-./0-9:<=>?@A-Z\\[\\]a-z{-}]+';
   const spaces = '\\s*';
+  // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- RegExp for readability, no user input
   const baggageRegex = new RegExp(
     `^${keyRegex}${spaces}=${spaces}${valueRegex}(${spaces},${spaces}${keyRegex}${spaces}=${spaces}${valueRegex})*$`,
   );

packages/eslint-config-sdk/src/base.js

@@ -135,6 +135,11 @@ module.exports = {
         '@sentry-internal/sdk/no-optional-chaining': 'error',
         '@sentry-internal/sdk/no-nullish-coalescing': 'error',
 
+        // We want to avoid using the RegExp constructor as it can lead to invalid or dangerous regular expressions
+        // if end user input is used in the constructor. It's fine to ignore this rule if it is safe to use the RegExp.
+        // However, we want to flag each use case so that we're aware of the potential danger.
+        '@sentry-internal/sdk/no-regexp-constructor': 'error',
+
         // JSDOC comments are required for classes and methods. As we have a public facing codebase, documentation,
         // even if it may seems excessive at times, is important to emphasize. Turned off in tests.
         'jsdoc/require-jsdoc': [

packages/eslint-plugin-sdk/src/index.js

@@ -15,5 +15,6 @@ module.exports = {
     'no-eq-empty': require('./rules/no-eq-empty'),
     'no-unsupported-es6-methods': require('./rules/no-unsupported-es6-methods'),
     'no-class-field-initializers': require('./rules/no-class-field-initializers'),
+    'no-regexp-constructor': require('./rules/no-regexp-constructor'),
   },
 };

packages/eslint-plugin-sdk/src/rules/no-regexp-constructor.js

@@ -0,0 +1,29 @@
+'use strict';
+
+/**
+ * This rule was created to flag usages of the RegExp constructor.
+ * It is fine to use `new RegExp` if it is necessary and safe to do so.
+ * "safe" means, that the regular expression is NOT created from unchecked or unescaped user input.
+ * Avoid creating regular expressions from user input, because it can lead to invalid expressions or vulnerabilities.
+ */
+module.exports = {
+  meta: {
+    docs: {
+      description:
+        "Avoid using `new RegExp` constructor to ensure we don't accidentally create invalid or dangerous regular expressions from user input.",
+    },
+    schema: [],
+  },
+  create: function (context) {
+    return {
+      NewExpression(node) {
+        if (node.callee.type === 'Identifier' && node.callee.name === 'RegExp') {
+          context.report({
+            node,
+            message: 'Avoid using the RegExp() constructor with unsafe user input.',
+          });
+        }
+      },
+    };
+  },
+};

packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts

@@ -252,6 +252,7 @@ function convertNextRouteToRegExp(route: string): RegExp {
     )
     .join('/');
 
+  // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- routeParts are from the build manifest, so no raw user input
   return new RegExp(
     `^${rejoinedRouteParts}${optionalCatchallWildcardRegex}(?:/)?$`, // optional slash at the end
   );

packages/nextjs/src/config/loaders/prefixLoader.ts

@@ -30,6 +30,7 @@ export default function prefixLoader(this: LoaderThis<LoaderOptions>, userCode:
   // Fill in placeholders
   let templateCode = fs.readFileSync(templatePath).toString();
   replacements.forEach(([placeholder, value]) => {
+    // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped
     const placeholderRegex = new RegExp(escapeStringForRegex(placeholder), 'g');
     templateCode = templateCode.replace(placeholderRegex, value);
   });

packages/nextjs/src/config/loaders/wrappingLoader.ts

@@ -115,6 +115,7 @@ export default function wrappingLoader(
       // Add a slash at the beginning
       .replace(/(.*)/, '/$1')
       // Pull off the file extension
+      // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- not end user input
       .replace(new RegExp(`\\.(${pageExtensionRegex})`), '')
       // Any page file named `index` corresponds to root of the directory its in, URL-wise, so turn `/xyz/index` into
       // just `/xyz`

packages/nextjs/src/config/webpack.ts

@@ -462,7 +462,7 @@ function isMatchingRule(rule: WebpackModuleRule, projectDir: string): boolean {
   //
   // The next 11 option is ugly, but thankfully 'next', 'babel', and 'loader' do appear in it in the same order as in
   // 'next-babel-loader', so we can use the same regex to test for both.
-  if (!useEntries.some(entry => entry?.loader && new RegExp('next.*(babel|swc).*loader').test(entry.loader))) {
+  if (!useEntries.some(entry => entry?.loader && /next.*(babel|swc).*loader/.test(entry.loader))) {
     return false;
   }
 

packages/nextjs/src/edge/index.ts

@@ -48,6 +48,7 @@ export function init(options: VercelEdgeOptions = {}): void {
 
     // Normally we would use `path.resolve` to obtain the absolute path we will strip from the stack frame to align with
     // the uploaded artifacts, however we don't have access to that API in edge so we need to be a bit more lax.
+    // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped
     const SOURCEMAP_FILENAME_REGEX = new RegExp(`.*${escapeStringForRegex(distDirAbsPath)}`);
 
     const defaultRewriteFramesIntegration = new RewriteFrames({

packages/nextjs/src/server/index.ts

@@ -131,6 +131,7 @@ function addServerIntegrations(options: NodeOptions): void {
     // nextjs always puts the build directory at the project root level, which is also where you run `next start` from, so
     // we can read in the project directory from the currently running process
     const distDirAbsPath = path.resolve(distDirName).replace(/(\/|\\)$/, ''); // We strip trailing slashes because "app:///_next" also doesn't have one
+    // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor -- user input is escaped
     const SOURCEMAP_FILENAME_REGEX = new RegExp(escapeStringForRegex(distDirAbsPath));
 
     const defaultRewriteFramesIntegration = new RewriteFrames({