feat(COD-4173): Inline comments added

src/actions.ts

@@ -1,8 +1,9 @@
 import { create } from '@actions/artifact'
-import { startGroup, endGroup, getInput } from '@actions/core'
+import { startGroup, endGroup, getInput, info } from '@actions/core'
 import { context, getOctokit } from '@actions/github'
 import { retry } from '@octokit/plugin-retry'
 import { Md5 } from 'ts-md5'
+import { VulnerabilityEntry, calculatePosition, generateCombinedReviewBody } from './util'
 
 export async function uploadArtifact(artifactName: string, ...files: string[]) {
   startGroup('Uploading artifact ' + artifactName)
@@ -30,6 +31,7 @@ export async function postCommentIfInPr(message: string): Promise<string | undef
     const foundComment = await findExistingComment(stepHash)
     const escapedMessage = message.replaceAll(/(\s)#([0-9]+\s)/g, '$1#&#8203;$2')
     const messageWithHash = appendHash(escapedMessage, stepHash)
+
     if (foundComment === undefined) {
       return (
         await getIssuesApi().createComment({
@@ -114,6 +116,104 @@ async function findExistingComment(stepHash: string): Promise<number | undefined
   return undefined
 }
 
+// Function that looks if a review comment exists already.
+async function findExistingReviewComment(stepHash: string): Promise<any | undefined> {
+  const { owner, repo } = context.repo
+  const pullNumber = context.payload.pull_request?.number
+
+  if (!pullNumber) return undefined
+
+  // List comments on the pull request
+  const { data: comments } = await getPrApi().listReviewComments({
+    owner,
+    repo,
+    pull_number: pullNumber,
+  })
+
+  // Find the comment matching the unique stepHash in the body.
+  return comments.find((comment) => comment.body?.includes(stepHash))
+}
+
+// This function will take in a VulnerabilityEntry and create or update a review comment on the PR.
+export async function postReviewComment(
+  groupedVulnerabilities: { CVE: VulnerabilityEntry[]; CWE: VulnerabilityEntry[] },
+  filePath: string,
+  line: number
+) {
+  if (context.payload.pull_request) {
+    // Post a review comment to the PR.
+    try {
+      // Extract necessary data from context
+      const { owner, repo } = context.repo
+      const pullNumber = context.payload.pull_request.number
+      const commitId = context.payload.pull_request.head.sha
+
+      if (!pullNumber || !commitId) {
+        throw new Error('Pull request number or commit SHA is missing from the context.')
+      }
+
+      const stepHash = `<!-- Vulnerabilities: ${filePath}-${line} -->` // Unique comment identifier
+
+      // Fetch the PR and look for for the diff incorporating the current entry's file.
+      const files = await getPrApi().listFiles({
+        ...context.repo,
+        pull_number: context.payload.pull_request.number,
+      })
+
+      const file = files.data.find((f) => f.filename === filePath)
+
+      if (!file || !file.patch) {
+        throw new Error(`Patch not found for file: ${filePath}`)
+      }
+
+      // Calculate position in the diff
+      const position = calculatePosition(file.patch, line)
+      if (!position) {
+        info('Could not find an appropriate position in the diff. Skipping review comment.')
+        return
+      }
+
+      // Check for an existing comment
+      const foundComment = await findExistingReviewComment(stepHash)
+
+      // Comment body
+      const commentBody = generateCombinedReviewBody(
+        groupedVulnerabilities,
+        filePath,
+        line,
+        stepHash
+      )
+
+      if (foundComment) {
+        info('Found existing review comment.')
+        // Update the existing comment
+        await getPrApi().updateReviewComment({
+          owner,
+          repo,
+          comment_id: foundComment.id,
+          body: commentBody,
+        })
+        info(`Updated comment for ${filePath}:${line}`)
+      } else {
+        info('Trying to create new review comment.')
+        // Create a new review comment
+        await getPrApi().createReviewComment({
+          owner,
+          repo,
+          pull_number: pullNumber,
+          commit_id: commitId,
+          path: filePath,
+          position: position, // Will need position mapping later
+          body: commentBody,
+        })
+        info(`Created comment for ${filePath}:${line}`)
+      }
+    } catch (error) {
+      info(`Failed to post or update comment for ${filePath}:${line}:` + error)
+    }
+  }
+}
+
 function makeOctokit() {
   return getOctokit(
     getInput('token'),

src/index.ts

@@ -3,6 +3,7 @@ import { existsSync, appendFileSync } from 'fs'
 import {
   downloadArtifact,
   postCommentIfInPr,
+  postReviewComment,
   resolveExistingCommentIfFound,
   uploadArtifact,
 } from './actions'
@@ -18,9 +19,12 @@ import {
   getOrDefault,
   getRequiredEnvVariable,
   getRunUrl,
+  groupVulnerabilitiesByLineAndType,
+  parseVulnerabilities,
   telemetryCollector,
 } from './util'
 import { downloadKeys, trustedKeys } from './keys'
+import { group } from 'console'
 
 const scaSarifReport = 'scaReport/output.sarif'
 const scaReport = 'sca.sarif'
@@ -117,7 +121,20 @@ async function displayResults() {
     if (getInput('footer') !== '') {
       message += '\n\n' + getInput('footer')
     }
-    info(message)
+
+    // Breaking the message into individual vulnerability entries.
+    var entries = parseVulnerabilities(message)
+
+    const groupedVulnerabilities = groupVulnerabilitiesByLineAndType(entries)
+
+    for (const key of Object.keys(groupedVulnerabilities)) {
+      const [filePath, line] = key.split(':')
+      const groupedEntries = groupedVulnerabilities[key]
+      // For each file/line, we will post the batch of vulnerabilities affecting it.
+      info('Posting a review comment for ' + filePath + ' ' + line)
+      await postReviewComment(groupedEntries, filePath, parseInt(line, 10))
+    }
+
     const commentUrl = await postCommentIfInPr(message)
     if (commentUrl !== undefined) {
       setOutput('posted-comment', commentUrl)

src/util.ts

@@ -90,3 +90,227 @@ export function getOrDefault(name: string, defaultValue: string) {
   if (setTo !== undefined && setTo.length > 0) return setTo
   return defaultValue
 }
+
+// This interface will be used to store the vulnerabilities found in the comparison report.
+export interface VulnerabilityEntry {
+  name: string // Title of the vulnerability (e.g., CVE-2021-1234, cookie-without-domain-js) - SCA/SAST tool specific.
+  type: 'CVE' | 'CWE' // Type of the vulnerability.
+  url: string // Where to find the vulnerability inside the codebase.
+  line: number // Line number where the vulnerability was - extracted from the URL.
+  details: string // Description of the vulnerability.
+  filePath?: string // Path to the file where the vulnerability was found.
+  SmartFix?: string // Specific to SCA - will be used to suggest the right version to upgrade to and contains supporting text.
+  SmartFixVersion?: string // Specific to SCA - will be used to suggest the right version to upgrade to.
+  moreDetails?: string // Additional details about the vulnerability to be printed in the PR.
+}
+
+// This function is used to break the vulnerabilities clumped together into individual vulnerabilities. We aim to store information such as SmartFix version, line number, etc.
+export function parseVulnerabilities(message: string) {
+  const entries: VulnerabilityEntry[] = []
+  const lines = message.split('\n')
+
+  let currentEntry: Partial<VulnerabilityEntry> | null = null
+
+  for (const line of lines) {
+    const trimmedLine = line.trim()
+
+    // Start of a new vulnerability entry
+    const match = /^\*\s*([^\s]+)\s+(.*)$/.exec(trimmedLine)
+    if (match) {
+      // Push the previous entry to the list, if any
+      if (currentEntry && currentEntry.name && currentEntry.details) {
+        entries.push(currentEntry as VulnerabilityEntry)
+      }
+
+      // Create a new entry
+      currentEntry = {
+        name: match[1], // Vulnerability name (e.g., CVE-2021-1234)
+        details: match[2], // Details after name
+      }
+
+      // Determine the type of vulnerability based on naming.
+      if (currentEntry.name) {
+        if (currentEntry.name.startsWith('CVE')) {
+          currentEntry.type = 'CVE'
+        } else {
+          currentEntry.type = 'CWE'
+        }
+      }
+
+      if (currentEntry.details) {
+        const url = extractUrl(currentEntry.details)
+        if (url) {
+          currentEntry.url = url
+          // Extract the line number from the URL.
+          const lineNumber = extractLineNumber(url)
+          if (lineNumber) {
+            currentEntry.line = lineNumber
+          }
+        }
+
+        const filePath = extractFilePath(currentEntry.details)
+        if (filePath) {
+          currentEntry.filePath = filePath
+        }
+
+        const moreDetails = extractMoreDetails(currentEntry.details)
+        if (moreDetails) {
+          currentEntry.moreDetails = moreDetails
+        } else {
+          currentEntry.moreDetails = 'No more details provided.'
+        }
+      }
+    }
+
+    // Parse SmartFix field
+    if (currentEntry) {
+      const smartFixMatch = /^SmartFix:\s*(\d+\.\d+\.\d+)(.*)$/.exec(trimmedLine)
+      if (smartFixMatch) {
+        currentEntry.SmartFix = smartFixMatch[0].trim() // Full SmartFix text
+        currentEntry.SmartFixVersion = smartFixMatch[1] // Extracted version
+      }
+    }
+
+    // Skip unsupported lines
+    const isSupportedLine = /^\*|SmartFix:/.test(trimmedLine)
+    if (!isSupportedLine) {
+      continue
+    }
+  }
+
+  // Push the last entry, if any
+  if (currentEntry && currentEntry.name && currentEntry.details) {
+    entries.push(currentEntry as VulnerabilityEntry)
+  }
+
+  return entries
+}
+
+// This function identifies the URL from the details of the vulnerability. CVEs and CWEs have the URLs in the second block of paranthesis. For reference, here are examples:
+// CVE-XX-YY ([pom.xml: com.artifact:artifact@1.4.6](https://github.com/lacework-dev/WebGoat/blob/faf0ff128a287a3a341c90e61720313b98d43ea3/pom.xml#L308))
+// no-csrf-protection-in-express-js ([vuln.js: https://github.com/lacework-dev/WebGoat](https://github.com/lacework-dev/WebGoat/blob/faf0ff128a287a3a341c90e61720313b98d43ea3/vuln.js#L2))
+function extractUrl(details: string): string | undefined {
+  // Match the URL inside parentheses at the end of the details string
+  const match = /\((https?:\/\/[^\s)]+)\)/.exec(details)
+  if (match) {
+    return match[1] // Extracted URL
+  }
+  return undefined // Fallback if no URL is found
+}
+
+// This function will take in the URL as input and extract the line number from it. The format is https://.....#L<number>
+function extractLineNumber(url: string): number | undefined {
+  const match = /#L(\d+)$/.exec(url) // Match #L<number> at the end of the URL
+  return match ? parseInt(match[1], 10) : undefined
+}
+
+// This function will extract the file path from the details string. The file path is the text before the ':' in square brackets.
+function extractFilePath(details: string): string | undefined {
+  const match = /\[([^\s:]+):/.exec(details) // Match the text before ':' in square brackets
+  return match ? match[1] : undefined
+}
+
+// This function will extract the entire <details> block from the details string. This will be used to provide more context in the PR review comment.
+function extractMoreDetails(details: string): string | undefined {
+  const parts = details.split('<details><summary>More details</summary>')
+
+  if (parts.length > 1) {
+    // If the <details> block is present, append </details> to the second part
+    const moreDetailsContent = parts[1].trim()
+    if (!moreDetailsContent.endsWith('</details>')) {
+      return `<details><summary>More details</summary>${moreDetailsContent}</details>`
+    }
+    return `<details><summary>More details</summary>${moreDetailsContent}`
+  }
+
+  return undefined // No <details> block found
+}
+
+// This function will calculate the "position" parameter based on the diff hunk and the target line number.
+export function calculatePosition(patch: string, targetLine: number): number | undefined {
+  const patchLines = patch.split('\n')
+  let position = 0 // Position in the diff
+  let currentLine = 0 // Tracks the file's line number
+
+  for (const line of patchLines) {
+    position++
+
+    if (line.startsWith('@@')) {
+      // Extract the starting line number from the diff hunk header
+      const match = /@@ -\d+,\d+ \+(\d+),(\d+) @@/.exec(line)
+      if (match) {
+        currentLine = parseInt(match[1], 10) - 1 // Start line in the new file
+      }
+    } else if (!line.startsWith('-')) {
+      // Skip lines removed from the old file
+      currentLine++
+    }
+
+    // Check if we've reached the target line
+    if (currentLine === targetLine) {
+      return position // Return the position in the diff
+    }
+  }
+
+  return undefined // Line not found in the patch
+}
+
+// This function will group the vulnerabilities by file path and line number information, as well as type (CVE or CWE).
+export function groupVulnerabilitiesByLineAndType(vulnerabilities: VulnerabilityEntry[]) {
+  // The key will be a combination of the file path and line number. This will be used later in identifying PR review comments.
+  const groupedVulnerabilities: Record<
+    string,
+    { CVE: VulnerabilityEntry[]; CWE: VulnerabilityEntry[] }
+  > = {}
+
+  for (const entry of vulnerabilities) {
+    const key = `${entry.filePath}:${entry.line}`
+    if (!groupedVulnerabilities[key]) {
+      groupedVulnerabilities[key] = { CVE: [], CWE: [] }
+    }
+    groupedVulnerabilities[key][entry.type].push(entry)
+  }
+
+  return groupedVulnerabilities
+}
+
+// This function will generate a body message reflecting all the vulnerabilities found for a particular line inside a specific file.
+export function generateCombinedReviewBody(
+  groupedVulnerabilities: { CVE: VulnerabilityEntry[]; CWE: VulnerabilityEntry[] },
+  filePath: string,
+  line: number,
+  stepHash: string
+): string {
+  let body = `${stepHash}\n\n`
+
+  if (groupedVulnerabilities.CVE.length > 0) {
+    body += `\n#### CVEs:\n`
+    groupedVulnerabilities.CVE.forEach((entry) => {
+      body += `- **${entry.name}**: ${entry.moreDetails || 'No more details provided.'}\n`
+    })
+  }
+
+  if (groupedVulnerabilities.CWE.length > 0) {
+    body += `\n#### CWEs:\n`
+    groupedVulnerabilities.CWE.forEach((entry) => {
+      body += `- **${entry.name}**: ${entry.moreDetails || 'No more details provided.'}\n`
+    })
+  }
+
+  return body
+}
+
+export function printEntries(entries: VulnerabilityEntry[]) {
+  for (const entry of entries) {
+    info('Here is an entry: ')
+    info('Name: ' + entry.name)
+    info('Type: ' + entry.type)
+    info('Details: ' + entry.details)
+    info('SmartFix: ' + (entry.SmartFix ?? 'No SmartFix'))
+    info('SmartFixVersion: ' + (entry.SmartFixVersion ?? 'No SmartFixVersion'))
+    info('URL: ' + entry.url)
+    info('Line: ' + entry.line)
+    info('FilePath: ' + (entry.filePath ?? 'No FilePath'))
+    info('More Details: ' + (entry.moreDetails ?? 'No More Details'))
+  }
+}