Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat: handle empty package.json dependencies without error
  • Loading branch information
lili2311 committed Aug 28, 2020
1 parent 2780cac commit a234c41
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 22 deletions.
1 change: 1 addition & 0 deletions src/cli/args.ts
Expand Up @@ -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',
Expand Down
4 changes: 2 additions & 2 deletions src/lib/get-file-contents.ts
Expand Up @@ -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)) {
Expand All @@ -17,6 +17,6 @@ export function getFileContents(
const content = fs.readFileSync(fullPath, 'utf-8');
return {
content,
name: fileName,
fileName,
};
}
54 changes: 42 additions & 12 deletions 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<resolveNodeDeps.PackageExpanded> {
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
Expand All @@ -36,17 +77,6 @@ export async function parse(
try {
await spinner.clear<void>(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 }),
Expand Down
6 changes: 3 additions & 3 deletions src/lib/plugins/nodejs-plugin/yarn-workspaces-parser.ts
Expand Up @@ -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',
Expand All @@ -110,7 +110,7 @@ interface YarnWorkspacesMap {

export function getWorkspacesMap(file: {
content: string;
name: string;
fileName: string;
}): YarnWorkspacesMap {
const yarnWorkspacesMap = {};
if (!file) {
Expand All @@ -123,7 +123,7 @@ export function getWorkspacesMap(file: {
);

if (rootFileWorkspacesDefinitions && rootFileWorkspacesDefinitions.length) {
yarnWorkspacesMap[file.name] = {
yarnWorkspacesMap[file.fileName] = {
workspaces: rootFileWorkspacesDefinitions,
};
}
Expand Down
12 changes: 12 additions & 0 deletions 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"
}
3 changes: 2 additions & 1 deletion 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",
Expand All @@ -10,5 +10,6 @@
"author": "",
"license": "ISC",
"dependencies": {
"debug": "3.6.7"
}
}
61 changes: 57 additions & 4 deletions 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;
Expand All @@ -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) => {
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit a234c41

Please sign in to comment.