Skip to content

Commit

Permalink
fix: do not offer remediation advice when scanning a non-local package
Browse files Browse the repository at this point in the history
  • Loading branch information
Konstantin Yegupov committed May 1, 2019
1 parent 8c18c6f commit df104e3
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 10 deletions.
23 changes: 16 additions & 7 deletions src/cli/commands/test.ts
Expand Up @@ -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<string> {
const resultOptions = [] as any[];
let results = [] as any[];
let options = {} as any as TestOptions;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.');
}

Expand Down Expand Up @@ -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));
Expand All @@ -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(
Expand Down Expand Up @@ -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) : '',
Expand Down
2 changes: 1 addition & 1 deletion src/lib/detect.ts
Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions test/acceptance/cli.acceptance.test.ts
Expand Up @@ -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) => {
Expand Down

0 comments on commit df104e3

Please sign in to comment.