Skip to content

Commit

Permalink
Merge pull request #60 from snyk/fix/wrong-number-of-issues-on-sarif
Browse files Browse the repository at this point in the history
[COD-254] fix: wrong number of calculated issues as part of the Sarif conversion
  • Loading branch information
orkamara authored Mar 4, 2021
2 parents 0d61fd8 + 48fe735 commit 723a957
Show file tree
Hide file tree
Showing 4 changed files with 196 additions and 55 deletions.
97 changes: 53 additions & 44 deletions src/sarif_converter.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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,
},
Expand All @@ -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 = <Result.level>{
1: 'note',
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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],
},
},
},
Expand Down Expand Up @@ -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 } };
}
96 changes: 96 additions & 0 deletions tests/__snapshots__/git.analysis.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -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",
},
]
`;
38 changes: 35 additions & 3 deletions tests/git.analysis.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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');
Expand Down Expand Up @@ -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: '[email protected]: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
Expand All @@ -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;
}
20 changes: 12 additions & 8 deletions tests/sarif_converter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

});

Expand Down

0 comments on commit 723a957

Please sign in to comment.