Regex suggestions (#89)

* feat(regex): initial commit

* feat(regex): generateRegexSuggestions function

* feat(regex): fix

* feat(regex): regex search working

* feat: add tests for regexp check

* fix: typo in one of the regex rules

* chore: merge regexp code with main branch

* chore: merge regexp code with main branch

* fix: correct call to createRegexComments

* fix: remove stale tests

---------

Co-authored-by: Alfonso Graziano <[email protected]>

functions/createReview/app.js

@@ -1,6 +1,7 @@
 import * as dotenv from 'dotenv'
 import { createLLMPRComments } from './llm/index.js'
 import { createASTPRComments } from './astParsing/index.js'
+import { createRegexComments } from './regex/index.js'
 import getOctokit from './oktokit/index.js'
 import { filterOutInvalidComments } from './utils.js'
 
@@ -18,19 +19,26 @@ export default async function app(message) {
   )
 
   console.log('[reviewbot] - creating suggestions')
-  // const llmComments = []
+
   const llmComments = await createLLMPRComments(
     messageContext.files,
     messageContext.diff
   )
-  // const astComments = []
+
   const astComments = createASTPRComments(
     messageContext.files,
     messageContext.fullFiles,
     messageContext.diff
   )
 
-  const comments = filterOutInvalidComments(llmComments.concat(astComments))
+  const regexpComments = createRegexComments(
+    messageContext.files,
+    messageContext.diff
+  )
+
+  const comments = filterOutInvalidComments(
+    llmComments.concat(astComments, regexpComments)
+  )
 
   console.log(
     `[reviewbot] - creating review with ${comments.length} comments for commit ${messageContext.latestCommit}`

functions/createReview/llm/index.js

@@ -27,10 +27,10 @@ async function createLLMSuggestions(gitDiff) {
           suggestions: suggestion
         }
       })
-
       return suggestionsForFile
     })
   )
+
   return response
 }
 

functions/createReview/llm/prompt-engine.js

@@ -1,22 +1,6 @@
-import path from 'node:path'
+import { filterAcceptedFiles, filterOnlyModified } from '../utils.js'
 
-function filterOnlyModified(files) {
-  return files
-    .map(file => ({
-      ...file,
-      modifiedLines: file.modifiedLines.filter(line => line.added)
-    }))
-    .filter(file => file.modifiedLines.length > 0)
-}
-
-function filterAcceptedFiles(files) {
-  const filteredFiles = files.filter(f =>
-    /\.[tj]sx?$/g.test(path.extname(f.afterName))
-  )
-  return filteredFiles
-}
-
-function groupByLineRange({ modifiedLines }) {
+export function groupByLineRange({ modifiedLines }) {
   const output = []
   let range = { start: 0, end: 0 }
   let diff = ''

functions/createReview/regex/index.js

@@ -0,0 +1,70 @@
+import { filterAcceptedFiles, filterOnlyModified } from '../utils.js'
+import regexRules from './rules.js'
+import { mapLineToDiff } from 'map-line-to-diff'
+
+/**
+ * Finds and returns issues from a given diff string based on a set of regex rules.
+ *
+ * @param {string} diff - The diff string to analyze.
+ * @param {Array<Object>} rules - An array of rules where each rule object should contain:
+ *   - `id` {string} The unique identifier for the rule.
+ *   - `regex` {RegExp} The regular expression to test against the diff content.
+ *   - `prompt` {string} The description or prompt for the rule.
+ * @returns {Array<Object>} An array of issues found. Each issue object contains:
+ *   - `lineNumber` {number} The line number in the diff where the issue was found.
+ *   - `id` {string} The id of the rule that matched.
+ *   - `description` {string} The description or prompt of the rule that matched.
+ */
+export const findRegexRulesInLine = (lineContent, rules) => {
+  const issues = []
+
+  for (let rule of rules) {
+    if (rule.regex.test(lineContent)) {
+      issues.push(rule)
+    }
+  }
+
+  return issues
+}
+
+export const createRegexSuggestions = gitDiff => {
+  const acceptedFiles = filterAcceptedFiles(gitDiff)
+  const filesWithModifiedLines = filterOnlyModified(acceptedFiles)
+  const filesWithAddDiff = filesWithModifiedLines.filter(file => file.added)
+
+  const comments = []
+
+  filesWithAddDiff.forEach(file => {
+    file.modifiedLines.forEach(line => {
+      if (!line.added) return
+      const lineSuggestions = findRegexRulesInLine(line.line, regexRules).map(
+        rule => rule.description
+      )
+      if (lineSuggestions.length === 0) return
+
+      comments.push({
+        path: file.afterName,
+        lineNumber: line.lineNumber,
+        body: lineSuggestions
+      })
+    })
+  })
+
+  return comments
+}
+
+function transformSuggestionsIntoComments(suggestions, rawDiff) {
+  const comments = suggestions.map(s => {
+    return {
+      path: s.path,
+      position: mapLineToDiff(rawDiff, s.filename, s.lineNumber),
+      body: s.body
+    }
+  })
+  return comments
+}
+
+export const createRegexComments = (gitDiff, rawDiff) => {
+  const suggestions = createRegexSuggestions(gitDiff)
+  return transformSuggestionsIntoComments(suggestions, rawDiff)
+}

functions/createReview/regex/rules.js

@@ -0,0 +1,25 @@
+export default [
+  {
+    id: 'JSON_PARSE_USAGE',
+    regex: /\bJSON\.parse\b/,
+    description:
+      'JSON.parse should be avoided in favor of a more secure alternative like fastify/secure-json-parse.'
+  },
+  {
+    id: 'MD5_USAGE',
+    regex: /\bMD5\b/i,
+    description: 'Do not use MD5, use a better hashing algorithm like SHA256.'
+  },
+  {
+    id: 'LOCALSTORAGE_USAGE',
+    regex: /\blocalStorage\b/,
+    description:
+      "Avoid using localStorage for sensitive data as it's vulnerable to XSS attacks. Consider using HttpOnly cookies or other server-side storage solutions."
+  },
+  {
+    id: 'EVAL_USAGE',
+    regex: /eval\(/,
+    description:
+      'Avoid the use of `eval()`, as it poses security risks and can execute arbitrary code. Consider safer alternatives, or parse data without execution.'
+  }
+]

functions/createReview/utils.js

@@ -1,3 +1,5 @@
+import path from 'node:path'
+
 export function filterOutInvalidComments(comments) {
   const validComments = []
   const invalidComments = []
@@ -14,3 +16,19 @@ export function filterOutInvalidComments(comments) {
   }
   return validComments
 }
+
+export function filterOnlyModified(files) {
+  return files
+    .map(file => ({
+      ...file,
+      modifiedLines: file.modifiedLines.filter(line => line.added)
+    }))
+    .filter(file => file.modifiedLines.length > 0)
+}
+
+export function filterAcceptedFiles(files) {
+  const filteredFiles = files.filter(f =>
+    /\.[tj]sx?$/g.test(path.extname(f.afterName))
+  )
+  return filteredFiles
+}

tests/create-regex-suggestions/createRegexSuggestions.test.js

@@ -0,0 +1,60 @@
+import { describe, test } from 'node:test'
+import assert from 'assert'
+import { createRegexSuggestions } from '../../functions/createReview/regex/index.js'
+
+describe('createRegexSuggestions tests', () => {
+  test('Avoid JSON.parse()', () => {
+    const gitDiff = [
+      {
+        added: true,
+        deleted: false,
+        beforeName: 'functions/createReview/astParsing/rules/J3.example.js',
+        afterName: 'functions/createReview/astParsing/rules/J3.example.js',
+        modifiedLines: [
+          {
+            added: true,
+            lineNumber: 2,
+            line: 'export function example() {'
+          },
+          {
+            added: true,
+            lineNumber: 3,
+            line: "  const obj = {'foo': 'bar'}"
+          },
+          {
+            added: true,
+            lineNumber: 4,
+            line: ''
+          },
+          {
+            added: true,
+            lineNumber: 5,
+            line: '  // expect: J3'
+          },
+          {
+            added: true,
+            lineNumber: 6,
+            line: '  JSON.parse(JSON.stringify(obj))'
+          },
+          {
+            added: true,
+            lineNumber: 7,
+            line: '}'
+          }
+        ]
+      }
+    ]
+
+    const output = createRegexSuggestions(gitDiff)
+
+    assert.deepEqual(output, [
+      {
+        path: 'functions/createReview/astParsing/rules/J3.example.js',
+        lineNumber: 6,
+        body: [
+          'JSON.parse should be avoided in favor of a more secure alternative like fastify/secure-json-parse.'
+        ]
+      }
+    ])
+  })
+})

tests/create-regex-suggestions/regexMatching.test.js

@@ -0,0 +1,53 @@
+import { describe, test } from 'node:test'
+import assert from 'node:assert'
+import { findRegexRulesInLine } from '../../functions/createReview/regex/index.js'
+import regexRules from '../../functions/createReview/regex/rules.js'
+
+describe('findRegexRulesInLine', () => {
+  test('MD5_USAGE', () => {
+    const rules = findRegexRulesInLine('    MD5(myString) ;', regexRules)
+    assert.deepEqual(
+      rules.map(r => {
+        return { id: r.id }
+      }),
+      [
+        {
+          id: 'MD5_USAGE'
+        }
+      ]
+    )
+  })
+
+  test('LOCALSTORAGE_USAGE', () => {
+    const rules = findRegexRulesInLine(
+      '    localStorage.setItem("foo", "bar")',
+      regexRules
+    )
+
+    assert.deepEqual(
+      rules.map(r => {
+        return { id: r.id }
+      }),
+      [
+        {
+          id: 'LOCALSTORAGE_USAGE'
+        }
+      ]
+    )
+  })
+
+  test('EVAL_USAGE', () => {
+    const rules = findRegexRulesInLine('    eval("const f = 2")', regexRules)
+
+    assert.deepEqual(
+      rules.map(r => {
+        return { id: r.id }
+      }),
+      [
+        {
+          id: 'EVAL_USAGE'
+        }
+      ]
+    )
+  })
+})

tests/create-suggestions/index.test.js

@@ -151,7 +151,6 @@ describe('createLLMSuggestions()', () => {
         suggestions: 'mockSuggestion'
       }
     ]
-    assert.strictEqual(response.length, 2)
     assert.deepStrictEqual(response, expectedResponse)
   })