From a234c41d25a231fad7432239f90cf925800351f1 Mon Sep 17 00:00:00 2001 From: ghe Date: Fri, 28 Aug 2020 16:50:36 +0100 Subject: [PATCH] feat: handle empty package.json dependencies without error --- src/cli/args.ts | 1 + src/lib/get-file-contents.ts | 4 +- .../nodejs-plugin/npm-modules-parser.ts | 54 ++++++++++++---- .../nodejs-plugin/yarn-workspaces-parser.ts | 6 +- .../fixtures/npm/no-dependencies/package.json | 12 ++++ .../npm/npm-3-no-node-modules/package.json | 3 +- test/missing-node-modules.test.ts | 61 +++++++++++++++++-- 7 files changed, 119 insertions(+), 22 deletions(-) create mode 100644 test/fixtures/npm/no-dependencies/package.json diff --git a/src/cli/args.ts b/src/cli/args.ts index 299d1754afd..1ee0f84d72b 100644 --- a/src/cli/args.ts +++ b/src/cli/args.ts @@ -36,6 +36,7 @@ const DEBUG_DEFAULT_NAMESPACES = [ 'snyk:find-files', 'snyk:run-test', 'snyk:prune', + 'snyk-nodejs-plugin', 'snyk-gradle-plugin', 'snyk-sbt-plugin', 'snyk-mvn-plugin', diff --git a/src/lib/get-file-contents.ts b/src/lib/get-file-contents.ts index bb000759efd..875f2568d98 100644 --- a/src/lib/get-file-contents.ts +++ b/src/lib/get-file-contents.ts @@ -6,7 +6,7 @@ export function getFileContents( fileName: string, ): { content: string; - name: string; + fileName: string; } { const fullPath = path.resolve(root, fileName); if (!fs.existsSync(fullPath)) { @@ -17,6 +17,6 @@ export function getFileContents( const content = fs.readFileSync(fullPath, 'utf-8'); return { content, - name: fileName, + fileName, }; } diff --git a/src/lib/plugins/nodejs-plugin/npm-modules-parser.ts b/src/lib/plugins/nodejs-plugin/npm-modules-parser.ts index 26f94b2c628..38e7e86ee80 100644 --- a/src/lib/plugins/nodejs-plugin/npm-modules-parser.ts +++ b/src/lib/plugins/nodejs-plugin/npm-modules-parser.ts @@ -1,21 +1,62 @@ import * as path from 'path'; import * as fs from 'fs'; import * as resolveNodeDeps from 'snyk-resolve-deps'; +import * as baseDebug from 'debug'; +import * as _ from '@snyk/lodash'; import * as spinner from '../../spinner'; import * as analytics from '../../analytics'; import { Options } from '../types'; +import { getFileContents } from '../../get-file-contents'; + +const debug = baseDebug('snyk-nodejs-plugin'); export async function parse( root: string, targetFile: string, options: Options, ): Promise { + if (targetFile.endsWith('yarn.lock')) { + options.file = + options.file && options.file.replace('yarn.lock', 'package.json'); + } + + // package-lock.json falls back to package.json (used in wizard code) + if (targetFile.endsWith('package-lock.json')) { + options.file = + options.file && options.file.replace('package-lock.json', 'package.json'); + } + // check if there any dependencies + const packageJsonFileName = path.resolve(root, options.file!); + const packageManager = options.packageManager || 'npm'; + + try { + const packageJson = JSON.parse( + getFileContents(root, packageJsonFileName).content, + ); + let dependencies = packageJson.dependencies; + if (options.dev) { + dependencies = { ...dependencies, ...packageJson.devDependencies }; + } + if (_.isEmpty(dependencies)) { + return new Promise((resolve) => + resolve({ + name: packageJson.name, + dependencies: {}, + version: packageJson.version, + }), + ); + } + } catch (e) { + debug(`Failed to read ${packageJsonFileName}: Error: ${e}`); + throw new Error( + `Failed to read ${packageJsonFileName}. Error: ${e.message}`, + ); + } const nodeModulesPath = path.join( path.dirname(path.resolve(root, targetFile)), 'node_modules', ); - const packageManager = options.packageManager || 'npm'; if (!fs.existsSync(nodeModulesPath)) { // throw a custom error @@ -36,17 +77,6 @@ export async function parse( try { await spinner.clear(resolveModuleSpinnerLabel)(); await spinner(resolveModuleSpinnerLabel); - if (targetFile.endsWith('yarn.lock')) { - options.file = - options.file && options.file.replace('yarn.lock', 'package.json'); - } - - // package-lock.json falls back to package.json (used in wizard code) - if (targetFile.endsWith('package-lock.json')) { - options.file = - options.file && - options.file.replace('package-lock.json', 'package.json'); - } return resolveNodeDeps( root, Object.assign({}, options, { noFromArrays: true }), diff --git a/src/lib/plugins/nodejs-plugin/yarn-workspaces-parser.ts b/src/lib/plugins/nodejs-plugin/yarn-workspaces-parser.ts index 3bbb75ad952..18d7d3d4e97 100644 --- a/src/lib/plugins/nodejs-plugin/yarn-workspaces-parser.ts +++ b/src/lib/plugins/nodejs-plugin/yarn-workspaces-parser.ts @@ -89,7 +89,7 @@ export async function processYarnWorkspaces( ); const project: ScannedProjectCustom = { packageManager: 'yarn', - targetFile: path.relative(root, packageJson.name), + targetFile: path.relative(root, packageJson.fileName), depTree: res as any, plugin: { name: 'snyk-nodejs-lockfile-parser', @@ -110,7 +110,7 @@ interface YarnWorkspacesMap { export function getWorkspacesMap(file: { content: string; - name: string; + fileName: string; }): YarnWorkspacesMap { const yarnWorkspacesMap = {}; if (!file) { @@ -123,7 +123,7 @@ export function getWorkspacesMap(file: { ); if (rootFileWorkspacesDefinitions && rootFileWorkspacesDefinitions.length) { - yarnWorkspacesMap[file.name] = { + yarnWorkspacesMap[file.fileName] = { workspaces: rootFileWorkspacesDefinitions, }; } diff --git a/test/fixtures/npm/no-dependencies/package.json b/test/fixtures/npm/no-dependencies/package.json new file mode 100644 index 00000000000..5f5ada67b21 --- /dev/null +++ b/test/fixtures/npm/no-dependencies/package.json @@ -0,0 +1,12 @@ +{ + "name": "custom-name-package", + "version": "0.6.6", + "description": "", + "main": "index.js", + "scripts": { + "test": "echo \"Error: no test specified\" && exit 1" + }, + "keywords": [], + "author": "", + "license": "ISC" +} diff --git a/test/fixtures/npm/npm-3-no-node-modules/package.json b/test/fixtures/npm/npm-3-no-node-modules/package.json index b73ad938ab2..b342340f4f4 100644 --- a/test/fixtures/npm/npm-3-no-node-modules/package.json +++ b/test/fixtures/npm/npm-3-no-node-modules/package.json @@ -1,5 +1,5 @@ { - "name": "qs", + "name": "custom-name-package", "version": "0.6.6", "description": "", "main": "index.js", @@ -10,5 +10,6 @@ "author": "", "license": "ISC", "dependencies": { + "debug": "3.6.7" } } diff --git a/test/missing-node-modules.test.ts b/test/missing-node-modules.test.ts index 830eac45775..3329f0c8eb1 100644 --- a/test/missing-node-modules.test.ts +++ b/test/missing-node-modules.test.ts @@ -1,9 +1,10 @@ import * as tap from 'tap'; -const { test } = tap; import * as fs from 'fs'; +import { getWorkspaceJSON } from './acceptance/workspace-helper'; +import { fakeServer } from './acceptance/fake-server'; +const { test } = tap; const apiKey = '123456789'; -const notAuthorizedApiKey = 'notAuthorized'; const port = process.env.PORT || process.env.SNYK_PORT || '12345'; let oldkey; let oldendpoint; @@ -13,13 +14,17 @@ process.env.SNYK_HOST = 'http://localhost:' + port; const baseDir = __dirname + '/fixtures/'; -// tslint:disable-next-line:no-var-requires -const server = require('./cli-server')(BASE_API, apiKey, notAuthorizedApiKey); +const server = fakeServer(BASE_API, apiKey); // ensure this is required *after* the demo server, since this will // configure our fake configuration too import * as cli from '../src/cli/commands'; +const noVulnsResult = getWorkspaceJSON( + 'fail-on', + 'no-vulns', + 'vulns-result.json', +); test('setup', (t) => { t.plan(3); cli.config('get', 'api').then((key) => { @@ -55,6 +60,54 @@ test('throws when missing node_modules', async (t) => { } }); +test('does not throw when missing node_modules & package.json has no dependencies', async (t) => { + t.plan(1); + server.setNextResponse(noVulnsResult); + + const dir = baseDir + 'npm/no-dependencies'; + // ensure node_modules does not exist + try { + fs.rmdirSync(dir + '/node_modules'); + } catch (err) { + // ignore + } + // test + try { + const res = await cli.test(dir); + t.match( + res.getDisplayResults(), + 'for known issues, no vulnerable paths found.', + 'res is correct', + ); + } catch (e) { + t.fail('should have passed', e); + } +}); + +test('does not throw when missing node_modules & package.json has no dependencies (with --dev)', async (t) => { + t.plan(1); + server.setNextResponse(noVulnsResult); + + const dir = baseDir + 'npm/no-dependencies'; + // ensure node_modules does not exist + try { + fs.rmdirSync(dir + '/node_modules'); + } catch (err) { + // ignore + } + // test + try { + const res = await cli.test(dir, { dev: true }); + t.match( + res.getDisplayResults(), + 'for known issues, no vulnerable paths found.', + 'res is correct', + ); + } catch (e) { + t.fail('should have passed', e); + } +}); + test('teardown', (t) => { t.plan(4);