Merge pull request #1550 from snyk/fix/iac-sarif-output-CC-525

fix: Include all results under a single run

src/cli/commands/test/iac-output.ts

@@ -120,11 +120,11 @@ export function getIacDisplayErrorFileOutput(
 -------------------------------------------------------
 
 Testing ${fileName}...
-  
+
 ${iacFileResult.failureReason}`;
 }
 
-export function capitalizePackageManager(type) {
+export function capitalizePackageManager(type: string | undefined) {
   switch (type) {
     case 'k8sconfig': {
       return 'Kubernetes';
@@ -141,24 +141,58 @@ export function capitalizePackageManager(type) {
   }
 }
 
+type ResponseIssues = { issue: AnnotatedIacIssue; targetPath: string }[];
+
+// Used to reference the base path in results.
+const PROJECT_ROOT_KEY = 'PROJECTROOT';
+
 export function createSarifOutputForIac(
   iacTestResponses: IacTestResponse[],
 ): sarif.Log {
-  const sarifRes: sarif.Log = {
-    version: '2.1.0',
-    runs: [],
-  };
+  const issues = iacTestResponses.reduce((collect: ResponseIssues, res) => {
+    if (res.result) {
+      // FIXME: For single file tests the targetFile includes the full file
+      // path, for directory tests only the filename is returned and we need
+      // too build the path manually.
+      const targetPath = res.targetFile.startsWith(res.path)
+        ? pathLib.join(res.targetFile)
+        : pathLib.join(res.path, res.targetFile);
+      const mapped = res.result.cloudConfigResults.map((issue) => ({
+        issue,
+        targetPath,
+      }));
+      collect.push(...mapped);
+    }
+    return collect;
+  }, []);
 
-  iacTestResponses
-    .filter((iacTestResponse) => iacTestResponse.result?.cloudConfigResults)
-    .forEach((iacTestResponse) => {
-      sarifRes.runs.push({
-        tool: mapIacTestResponseToSarifTool(iacTestResponse),
-        results: mapIacTestResponseToSarifResults(iacTestResponse),
-      });
-    });
+  const tool: sarif.Tool = {
+    driver: {
+      name: 'Snyk Infrastructure as Code',
+      rules: extractReportingDescriptor(issues),
+    },
+  };
+  return {
+    version: '2.1.0',
+    runs: [
+      {
+        // https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317498
+        originalUriBaseIds: {
+          [PROJECT_ROOT_KEY]: {
+            // The base path is the current working directory.
+            // See: https://github.com/snyk/snyk/blob/6408c730c88902f0f6a00e732ee83c812903f240/src/lib/iac/detect-iac.ts#L94
+            uri: 'file://' + pathLib.join(pathLib.resolve('.'), '/'),
+            description: {
+              text: 'The root directory for all project files.',
+            },
+          },
+        },
 
-  return sarifRes;
+        tool,
+        results: mapIacTestResponseToSarifResults(issues),
+      },
+    ],
+  };
 }
 
 function getIssueLevel(severity: SEVERITY): sarif.ReportingConfiguration.level {
@@ -170,72 +204,63 @@ const iacTypeToText = {
   terraform: 'Terraform',
 };
 
-export function mapIacTestResponseToSarifTool(
-  iacTestResponse: IacTestResponse,
-): sarif.Tool {
-  const tool: sarif.Tool = {
-    driver: {
-      name: 'Snyk Infrastructure as Code',
-      rules: [],
-    },
-  };
+export function extractReportingDescriptor(
+  results: ResponseIssues,
+): sarif.ReportingDescriptor[] {
+  const tool: Record<string, sarif.ReportingDescriptor> = {};
 
-  const pushedIds = {};
-  iacTestResponse.result.cloudConfigResults.forEach(
-    (iacIssue: AnnotatedIacIssue) => {
-      if (pushedIds[iacIssue.id]) {
-        return;
-      }
-      tool.driver.rules?.push({
-        id: iacIssue.id,
-        shortDescription: {
-          text: `${upperFirst(iacIssue.severity)} severity - ${iacIssue.title}`,
-        },
-        fullDescription: {
-          text: `${iacTypeToText[iacIssue.type]} ${iacIssue.subType}`,
-        },
-        help: {
-          text: '',
-          markdown: iacIssue.description,
-        },
-        defaultConfiguration: {
-          level: getIssueLevel(iacIssue.severity),
-        },
-        properties: {
-          tags: ['security', `${iacIssue.type}/${iacIssue.subType}`],
-        },
-      });
-      pushedIds[iacIssue.id] = true;
-    },
-  );
-  return tool;
+  results.forEach(({ issue }) => {
+    if (tool[issue.id]) {
+      return;
+    }
+    tool[issue.id] = {
+      id: issue.id,
+      shortDescription: {
+        text: `${upperFirst(issue.severity)} severity - ${issue.title}`,
+      },
+      fullDescription: {
+        text: `${iacTypeToText[issue.type]} ${issue.subType}`,
+      },
+      help: {
+        text: '',
+        markdown: issue.description,
+      },
+      defaultConfiguration: {
+        level: getIssueLevel(issue.severity),
+      },
+      properties: {
+        tags: ['security', `${issue.type}/${issue.subType}`],
+      },
+    };
+  });
+
+  return Object.values(tool);
 }
 
 export function mapIacTestResponseToSarifResults(
-  iacTestResponse: IacTestResponse,
+  issues: ResponseIssues,
 ): sarif.Result[] {
-  return iacTestResponse.result.cloudConfigResults.map(
-    (iacIssue: AnnotatedIacIssue) => ({
-      ruleId: iacIssue.id,
-      message: {
-        text: `This line contains a potential ${
-          iacIssue.severity
-        } severity misconfiguration affecting the ${
-          iacTypeToText[iacIssue.type]
-        } ${iacIssue.subType}`,
-      },
-      locations: [
-        {
-          physicalLocation: {
-            artifactLocation: {
-              uri: iacTestResponse.targetFile,
-            },
-            region: {
-              startLine: iacIssue.lineNumber,
-            },
+  return issues.map(({ targetPath, issue }) => ({
+    ruleId: issue.id,
+    message: {
+      text: `This line contains a potential ${
+        issue.severity
+      } severity misconfiguration affecting the ${iacTypeToText[issue.type]} ${
+        issue.subType
+      }`,
+    },
+    locations: [
+      {
+        physicalLocation: {
+          artifactLocation: {
+            uri: targetPath,
+            uriBaseId: PROJECT_ROOT_KEY,
+          },
+          region: {
+            startLine: issue.lineNumber,
           },
         },
-      ],
-    }),
-  );
+      },
+    ],
+  }));
 }

src/lib/snyk-test/iac-test-result.ts

@@ -19,6 +19,7 @@ export interface AnnotatedIacIssue {
 type FILTERED_OUT_FIELDS = 'cloudConfigPath' | 'name' | 'from';
 
 export interface IacTestResponse extends BasicResultData {
+  path: string;
   targetFile: string;
   projectName: string;
   displayTargetFile: string; // used for display only

test/acceptance/cli-test/iac/cli-test.iac-dir.spec.ts

@@ -1,4 +1,3 @@
-import * as _ from 'lodash';
 import {
   iacTest,
   iacTestJson,

test/acceptance/cli-test/iac/cli-test.iac-utils.ts

@@ -266,6 +266,11 @@ export function iacTestSarifAssertions(
 
     const messageText = `This line contains a potential ${expectedIssue.severity} severity misconfiguration affecting the Kubernetes ${expectedIssue.subType}`;
     t.deepEqual(sarifIssue.message.text, messageText, 'issue message is ok');
+    t.deepEqual(
+      sarifIssue.locations![0]!.physicalLocation!.artifactLocation!.uri,
+      'iac-kubernetes/multi-file.yaml',
+      'issue uri is ok',
+    );
     t.deepEqual(
       sarifIssue.locations![0].physicalLocation!.region!.startLine,
       expectedIssue.lineNumber,
@@ -301,6 +306,7 @@ function generateDummyTestData(
   cloudConfigResults: Array<AnnotatedIacIssue>,
 ): IacTestResponse {
   return {
+    path: '',
     targetFile: '',
     projectName: '',
     displayTargetFile: '',