From 0ea08e23ab96b763dc1aaca9998ce1ac69e5f32e Mon Sep 17 00:00:00 2001 From: Teodor-Ioan Baltoi Date: Fri, 13 Dec 2024 16:07:43 +0000 Subject: [PATCH] commit to start adding inline coms trying syntax trying syntax trying syntax parsing and printing url parsing and printing line parsing and printing SmartFixVersion filepath parsing logs to reviews and creation trying a mapping between lines and positions trying a mapping between lines and positions debug trying a mapping between lines and positions debug trying a mapping between lines and positions debug continue trying a mapping between lines and positions debug continue changing body of comments grouping the vulnerabilities based on file and line reformat reformat delete experimentations reformat details reformat details reformat details reformat details retry a push for updates retry more details retry more details retry more details other CVE attempt reformatting reformatting reformatting reformatting --- src/actions.ts | 102 +++++++++++++++++++++- src/index.ts | 19 ++++- src/util.ts | 224 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 343 insertions(+), 2 deletions(-) diff --git a/src/actions.ts b/src/actions.ts index a9254b24..af465f2d 100644 --- a/src/actions.ts +++ b/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 { + 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 = `` // 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'), diff --git a/src/index.ts b/src/index.ts index a7441dfb..094bf5cb 100644 --- a/src/index.ts +++ b/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) diff --git a/src/util.ts b/src/util.ts index de8cb5b9..87627ea1 100644 --- a/src/util.ts +++ b/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 | 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 +function extractLineNumber(url: string): number | undefined { + const match = /#L(\d+)$/.exec(url) // Match #L 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
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('
More details') + + if (parts.length > 1) { + // If the
block is present, append
to the second part + const moreDetailsContent = parts[1].trim() + if (!moreDetailsContent.endsWith('
')) { + return `
More details${moreDetailsContent}
` + } + return `
More details${moreDetailsContent}` + } + + return undefined // No
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')) + } +}