From 5d704e27a955edc81967efb08720ca9f3dbc7b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ramon=20R=C3=BCttimann?= Date: Tue, 27 Sep 2022 15:55:34 +0200 Subject: [PATCH 1/2] feat: add flag to exclude app vulnerabilities Although we already have a `--app-vulns` flag, the plan is to enable app-vulnerability scanning in containers by default. Before doing that, we want to give customers the opportunity to explicitly opt-out of the change in the default by introducing a new flag `--exclude-app-vulns`. This flag will co-exist with the `--app-vulns` flag until it's enabled by default, at which point the `--app-vulns` flag can be removed. --- src/cli/commands/monitor/index.ts | 5 +++- src/cli/commands/test/index.ts | 5 +++- src/lib/types.ts | 2 ++ .../app-vuln-container-project.spec.ts | 24 +++++++++++++++++++ 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/cli/commands/monitor/index.ts b/src/cli/commands/monitor/index.ts index 4256991afc..e7a04c2eaa 100644 --- a/src/cli/commands/monitor/index.ts +++ b/src/cli/commands/monitor/index.ts @@ -88,7 +88,10 @@ export default async function monitor(...args0: MethodArgs): Promise { } // TODO remove once https://github.com/snyk/cli/pull/3433 is merged - if (options.docker && !options['app-vulns']) { + if ( + options.docker && + (!options['app-vulns'] || options['exclude-app-vulns']) + ) { options['exclude-app-vulns'] = true; } diff --git a/src/cli/commands/test/index.ts b/src/cli/commands/test/index.ts index cbad39fd0f..3e703202f5 100644 --- a/src/cli/commands/test/index.ts +++ b/src/cli/commands/test/index.ts @@ -89,7 +89,10 @@ export default async function test( } // TODO remove once https://github.com/snyk/cli/pull/3433 is merged - if (options.docker && !options['app-vulns']) { + if ( + options.docker && + (!options['app-vulns'] || options['exclude-app-vulns']) + ) { options['exclude-app-vulns'] = true; } diff --git a/src/lib/types.ts b/src/lib/types.ts index 3705e27bf4..e206643d40 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -73,6 +73,7 @@ export interface Options { experimental?: boolean; // Used with the Docker plugin only. Allows application scanning. 'app-vulns'?: boolean; + 'exclude-app-vulns'?: boolean; debug?: boolean; sarif?: boolean; 'group-issues'?: boolean; @@ -107,6 +108,7 @@ export interface MonitorOptions { experimental?: boolean; // Used with the Docker plugin only. Allows application scanning. 'app-vulns'?: boolean; + 'exclude-app-vulns'?: boolean; initScript?: string; yarnWorkspaces?: boolean; 'max-depth'?: number; diff --git a/test/jest/acceptance/snyk-test/app-vuln-container-project.spec.ts b/test/jest/acceptance/snyk-test/app-vuln-container-project.spec.ts index 999584c41b..8aeec159bb 100644 --- a/test/jest/acceptance/snyk-test/app-vuln-container-project.spec.ts +++ b/test/jest/acceptance/snyk-test/app-vuln-container-project.spec.ts @@ -24,6 +24,30 @@ describe('container test projects behavior with --app-vulns, --file and --exclud expect(jsonOutput[1].uniqueCount).toBeGreaterThan(0); expect(code).toEqual(1); }, 10000); + it('should find nothing when app-vulns are explicitly disabled', async () => { + const { code, stdout } = await runSnykCLI( + `container test docker-archive:test/fixtures/container-projects/os-packages-and-app-vulns.tar --json --exclude-app-vulns`, + ); + const jsonOutput = JSON.parse(stdout); + expect(Array.isArray(jsonOutput)).toBeFalsy(); + expect(jsonOutput.applications).toBeUndefined(); + expect(jsonOutput.ok).toEqual(false); + expect(jsonOutput.uniqueCount).toBeGreaterThan(0); + expect(code).toEqual(1); + }, 10000); + it('should find nothing on conflicting app-vulns flags', async () => { + // if both flags are set, --exclude-app-vulns should take precedence and + // disable it. + const { code, stdout } = await runSnykCLI( + `container test docker-archive:test/fixtures/container-projects/os-packages-and-app-vulns.tar --json --app-vulns --exclude-app-vulns --experimental`, + ); + const jsonOutput = JSON.parse(stdout); + expect(Array.isArray(jsonOutput)).toBeFalsy(); + expect(jsonOutput.applications).toBeUndefined(); + expect(jsonOutput.ok).toEqual(false); + expect(jsonOutput.uniqueCount).toBeGreaterThan(0); + expect(code).toEqual(1); + }, 10000); it('should find all vulns when using --app-vulns without experimental flag', async () => { const { code, stdout } = await runSnykCLI( `container test docker-archive:test/fixtures/container-projects/os-packages-and-app-vulns.tar --json --app-vulns`, From 9216c49e5126decdbeee2dfd34bbe27f2fc7f7e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ramon=20R=C3=BCttimann?= Date: Thu, 29 Sep 2022 09:28:15 +0200 Subject: [PATCH 2/2] feat: print warning message on app-vulns enablement We want to notify customers about the upcoming change to scan app vulns by default in the CLI. As such, this commit adds a warning message whenever `container monitor` or `container test` is executed. --- src/cli/commands/monitor/index.ts | 24 ++++++++++++++++++------ src/cli/commands/test/index.ts | 25 +++++++++++++++++++------ 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/cli/commands/monitor/index.ts b/src/cli/commands/monitor/index.ts index e7a04c2eaa..a328380418 100644 --- a/src/cli/commands/monitor/index.ts +++ b/src/cli/commands/monitor/index.ts @@ -4,6 +4,7 @@ import * as Debug from 'debug'; import * as pathUtil from 'path'; import { legacyPlugin as pluginApi } from '@snyk/cli-interface'; import { checkOSSPaths } from '../../../lib/check-paths'; +import * as theme from '../../../lib/theme'; import { MonitorOptions, @@ -50,6 +51,11 @@ import { processCommandArgs } from '../process-command-args'; const SEPARATOR = '\n-------------------------------------------------------\n'; const debug = Debug('snyk'); +const appVulnsReleaseWarningMsg = `${theme.icon.WARNING} Important: Beginning January 24th, 2023, application dependencies in container +images will be scanned by default when using the snyk container test/monitor +commands. If you are using Snyk in a CI pipeline, action may be required. Read +https://snyk.io/blog/securing-container-applications-using-the-snyk-cli/ for +more info.`; // This is used instead of `let x; try { x = await ... } catch { cleanup }` to avoid // declaring the type of x as possibly undefined. @@ -87,12 +93,18 @@ export default async function monitor(...args0: MethodArgs): Promise { throw new Error('`--remote-repo-url` is not supported for container scans'); } - // TODO remove once https://github.com/snyk/cli/pull/3433 is merged - if ( - options.docker && - (!options['app-vulns'] || options['exclude-app-vulns']) - ) { - options['exclude-app-vulns'] = true; + // TODO remove 'app-vulns' options and warning message once + // https://github.com/snyk/cli/pull/3433 is merged + if (options.docker) { + if (!options['app-vulns'] || options['exclude-app-vulns']) { + options['exclude-app-vulns'] = true; + } + + // we can't print the warning message with JSON output as that would make + // the JSON output invalid. + if (!options['app-vulns'] && !options['json']) { + console.log(theme.color.status.warn(appVulnsReleaseWarningMsg)); + } } // Handles no image arg provided to the container command until diff --git a/src/cli/commands/test/index.ts b/src/cli/commands/test/index.ts index 3e703202f5..631bb32f11 100644 --- a/src/cli/commands/test/index.ts +++ b/src/cli/commands/test/index.ts @@ -4,6 +4,7 @@ const cloneDeep = require('lodash.clonedeep'); const assign = require('lodash.assign'); import chalk from 'chalk'; import { MissingArgError } from '../../../lib/errors'; +import * as theme from '../../../lib/theme'; import * as snyk from '../../../lib'; import { Options, TestOptions } from '../../../lib/types'; @@ -48,6 +49,12 @@ import { checkOSSPaths } from '../../../lib/check-paths'; const debug = Debug('snyk-test'); const SEPARATOR = '\n-------------------------------------------------------\n'; +const appVulnsReleaseWarningMsg = `${theme.icon.WARNING} Important: Beginning January 24th, 2023, application dependencies in container +images will be scanned by default when using the snyk container test/monitor +commands. If you are using Snyk in a CI pipeline, action may be required. Read +https://snyk.io/blog/securing-container-applications-using-the-snyk-cli/ for +more info.`; + // TODO: avoid using `as any` whenever it's possible export default async function test( @@ -88,12 +95,18 @@ export default async function test( throw new MissingArgError(); } - // TODO remove once https://github.com/snyk/cli/pull/3433 is merged - if ( - options.docker && - (!options['app-vulns'] || options['exclude-app-vulns']) - ) { - options['exclude-app-vulns'] = true; + // TODO remove 'app-vulns' options and warning message once + // https://github.com/snyk/cli/pull/3433 is merged + if (options.docker) { + if (!options['app-vulns'] || options['exclude-app-vulns']) { + options['exclude-app-vulns'] = true; + } + + // we can't print the warning message with JSON output as that would make + // the JSON output invalid. + if (!options['app-vulns'] && !options['json']) { + console.log(theme.color.status.warn(appVulnsReleaseWarningMsg)); + } } const ecosystem = getEcosystemForTest(options);