Skip to content

Commit

Permalink
fix: default limit to max vulnerable paths per vuln, add override option
Browse files Browse the repository at this point in the history
In CLI-261 an out-of-memory condition occurs when trying to report SCA
findings on a container test. This happens due to an unusually large
number of dependency paths to some vulnerabilities.

Such a large number of paths is unlikely to be useful or helpful.

If the user has specifically requested all vulnerable paths with
--show-vulnerable-paths=all, this takes precedence and the limit is
ignored.

Otherwise the user may opt-in to limiting the max number of vulnerable paths
with --max-vulnerable-paths=N.
  • Loading branch information
cmars committed Sep 24, 2024
1 parent 0a6a560 commit 0836ef9
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/cli/commands/test/set-default-test-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ export function setDefaultTestOptions<CommandOptions>(
.toLowerCase();

delete options['show-vulnerable-paths'];
const showVulnPaths = showVulnPathsMapping[svpSupplied] || 'some';
const maxVulnPaths = options['max-vulnerable-paths'];
return {
...options,
// org fallback to config unless specified
org: options.org || config.org,
// making `show-vulnerable-paths` 'some' by default.
showVulnPaths: showVulnPathsMapping[svpSupplied] || 'some',
showVulnPaths,
maxVulnPaths,
};
}

Expand Down
5 changes: 4 additions & 1 deletion src/lib/snyk-test/legacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,10 @@ function convertTestDepGraphResultToLegacy(
const vulns: AnnotatedIssue[] = [];

for (const pkgInfo of values(result.affectedPkgs)) {
for (const vulnPkgPath of depGraph.pkgPathsToRoot(pkgInfo.pkg)) {
const pkgPathsToRoot = depGraph.pkgPathsToRoot(pkgInfo.pkg, {
limit: options.maxVulnPaths,
});
for (const vulnPkgPath of pkgPathsToRoot) {
const legacyFromPath = pkgPathToLegacyPath(vulnPkgPath.reverse());
for (const pkgIssue of values(pkgInfo.issues)) {
const vulnPathString = getVulnPathString(
Expand Down
1 change: 1 addition & 0 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface TestOptions {
traverseNodeModules?: boolean;
pruneRepeatedSubdependencies?: boolean;
showVulnPaths: ShowVulnPaths;
maxVulnPaths?: number;
failOn?: FailOn;
initScript?: string;
yarnWorkspaces?: boolean;
Expand Down
24 changes: 24 additions & 0 deletions test/jest/unit/cli/commands/test/set-default-test-options.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { setDefaultTestOptions } from '../../../../../../src/cli/commands/test/set-default-test-options';

describe('setDefaultTestOptions', () => {
it('default options', () => {
const options = {};
const result = setDefaultTestOptions(options as any);
expect(result.showVulnPaths).toEqual('some');
expect(result.maxVulnPaths).toBeUndefined();
});

it('explicit max-vulnerable-paths', () => {
const options = { 'max-vulnerable-paths': 42 };
const result = setDefaultTestOptions(options as any);
expect(result.showVulnPaths).toEqual('some');
expect(result.maxVulnPaths).toEqual(42);
});

it('explicit show-vulnerable-paths', () => {
const options = { 'show-vulnerable-paths': 'all' };
const result = setDefaultTestOptions(options as any);
expect(result.showVulnPaths).toEqual('all');
expect(result.maxVulnPaths).toBeUndefined();
});
});

0 comments on commit 0836ef9

Please sign in to comment.