From df104e3c873fe1092cb5e828e2413807497121e1 Mon Sep 17 00:00:00 2001 From: Konstantin Yegupov Date: Tue, 30 Apr 2019 17:45:05 +0100 Subject: [PATCH] fix: do not offer remediation advice when scanning a non-local package --- src/cli/commands/test.ts | 23 ++++++++++++++++------- src/lib/detect.ts | 2 +- test/acceptance/cli.acceptance.test.ts | 6 ++++-- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/cli/commands/test.ts b/src/cli/commands/test.ts index 4378e606e2e..6356576628e 100644 --- a/src/cli/commands/test.ts +++ b/src/cli/commands/test.ts @@ -9,16 +9,21 @@ import {exists as apiTokenExists} from '../../lib/api-token'; import {SEVERITIES, WIZARD_SUPPORTED_PMS} from '../../lib/snyk-test/common'; import * as docker from '../../lib/docker-promotion'; import * as Debug from 'debug'; -import { TestOptions } from '../../lib/types'; +import {TestOptions} from '../../lib/types'; +import {isLocalFolder} from '../../lib/detect'; const debug = Debug('snyk'); const SEPARATOR = '\n-------------------------------------------------------\n'; +interface OptionsAtDisplayStage { + canSuggestRemediation: boolean; +} + // TODO: avoid using `as any` whenever it's possible // arguments array is 0 or more `path` strings followed by // an optional `option` object -async function test(...args) { +async function test(...args): Promise { const resultOptions = [] as any[]; let results = [] as any[]; let options = {} as any as TestOptions; @@ -192,10 +197,11 @@ function summariseErrorResults(errorResults) { return ''; } -function displayResult(res, options: TestOptions) { +function displayResult(res, options: TestOptions & OptionsAtDisplayStage) { const meta = metaForDisplay(res, options) + '\n\n'; const dockerAdvice = dockerRemediationForDisplay(res); const packageManager = options.packageManager; + options.canSuggestRemediation = isLocalFolder(options.path); const prefix = chalk.bold.white('\nTesting ' + options.path + '...\n\n'); // handle errors by extracting their message @@ -270,7 +276,7 @@ function displayResult(res, options: TestOptions) { } let summary = testedInfoText + ', ' + chalk.red.bold(vulnCountText); - if (WIZARD_SUPPORTED_PMS.indexOf(packageManager) > -1) { + if (options.canSuggestRemediation && WIZARD_SUPPORTED_PMS.indexOf(packageManager) > -1) { summary += chalk.bold.green('\n\nRun `snyk wizard` to address these issues.'); } @@ -312,7 +318,10 @@ function displayResult(res, options: TestOptions) { return prefix + body + multiProjAdvice + dockerAdvice + dockerSuggestion; } -function formatDockerBinariesIssues(dockerBinariesSortedGroupedVulns, binariesVulns, options) { +function formatDockerBinariesIssues( + dockerBinariesSortedGroupedVulns, + binariesVulns, + options: TestOptions & OptionsAtDisplayStage) { const binariesIssuesOutput = [] as string[]; for (const pkgInfo of _.values(binariesVulns.affectedPkgs)) { binariesIssuesOutput.push(createDockerBinaryHeading(pkgInfo)); @@ -334,7 +343,7 @@ function createDockerBinaryHeading(pkgInfo) { ` for ${binaryName}@${binaryVersion} ------------`, '\n') : ''; } -function formatIssues(vuln, options) { +function formatIssues(vuln, options: TestOptions & OptionsAtDisplayStage) { const vulnID = vuln.list[0].id; const packageManager = options.packageManager; const uniquePackages = _.uniq( @@ -364,7 +373,7 @@ function formatIssues(vuln, options) { fromPaths: options.showVulnPaths ? createTruncatedVulnsPathsText(vuln.list) : '', extraInfo: vuln.note ? chalk.bold('\n Note: ' + vuln.note) : '', - remediationInfo: vuln.metadata.type !== 'license' + remediationInfo: vuln.metadata.type !== 'license' && options.canSuggestRemediation ? createRemediationText(vuln, packageManager) : '', fixedIn: options.docker ? createFixedInText(vuln) : '', diff --git a/src/lib/detect.ts b/src/lib/detect.ts index b56d6aa34a7..e90046ad967 100644 --- a/src/lib/detect.ts +++ b/src/lib/detect.ts @@ -110,7 +110,7 @@ function localFileSuppliedButNotFound(root, file) { !fs.existsSync(pathLib.resolve(root, file)); } -function isLocalFolder(root) { +export function isLocalFolder(root) { try { return fs.lstatSync(root).isDirectory(); } catch (e) { diff --git a/test/acceptance/cli.acceptance.test.ts b/test/acceptance/cli.acceptance.test.ts index 20dbb365563..d817e341ccf 100644 --- a/test/acceptance/cli.acceptance.test.ts +++ b/test/acceptance/cli.acceptance.test.ts @@ -106,13 +106,15 @@ test('userMessage correctly bubbles with everything other than npm', async (t) = */ test('`test semver` sends remote NPM request:', async (t) => { - t.plan(3); // We care about the request here, not the response - await cli.test('semver', {registry: 'npm', org: 'EFF'}); + let output = await cli.test('semver', {registry: 'npm', org: 'EFF'}); const req = server.popRequest(); t.equal(req.method, 'GET', 'makes GET request'); t.match(req.url, '/vuln/npm/semver', 'gets from correct url'); t.equal(req.query.org, 'EFF', 'org sent as a query in request'); + t.match(output, 'Testing semver', 'has "Testing semver" message'); + t.notMatch(output, 'Remediation', 'shows no remediation advice'); + t.notMatch(output, 'snyk wizard', 'does not suggest `snyk wizard`'); }); test('`test sinatra --registry=rubygems` sends remote Rubygems request:', async (t) => {