fix: container sarif replace colon within location uri

When using sarif output and the Dockerfile is not specified, the path is used as the sarif results location uri. But the path can contain colon characters (e.g. between repo and tag) and GitHub Code Scanning seems to return an error in this case. This fix replaces the colon characters from the sarif results location uri with underscore characters. Issue: LUM-257
snyk · Jun 20, 2023 · a0c29cf · a0c29cf
1 parent b965894
commit a0c29cf
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 76 deletions.
diff --git a/src/lib/formatters/get-sarif-result.ts b/src/lib/formatters/get-sarif-result.ts
@@ -18,7 +18,10 @@ export function getResults(testResult): sarif.Result[] {
         {
           physicalLocation: {
             artifactLocation: {
-              uri: testResult.displayTargetFile || testResult.path,
+              uri: getArtifactLocationUri(
+                testResult.displayTargetFile,
+                testResult.path,
+              ),
             },
             region: {
               startLine: vuln.lineNumber || 1,
@@ -42,3 +45,15 @@ export function getLevel(vuln: AnnotatedIssue) {
       return 'note';
   }
 }
+
+function getArtifactLocationUri(targetFile: string, path: string): string {
+  if (targetFile) {
+    return targetFile;
+  }
+
+  // For container tests there might be cases when the target file (i.e. Dockerfile passed with the --file flag) is not
+  // present. In this case we use the test result path which contains the image reference (e.g. alpine:3.18.0).
+  // Also, Github Code Scanning returns an error when the artifact location uri from the uploaded sarif file contains
+  // a colon (e.g. alpine:3.18.0 is not valid, but alpine_3.18.0 is valid), so we are replacing colon characters.
+  return path.replace(/:/g, '_');
+}
diff --git a/test/jest/unit/lib/formatters/get-sarif-result.spec.ts b/test/jest/unit/lib/formatters/get-sarif-result.spec.ts
@@ -2,89 +2,48 @@ import { getResults } from '../../../../../src/lib/formatters/get-sarif-result';
 import { SEVERITY, TestResult } from '../../../../../src/lib/snyk-test/legacy';
 
 describe('Retrieving sarif result', () => {
-  it('should use the test results path as the location uri when target file is not present', () => {
-    let result = getResults(
-      getTestResult({
-        path: 'alpine:3.18.0',
-      }),
-    );
-    expect(result).toEqual([
+  const cases: Array<[
+    string,
+    { path: string; displayTargetFile?: string },
+    { resultLocationUri: string },
+  ]> = [
+    [
+      'should return the path given there is no target file present',
+      { path: 'alpine' },
+      { resultLocationUri: 'alpine' },
+    ],
+    [
+      'should return the path without colon characters given there is no target file present and the path contains a tag',
+      { path: 'alpine:3.18.0' },
+      { resultLocationUri: 'alpine_3.18.0' },
+    ],
+    [
+      'should return the path without colon characters given there is no target file present and the path contains a digest',
       {
-        ruleId: 'SNYK-LINUX-EXPAT-450908',
-        level: 'error',
-        message: {
-          text:
-            'This file introduces a vulnerable expat package with a critical severity vulnerability.',
-        },
-        locations: [
-          {
-            physicalLocation: {
-              artifactLocation: { uri: 'alpine:3.18.0' },
-              region: { startLine: 1 },
-            },
-          },
-        ],
+        path:
+          'alpine@sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda',
       },
-    ]);
-
-    result = getResults(
-      getTestResult({
-        path: 'alpine:3.18.0',
-        displayTargetFile: undefined,
-      }),
-    );
-    expect(result).toEqual([
       {
-        ruleId: 'SNYK-LINUX-EXPAT-450908',
-        level: 'error',
-        message: {
-          text:
-            'This file introduces a vulnerable expat package with a critical severity vulnerability.',
-        },
-        locations: [
-          {
-            physicalLocation: {
-              artifactLocation: { uri: 'alpine:3.18.0' },
-              region: { startLine: 1 },
-            },
-          },
-        ],
+        resultLocationUri:
+          'alpine@sha256_c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda',
       },
-    ]);
+    ],
+    [
+      'should return the target file given there is a target file present',
+      { path: 'alpine', displayTargetFile: 'Dockerfile.test' },
+      { resultLocationUri: 'Dockerfile.test' },
+    ],
+  ];
 
-    result = getResults(
+  it.each(cases)('%s', (_, input, want) => {
+    const result = getResults(
       getTestResult({
-        path: 'alpine:3.18.0',
-        displayTargetFile: null,
+        displayTargetFile: input.displayTargetFile,
+        path: input.path,
       }),
     );
-    expect(result).toEqual([
-      {
-        ruleId: 'SNYK-LINUX-EXPAT-450908',
-        level: 'error',
-        message: {
-          text:
-            'This file introduces a vulnerable expat package with a critical severity vulnerability.',
-        },
-        locations: [
-          {
-            physicalLocation: {
-              artifactLocation: { uri: 'alpine:3.18.0' },
-              region: { startLine: 1 },
-            },
-          },
-        ],
-      },
-    ]);
-  });
 
-  it('should use the target file as the location uri when target file is present', () => {
-    const actualResult = getResults(
-      getTestResult({
-        displayTargetFile: 'Dockerfile.test',
-      }),
-    );
-    expect(actualResult).toEqual([
+    expect(result).toEqual([
       {
         ruleId: 'SNYK-LINUX-EXPAT-450908',
         level: 'error',
@@ -95,7 +54,7 @@ describe('Retrieving sarif result', () => {
         locations: [
           {
             physicalLocation: {
-              artifactLocation: { uri: 'Dockerfile.test' },
+              artifactLocation: { uri: want.resultLocationUri },
               region: { startLine: 1 },
             },
           },