Skip to content

Commit

Permalink
feat: CLI ignores
Browse files Browse the repository at this point in the history
  • Loading branch information
Craig Furman committed Aug 6, 2021
1 parent f30b7f4 commit ac10cdd
Show file tree
Hide file tree
Showing 19 changed files with 1,154 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ const keys: (keyof IaCTestFlags)[] = [
'quiet',
'scan',
'legacy',

// PolicyOptions
'ignore-policy',
'policy-path',
];
const allowed = new Set<string>(keys);

Expand Down
12 changes: 9 additions & 3 deletions src/cli/commands/test/iac-local-execution/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from './types';
import { addIacAnalytics } from './analytics';
import { TestLimitReachedError } from './usage-tracking';
import { filterIgnoredIssues } from './policy';
import { TestResult } from '../../../../lib/snyk-test/legacy';
import {
initLocalCache,
Expand All @@ -24,6 +25,7 @@ import {
import { isFeatureFlagSupportedForOrg } from '../../../../lib/feature-flags';
import { FlagError } from './assert-iac-options-flag';
import config = require('../../../../lib/config');
import { findAndLoadPolicy } from '../../../../lib/policy/find-and-load-policy';

// this method executes the local processing engine and then formats the results to adapt with the CLI output.
// this flow is the default GA flow for IAC scanning.
Expand All @@ -38,6 +40,8 @@ export async function test(

await initLocalCache({ customRulesPath });

const policy = await findAndLoadPolicy(pathToScan, 'iac', options);

const filesToParse = await loadFiles(pathToScan, options);
const { parsedFiles, failedFiles } = await parseFiles(
filesToParse,
Expand Down Expand Up @@ -65,8 +69,10 @@ export async function test(
iacOrgSettings.meta,
);

const filteredResults = filterIgnoredIssues(policy, formattedResults);

try {
await trackUsage(formattedResults);
await trackUsage(filteredResults);
} catch (e) {
if (e instanceof TestLimitReachedError) {
throw e;
Expand All @@ -75,11 +81,11 @@ export async function test(
// run their tests by squashing the error.
}

addIacAnalytics(formattedResults);
addIacAnalytics(filteredResults);

// TODO: add support for proper typing of old TestResult interface.
return {
results: (formattedResults as unknown) as TestResult[],
results: (filteredResults as unknown) as TestResult[],
// NOTE: No file or parsed file data should leave this function.
failures: isLocalFolder(pathToScan)
? failedFiles.map(removeFileContent)
Expand Down
87 changes: 87 additions & 0 deletions src/cli/commands/test/iac-local-execution/policy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { FormattedResult, PolicyMetadata } from './types';
import { Policy } from '../../../../lib/policy/find-and-load-policy';

export function filterIgnoredIssues(
policy: Policy | undefined,
results: FormattedResult[],
): FormattedResult[] {
if (!policy) {
return results;
}
return results
.map((res) => policy.filter(toIaCVulnAdapter(res)))
.map((vuln) => toFormattedResult(vuln));
}

type IacVulnAdapter = {
vulnerabilities: {
id: string;
from: string[];
}[];
originalResult: FormattedResult;
};

// This is a total cop-out. The type I really want is AnnotatedIacIssue from
// src/lib/snyk-test/iac-test-result.ts, but that appears to only be used in the
// legacy flow and I gave up on adapting it to work in both flows. By the time
// this code is called, cloudConfigPath is present on the result object.
type AnnotatedResult = PolicyMetadata & {
cloudConfigPath: string[];
};

function toIaCVulnAdapter(result: FormattedResult): IacVulnAdapter {
return {
vulnerabilities: result.result.cloudConfigResults.map(
(cloudConfigResult) => {
const annotatedResult = cloudConfigResult as AnnotatedResult;

// Copy the cloudConfigPath array to avoid modifying the original with
// splice.
// Insert the targetFile into the path so that it is taken into account
// when determining whether an ignore rule should be applied.
// Insert garbage into the first element because the policy library
// ignores it.
const path = [...annotatedResult.cloudConfigPath];
path.splice(0, 0, 'GARBAGE', result.targetFile);

return {
id: cloudConfigResult.id,
from: path,
};
},
),
originalResult: result,
};
}

function toFormattedResult(adapter: IacVulnAdapter): FormattedResult {
const original = adapter.originalResult;
const filteredCloudConfigResults = original.result.cloudConfigResults.filter(
(res) => {
return adapter.vulnerabilities.some((vuln) => {
if (vuln.id !== res.id) {
return false;
}

// Unfortunately we are forced to duplicate the logic in
// toIaCVulnAdapter so that we're comparing path components properly,
// including target file context. As that logic changes, so must this.
const annotatedResult = res as AnnotatedResult;
const significantPath = [...annotatedResult.cloudConfigPath];
significantPath.splice(0, 0, 'GARBAGE', original.targetFile);

if (vuln.from.length !== significantPath.length) {
return false;
}
for (let i = 0; i < vuln.from.length; i++) {
if (vuln.from[i] !== significantPath[i]) {
return false;
}
}
return true;
});
},
);
original.result.cloudConfigResults = filteredCloudConfigResults;
return original;
}
7 changes: 6 additions & 1 deletion src/cli/commands/test/iac-local-execution/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
IacFileInDirectory,
Options,
TestOptions,
PolicyOptions,
} from '../../../../lib/types';

export interface IacFileData extends IacFileInDirectory {
Expand Down Expand Up @@ -117,7 +118,7 @@ export interface PolicyMetadata {
// Collection of all options supported by `iac test` command.
// TODO: Needs to be fixed at the args module level.
export type IaCTestFlags = Pick<
Options & TestOptions,
Options & TestOptions & PolicyOptions,
| 'org'
| 'insecure'
| 'debug'
Expand All @@ -126,6 +127,10 @@ export type IaCTestFlags = Pick<
| 'severityThreshold'
| 'json'
| 'sarif'

// PolicyOptions
| 'ignore-policy'
| 'policy-path'
> & {
// Supported flags not yet covered by Options or TestOptions
'json-file-output'?: string;
Expand Down
10 changes: 7 additions & 3 deletions src/lib/policy/find-and-load-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ const debug = debugModule('snyk');

export async function findAndLoadPolicy(
root: string,
scanType: SupportedPackageManagers | 'docker',
scanType: SupportedPackageManagers | 'docker' | 'iac',
options: PolicyOptions,
pkg?: PackageExpanded,
scannedProjectFolder?: string,
): Promise<string | undefined> {
): Promise<Policy | undefined> {
const isDocker = scanType === 'docker';
const isNodeProject = ['npm', 'yarn'].includes(scanType);
// monitor
Expand Down Expand Up @@ -43,9 +43,13 @@ export async function findAndLoadPolicy(
} catch (err) {
// note: inline catch, to handle error from .load
// if the .snyk file wasn't found, it is fine
if (err.code !== 'ENOENT') {
if (err.code !== 'ENOENT' && err.code !== 'ENOTDIR') {
throw err;
}
}
return policy;
}

export interface Policy {
filter(vulns: any): any;
}
4 changes: 2 additions & 2 deletions test/jest/acceptance/iac/custom-rules.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ describe('iac test --rules', () => {
);
expect(exitCode).toBe(1);

expect(stdout).toContain('Testing sg_open_ssh.tf');
expect(stdout).toContain('Testing ./iac/terraform/sg_open_ssh.tf');
expect(stdout).toContain('Infrastructure as code issues:');
expect(stdout).toContain('Missing tags');
expect(stdout).toContain('CUSTOM-123');
expect(stdout).toContain(
'introduced by resource > aws_security_group[allow_ssh] > tags',
'introduced by input > resource > aws_security_group[allow_ssh] > tags',
);
});

Expand Down

0 comments on commit ac10cdd

Please sign in to comment.