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 c65de60
Show file tree
Hide file tree
Showing 3 changed files with 32 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
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 c65de60

Please sign in to comment.