Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: detect other files & suggest to use --all-projects #1233

Merged
merged 1 commit into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ export function formatMonitorOutput(
const manageUrl = buildManageUrl(res.id, res.org);
const advertiseGradleSubProjectsCount =
packageManager === 'gradle' && !options['gradle-sub-project'];

const advertiseAllProjectsCount =
packageManager !== 'gradle' && !options.allProjects && foundProjectCount;
const issues = res.licensesPolicy ? 'issues' : 'vulnerabilities';
const humanReadableName = projectName
? `${res.path} (${projectName})`
Expand All @@ -31,6 +32,12 @@ export function formatMonitorOutput(
'use --all-sub-projects flag to scan all sub-projects.\n\n',
)
: '') +
(advertiseAllProjectsCount && foundProjectCount
? chalk.bold.white(
`Tip: Detected multiple supported manifests (${foundProjectCount}), ` +
'use --all-projects to scan all of them at once.\n\n',
)
: '') +
(res.isMonitored
? 'Notifications about newly disclosed ' +
issues +
Expand Down
4 changes: 2 additions & 2 deletions src/cli/commands/monitor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { processJsonMonitorResponse } from './process-json-monitor';
import snyk = require('../../../lib'); // TODO(kyegupov): fix import
import { formatMonitorOutput } from './formatters/format-monitor-response';
import { getDepsFromPlugin } from '../../../lib/plugins/get-deps-from-plugin';
import { getSubProjectCount } from '../../../lib/plugins/get-sub-project-count';
import { getExtraProjectCount } from '../../../lib/plugins/get-extra-project-count';
import { extractPackageManager } from '../../../lib/plugins/extract-package-manager';
import { MultiProjectResultCustom } from '../../../lib/plugins/get-multi-plugin-result';
import { convertMultiResultToMultiCustom } from '../../../lib/plugins/convert-multi-plugin-res-to-multi-custom';
Expand Down Expand Up @@ -241,7 +241,7 @@ async function monitor(...args0: MethodArgs): Promise<any> {
res,
options,
projectName,
getSubProjectCount(inspectResult),
await getExtraProjectCount(path, options, inspectResult),
);
// push a good result
results.push({ ok: true, data: monOutput, path, projectName });
Expand Down
15 changes: 12 additions & 3 deletions src/cli/commands/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ async function test(...args: MethodArgs): Promise<TestCommandResult> {
let response = results
.map((result, i) => {
resultOptions[i].pinningSupported = pinningSupported;

return displayResult(
results[i] as LegacyVulnApiResult,
resultOptions[i],
Expand Down Expand Up @@ -425,13 +424,23 @@ function displayResult(
let multiProjAdvice = '';

const advertiseGradleSubProjectsCount =
projectType === 'gradle' && !options['gradle-sub-project'];
if (advertiseGradleSubProjectsCount && foundProjectCount) {
projectType === 'gradle' &&
!options['gradle-sub-project'] &&
foundProjectCount;
if (advertiseGradleSubProjectsCount) {
multiProjAdvice = chalk.bold.white(
`\n\nTip: This project has multiple sub-projects (${foundProjectCount}), ` +
'use --all-sub-projects flag to scan all sub-projects.',
);
}
const advertiseAllProjectsCount =
projectType !== 'gradle' && !options.allProjects && foundProjectCount;
if (advertiseAllProjectsCount) {
multiProjAdvice = chalk.bold.white(
`\n\nTip: Detected multiple supported manifests (${foundProjectCount}), ` +
'use --all-projects to scan all of them at once.',
);
}

// OK => no vulns found, return
if (res.ok && res.vulnerabilities.length === 0) {
Expand Down
29 changes: 29 additions & 0 deletions src/lib/plugins/get-extra-project-count.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { legacyPlugin as pluginApi } from '@snyk/cli-interface';
import { find } from '../find-files';
import { AUTO_DETECTABLE_FILES } from '../detect';
import { Options } from '../types';

export async function getExtraProjectCount(
root: string,
options: Options,
inspectResult: pluginApi.InspectResult,
): Promise<number | undefined> {
if (options.docker) {
return undefined;
}
if (
inspectResult.plugin.meta &&
inspectResult.plugin.meta.allSubProjectNames &&
inspectResult.plugin.meta.allSubProjectNames.length > 1
) {
return inspectResult.plugin.meta.allSubProjectNames.length;
}
try {
const extraTargetFiles = await find(root, [], AUTO_DETECTABLE_FILES);
const foundProjectsCount =
extraTargetFiles.length > 1 ? extraTargetFiles.length - 1 : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it more interesting to use which files (their path and name) we found? rather than just a count?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, how would you stop this list from getting too big? file-a, file-b and 24 more... or something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea maybe truncate to a reasonable limit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so changing both the message for gradle and this one? They both use the same ish wording & show how many more you could scan with an different option? This function handles both discovered gradle sub-proejcts (where we have the sub-project names) and now also extra manifests that you could scan with --all-projects

return foundProjectsCount;
} catch (e) {
return undefined;
}
}
15 changes: 0 additions & 15 deletions src/lib/plugins/get-sub-project-count.ts

This file was deleted.

4 changes: 2 additions & 2 deletions src/lib/snyk-test/run-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import { ScannedProjectCustom } from '../plugins/get-multi-plugin-result';
import request = require('../request');
import spinner = require('../spinner');
import { extractPackageManager } from '../plugins/extract-package-manager';
import { getSubProjectCount } from '../plugins/get-sub-project-count';
import { getExtraProjectCount } from '../plugins/get-extra-project-count';
import { serializeCallGraphWithMetrics } from '../reachable-vulns';
import { validateOptions } from '../options-validator';
import { findAndLoadPolicy } from '../policy';
Expand Down Expand Up @@ -471,7 +471,7 @@ async function assembleLocalPayloads(
projectNameOverride: options.projectName,
originalProjectName,
policy: policy ? policy.toString() : undefined,
foundProjectCount: getSubProjectCount(deps),
foundProjectCount: await getExtraProjectCount(root, options, deps),
displayTargetFile: targetFile,
docker: (pkg as DepTree).docker,
hasDevDependencies: (pkg as any).hasDevDependencies,
Expand Down
6 changes: 6 additions & 0 deletions test/acceptance/cli-monitor/cli-monitor.acceptance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,12 @@ test('`monitor cocoapods-app --file=Podfile`', async (t) => {
);
});

test('`monitor large-mono-repo --file=bundler-app/Gemfile` suggest to use --all-projects', async (t) => {
chdirWorkspaces('large-mono-repo');
const res = await cli.monitor({ file: 'bundler-app/Gemfile' });
t.match(res, '--all-projects', 'Suggest using --all-projects');
});

test('`monitor cocoapods-app --file=Podfile.lock`', async (t) => {
chdirWorkspaces('cocoapods-app');
const plugin = {
Expand Down
14 changes: 14 additions & 0 deletions test/acceptance/cli-test/cli-test.ruby.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -568,5 +568,19 @@ export const RubyTests: AcceptanceTests = {
);
}
},
'`test large-mono-repo --file=bundler-app/Gemfile`': (
params,
utils,
) => async (t) => {
utils.chdirWorkspaces();
const res = await params.cli.test('large-mono-repo', {
file: 'bundler-app/Gemfile',
});
t.match(
res.getDisplayResults(),
'--all-projects',
'Suggest using --all-projects',
);
},
},
};