From c65de6075930283cb8a6d5ac5c051885401ad234 Mon Sep 17 00:00:00 2001 From: Casey Marshall Date: Fri, 30 Aug 2024 17:08:10 -0500 Subject: [PATCH] fix: default limit to max vulnerable paths per vuln, add override option 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. --- .../commands/test/set-default-test-options.ts | 5 +++- src/lib/snyk-test/legacy.ts | 5 +++- .../test/set-default-test-options.spec.ts | 24 +++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 test/jest/unit/cli/commands/test/set-default-test-options.spec.ts diff --git a/src/cli/commands/test/set-default-test-options.ts b/src/cli/commands/test/set-default-test-options.ts index 62504a31d0..e86b5c9cdf 100644 --- a/src/cli/commands/test/set-default-test-options.ts +++ b/src/cli/commands/test/set-default-test-options.ts @@ -9,12 +9,15 @@ export function setDefaultTestOptions( .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, }; } diff --git a/src/lib/snyk-test/legacy.ts b/src/lib/snyk-test/legacy.ts index bbd88f80b2..e82b587f44 100644 --- a/src/lib/snyk-test/legacy.ts +++ b/src/lib/snyk-test/legacy.ts @@ -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( diff --git a/test/jest/unit/cli/commands/test/set-default-test-options.spec.ts b/test/jest/unit/cli/commands/test/set-default-test-options.spec.ts new file mode 100644 index 0000000000..1aeaea86db --- /dev/null +++ b/test/jest/unit/cli/commands/test/set-default-test-options.spec.ts @@ -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(); + }); +})