From fb0d50621236e7d8e59b6d052f11b844a5d54289 Mon Sep 17 00:00:00 2001 From: shati-patel <42641846+shati-patel@users.noreply.github.com> Date: Mon, 14 Mar 2022 17:48:05 +0000 Subject: [PATCH 1/5] Create remote file link for alert queries --- .../analyses-results-manager.ts | 14 +++++--- .../src/remote-queries/sarif-processing.ts | 32 +++++++++++++------ .../remote-queries/shared/analysis-result.ts | 11 +++++-- .../view/AnalysisAlertResult.tsx | 2 +- .../src/remote-queries/view/CodePaths.tsx | 4 +-- .../remote-queries/view/FileCodeSnippet.tsx | 23 ++++++++++--- 6 files changed, 61 insertions(+), 25 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts b/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts index 97fa3c45aff..8b0c6bb3429 100644 --- a/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts +++ b/extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts @@ -121,10 +121,12 @@ export class AnalysesResultsManager { throw new Error(`Could not download the analysis results for ${analysis.nwo}: ${e.message}`); } + const fileLinkPrefix = this.createFileLinkPrefix(analysis.nwo, analysis.databaseSha); + let newAnaysisResults: AnalysisResults; const fileExtension = path.extname(artifactPath); if (fileExtension === '.sarif') { - const queryResults = await this.readSarifResults(artifactPath); + const queryResults = await this.readSarifResults(artifactPath, fileLinkPrefix); newAnaysisResults = { ...analysisResults, interpretedResults: queryResults, @@ -152,11 +154,11 @@ export class AnalysesResultsManager { return await extractRawResults(this.cliServer, this.logger, filePath); } - private async readSarifResults(filePath: string): Promise { + private async readSarifResults(filePath: string, fileLinkPrefix: string): Promise { const sarifLog = await sarifParser(filePath); - const processedSarif = extractAnalysisAlerts(sarifLog); - if (processedSarif.errors) { + const processedSarif = extractAnalysisAlerts(sarifLog, fileLinkPrefix); + if (processedSarif.errors.length) { void this.logger.log(`Error processing SARIF file: ${os.EOL}${processedSarif.errors.join(os.EOL)}`); } @@ -166,4 +168,8 @@ export class AnalysesResultsManager { private isAnalysisInMemory(analysis: AnalysisSummary): boolean { return this.internalGetAnalysesResults(analysis.downloadLink.queryId).some(x => x.nwo === analysis.nwo); } + + private createFileLinkPrefix(nwo: string, sha: string): string { + return `https://github.com/${nwo}/blob/${sha}`; + } } diff --git a/extensions/ql-vscode/src/remote-queries/sarif-processing.ts b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts index 0ae886fd1ff..25de2f84f43 100644 --- a/extensions/ql-vscode/src/remote-queries/sarif-processing.ts +++ b/extensions/ql-vscode/src/remote-queries/sarif-processing.ts @@ -15,7 +15,8 @@ import { const defaultSeverity = 'Warning'; export function extractAnalysisAlerts( - sarifLog: sarif.Log + sarifLog: sarif.Log, + fileLinkPrefix: string ): { alerts: AnalysisAlert[], errors: string[] @@ -26,7 +27,7 @@ export function extractAnalysisAlerts( for (const run of sarifLog.runs ?? []) { for (const result of run.results ?? []) { try { - alerts.push(...extractResultAlerts(run, result)); + alerts.push(...extractResultAlerts(run, result, fileLinkPrefix)); } catch (e) { errors.push(`Error when processing SARIF result: ${e}`); continue; @@ -39,14 +40,15 @@ export function extractAnalysisAlerts( function extractResultAlerts( run: sarif.Run, - result: sarif.Result + result: sarif.Result, + fileLinkPrefix: string ): AnalysisAlert[] { const alerts: AnalysisAlert[] = []; - const message = getMessage(result); + const message = getMessage(result, fileLinkPrefix); const rule = tryGetRule(run, result); const severity = tryGetSeverity(run, result, rule) || defaultSeverity; - const codeFlows = getCodeFlows(result); + const codeFlows = getCodeFlows(result, fileLinkPrefix); const shortDescription = getShortDescription(rule, message!); for (const location of result.locations ?? []) { @@ -60,7 +62,10 @@ function extractResultAlerts( const analysisAlert: AnalysisAlert = { message, shortDescription, - filePath, + fileLink: { + fileLinkPrefix, + filePath, + }, severity, codeSnippet, highlightedRegion, @@ -174,7 +179,8 @@ function getHighlightedRegion(region: sarif.Region): HighlightedRegion { } function getCodeFlows( - result: sarif.Result + result: sarif.Result, + fileLinkPrefix: string ): CodeFlow[] { const codeFlows = []; @@ -192,7 +198,10 @@ function getCodeFlows( : undefined; threadFlows.push({ - filePath, + fileLink: { + fileLinkPrefix, + filePath, + }, codeSnippet, highlightedRegion } as ThreadFlow); @@ -206,7 +215,7 @@ function getCodeFlows( return codeFlows; } -function getMessage(result: sarif.Result): AnalysisMessage { +function getMessage(result: sarif.Result, fileLinkPrefix: string): AnalysisMessage { const tokens: AnalysisMessageToken[] = []; const messageText = result.message!.text!; @@ -221,7 +230,10 @@ function getMessage(result: sarif.Result): AnalysisMessage { t: 'location', text: messagePart.text, location: { - filePath: relatedLocation!.physicalLocation!.artifactLocation!.uri!, + fileLink: { + fileLinkPrefix: fileLinkPrefix, + filePath: relatedLocation!.physicalLocation!.artifactLocation!.uri!, + }, highlightedRegion: getHighlightedRegion(relatedLocation!.physicalLocation!.region!), } }); diff --git a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts index 52c5177896c..cdd622a0fc2 100644 --- a/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts +++ b/extensions/ql-vscode/src/remote-queries/shared/analysis-result.ts @@ -19,12 +19,17 @@ export interface AnalysisAlert { message: AnalysisMessage; shortDescription: string; severity: ResultSeverity; - filePath: string; + fileLink: FileLink; codeSnippet: CodeSnippet; highlightedRegion?: HighlightedRegion; codeFlows: CodeFlow[]; } +export interface FileLink { + fileLinkPrefix: string; + filePath: string; +} + export interface CodeSnippet { startLine: number; endLine: number; @@ -43,7 +48,7 @@ export interface CodeFlow { } export interface ThreadFlow { - filePath: string; + fileLink: FileLink; codeSnippet: CodeSnippet; highlightedRegion?: HighlightedRegion; message?: AnalysisMessage; @@ -66,7 +71,7 @@ export interface AnalysisMessageLocationToken { t: 'location'; text: string; location: { - filePath: string; + fileLink: FileLink; highlightedRegion?: HighlightedRegion; }; } diff --git a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx index 408511d1666..736fe49c350 100644 --- a/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/AnalysisAlertResult.tsx @@ -7,7 +7,7 @@ const AnalysisAlertResult = ({ alert }: { alert: AnalysisAlert }) => { const showPathsLink = alert.codeFlows.length > 0; return { - const filePath = codeFlow.threadFlows[codeFlow.threadFlows.length - 1].filePath; + const filePath = codeFlow.threadFlows[codeFlow.threadFlows.length - 1].fileLink.filePath; return filePath.substring(filePath.lastIndexOf('/') + 1); }; diff --git a/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx b/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx index 1e0db50a5ea..9505a1f0d9e 100644 --- a/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import styled from 'styled-components'; -import { CodeSnippet, HighlightedRegion, AnalysisMessage, ResultSeverity } from '../shared/analysis-result'; +import { CodeSnippet, FileLink, HighlightedRegion, AnalysisMessage, ResultSeverity } from '../shared/analysis-result'; import { Box, Link } from '@primer/react'; import VerticalSpace from './VerticalSpace'; @@ -8,6 +8,14 @@ const borderColor = 'var(--vscode-editor-snippetFinalTabstopHighlightBorder)'; const warningColor = '#966C23'; const highlightColor = '#534425'; +const createFileLink = (fileLink: FileLink, startLine?: number, endLine?: number) => { + if (startLine && endLine) { + return `${fileLink.fileLinkPrefix}/${fileLink.filePath}#L${startLine}-L${endLine}`; + } else { + return `${fileLink.fileLinkPrefix}/${fileLink.filePath}`; + } +}; + const getSeverityColor = (severity: ResultSeverity) => { switch (severity) { case 'Recommendation': @@ -106,7 +114,11 @@ const Message = ({ case 'text': return {token.text}; case 'location': - return {token.text}; + return + {token.text} + ; default: return <>; } @@ -165,14 +177,14 @@ const CodeLine = ({ }; const FileCodeSnippet = ({ - filePath, + fileLink, codeSnippet, highlightedRegion, severity, message, messageChildren, }: { - filePath: string, + fileLink: FileLink, codeSnippet: CodeSnippet, highlightedRegion?: HighlightedRegion, severity?: ResultSeverity, @@ -183,11 +195,12 @@ const FileCodeSnippet = ({ const code = codeSnippet.text.split('\n'); const startingLine = codeSnippet.startLine; + const endingLine = codeSnippet.endLine; return ( - {filePath} + {fileLink.filePath} {code.map((line, index) => ( From 48374d0c7da6865285f8b1ef7e89dc20284b9f20 Mon Sep 17 00:00:00 2001 From: shati-patel <42641846+shati-patel@users.noreply.github.com> Date: Mon, 14 Mar 2022 18:02:15 +0000 Subject: [PATCH 2/5] temp changes for testing --- extensions/ql-vscode/src/remote-queries/run-remote-query.ts | 2 +- extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts b/extensions/ql-vscode/src/remote-queries/run-remote-query.ts index f21f8876a33..c76410adcc0 100644 --- a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts +++ b/extensions/ql-vscode/src/remote-queries/run-remote-query.ts @@ -302,7 +302,7 @@ export async function runRemoteQuery( message: 'Sending request' }); - const workflowRunId = await runRemoteQueriesApiRequest(credentials, 'main', language, repositories, owner, repo, base64Pack, dryRun); + const workflowRunId = await runRemoteQueriesApiRequest(credentials, 'get-db-sha', language, repositories, owner, repo, base64Pack, dryRun); const queryStartTime = Date.now(); const queryMetadata = await tryGetQueryMetadata(cliServer, queryFile); diff --git a/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx b/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx index 9bf2c746bd5..116569bd932 100644 --- a/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx +++ b/extensions/ql-vscode/src/remote-queries/view/RemoteQueries.tsx @@ -369,7 +369,7 @@ export function RemoteQueries(): JSX.Element { return
Waiting for results to load.
; } - const showAnalysesResults = false; + const showAnalysesResults = true; try { return
From 91a5d208f101aed8629e7a3ad80ccb132d92758d Mon Sep 17 00:00:00 2001 From: shati-patel <42641846+shati-patel@users.noreply.github.com> Date: Tue, 15 Mar 2022 10:48:11 +0000 Subject: [PATCH 3/5] =?UTF-8?q?Fix=20tests=20(hopefully=20=F0=9F=A4=9E?= =?UTF-8?q?=F0=9F=8F=BD)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../test/pure-tests/sarif-processing.test.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts b/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts index 208db88d124..0ba8a387041 100644 --- a/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts +++ b/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts @@ -378,12 +378,13 @@ describe('SARIF processing', () => { }); describe('extractAnalysisAlerts', () => { + const fakefileLinkPrefix = 'https://example.com'; it('should not return any results if no runs found in the SARIF', () => { const sarif = { // Runs are missing here. } as sarif.Log; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.alerts.length).to.equal(0); @@ -401,7 +402,7 @@ describe('SARIF processing', () => { ] } as sarif.Log; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.alerts.length).to.equal(0); @@ -411,7 +412,7 @@ describe('SARIF processing', () => { const sarif = buildValidSarifLog(); sarif.runs![0]!.results![0]!.message.text = undefined; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(1); @@ -422,7 +423,7 @@ describe('SARIF processing', () => { const sarif = buildValidSarifLog(); sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.contextRegion = undefined; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(1); @@ -433,7 +434,7 @@ describe('SARIF processing', () => { const sarif = buildValidSarifLog(); sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.region = undefined; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.alerts.length).to.equal(1); @@ -443,7 +444,7 @@ describe('SARIF processing', () => { const sarif = buildValidSarifLog(); sarif.runs![0]!.results![0]!.locations![0]!.physicalLocation!.artifactLocation = undefined; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(1); @@ -532,7 +533,7 @@ describe('SARIF processing', () => { ] } as sarif.Log; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(0); expect(result.alerts.length).to.equal(3); @@ -562,7 +563,7 @@ describe('SARIF processing', () => { } ]; - const result = extractAnalysisAlerts(sarif); + const result = extractAnalysisAlerts(sarif, fakefileLinkPrefix); expect(result).to.be.ok; expect(result.errors.length).to.equal(0); From 3c83d4cf8fe657882ddac0b091ce6add32063435 Mon Sep 17 00:00:00 2001 From: shati-patel <42641846+shati-patel@users.noreply.github.com> Date: Tue, 15 Mar 2022 12:22:59 +0000 Subject: [PATCH 4/5] revert branch name --- extensions/ql-vscode/src/remote-queries/run-remote-query.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts b/extensions/ql-vscode/src/remote-queries/run-remote-query.ts index c76410adcc0..f21f8876a33 100644 --- a/extensions/ql-vscode/src/remote-queries/run-remote-query.ts +++ b/extensions/ql-vscode/src/remote-queries/run-remote-query.ts @@ -302,7 +302,7 @@ export async function runRemoteQuery( message: 'Sending request' }); - const workflowRunId = await runRemoteQueriesApiRequest(credentials, 'get-db-sha', language, repositories, owner, repo, base64Pack, dryRun); + const workflowRunId = await runRemoteQueriesApiRequest(credentials, 'main', language, repositories, owner, repo, base64Pack, dryRun); const queryStartTime = Date.now(); const queryMetadata = await tryGetQueryMetadata(cliServer, queryFile); From d02246a04b418c459f480408d758d8064ae0d26e Mon Sep 17 00:00:00 2001 From: shati-patel <42641846+shati-patel@users.noreply.github.com> Date: Tue, 15 Mar 2022 12:27:49 +0000 Subject: [PATCH 5/5] fix test --- .../ql-vscode/test/pure-tests/sarif-processing.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts b/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts index 0ba8a387041..9315f28ecd9 100644 --- a/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts +++ b/extensions/ql-vscode/test/pure-tests/sarif-processing.test.ts @@ -575,7 +575,10 @@ describe('SARIF processing', () => { expect(message.tokens[1].t).to.equal('location'); expect(message.tokens[1].text).to.equal('absolute path'); expect((message.tokens[1] as AnalysisMessageLocationToken).location).to.deep.equal({ - filePath: 'npm-packages/meteor-installer/config.js', + fileLink: { + fileLinkPrefix: fakefileLinkPrefix, + filePath: 'npm-packages/meteor-installer/config.js', + }, highlightedRegion: { startLine: 35, startColumn: 20,