diff --git a/package.json b/package.json index c0750080d9..5403323c18 100644 --- a/package.json +++ b/package.json @@ -75,7 +75,7 @@ "dependencies": { "@open-policy-agent/opa-wasm": "^1.2.0", "@snyk/cli-interface": "2.11.0", - "@snyk/cloud-config-parser": "^1.9.2", + "@snyk/cloud-config-parser": "^1.10.2", "@snyk/code-client": "4.0.0", "@snyk/dep-graph": "^1.27.1", "@snyk/fix": "1.650.0", @@ -147,7 +147,7 @@ "devDependencies": { "@types/cross-spawn": "^6.0.2", "@types/fs-extra": "^9.0.11", - "@types/jest": "^26.0.20", + "@types/jest": "^27.0.1", "@types/lodash": "^4.14.161", "@types/needle": "^2.0.4", "@types/node": "^14.14.31", diff --git a/src/cli/commands/test/iac-local-execution/extract-line-number.ts b/src/cli/commands/test/iac-local-execution/extract-line-number.ts index dd4e75feaa..106ddb7181 100644 --- a/src/cli/commands/test/iac-local-execution/extract-line-number.ts +++ b/src/cli/commands/test/iac-local-execution/extract-line-number.ts @@ -2,7 +2,8 @@ import { IaCErrorCodes } from './types'; import { CustomError } from '../../../../lib/errors'; import { CloudConfigFileTypes, - issuesToLineNumbers, + MapsDocIdToTree, + getLineNumber, } from '@snyk/cloud-config-parser'; import { UnsupportedFileTypeError } from './file-parser'; import * as analytics from '../../../../lib/analytics'; @@ -10,7 +11,7 @@ import * as Debug from 'debug'; import { getErrorStringCode } from './error-utils'; const debug = Debug('iac-extract-line-number'); -function getFileTypeForLineNumber(fileType: string): CloudConfigFileTypes { +export function getFileTypeForParser(fileType: string): CloudConfigFileTypes { switch (fileType) { case 'yaml': case 'yml': @@ -25,16 +26,12 @@ function getFileTypeForLineNumber(fileType: string): CloudConfigFileTypes { } export function extractLineNumber( - fileContent: string, - fileType: string, cloudConfigPath: string[], + fileType: CloudConfigFileTypes, + treeByDocId: MapsDocIdToTree, ): number { try { - return issuesToLineNumbers( - fileContent, - getFileTypeForLineNumber(fileType), - cloudConfigPath, - ); + return getLineNumber(cloudConfigPath, fileType, treeByDocId); } catch { const err = new FailedToExtractLineNumberError(); analytics.add('error-code', err.code); diff --git a/src/cli/commands/test/iac-local-execution/results-formatter.ts b/src/cli/commands/test/iac-local-execution/results-formatter.ts index f171a2c1d1..d1adb50b07 100644 --- a/src/cli/commands/test/iac-local-execution/results-formatter.ts +++ b/src/cli/commands/test/iac-local-execution/results-formatter.ts @@ -12,9 +12,10 @@ import * as path from 'path'; import { SEVERITY } from '../../../../lib/snyk-test/common'; import { IacProjectType } from '../../../../lib/iac/constants'; import { CustomError } from '../../../../lib/errors'; -import { extractLineNumber } from './extract-line-number'; +import { extractLineNumber, getFileTypeForParser } from './extract-line-number'; import { getErrorStringCode } from './error-utils'; import { isLocalFolder } from '../../../../lib/detect'; +import { MapsDocIdToTree, getTrees } from '@snyk/cloud-config-parser'; const SEVERITIES = [SEVERITY.LOW, SEVERITY.MEDIUM, SEVERITY.HIGH]; @@ -53,27 +54,24 @@ function formatScanResult( meta: TestMeta, options: IaCTestFlags, ): FormattedResult { + const fileType = getFileTypeForParser(scanResult.fileType); + let treeByDocId: MapsDocIdToTree; + try { + treeByDocId = getTrees(fileType, scanResult.fileContent); + } catch (err) { + // we do nothing intentionally. + // Even if the building of the tree fails in the external parser, + // we still pass an undefined tree and not calculated line number for those + } + const formattedIssues = scanResult.violatedPolicies.map((policy) => { const cloudConfigPath = scanResult.docId !== undefined ? [`[DocId: ${scanResult.docId}]`].concat(parsePath(policy.msg)) : policy.msg.split('.'); - const flagsRequiringLineNumber = [ - 'json', - 'sarif', - 'json-file-output', - 'sarif-file-output', - ]; - const shouldExtractLineNumber = flagsRequiringLineNumber.some( - (flag) => options[flag], - ); - const lineNumber: number = shouldExtractLineNumber - ? extractLineNumber( - scanResult.fileContent, - scanResult.fileType, - cloudConfigPath, - ) + const lineNumber: number = treeByDocId + ? extractLineNumber(cloudConfigPath, fileType, treeByDocId) : -1; return { diff --git a/test/jest/unit/iac-unit-tests/results-formatter.spec.ts b/test/jest/unit/iac-unit-tests/results-formatter.spec.ts index 2be08000ee..1d65fada79 100644 --- a/test/jest/unit/iac-unit-tests/results-formatter.spec.ts +++ b/test/jest/unit/iac-unit-tests/results-formatter.spec.ts @@ -10,16 +10,18 @@ import { policyStub, generateScanResults, } from './results-formatter.fixtures'; -import { issuesToLineNumbers } from '@snyk/cloud-config-parser'; +import * as cloudConfigParserModule from '@snyk/cloud-config-parser'; import { PolicyMetadata } from '../../../../src/cli/commands/test/iac-local-execution/types'; -jest.mock('@snyk/cloud-config-parser'); - +jest.mock('@snyk/cloud-config-parser', () => ({ + ...jest.requireActual('@snyk/cloud-config-parser'), +})); +const validTree = { '0': { nodes: [] } }; describe('formatScanResults', () => { it.each([ [ { severityThreshold: SEVERITY.HIGH }, - expectedFormattedResultsWithoutLineNumber, + expectedFormattedResultsWithLineNumber, ], [ { severityThreshold: SEVERITY.HIGH, sarif: true }, @@ -40,7 +42,10 @@ describe('formatScanResults', () => { ])( 'given %p options object, returns the expected results', (optionsObject, expectedResult) => { - (issuesToLineNumbers as jest.Mock).mockReturnValue(3); + jest + .spyOn(cloudConfigParserModule, 'getTrees') + .mockReturnValue(validTree); + jest.spyOn(cloudConfigParserModule, 'getLineNumber').mockReturnValue(3); const formattedResults = formatScanResults( generateScanResults(), optionsObject, @@ -51,9 +56,50 @@ describe('formatScanResults', () => { expect(formattedResults[0]).toEqual(expectedResult); }, ); - // TODO: add tests for the multi-doc yaml grouping }); +describe('parser failures should return -1 for lineNumber', () => { + beforeEach(async () => { + jest.restoreAllMocks(); + }); + + it('creates a valid tree, but the getLineNumber() fails', () => { + jest.spyOn(cloudConfigParserModule, 'getTrees').mockReturnValue(validTree); + jest + .spyOn(cloudConfigParserModule, 'getLineNumber') + .mockImplementation(() => { + throw new Error(); + }); + const formattedResults = formatScanResults( + generateScanResults(), + { severityThreshold: SEVERITY.HIGH }, + meta, + ); + + expect(formattedResults.length).toEqual(1); + expect(formattedResults[0]).toEqual( + expectedFormattedResultsWithoutLineNumber, + ); + }); + + it('sends an invalid tree and getLineNumber() fails', () => { + jest + .spyOn(cloudConfigParserModule, 'getTrees') + .mockReturnValue(null as any); + const formattedResults = formatScanResults( + generateScanResults(), + { severityThreshold: SEVERITY.HIGH }, + meta, + ); + + expect(formattedResults.length).toEqual(1); + expect(formattedResults[0]).toEqual( + expectedFormattedResultsWithoutLineNumber, + ); + }); +}); + +// TODO: add tests for the multi-doc yaml grouping describe('filterPoliciesBySeverity', () => { it('returns the formatted results filtered by severity - no default threshold', () => { const scanResults = generateScanResults();