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.

To mitigate this, vulnerable paths is limited to a maximum of 20 per
vulnerability, which can be raised or lowered using a new
--max-vulnerable-paths option.

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

With these options and additional documentation, OOM issues will be less
likely to occur with default settings, and users interested in vulnerable
path information have more flexibility to mitigate memory issues.
  • Loading branch information
cmars committed Sep 24, 2024
1 parent 0a6a560 commit 9b5ca04
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 2 deletions.
8 changes: 7 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,15 +9,21 @@ export function setDefaultTestOptions<CommandOptions>(
.toLowerCase();

delete options['show-vulnerable-paths'];
const showVulnPaths = showVulnPathsMapping[svpSupplied] || 'some';
const maxVulnPaths =
showVulnPaths === 'all' ? undefined : options['max-vulnerable-paths'] ?? defaultMaxVulnPaths;
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,
};
}

const defaultMaxVulnPaths = 20;

const showVulnPathsMapping: Record<string, ShowVulnPaths> = {
false: 'none',
none: 'none',
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

0 comments on commit 9b5ca04

Please sign in to comment.