fix: Skip specific errors when parsing yaml IaC files

The new yaml parser library does some validations that we do not do in Policy Engine, so we return less results in the CLI. These are not critical errors, so we decided to skip them and return the parsed file for consistency with the Policy Engine.

src/cli/commands/test/iac-local-execution/file-parser.ts

@@ -23,6 +23,7 @@ import {
 import * as analytics from '../../../../lib/analytics';
 import { CustomError } from '../../../../lib/errors';
 import { getErrorStringCode } from './error-utils';
+import { shouldThrowErrorFor } from './file-utils';
 
 export async function parseFiles(
   filesData: IacFileData[],
@@ -69,7 +70,7 @@ function parseYAMLOrJSONFileData(fileData: IacFileData): any[] {
     // the YAML library can parse both YAML and JSON content, as well as content with singe/multiple YAMLs
     // by using this library we don't have to disambiguate between these different contents ourselves
     yamlDocuments = YAML.parseAllDocuments(fileData.fileContent).map((doc) => {
-      if (doc.errors.length !== 0) {
+      if (shouldThrowErrorFor(doc)) {
         throw doc.errors[0];
       }
       return doc.toJSON();

src/cli/commands/test/iac-local-execution/file-utils.ts

@@ -2,6 +2,7 @@ import * as fs from 'fs';
 import * as tar from 'tar';
 import * as path from 'path';
 import { FailedToInitLocalCacheError } from './local-cache';
+import * as YAML from 'yaml';
 
 export function createIacDir(): void {
   // this path will be able to be customised by the user in the future
@@ -29,3 +30,18 @@ export function extractBundle(response: NodeJS.ReadableStream): Promise<void> {
       .on('error', reject);
   });
 }
+
+const errorsToSkip = [
+  'Insufficient indentation in flow collection',
+  'Map keys must be unique',
+];
+
+// the YAML Parser is more strict than the Golang one in Policy Engine,
+// so we decided to skip specific errors in order to be consistent.
+// this function checks if the current error is one them
+export function shouldThrowErrorFor(doc: YAML.Document.Parsed) {
+  return (
+    doc.errors.length !== 0 &&
+    !errorsToSkip.some((e) => doc.errors[0].message.includes(e))
+  );
+}

src/lib/iac/iac-parser.ts

@@ -14,6 +14,7 @@ import {
   IacValidateTerraformResponse,
   IacValidationResponse,
 } from './constants';
+import { shouldThrowErrorFor } from '../../cli/commands/test/iac-local-execution/file-utils';
 
 const debug = debugLib('snyk-detect');
 
@@ -31,7 +32,7 @@ function parseYamlOrJson(fileContent: string, filePath: string): any {
     case 'yml':
       try {
         return YAML.parseAllDocuments(fileContent).map((doc) => {
-          if (doc.errors.length !== 0) {
+          if (shouldThrowErrorFor(doc)) {
             throw doc.errors[0];
           }
           return doc.toJSON();

test/jest/unit/iac-unit-tests/file-parser.fixtures.ts

@@ -83,6 +83,73 @@ const invalidYamlFile = `
 foo: "bar
 `;
 
+const semanticYamlWithDuplicateKeyFile = `
+apiVersion: v1
+kind: Pod
+metadata:
+  name: myapp-pod
+spec:
+  containers:
+  something: here
+metadata:
+  another: thing
+`;
+
+const semanticYamlErrorFileJSON = {
+  apiVersion: 'v1',
+  kind: 'Pod',
+  metadata: {
+    another: 'thing', //in a case of a duplicate key, the last one will be the one used when parsed
+  },
+  spec: {
+    containers: null,
+    something: 'here',
+  },
+};
+
+const yamlWithInsufficientIndentationFile = `
+Resources:
+ Denied:
+   Type: "AWS::IAM::Role"
+   Properties:
+     AssumeRolePolicyDocument: {
+ "Version": "2012-10-17",
+ "Statement": [
+   {
+     "Action": "sts:AssumeRole",
+     "Principal": {
+       "AWS": "arn:aws:iam::123456789012:root"
+     },
+     "Effect": "Allow",
+     "Sid": ""
+   }
+ ]
+}
+`;
+
+const yamlWithInsufficientIndentationFileJSON = {
+  Resources: {
+    Denied: {
+      Type: 'AWS::IAM::Role',
+      Properties: {
+        AssumeRolePolicyDocument: {
+          Version: '2012-10-17',
+          Statement: [
+            {
+              Action: 'sts:AssumeRole',
+              Principal: {
+                AWS: 'arn:aws:iam::123456789012:root',
+              },
+              Effect: 'Allow',
+              Sid: '',
+            },
+          ],
+        },
+      },
+    },
+  },
+};
+
 export const kubernetesYamlFileDataStub: IacFileData = {
   fileContent: kubernetesYamlFileContent,
   filePath: 'dont-care',
@@ -119,6 +186,33 @@ export const invalidYamlFileDataStub: IacFileData = {
   fileType: 'yml',
 };
 
+export const duplicateKeyYamlErrorFileDataStub: IacFileData = {
+  fileContent: semanticYamlWithDuplicateKeyFile,
+  filePath: 'dont-care',
+  fileType: 'yml',
+};
+
+export const expectedDuplicateKeyYamlErrorFileParsingResult: IacFileParsed = {
+  ...duplicateKeyYamlErrorFileDataStub,
+  docId: 0,
+  projectType: IacProjectType.K8S,
+  engineType: EngineType.Kubernetes,
+  jsonContent: semanticYamlErrorFileJSON,
+};
+
+export const insufficientIndentationYamlErrorFileDataStub: IacFileData = {
+  fileContent: yamlWithInsufficientIndentationFile,
+  filePath: 'dont-care',
+  fileType: 'yml',
+};
+export const expectedInsufficientIndentationYamlErrorFileParsingResult: IacFileParsed = {
+  ...insufficientIndentationYamlErrorFileDataStub,
+  docId: 0,
+  projectType: IacProjectType.CLOUDFORMATION,
+  engineType: EngineType.CloudFormation,
+  jsonContent: yamlWithInsufficientIndentationFileJSON,
+};
+
 export const expectedKubernetesYamlParsingResult: IacFileParsed = {
   ...kubernetesYamlFileDataStub,
   docId: 0,

test/jest/unit/iac-unit-tests/file-parser.spec.ts

@@ -27,6 +27,10 @@ import {
   expectedMultipleKubernetesYamlsParsingResult,
   invalidYamlFileDataStub,
   invalidJsonFileDataStub,
+  duplicateKeyYamlErrorFileDataStub,
+  expectedDuplicateKeyYamlErrorFileParsingResult,
+  expectedInsufficientIndentationYamlErrorFileParsingResult,
+  insufficientIndentationYamlErrorFileDataStub,
 } from './file-parser.fixtures';
 import { IacFileData } from '../../../../src/cli/commands/test/iac-local-execution/types';
 import { IacFileTypes } from '../../../../dist/lib/iac/constants';
@@ -119,12 +123,35 @@ describe('parseFiles', () => {
     );
   });
 
-  it('throws an error for invalid YAML file types', async () => {
+  it('throws an error for invalid (syntax) YAML file types', async () => {
     await expect(parseFiles([invalidYamlFileDataStub])).rejects.toThrow(
       InvalidYamlFileError,
     );
   });
 
+  // the npm yaml parser by default fails on SemanticErrors like duplicate keys
+  // but we decided to skip this error in order to be consistent with the Policy Engine
+  it.each([
+    [
+      {
+        fileStub: duplicateKeyYamlErrorFileDataStub,
+        expectedParsingResult: expectedDuplicateKeyYamlErrorFileParsingResult,
+      },
+    ],
+    [
+      {
+        fileStub: insufficientIndentationYamlErrorFileDataStub,
+        expectedParsingResult: expectedInsufficientIndentationYamlErrorFileParsingResult,
+      },
+    ],
+  ])(
+    `given an $fileStub with one of the errors to skip, it returns $expectedParsingResult`,
+    async ({ fileStub, expectedParsingResult }) => {
+      const { parsedFiles } = await parseFiles([fileStub]);
+      expect(parsedFiles[0]).toEqual(expectedParsingResult);
+    },
+  );
+
   it('throws an error for a Helm file', async () => {
     const helmFileData: IacFileData = {
       fileContent: ' {{ something }}',