From d74fafb6afd3741f99402ad198ac6f8445fc260f Mon Sep 17 00:00:00 2001 From: Robert Mutuleanu Date: Thu, 8 Jun 2023 10:58:29 +0300 Subject: [PATCH] feat: container sarif flag support without file Customers want to be able to save results of container scans to sarif without needing to specify the dockerfile Issue: LUM-257 --- src/cli/main.ts | 16 -- src/lib/formatters/get-sarif-result.ts | 2 +- .../docker/sarif-container-result.json | 4 +- .../sarif-with-file-container-result.json | 58 +++++++ .../__snapshots__/sarif-output.spec.ts.snap | 6 +- .../lib/formatters/get-sarif-result.spec.ts | 157 ++++++++++++++++++ .../unit/lib/formatters/sarif-output.spec.ts | 1 + test/tap/cli-test/cli-test.docker.spec.ts | 26 ++- 8 files changed, 244 insertions(+), 26 deletions(-) create mode 100644 test/fixtures/docker/sarif-with-file-container-result.json create mode 100644 test/jest/unit/lib/formatters/get-sarif-result.spec.ts diff --git a/src/cli/main.ts b/src/cli/main.ts index 14817a5fc3..007410b6e1 100755 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -423,22 +423,6 @@ function validateUnsupportedSarifCombinations(args) { 'sarif-file-output', ]); } - - if ( - args.options['sarif'] && - args.options['docker'] && - !args.options['file'] - ) { - throw new MissingOptionError('sarif', ['--file']); - } - - if ( - args.options['sarif-file-output'] && - args.options['docker'] && - !args.options['file'] - ) { - throw new MissingOptionError('sarif-file-output', ['--file']); - } } async function saveResultsToFile( diff --git a/src/lib/formatters/get-sarif-result.ts b/src/lib/formatters/get-sarif-result.ts index 0bc56ec849..1fee936729 100644 --- a/src/lib/formatters/get-sarif-result.ts +++ b/src/lib/formatters/get-sarif-result.ts @@ -18,7 +18,7 @@ export function getResults(testResult): sarif.Result[] { { physicalLocation: { artifactLocation: { - uri: testResult.displayTargetFile, + uri: testResult.displayTargetFile || testResult.path, }, region: { startLine: vuln.lineNumber || 1, diff --git a/test/fixtures/docker/sarif-container-result.json b/test/fixtures/docker/sarif-container-result.json index 024f9d8c22..104ae4c909 100644 --- a/test/fixtures/docker/sarif-container-result.json +++ b/test/fixtures/docker/sarif-container-result.json @@ -42,7 +42,9 @@ "locations": [ { "physicalLocation": { - "artifactLocation": {}, + "artifactLocation": { + "uri": "alpine/kubernetes-monitor" + }, "region": { "startLine": 1 } diff --git a/test/fixtures/docker/sarif-with-file-container-result.json b/test/fixtures/docker/sarif-with-file-container-result.json new file mode 100644 index 0000000000..036685386c --- /dev/null +++ b/test/fixtures/docker/sarif-with-file-container-result.json @@ -0,0 +1,58 @@ +{ + "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "Snyk Container", + "rules": [ + { + "id": "SNYK-LINUX-BZIP2-106947", + "shortDescription": { + "text": "Low severity - Denial of Service (DoS) vulnerability in bzip2" + }, + "fullDescription": { + "text": "(CVE-2016-3189) bzip2/libbz2-1.0@1.0.6-8.1" + }, + "help": { + "text": "", + "markdown": "## Overview\nUse-after-free vulnerability in bzip2recover in bzip2 1.0.6 allows remote attackers to cause a denial of service (crash) via a crafted bzip2 file, related to block ends set to before the start of the block.\n\n## References\n- [GENTOO](https://security.gentoo.org/glsa/201708-08)\n- [CONFIRM](https://bugzilla.redhat.com/show_bug.cgi?id=1319648)\n- [SECTRACK](http://www.securitytracker.com/id/1036132)\n- [BID](http://www.securityfocus.com/bid/91297)\n- [CONFIRM](http://www.oracle.com/technetwork/topics/security/bulletinjul2016-3090568.html)\n- [MLIST](http://www.openwall.com/lists/oss-security/2016/06/20/1)\n" + }, + "defaultConfiguration": { + "level": "warning" + }, + "properties": { + "tags": [ + "security", + "deb" + ] + } + } + ] + } + }, + "results": [ + { + "ruleId": "SNYK-LINUX-BZIP2-106947", + "level": "note", + "message": { + "text": "This file introduces a vulnerable bzip2 package with a low severity vulnerability." + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "Dockerfile" + }, + "region": { + "startLine": 1 + } + } + } + ] + } + ] + } + ] +} diff --git a/test/jest/unit/lib/formatters/__snapshots__/sarif-output.spec.ts.snap b/test/jest/unit/lib/formatters/__snapshots__/sarif-output.spec.ts.snap index 950826a11d..41bf1a9043 100644 --- a/test/jest/unit/lib/formatters/__snapshots__/sarif-output.spec.ts.snap +++ b/test/jest/unit/lib/formatters/__snapshots__/sarif-output.spec.ts.snap @@ -12,7 +12,7 @@ Object { Object { "physicalLocation": Object { "artifactLocation": Object { - "uri": undefined, + "uri": "Dockerfile", }, "region": Object { "startLine": 1, @@ -92,7 +92,7 @@ Object { Object { "physicalLocation": Object { "artifactLocation": Object { - "uri": undefined, + "uri": "Dockerfile", }, "region": Object { "startLine": 1, @@ -171,7 +171,7 @@ Object { Object { "physicalLocation": Object { "artifactLocation": Object { - "uri": undefined, + "uri": "Dockerfile", }, "region": Object { "startLine": 1, diff --git a/test/jest/unit/lib/formatters/get-sarif-result.spec.ts b/test/jest/unit/lib/formatters/get-sarif-result.spec.ts new file mode 100644 index 0000000000..7de7ec157c --- /dev/null +++ b/test/jest/unit/lib/formatters/get-sarif-result.spec.ts @@ -0,0 +1,157 @@ +import { getResults } from '../../../../../src/lib/formatters/get-sarif-result'; +import { SEVERITY, TestResult } from '../../../../../src/lib/snyk-test/legacy'; + +describe('Retrieving sarif result', () => { + it('should use the test results path as the location uri when target file is not present', () => { + let result = getResults( + getTestResult({ + path: 'alpine:3.18.0', + }), + ); + expect(result).toEqual([ + { + ruleId: 'SNYK-LINUX-EXPAT-450908', + level: 'error', + message: { + text: + 'This file introduces a vulnerable expat package with a critical severity vulnerability.', + }, + locations: [ + { + physicalLocation: { + artifactLocation: { uri: 'alpine:3.18.0' }, + region: { startLine: 1 }, + }, + }, + ], + }, + ]); + + result = getResults( + getTestResult({ + path: 'alpine:3.18.0', + displayTargetFile: undefined, + }), + ); + expect(result).toEqual([ + { + ruleId: 'SNYK-LINUX-EXPAT-450908', + level: 'error', + message: { + text: + 'This file introduces a vulnerable expat package with a critical severity vulnerability.', + }, + locations: [ + { + physicalLocation: { + artifactLocation: { uri: 'alpine:3.18.0' }, + region: { startLine: 1 }, + }, + }, + ], + }, + ]); + + result = getResults( + getTestResult({ + path: 'alpine:3.18.0', + displayTargetFile: null, + }), + ); + expect(result).toEqual([ + { + ruleId: 'SNYK-LINUX-EXPAT-450908', + level: 'error', + message: { + text: + 'This file introduces a vulnerable expat package with a critical severity vulnerability.', + }, + locations: [ + { + physicalLocation: { + artifactLocation: { uri: 'alpine:3.18.0' }, + region: { startLine: 1 }, + }, + }, + ], + }, + ]); + }); + + it('should use the target file as the location uri when target file is present', () => { + const actualResult = getResults( + getTestResult({ + displayTargetFile: 'Dockerfile.test', + }), + ); + expect(actualResult).toEqual([ + { + ruleId: 'SNYK-LINUX-EXPAT-450908', + level: 'error', + message: { + text: + 'This file introduces a vulnerable expat package with a critical severity vulnerability.', + }, + locations: [ + { + physicalLocation: { + artifactLocation: { uri: 'Dockerfile.test' }, + region: { startLine: 1 }, + }, + }, + ], + }, + ]); + }); +}); + +function getTestResult(testResultOverride = {}, vulnOverride = {}): TestResult { + return { + vulnerabilities: [ + { + id: 'SNYK-LINUX-EXPAT-450908', + packageName: 'expat', + version: '2.2.5-r0', + below: '', + semver: { + vulnerable: ['<2.2.7-r0'], + }, + patches: [], + isNew: false, + description: '## Overview\nIn libexpat in Expat before 2.2.7...', + title: 'XML External Entity (XXE) Injection', + severity: SEVERITY.CRITICAL, + fixedIn: ['2.2.7-r0'], + credit: [''], + name: 'expat', + from: [ + 'docker-image|garethr/snyky@alpine', + '.python-rundeps@0', + 'expat@2.2.5-r0', + ], + upgradePath: [], + isUpgradable: false, + isPatchable: false, + parentDepType: 'prod', + identifiers: { + ALTERNATIVE: [ + 'SNYK-DEBIAN8-EXPAT-450909', + 'SNYK-DEBIAN9-EXPAT-450910', + ], + CVE: ['CVE-2018-20843'], + CWE: ['CWE-611'], + }, + ...vulnOverride, + }, + ], + ok: false, + dependencyCount: 969, + org: 'ORG', + policy: '', + isPrivate: true, + licensesPolicy: null, + ignoreSettings: null, + summary: '165 vulnerable dependencies', + ...testResultOverride, + }; +} diff --git a/test/jest/unit/lib/formatters/sarif-output.spec.ts b/test/jest/unit/lib/formatters/sarif-output.spec.ts index 4e60f4ef95..77d73fa7ec 100644 --- a/test/jest/unit/lib/formatters/sarif-output.spec.ts +++ b/test/jest/unit/lib/formatters/sarif-output.spec.ts @@ -155,5 +155,6 @@ function getTestResult( uniqueCount: 42, projectName: 'PROJECT_NAME', foundProjectCount: 22, + displayTargetFile: 'Dockerfile', }; } diff --git a/test/tap/cli-test/cli-test.docker.spec.ts b/test/tap/cli-test/cli-test.docker.spec.ts index 11c464ee47..c2f2af942a 100644 --- a/test/tap/cli-test/cli-test.docker.spec.ts +++ b/test/tap/cli-test/cli-test.docker.spec.ts @@ -745,10 +745,10 @@ export const DockerTests: AcceptanceTests = { } }, - '`test --docker --file=Dockerfile --sarif `': (params, utils) => async ( - t, - ) => { - const testableObject = await testSarif(t, utils, params, { sarif: true }); + '`container test alpine --sarif `': (params, utils) => async (t) => { + const testableObject = await testSarif(t, utils, params, { + sarif: true, + }); const results = JSON.parse(testableObject.message); const sarifResults = require(getFixturePath( 'docker/sarif-container-result.json', @@ -757,6 +757,22 @@ export const DockerTests: AcceptanceTests = { t.end(); }, + '`container test alpine --file=Dockerfile --sarif `': ( + params, + utils, + ) => async (t) => { + const testableObject = await testSarif(t, utils, params, { + sarif: true, + file: 'Dockerfile', + }); + const results = JSON.parse(testableObject.message); + const sarifResults = require(getFixturePath( + 'docker/sarif-with-file-container-result.json', + )); + t.deepEqual(results, sarifResults, 'stdout containing sarif results'); + t.end(); + }, + '`test --docker --file=Dockerfile --sarif --sarif-output-file`': ( params, utils, @@ -851,7 +867,7 @@ async function testPrep(t, utils, params, additionaLpropsForCli) { params.server.setNextResponse(vulns); try { - await params.cli.test('test alpine', { + await params.cli.test('alpine', { docker: true, ...additionaLpropsForCli, });