Skip to content

Commit

Permalink
Merge pull request #1224 from snyk/fix/policies-all-projects
Browse files Browse the repository at this point in the history
fix: policies to be correctly picked up from nested scans
  • Loading branch information
lili2311 committed Jun 23, 2020
2 parents 32c3dd7 + 9f50581 commit 14bbc0d
Show file tree
Hide file tree
Showing 19 changed files with 361 additions and 16 deletions.
27 changes: 27 additions & 0 deletions src/lib/monitor/index.ts
@@ -1,4 +1,6 @@
import * as Debug from 'debug';
import * as path from 'path';

import * as depGraphLib from '@snyk/dep-graph';
import * as snyk from '..';
import { apiTokenExists } from '../api-token';
Expand Down Expand Up @@ -185,13 +187,21 @@ async function monitorDepTree(
treeMissingDeps = missingDeps;
}

let targetFileDir;

if (targetFileRelativePath) {
const { dir } = path.parse(targetFileRelativePath);
targetFileDir = dir;
}

const policy = await findAndLoadPolicy(
root,
meta.isDocker ? 'docker' : packageManager!,
options,
// TODO: fix this and send only send when we used resolve-deps for node
// it should be a ExpandedPkgTree type instead
depTree,
targetFileDir,
);

const target = await projectMetadata.getInfo(scannedProject, meta, depTree);
Expand Down Expand Up @@ -328,10 +338,19 @@ export async function monitorDepGraph(
);
}

let targetFileDir;

if (targetFileRelativePath) {
const { dir } = path.parse(targetFileRelativePath);
targetFileDir = dir;
}

const policy = await findAndLoadPolicy(
root,
meta.isDocker ? 'docker' : packageManager!,
options,
undefined,
targetFileDir,
);

const target = await projectMetadata.getInfo(scannedProject, meta);
Expand Down Expand Up @@ -443,13 +462,21 @@ async function experimentalMonitorDepGraphFromDepTree(
);
}

let targetFileDir;

if (targetFileRelativePath) {
const { dir } = path.parse(targetFileRelativePath);
targetFileDir = dir;
}

const policy = await findAndLoadPolicy(
root,
meta.isDocker ? 'docker' : packageManager!,
options,
// TODO: fix this and send only send when we used resolve-deps for node
// it should be a ExpandedPkgTree type instead
depTree,
targetFileDir,
);

if (['npm', 'yarn'].includes(meta.packageManager)) {
Expand Down
6 changes: 5 additions & 1 deletion src/lib/policy/find-and-load-policy.ts
Expand Up @@ -14,18 +14,22 @@ export async function findAndLoadPolicy(
scanType: SupportedPackageManagers | 'docker',
options: PolicyOptions,
pkg?: PackageExpanded,
scannedProjectFolder?: string,
): Promise<string | undefined> {
const isDocker = scanType === 'docker';
const isNodeProject = ['npm', 'yarn'].includes(scanType);
// monitor
let policyLocations: string[] = [options['policy-path'] || root];
let policyLocations: string[] = [
options['policy-path'] || scannedProjectFolder || root,
];
if (isDocker) {
policyLocations = policyLocations.filter((loc) => loc !== root);
} else if (isNodeProject) {
// TODO: pluckPolicies expects a package.json object to
// find and apply policies in node_modules
policyLocations = policyLocations.concat(pluckPolicies(pkg as PackageJson));
}

debug('Potential policy locations found:', policyLocations);
analytics.add('policies', policyLocations.length);
analytics.add('policyLocations', policyLocations);
Expand Down
27 changes: 17 additions & 10 deletions src/lib/snyk-test/run-test.ts
Expand Up @@ -108,7 +108,6 @@ async function runTest(
_.get(payload, 'body.originalProjectName');
const foundProjectCount = _.get(payload, 'body.foundProjectCount');
const displayTargetFile = _.get(payload, 'body.displayTargetFile');

let dockerfilePackages;
if (
payload.body &&
Expand Down Expand Up @@ -402,13 +401,30 @@ async function assembleLocalPayloads(
}
}

// todo: normalize what target file gets used across plugins and functions
const targetFile =
scannedProject.targetFile || deps.plugin.targetFile || options.file;

// Forcing options.path to be a string as pathUtil requires is to be stringified
const targetFileRelativePath = targetFile
? pathUtil.join(pathUtil.resolve(`${options.path || root}`), targetFile)
: '';

let targetFileDir;

if (targetFileRelativePath) {
const { dir } = path.parse(targetFileRelativePath);
targetFileDir = dir;
}

const policy = await findAndLoadPolicy(
root,
options.docker ? 'docker' : packageManager!,
options,
// TODO: fix this and send only send when we used resolve-deps for node
// it should be a ExpandedPkgTree type instead
pkg,
targetFileDir,
);

analytics.add('packageManager', packageManager);
Expand All @@ -421,15 +437,6 @@ async function assembleLocalPayloads(
addPackageAnalytics(depTree.name!, depTree.version!);
}

// todo: normalize what target file gets used across plugins and functions
const targetFile =
scannedProject.targetFile || deps.plugin.targetFile || options.file;

// Forcing options.path to be a string as pathUtil requires is to be stringified
const targetFileRelativePath = targetFile
? pathUtil.join(pathUtil.resolve(`${options.path}`), targetFile)
: '';

let target: GitTarget | ContainerTarget | null;
if (scannedProject.depGraph) {
target = await projectMetadata.getInfo(scannedProject, options);
Expand Down
1 change: 0 additions & 1 deletion test/acceptance/cli-monitor/cli-monitor.acceptance.test.ts
Expand Up @@ -13,7 +13,6 @@ import * as _ from '@snyk/lodash';

// ensure this is required *after* the demo server, since this will
// configure our fake configuration too
import * as snykPolicy from 'snyk-policy';
import { AllProjectsTests } from './cli-monitor.all-projects.spec';

const { test, only } = tap;
Expand Down
31 changes: 31 additions & 0 deletions test/acceptance/cli-monitor/cli-monitor.all-projects.spec.ts
Expand Up @@ -12,6 +12,37 @@ interface AcceptanceTests {
export const AllProjectsTests: AcceptanceTests = {
language: 'Mixed',
tests: {
'`monitor mono-repo-with-ignores --all-projects` respects .snyk policy': (
params,
utils,
) => async (t) => {
utils.chdirWorkspaces();
await params.cli.monitor('mono-repo-with-ignores', {
allProjects: true,
detectionDepth: 2,
});
// Pop all calls to server and filter out calls to `featureFlag` endpoint
const requests = params.server
.popRequests(4)
.filter((req) => req.url.includes('/monitor/'));
let policyCount = 0;
requests.forEach((req) => {
const vulnerableFolderPath =
process.platform === 'win32'
? 'vulnerable\\package-lock.json'
: 'vulnerable/package-lock.json';

if (req.body.targetFileRelativePath.endsWith(vulnerableFolderPath)) {
t.match(
req.body.policy,
'npm:node-uuid:20160328',
'body contains policy',
);
policyCount += 1;
}
});
t.equal(policyCount, 1, 'one policy found');
},
'`monitor mono-repo-project --all-projects --detection-depth=1`': (
params,
utils,
Expand Down
61 changes: 61 additions & 0 deletions test/acceptance/cli-test/cli-test.all-projects.spec.ts
Expand Up @@ -7,6 +7,67 @@ import { CommandResult } from '../../../src/cli/commands/types';
export const AllProjectsTests: AcceptanceTests = {
language: 'Mixed',
tests: {
'`test mono-repo-with-ignores --all-projects` respects .snyk policy': (
params,
utils,
) => async (t) => {
utils.chdirWorkspaces();
const loadPlugin = sinon.spy(params.plugins, 'loadPlugin');
t.teardown(loadPlugin.restore);

const result: CommandResult = await params.cli.test(
'mono-repo-with-ignores',
{
allProjects: true,
detectionDepth: 3,
},
);
t.ok(loadPlugin.withArgs('npm').calledTwice, 'calls npm plugin');
let policyCount = 0;
params.server.popRequests(2).forEach((req) => {
t.equal(req.method, 'POST', 'makes POST request');
t.equal(
req.headers['x-snyk-cli-version'],
params.versionNumber,
'sends version number',
);
t.match(req.url, '/api/v1/test-dep-graph', 'posts to correct url');
t.ok(req.body.depGraph, 'body contains depGraph');
const vulnerableFolderPath =
process.platform === 'win32'
? 'vulnerable\\package-lock.json'
: 'vulnerable/package-lock.json';
if (req.body.targetFileRelativePath.endsWith(vulnerableFolderPath)) {
t.match(
req.body.policy,
'npm:node-uuid:20160328',
'body contains policy',
);
policyCount += 1;
}
t.match(
req.body.depGraph.pkgManager.name,
/(npm)/,
'depGraph has package manager',
);
});

t.match(policyCount, 1, 'one policy should have been found');
// results should contain test results from both package managers
// and show only 1/2 vulnerable paths for nested one since we ignore
// it in the .snyk file

t.match(
result.getDisplayResults(),
'Package manager: npm',
'contains package manager npm',
);
t.match(
result.getDisplayResults(),
'Target file: package-lock.json',
'contains target file package-lock.json',
);
},
'`test mono-repo-project with lockfiles --all-projects`': (
params,
utils,
Expand Down
44 changes: 44 additions & 0 deletions test/acceptance/cli-test/cli-test.gradle.spec.ts
Expand Up @@ -112,6 +112,50 @@ export const GradleTests: AcceptanceTests = {
});
t.true(((spyPlugin.args[0] as any)[2] as any).allSubProjects);
},
'`test gradle-app --all-sub-projects` with policy': (
params,
utils,
) => async (t) => {
utils.chdirWorkspaces();
const plugin = {
async inspect() {
return { plugin: { name: 'gradle' }, package: {} };
},
};
const spyPlugin = sinon.spy(plugin, 'inspect');
const loadPlugin = sinon.stub(params.plugins, 'loadPlugin');
t.teardown(loadPlugin.restore);
loadPlugin.withArgs('gradle').returns(plugin);

await params.cli.test('gradle-app', {
allSubProjects: true,
});
t.true(((spyPlugin.args[0] as any)[2] as any).allSubProjects);
const requests = params.server.popRequests(2);
let policyCount = 0;
requests.forEach((req) => {
if (
req.body.displayTargetFile.endsWith('gradle-multi-project/subproj')
) {
// TODO: this should return 1 policy when fixed
// uncomment then
// t.match(
// req.body.policy,
// 'SNYK-JAVA-ORGBOUNCYCASTLE-32364',
// 'policy is found & sent',
// );
t.ok(
req.body.policy,
undefined,
'policy is not found even though it should be',
);
policyCount += 1;
}
t.match(req.url, '/test-dep-graph', 'posts to correct url');
});
// TODO: this should return 1 policy when fixed
t.equal(policyCount, 0, 'one sub-project policy found & sent');
},

'`test gradle-app` plugin fails to return package or scannedProjects': (
params,
Expand Down
37 changes: 35 additions & 2 deletions test/acceptance/cli-test/cli-test.yarn.spec.ts
Expand Up @@ -187,15 +187,48 @@ export const YarnTests: AcceptanceTests = {
'depGraph looks fine',
);
},

'`test yarn-package --file=yarn.lock ` sends pkg info': (
'`test yarn-package --file=yarn-package/yarn.lock ` sends pkg info & policy': (
params,
utils,
) => async (t) => {
utils.chdirWorkspaces();
await params.cli.test({ file: 'yarn-package/yarn.lock' });
const req = params.server.popRequest();
t.match(req.url, '/test-dep-graph', 'posts to correct url');
t.match(req.body.policy, 'npm:debug:20170905', 'policy is found & sent');
t.match(req.body.targetFile, undefined, 'target is undefined');
const depGraph = req.body.depGraph;
t.same(
depGraph.pkgs.map((p) => p.id).sort(),
['npm-package@1.0.0', 'ms@0.7.1', 'debug@2.2.0'].sort(),
'depGraph looks fine',
);
},
'`test yarn-package --file=yarn.lock ` sends pkg info & policy': (
params,
utils,
) => async (t) => {
utils.chdirWorkspaces();
await params.cli.test('yarn-package', { file: 'yarn.lock' });
const req = params.server.popRequest();
t.match(req.url, '/test-dep-graph', 'posts to correct url');
t.match(req.body.policy, 'npm:debug:20170905', 'policy is found & sent');
t.match(req.body.targetFile, undefined, 'target is undefined');
const depGraph = req.body.depGraph;
t.same(
depGraph.pkgs.map((p) => p.id).sort(),
['npm-package@1.0.0', 'ms@0.7.1', 'debug@2.2.0'].sort(),
'depGraph looks fine',
);
},
'`test yarn-package` sends pkg info & policy': (params, utils) => async (
t,
) => {
utils.chdirWorkspaces('yarn-package');
await params.cli.test();
const req = params.server.popRequest();
t.match(req.url, '/test-dep-graph', 'posts to correct url');
t.match(req.body.policy, 'npm:debug:20170905', 'policy is found & sent');
t.match(req.body.targetFile, undefined, 'target is undefined');
const depGraph = req.body.depGraph;
t.same(
Expand Down
Empty file.
@@ -0,0 +1,5 @@
rootProject.name = 'root-proj'

include 'subproj'


9 changes: 9 additions & 0 deletions test/acceptance/workspaces/gradle-multi-project/subproj/.snyk
@@ -0,0 +1,9 @@
# Snyk (https://snyk.io) policy file, patches or ignores known vulnerabilities.
version: v1.14.1
# ignores vulnerabilities until expiry date; change duration by modifying expiry date
ignore:
SNYK-JAVA-ORGBOUNCYCASTLE-32364:
- '*':
reason: None Given
expires: 2020-07-19T11:33:19.028Z
patch: {}

0 comments on commit 14bbc0d

Please sign in to comment.