From 48fe7357acfc97252eb3a46c5ad3d9f32ba70144 Mon Sep 17 00:00:00 2001 From: orkamara Date: Wed, 3 Mar 2021 19:11:55 +0200 Subject: [PATCH] fix: wrong number of calculated issues as part of the Sarif conversion --- src/sarif_converter.ts | 97 ++++++++++--------- tests/__snapshots__/git.analysis.spec.ts.snap | 96 ++++++++++++++++++ tests/git.analysis.spec.ts | 38 +++++++- tests/sarif_converter.spec.ts | 20 ++-- 4 files changed, 196 insertions(+), 55 deletions(-) create mode 100644 tests/__snapshots__/git.analysis.spec.ts.snap diff --git a/src/sarif_converter.ts b/src/sarif_converter.ts index 21c9f7f6..4c258e56 100644 --- a/src/sarif_converter.ts +++ b/src/sarif_converter.ts @@ -1,5 +1,5 @@ // eslint-disable-next-line import/no-unresolved -import { Log, ReportingConfiguration, ReportingDescriptor, Result } from 'sarif'; +import { Log, ReportingConfiguration, ReportingDescriptor, Result, Tool } from 'sarif'; import { IAnalysisResult, IFileSuggestion, RuleProperties } from './interfaces/analysis-result.interface'; @@ -19,19 +19,19 @@ interface ISarifSuggestion extends IFileSuggestion { } interface ISarifSuggestions { - [suggestionIndex: number]: ISarifSuggestion; + [suggestionIndex: number]: ISarifSuggestion[]; } export default function getSarif(analysisResults: IAnalysisResult): Log { - const { tool, suggestions } = getTools(analysisResults, getSuggestions(analysisResults)); - const results = getResults(suggestions); + const allIssuesBySuggestion: ISarifSuggestions = getSuggestions(analysisResults); + const { rules, allIssues } = getRulesAndAllIssues(analysisResults, allIssuesBySuggestion); return { $schema: 'https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json', version: '2.1.0', runs: [ { - tool, - results, + tool: getTool(rules), + results: getResults(allIssues), properties: { coverage: analysisResults.coverage, }, @@ -40,23 +40,23 @@ export default function getSarif(analysisResults: IAnalysisResult): Log { }; } -const getSuggestions = (analysisResults: IAnalysisResult): ISarifSuggestions => { - const suggestions = {}; +function getSuggestions(analysisResults: IAnalysisResult): ISarifSuggestions { + const suggestions: ISarifSuggestions = {}; for (const [file] of Object.entries(analysisResults.files)) { for (const [issueId, issues] of Object.entries(analysisResults.files[file])) { - if (!suggestions || !Object.keys(suggestions).includes(issueId)) { - suggestions[issueId] = { ...issues[0], file: file.substring(1) }; + if (!Object.keys(suggestions).includes(issueId)) { + suggestions[issueId] = []; } + suggestions[issueId].push({ ...issues[0], file: file.substring(1) }); } } return suggestions; }; -const getTools = (analysisResults: IAnalysisResult, suggestions: ISarifSuggestions) => { - const output = { driver: { name: 'SnykCode', semanticVersion: '1.0.0', version: '1.0.0' } }; - const rules = []; +function getRulesAndAllIssues(analysisResults: IAnalysisResult, allIssuesBySuggestions: ISarifSuggestions): { rules: ReportingDescriptor[], allIssues: ISarifSuggestion[] } { let ruleIndex = 0; - const result: ISarifSuggestions = {}; + const rules: ReportingDescriptor[] = []; + const allIssues: ISarifSuggestion[] = []; for (const [suggestionIndex, suggestion] of Object.entries(analysisResults.suggestions)) { const severity = { 1: 'note', @@ -96,61 +96,65 @@ const getTools = (analysisResults: IAnalysisResult, suggestions: ISarifSuggestio rules.push(rule); // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - result[suggestionIndex] = { - ...suggestions[suggestionIndex], - ruleIndex, - rule, - level: severity, - id: suggestionId, - text: suggestion.message, - }; + const allIssuesOfSuggestion: ISarifSuggestion[] = allIssuesBySuggestions[suggestionIndex]; + allIssuesOfSuggestion.forEach(issue => { + allIssues.push({ + ...issue, + ruleIndex, + rule, + level: severity, + id: suggestionId, + text: suggestion.message, + }); + + }); ruleIndex += 1; } - return { tool: { driver: { ...output.driver, rules } }, suggestions: result }; + return { rules, allIssues }; }; -function getResults(suggestions: ISarifSuggestions): Result[] { +function getResults(allIssues: ISarifSuggestion[]): Result[] { const output = []; - for (const [, suggestion] of <[string, ISarifSuggestion][]>Object.entries(suggestions)) { + for (const issue of allIssues) { let helpers: any[] = []; let result: Result = { - ruleId: suggestion.id, - ruleIndex: suggestion.ruleIndex, - level: suggestion.level ? suggestion.level : 'none', + ruleId: issue.id, + ruleIndex: issue.ruleIndex, + level: issue.level ? issue.level : 'none', message: { - text: suggestion.text, - markdown: suggestion.text, + text: issue.text, + markdown: issue.text, arguments: [''], }, locations: [ { physicalLocation: { artifactLocation: { - uri: suggestion.file, + uri: issue.file, uriBaseId: '%SRCROOT%', }, region: { - startLine: suggestion.rows[0], - endLine: suggestion.rows[1], - startColumn: suggestion.cols[0], - endColumn: suggestion.cols[1], + startLine: issue.rows[0], + endLine: issue.rows[1], + startColumn: issue.cols[0], + endColumn: issue.cols[1], }, }, }, ] }; - if (suggestion.fingerprints) { + if (issue.fingerprints) { result.fingerprints = {}; - suggestion.fingerprints.forEach(fingerprinting => { + issue.fingerprints.forEach(fingerprinting => { (result.fingerprints as any)[`${fingerprinting.version}`] = fingerprinting.fingerprint; }); } const codeThreadFlows = []; let i = 0; - if (suggestion.markers && suggestion.markers.length >= 1) { - for (const marker of suggestion.markers) { + if (issue.markers && issue.markers.length >= 1) { + for (const marker of issue.markers) { for (const position of marker.pos) { const helperIndex = helpers.findIndex(helper => helper.msg === marker.msg); if (helperIndex != -1) { @@ -184,14 +188,14 @@ function getResults(suggestions: ISarifSuggestions): Result[] { id: 0, physicalLocation: { artifactLocation: { - uri: suggestion.file, + uri: issue.file, uriBaseId: '%SRCROOT%', }, region: { - startLine: suggestion.rows[0], - endLine: suggestion.rows[1], - startColumn: suggestion.cols[0], - endColumn: suggestion.cols[1], + startLine: issue.rows[0], + endLine: issue.rows[1], + startColumn: issue.cols[0], + endColumn: issue.cols[1], }, }, }, @@ -256,3 +260,8 @@ export function getArgumentsAndMessage( return { message, argumentArray }; } + +function getTool(rules: ReportingDescriptor[]): Tool { + const output = { driver: { name: 'SnykCode', semanticVersion: '1.0.0', version: '1.0.0' } }; + return { driver: { ...output.driver, rules } }; +} diff --git a/tests/__snapshots__/git.analysis.spec.ts.snap b/tests/__snapshots__/git.analysis.spec.ts.snap new file mode 100644 index 00000000..9d9cb294 --- /dev/null +++ b/tests/__snapshots__/git.analysis.spec.ts.snap @@ -0,0 +1,96 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Functional test of analysis detailed sarif tests analyze remote git and formatter sarif with supported files 1`] = ` +Array [ + Object { + "files": 115, + "isSupported": false, + "lang": "Unknown", + }, + Object { + "files": 34, + "isSupported": false, + "lang": "Markdown", + }, + Object { + "files": 6, + "isSupported": false, + "lang": "YAML", + }, + Object { + "files": 3, + "isSupported": false, + "lang": "Shell", + }, + Object { + "files": 39, + "isSupported": false, + "lang": "Text", + }, + Object { + "files": 34, + "isSupported": false, + "lang": "XML", + }, + Object { + "files": 4, + "isSupported": false, + "lang": "INI", + }, + Object { + "files": 122, + "isSupported": true, + "lang": "JavaScript", + }, + Object { + "files": 81, + "isSupported": false, + "lang": "JSON", + }, + Object { + "files": 13, + "isSupported": false, + "lang": "CSS", + }, + Object { + "files": 21, + "isSupported": false, + "lang": "Java Properties", + }, + Object { + "files": 6, + "isSupported": false, + "lang": "SVG", + }, + Object { + "files": 78, + "isSupported": true, + "lang": "HTML", + }, + Object { + "files": 45, + "isSupported": false, + "lang": "Less", + }, + Object { + "files": 989, + "isSupported": true, + "lang": "Java", + }, + Object { + "files": 4, + "isSupported": false, + "lang": "SQLPL", + }, + Object { + "files": 8, + "isSupported": false, + "lang": "CSV", + }, + Object { + "files": 1, + "isSupported": false, + "lang": "Batchfile", + }, +] +`; diff --git a/tests/git.analysis.spec.ts b/tests/git.analysis.spec.ts index 39f5ac05..6deb8151 100644 --- a/tests/git.analysis.spec.ts +++ b/tests/git.analysis.spec.ts @@ -94,7 +94,7 @@ describe('Functional test of analysis', () => { }); // Test DC JSON format first - expect(Object.keys(bundle.analysisResults.suggestions).length).toEqual(121); + expect(Object.keys(bundle.analysisResults.suggestions).length).toEqual(120); const cweSuggestion = Object.values(bundle.analysisResults.suggestions).find( s => s.id === 'java%2Fdc_interfile_project%2FPT', @@ -104,8 +104,8 @@ describe('Functional test of analysis', () => { expect(cweSuggestion?.title).toBeTruthy(); expect(cweSuggestion?.text).toBeTruthy(); - expect(bundle.sarifResults?.runs[0].results?.length).toEqual(121); - expect(bundle.sarifResults?.runs[0].tool?.driver.rules?.length).toEqual(121); + expect(bundle.sarifResults?.runs[0].results?.length).toEqual(236); + expect(bundle.sarifResults?.runs[0].tool?.driver.rules?.length).toEqual(120); const cweRule = bundle.sarifResults?.runs[0].tool?.driver.rules?.find(r => r.id === 'java/PT'); expect(cweRule?.properties?.cwe).toContain('CWE-23'); @@ -160,6 +160,27 @@ describe('Functional test of analysis', () => { TEST_TIMEOUT, ); + it( + 'analyze remote git and formatter sarif with supported files', + async () => { + const bundle = await analyzeGit({ + baseURL, + sessionToken, + includeLint: false, + severity: 1, + gitUri: 'git@github.com:OpenRefine/OpenRefine.git@437dc4d74110ce006b9f829fe05f461cc8ed1170', + sarif: true, + }); + expect(bundle.sarifResults?.runs[0].properties?.coverage).toMatchSnapshot(); + + const numOfIssues = getNumOfIssues(bundle); + const numOfIssuesInSarif = bundle.sarifResults?.runs[0].results?.length; + + expect(numOfIssuesInSarif).toEqual(numOfIssues); + }, + TEST_TIMEOUT, + ); + it('should match sarif schema', () => { const validationResult = jsonschema.validate(sarifResults, sarifSchema); // this is to debug any errors found @@ -169,3 +190,14 @@ describe('Functional test of analysis', () => { }); }); }); + +function getNumOfIssues(bundle: IGitBundle): number { + let numberOfIssues = 0; + + Object.keys(bundle.analysisResults.files).forEach(filePath => { + const curFile = bundle.analysisResults.files[filePath]; + numberOfIssues += Object.keys(curFile).length; + }); + + return numberOfIssues; +} diff --git a/tests/sarif_converter.spec.ts b/tests/sarif_converter.spec.ts index 4f925552..5b8c4cdd 100644 --- a/tests/sarif_converter.spec.ts +++ b/tests/sarif_converter.spec.ts @@ -49,22 +49,26 @@ describe('Sarif Convertor', () => { expect(validationResult.errors.length).toEqual(0); //no fingerprinting - expect(Object.keys((sarifResults.runs[0].results as any)[2].fingerprints).length).toEqual(0); + expect(Object.keys((sarifResults.runs[0].results as any)[6].fingerprints).length).toEqual(0); //single fingerprinting - expect((sarifResults.runs[0].results as any)[1].fingerprints[0]).toEqual('c92fd0d8fbb0722ac17a8dfd6b9fd5c0770a64a3f965ed128ecc6ae45c20813b'); - expect((sarifResults.runs[0].results as any)[3].fingerprints[0]).toEqual('719d356d9a2efebe9142ca79825c6c0e4b0e6f5013a6e3f3b04630243416ac87'); - expect((sarifResults.runs[0].results as any)[5].fingerprints[0]).toEqual('2cc2fb05feccfae008104250873928e0d9f1de01f5878d485963f2966edc163e'); - expect((sarifResults.runs[0].results as any)[6].fingerprints[0]).toEqual('4aaff40942701a6acc92fafebea49f718bdc060012ca10a5f30e9462da6143b9'); + expect((sarifResults.runs[0].results as any)[1].fingerprints[0]).toEqual('c8ede54bd6611ca074c42baf78809b9fe198c762ba8a1a78571befdf191869e5'); + expect((sarifResults.runs[0].results as any)[2].fingerprints[0]).toEqual('8105a5e15b9d725e663009985913a6931ba94ddd56f4854d24ba3565067501b4'); + expect((sarifResults.runs[0].results as any)[3].fingerprints[0]).toEqual('8a870ceae63c99fb925bcdcbaca7ce5c8ccf29024898cd1694d930b5d687842e'); + expect((sarifResults.runs[0].results as any)[4].fingerprints[0]).toEqual('c92fd0d8fbb0722ac17a8dfd6b9fd5c0770a64a3f965ed128ecc6ae45c20813b'); + expect((sarifResults.runs[0].results as any)[5].fingerprints[0]).toEqual('2ea8d6c4f7096751e5e6f725a3deeac983fe5fac17cdd64e5f6aa83d49156d67'); + expect((sarifResults.runs[0].results as any)[7].fingerprints[0]).toEqual('719d356d9a2efebe9142ca79825c6c0e4b0e6f5013a6e3f3b04630243416ac87'); + expect((sarifResults.runs[0].results as any)[9].fingerprints[0]).toEqual('2cc2fb05feccfae008104250873928e0d9f1de01f5878d485963f2966edc163e'); + expect((sarifResults.runs[0].results as any)[10].fingerprints[0]).toEqual('4aaff40942701a6acc92fafebea49f718bdc060012ca10a5f30e9462da6143b9'); //2 fingerprinting expect((sarifResults.runs[0].results as any)[0].fingerprints[0]).toEqual('ae785dddd67d66083bf565a2ab826535bac183b3c477d249649c6596f2405dd6'); expect((sarifResults.runs[0].results as any)[0].fingerprints[1]).toEqual('12342'); //3 fingerprinting - expect((sarifResults.runs[0].results as any)[4].fingerprints[0]).toEqual('78d8a8779142d82cf9be9da7a167e8e06236ab0a67d324143494650a5e4badf2'); - expect((sarifResults.runs[0].results as any)[4].fingerprints[1]).toEqual('asdf'); - expect((sarifResults.runs[0].results as any)[4].fingerprints[2]).toEqual('78d8a8779142d82cf9be9da7a167e8e06236ab0a67d324143494650a5e4badf2'); + expect((sarifResults.runs[0].results as any)[8].fingerprints[0]).toEqual('78d8a8779142d82cf9be9da7a167e8e06236ab0a67d324143494650a5e4badf2'); + expect((sarifResults.runs[0].results as any)[8].fingerprints[1]).toEqual('asdf'); + expect((sarifResults.runs[0].results as any)[8].fingerprints[2]).toEqual('78d8a8779142d82cf9be9da7a167e8e06236ab0a67d324143494650a5e4badf2'); });