From 2a9c71b05d1e394d1e265c71177129c8fdc49cac Mon Sep 17 00:00:00 2001 From: orkamara Date: Tue, 30 Jun 2020 13:49:57 +0300 Subject: [PATCH] feat: improve IAC test output --- .github/CODEOWNERS | 3 +- help/iac.txt | 3 +- .../test/formatters/format-test-meta.ts | 14 ++- .../test/formatters/legacy-format-issue.ts | 2 +- .../remediation-based-format-issues.ts | 2 +- src/cli/commands/test/iac-output.ts | 94 +++++++++++++++---- src/cli/commands/test/index.ts | 9 +- src/lib/errors/invalid-iac-file.ts | 14 ++- src/lib/snyk-test/run-test.ts | 2 +- .../cli-test/cli-test.iac-k8s.spec.ts | 69 +++++++------- .../iac-kubernetes/test-iac-result.json | 6 +- 11 files changed, 148 insertions(+), 70 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 1696c543308..fdd66f68897 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -10,4 +10,5 @@ src/lib/iac/ @snyk/cloudconfig src/lib/snyk-test/iac-test-result.ts @snyk/cloudconfig src/lib/snyk-test/payload-schema.ts @snyk/cloudconfig src/lib/snyk-test/run-iac-test.ts @snyk/cloudconfig -test/acceptance/cli-test/cli-test.iac-k8s.spec.ts @snyk/cloudconfig \ No newline at end of file +test/acceptance/cli-test/cli-test.iac-k8s.spec.ts @snyk/cloudconfig +src/lib/errors/invalid-iac-file.ts @snyk/cloudconfig \ No newline at end of file diff --git a/help/iac.txt b/help/iac.txt index 31bc6da7955..c59b493b3d7 100644 --- a/help/iac.txt +++ b/help/iac.txt @@ -13,12 +13,11 @@ Options: -h, --help --json .................................. Return results in JSON format. --project-name= ................. Specify a custom Snyk project name. - --policy-path= .................... Manually pass a path to a snyk policy file. --severity-threshold=... Only report issues of provided level or higher. Examples: - $ snyk iac test /path/to/Kubernetes.yaml + $ snyk iac test --file=/path/to/Kubernetes.yaml For more information see https://support.snyk.io/hc/en-us/categories/360001342678-Infrastructure-as-code diff --git a/src/cli/commands/test/formatters/format-test-meta.ts b/src/cli/commands/test/formatters/format-test-meta.ts index 03b8056f93f..c7a6e95d220 100644 --- a/src/cli/commands/test/formatters/format-test-meta.ts +++ b/src/cli/commands/test/formatters/format-test-meta.ts @@ -3,6 +3,7 @@ import { rightPadWithSpaces } from '../../../../lib/right-pad'; import { TestOptions, Options } from '../../../../lib/types'; import { TestResult } from '../../../../lib/snyk-test/legacy'; import { IacTestResult } from '../../../../lib/snyk-test/iac-test-result'; +import { capitalizePackageManager } from '../iac-output'; export function formatTestMeta( res: TestResult | IacTestResult, @@ -14,9 +15,18 @@ export function formatTestMeta( const openSource = res.isPrivate ? 'no' : 'yes'; const meta = [ chalk.bold(rightPadWithSpaces('Organization: ', padToLength)) + res.org, - chalk.bold(rightPadWithSpaces('Package manager: ', padToLength)) + - packageManager, ]; + if (options.iac) { + meta.push( + chalk.bold(rightPadWithSpaces('Type: ', padToLength)) + + capitalizePackageManager(packageManager), + ); + } else { + meta.push( + chalk.bold(rightPadWithSpaces('Package manager: ', padToLength)) + + packageManager, + ); + } if (targetFile) { meta.push( chalk.bold(rightPadWithSpaces('Target file: ', padToLength)) + targetFile, diff --git a/src/cli/commands/test/formatters/legacy-format-issue.ts b/src/cli/commands/test/formatters/legacy-format-issue.ts index d81ed915ca8..b3953e9aeaf 100644 --- a/src/cli/commands/test/formatters/legacy-format-issue.ts +++ b/src/cli/commands/test/formatters/legacy-format-issue.ts @@ -107,7 +107,7 @@ function createSeverityBasedIssueHeading(severity, type, packageName, isNew) { ); } -function titleCaseText(text) { +export function titleCaseText(text) { return text[0].toUpperCase() + text.slice(1); } diff --git a/src/cli/commands/test/formatters/remediation-based-format-issues.ts b/src/cli/commands/test/formatters/remediation-based-format-issues.ts index 0faa31a3a07..85c23cb4cd2 100644 --- a/src/cli/commands/test/formatters/remediation-based-format-issues.ts +++ b/src/cli/commands/test/formatters/remediation-based-format-issues.ts @@ -414,7 +414,7 @@ function constructUnfixableText( return unfixableIssuesTextArray; } -function printPath(path: string[]) { +export function printPath(path: string[]) { return path.slice(1).join(' > '); } diff --git a/src/cli/commands/test/iac-output.ts b/src/cli/commands/test/iac-output.ts index 4f895f3726f..b52e02ed564 100644 --- a/src/cli/commands/test/iac-output.ts +++ b/src/cli/commands/test/iac-output.ts @@ -1,16 +1,72 @@ import chalk from 'chalk'; import * as Debug from 'debug'; -import { Options, TestOptions } from '../../../lib/types'; import { IacTestResult } from '../../../lib/snyk-test/iac-test-result'; import { getSeverityValue } from './formatters'; -import { formatIssue } from './formatters/remediation-based-format-issues'; +import { printPath } from './formatters/remediation-based-format-issues'; import { AnnotatedIacIssue } from '../../../lib/snyk-test/iac-test-result'; - +import { titleCaseText } from './formatters/legacy-format-issue'; const debug = Debug('iac-output'); +function formatIacIssue( + issue: AnnotatedIacIssue, + isNew: boolean, + path: string[], +): string { + const severitiesColourMapping = { + low: { + colorFunc(text) { + return chalk.blueBright(text); + }, + }, + medium: { + colorFunc(text) { + return chalk.yellowBright(text); + }, + }, + high: { + colorFunc(text) { + return chalk.redBright(text); + }, + }, + }; + const newBadge = isNew ? ' (new)' : ''; + const name = issue.subType ? ` in ${chalk.bold(issue.subType)}` : ''; + + let introducedBy = ''; + if (path) { + // In this mode, we show only one path by default, for compactness + const pathStr = printPath(path); + introducedBy = `\n introduced by ${pathStr}`; + } + + const description = extractOverview(issue.description).trim(); + const descriptionLine = `\n ${description}\n`; + + return ( + severitiesColourMapping[issue.severity].colorFunc( + ` ✗ ${chalk.bold(issue.title)}${newBadge} [${titleCaseText( + issue.severity, + )} Severity]`, + ) + + ` [${issue.id}]` + + name + + introducedBy + + descriptionLine + ); +} + +function extractOverview(description: string): string { + if (!description) { + return ''; + } + + const overviewRegExp = /## Overview([\s\S]*?)(?=##|(# Details))/m; + const overviewMatches = overviewRegExp.exec(description); + return (overviewMatches && overviewMatches[1]) || ''; +} + export function getIacDisplayedOutput( res: IacTestResult, - testOptions: Options & TestOptions, testedInfoText: string, meta: string, prefix: string, @@ -19,7 +75,6 @@ export function getIacDisplayedOutput( chalk.bold.white('\nInfrastructure as code issues:'), ]; - const NoNote = false; const NotNew = false; const issues: AnnotatedIacIssue[] = res.result.cloudConfigResults; @@ -28,18 +83,8 @@ export function getIacDisplayedOutput( issues .sort((a, b) => getSeverityValue(b.severity) - getSeverityValue(a.severity)) .forEach((issue) => { - const path: string[][] = [issue.cloudConfigPath]; issuesTextArray.push( - formatIssue( - issue.id, - issue.title, - issue.severity, - NotNew, - issue.subType, - path, - testOptions, - NoNote, - ), + formatIacIssue(issue, NotNew, issue.cloudConfigPath), ); }); @@ -58,3 +103,20 @@ export function getIacDisplayedOutput( return prefix + body; } + +export function capitalizePackageManager(type) { + switch (type) { + case 'k8sconfig': { + return 'Kubernetes'; + } + case 'helmconfig': { + return 'Helm'; + } + case 'terraformconfig': { + return 'Terraform'; + } + default: { + return 'Infrastracture as Code'; + } + } +} diff --git a/src/cli/commands/test/index.ts b/src/cli/commands/test/index.ts index 97ad01daa1c..96963e12bd2 100644 --- a/src/cli/commands/test/index.ts +++ b/src/cli/commands/test/index.ts @@ -399,7 +399,11 @@ function displayResult( const projectType = (res.packageManager as SupportedProjectTypes) || options.packageManager; const localPackageTest = isLocalFolder(options.path); - const prefix = chalk.bold.white('\nTesting ' + options.path + '...\n\n'); + let testingPath = options.path; + if (options.iac && options.file) { + testingPath = options.file; + } + const prefix = chalk.bold.white('\nTesting ' + testingPath + '...\n\n'); // handle errors by extracting their message if (res instanceof Error) { @@ -411,7 +415,7 @@ function displayResult( : 'vulnerabilities'; let pathOrDepsText = ''; - if (res.hasOwnProperty('dependencyCount')) { + if (res.dependencyCount) { pathOrDepsText += res.dependencyCount + ' dependencies'; } else { pathOrDepsText += options.path; @@ -464,7 +468,6 @@ function displayResult( if (res.packageManager === 'k8sconfig') { return getIacDisplayedOutput( (res as any) as IacTestResult, - options, testedInfoText, meta, prefix, diff --git a/src/lib/errors/invalid-iac-file.ts b/src/lib/errors/invalid-iac-file.ts index eda6d979795..32f2104a3b6 100644 --- a/src/lib/errors/invalid-iac-file.ts +++ b/src/lib/errors/invalid-iac-file.ts @@ -4,12 +4,11 @@ import { CustomError } from './custom-error'; export function NotSupportedIacFileError(atLocations: string[]) { const locationsStr = atLocations.join(', '); const errorMsg = - 'Not supported infrastruction as code target files in ' + + 'Not supported infrastructure as code target files in ' + locationsStr + - '.\nPlease see our documentation for supported languages and ' + - 'target files: ' + + '.\nPlease see our documentation for supported target files: ' + chalk.underline( - 'https://support.snyk.io/hc/en-us/articles/360000911957-Language-support', + 'https://support.snyk.io/hc/en-us/articles/360006368877-Scan-and-fix-security-issues-in-your-Kubernetes-configuration-files', ) + ' and make sure you are in the right directory.'; @@ -22,12 +21,11 @@ export function NotSupportedIacFileError(atLocations: string[]) { export function IllegalIacFileError(atLocations: string[]): CustomError { const locationsStr = atLocations.join(', '); const errorMsg = - 'Illegal infrastruction as code target file ' + + 'Illegal infrastructure as code target file ' + locationsStr + - '.\nPlease see our documentation for supported languages and ' + - 'target files: ' + + '.\nPlease see our documentation for supported target files: ' + chalk.underline( - 'https://support.snyk.io/hc/en-us/articles/360000911957-Language-support', + 'https://support.snyk.io/hc/en-us/articles/360006368877-Scan-and-fix-security-issues-in-your-Kubernetes-configuration-files', ) + ' and make sure you are in the right directory.'; diff --git a/src/lib/snyk-test/run-test.ts b/src/lib/snyk-test/run-test.ts index 40810158c9d..9ebaf3a960f 100644 --- a/src/lib/snyk-test/run-test.ts +++ b/src/lib/snyk-test/run-test.ts @@ -331,7 +331,7 @@ async function assembleLocalPayloads( if (options.docker) { analysisTypeText = 'docker dependencies for '; } else if (options.iac) { - analysisTypeText = 'Infrastruction as code configurations for '; + analysisTypeText = 'Infrastructure as code configurations for '; } else if (options.packageManager) { analysisTypeText = options.packageManager + ' dependencies for '; } diff --git a/test/acceptance/cli-test/cli-test.iac-k8s.spec.ts b/test/acceptance/cli-test/cli-test.iac-k8s.spec.ts index d0e906b8d4e..579a1accd00 100644 --- a/test/acceptance/cli-test/cli-test.iac-k8s.spec.ts +++ b/test/acceptance/cli-test/cli-test.iac-k8s.spec.ts @@ -1,9 +1,6 @@ -import * as sinon from 'sinon'; import * as _ from '@snyk/lodash'; import { getWorkspaceJSON } from '../workspace-helper'; import { CommandResult } from '../../../src/cli/commands/types'; -// import * as fs from 'fs'; -// import * as path from 'path'; import { AcceptanceTests } from './cli-test.acceptance.test'; @@ -82,11 +79,7 @@ export const IacK8sTests: AcceptanceTests = { const meta = res.slice(res.indexOf('Organization:')).split('\n'); t.match(meta[0], /Organization:\s+test-org/, 'organization displayed'); - t.match( - meta[1], - /Package manager:\s+k8sconfig/, - 'package manager displayed', - ); + t.match(meta[1], /Type:\s+Kubernetes/, 'Type displayed'); t.match( meta[2], /Target file:\s+multi-file.yaml/, @@ -124,17 +117,41 @@ export const IacK8sTests: AcceptanceTests = { t.match( res, - 'Tested 0 dependencies for known issues, found 3 issues', + 'Tested iac-kubernetes for known issues, found 3 issues', '3 issue', ); + const issues = res + .slice( + res.indexOf('Infrastructure as code issues:'), + res.indexOf('Organization:'), + ) + .split('\n'); + t.ok(issues[1].includes('[SNYK-CC-K8S-'), 'Snyk id'); + + t.ok( + issues[2].trim().startsWith('introduced by'), + 'Introduced by line', + ); + + t.ok(issues[3], 'description'); + + t.ok(issues[4] === '', 'Empty line after description'); + + t.ok(issues[5].includes('[SNYK-CC-K8S-'), 'Snyk id'); + + t.ok( + issues[6].trim().startsWith('introduced by'), + 'Introduced by line', + ); + + t.ok(issues[7], 'description'); + + t.ok(issues[8] === '', 'Empty line after description'); + const meta = res.slice(res.indexOf('Organization:')).split('\n'); t.match(meta[0], /Organization:\s+test-org/, 'organization displayed'); - t.match( - meta[1], - /Package manager:\s+k8sconfig/, - 'package manager displayed', - ); + t.match(meta[1], /Type:\s+Kubernetes/, 'Type displayed'); t.match( meta[2], /Target file:\s+multi-file.yaml/, @@ -177,17 +194,13 @@ export const IacK8sTests: AcceptanceTests = { t.match( res, - 'Tested 0 dependencies for known issues, found 3 issues', + 'Tested iac-kubernetes for known issues, found 3 issues', '3 issue', ); const meta = res.slice(res.indexOf('Organization:')).split('\n'); t.match(meta[0], /Organization:\s+test-org/, 'organization displayed'); - t.match( - meta[1], - /Package manager:\s+k8sconfig/, - 'package manager displayed', - ); + t.match(meta[1], /Type:\s+Kubernetes/, 'Type displayed'); t.match( meta[2], /Target file:\s+multi-file.yaml/, @@ -275,17 +288,13 @@ export const IacK8sTests: AcceptanceTests = { t.match( res, - 'Tested 0 dependencies for known issues, found 2 issues', + 'Tested iac-kubernetes for known issues, found 2 issues', '2 issue', ); const meta = res.slice(res.indexOf('Organization:')).split('\n'); t.match(meta[0], /Organization:\s+test-org/, 'organization displayed'); - t.match( - meta[1], - /Package manager:\s+k8sconfig/, - 'package manager displayed', - ); + t.match(meta[1], /Type:\s+Kubernetes/, 'Type displayed'); t.match( meta[2], /Target file:\s+multi-file.yaml/, @@ -373,17 +382,13 @@ export const IacK8sTests: AcceptanceTests = { t.match( res, - 'Tested 0 dependencies for known issues, found 1 issues', + 'Tested iac-kubernetes for known issues, found 1 issues', '1 issue', ); const meta = res.slice(res.indexOf('Organization:')).split('\n'); t.match(meta[0], /Organization:\s+test-org/, 'organization displayed'); - t.match( - meta[1], - /Package manager:\s+k8sconfig/, - 'package manager displayed', - ); + t.match(meta[1], /Type:\s+Kubernetes/, 'Type displayed'); t.match( meta[2], /Target file:\s+multi-file.yaml/, diff --git a/test/acceptance/workspaces/iac-kubernetes/test-iac-result.json b/test/acceptance/workspaces/iac-kubernetes/test-iac-result.json index 17f758a9548..17001a0b101 100644 --- a/test/acceptance/workspaces/iac-kubernetes/test-iac-result.json +++ b/test/acceptance/workspaces/iac-kubernetes/test-iac-result.json @@ -5,7 +5,7 @@ { "id": "SNYK-CC-K8S-1", "title": "Reducing the admission of containers with dropped capabilities", - "description": "The requiredDropCapabilities property (as part of the Pod Security Policy) provides a whitelist of capabilities that must be dropped from containers (https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities). These capabilities are removed from the default set, and must not be added. It’s recommended to drop all the capabilities (using ALL), or at least to drop NET_RAW (which allows a process to spy on packets on its network / to inject data onto the network).", + "description": "## Overview Privileged containers can do nearly everything a process on the host can do, and provide no isolation from other workloads. Avoid where possible. ## Remediation Change to `false` ## References ad", "cloudConfigPath": ["[DocId: 2]","input","spec","requiredDropCapabilities"], "severity": "high", "isIgnored": false, @@ -15,7 +15,7 @@ { "id": "SNYK-CC-K8S-2", "title": "Reducing the admission of containers with dropped capabilities", - "description": "The requiredDropCapabilities property (as part of the Pod Security Policy) provides a whitelist of capabilities that must be dropped from containers (https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities). These capabilities are removed from the default set, and must not be added. It’s recommended to drop all the capabilities (using ALL), or at least to drop NET_RAW (which allows a process to spy on packets on its network / to inject data onto the network).", + "description": "## Overview The requiredDropCapabilities property (as part of the Pod Security Policy) provides a whitelist of capabilities that must be dropped from containers (https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities). These capabilities are removed from the default set, and must not be added. It’s recommended to drop all the capabilities (using ALL), or at least to drop NET_RAW (which allows a process to spy on packets on its network / to inject data onto the network). ## Remediation Change to `false` ## References ad", "cloudConfigPath": ["[DocId: 2]","input","spec","requiredDropCapabilities"], "severity": "medium", "isIgnored": false, @@ -25,7 +25,7 @@ { "id": "SNYK-CC-K8S-3", "title": "Reducing the admission of containers with dropped capabilities", - "description": "The requiredDropCapabilities property (as part of the Pod Security Policy) provides a whitelist of capabilities that must be dropped from containers (https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities). These capabilities are removed from the default set, and must not be added. It’s recommended to drop all the capabilities (using ALL), or at least to drop NET_RAW (which allows a process to spy on packets on its network / to inject data onto the network).", + "description": "## Overview The requiredDropCapabilities property (as part of the Pod Security Policy) provides a whitelist of capabilities that must be dropped from containers (https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities). These capabilities are removed from the default set, and must not be added. It’s recommended to drop all the capabilities (using ALL), or at least to drop NET_RAW (which allows a process to spy on packets on its network / to inject data onto the network). ## Remediation Change to `false` ## References ad", "cloudConfigPath": ["[DocId: 2]","input","spec","requiredDropCapabilities"], "severity": "low", "isIgnored": false,