Skip to content

Commit 7e422c0

Browse files
authored
Merge pull request #4670 from snyk/fix/container-sarif-replace-colon-within-location-uri
fix: container sarif replace colon within location uri
2 parents 26a4247 + 6b3e046 commit 7e422c0

File tree

2 files changed

+50
-76
lines changed

2 files changed

+50
-76
lines changed

src/lib/formatters/get-sarif-result.ts

+16-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ export function getResults(testResult): sarif.Result[] {
1818
{
1919
physicalLocation: {
2020
artifactLocation: {
21-
uri: testResult.displayTargetFile || testResult.path,
21+
uri: getArtifactLocationUri(
22+
testResult.displayTargetFile,
23+
testResult.path,
24+
),
2225
},
2326
region: {
2427
startLine: vuln.lineNumber || 1,
@@ -42,3 +45,15 @@ export function getLevel(vuln: AnnotatedIssue) {
4245
return 'note';
4346
}
4447
}
48+
49+
function getArtifactLocationUri(targetFile: string, path: string): string {
50+
if (targetFile) {
51+
return targetFile;
52+
}
53+
54+
// For container tests there might be cases when the target file (i.e. Dockerfile passed with the --file flag) is not
55+
// present. In this case we use the test result path which contains the image reference (e.g. alpine:3.18.0).
56+
// Also, Github Code Scanning returns an error when the artifact location uri from the uploaded sarif file contains
57+
// a colon (e.g. alpine:3.18.0 is not valid, but alpine_3.18.0 is valid), so we are replacing colon characters.
58+
return path.replace(/:/g, '_');
59+
}

test/jest/unit/lib/formatters/get-sarif-result.spec.ts

+34-75
Original file line numberDiff line numberDiff line change
@@ -2,89 +2,48 @@ import { getResults } from '../../../../../src/lib/formatters/get-sarif-result';
22
import { SEVERITY, TestResult } from '../../../../../src/lib/snyk-test/legacy';
33

44
describe('Retrieving sarif result', () => {
5-
it('should use the test results path as the location uri when target file is not present', () => {
6-
let result = getResults(
7-
getTestResult({
8-
path: 'alpine:3.18.0',
9-
}),
10-
);
11-
expect(result).toEqual([
5+
const cases: Array<[
6+
string,
7+
{ path: string; displayTargetFile?: string },
8+
{ resultLocationUri: string },
9+
]> = [
10+
[
11+
'should return the path given there is no target file present',
12+
{ path: 'alpine' },
13+
{ resultLocationUri: 'alpine' },
14+
],
15+
[
16+
'should return the path without colon characters given there is no target file present and the path contains a tag',
17+
{ path: 'alpine:3.18.0' },
18+
{ resultLocationUri: 'alpine_3.18.0' },
19+
],
20+
[
21+
'should return the path without colon characters given there is no target file present and the path contains a digest',
1222
{
13-
ruleId: 'SNYK-LINUX-EXPAT-450908',
14-
level: 'error',
15-
message: {
16-
text:
17-
'This file introduces a vulnerable expat package with a critical severity vulnerability.',
18-
},
19-
locations: [
20-
{
21-
physicalLocation: {
22-
artifactLocation: { uri: 'alpine:3.18.0' },
23-
region: { startLine: 1 },
24-
},
25-
},
26-
],
23+
path:
24+
'alpine@sha256:c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda',
2725
},
28-
]);
29-
30-
result = getResults(
31-
getTestResult({
32-
path: 'alpine:3.18.0',
33-
displayTargetFile: undefined,
34-
}),
35-
);
36-
expect(result).toEqual([
3726
{
38-
ruleId: 'SNYK-LINUX-EXPAT-450908',
39-
level: 'error',
40-
message: {
41-
text:
42-
'This file introduces a vulnerable expat package with a critical severity vulnerability.',
43-
},
44-
locations: [
45-
{
46-
physicalLocation: {
47-
artifactLocation: { uri: 'alpine:3.18.0' },
48-
region: { startLine: 1 },
49-
},
50-
},
51-
],
27+
resultLocationUri:
28+
'alpine@sha256_c0669ef34cdc14332c0f1ab0c2c01acb91d96014b172f1a76f3a39e63d1f0bda',
5229
},
53-
]);
30+
],
31+
[
32+
'should return the target file given there is a target file present',
33+
{ path: 'alpine', displayTargetFile: 'Dockerfile.test' },
34+
{ resultLocationUri: 'Dockerfile.test' },
35+
],
36+
];
5437

55-
result = getResults(
38+
it.each(cases)('%s', (_, input, want) => {
39+
const result = getResults(
5640
getTestResult({
57-
path: 'alpine:3.18.0',
58-
displayTargetFile: null,
41+
displayTargetFile: input.displayTargetFile,
42+
path: input.path,
5943
}),
6044
);
61-
expect(result).toEqual([
62-
{
63-
ruleId: 'SNYK-LINUX-EXPAT-450908',
64-
level: 'error',
65-
message: {
66-
text:
67-
'This file introduces a vulnerable expat package with a critical severity vulnerability.',
68-
},
69-
locations: [
70-
{
71-
physicalLocation: {
72-
artifactLocation: { uri: 'alpine:3.18.0' },
73-
region: { startLine: 1 },
74-
},
75-
},
76-
],
77-
},
78-
]);
79-
});
8045

81-
it('should use the target file as the location uri when target file is present', () => {
82-
const actualResult = getResults(
83-
getTestResult({
84-
displayTargetFile: 'Dockerfile.test',
85-
}),
86-
);
87-
expect(actualResult).toEqual([
46+
expect(result).toEqual([
8847
{
8948
ruleId: 'SNYK-LINUX-EXPAT-450908',
9049
level: 'error',
@@ -95,7 +54,7 @@ describe('Retrieving sarif result', () => {
9554
locations: [
9655
{
9756
physicalLocation: {
98-
artifactLocation: { uri: 'Dockerfile.test' },
57+
artifactLocation: { uri: want.resultLocationUri },
9958
region: { startLine: 1 },
10059
},
10160
},

0 commit comments

Comments
 (0)