fix: container sarif replace colon within location uri #4670
Conversation
rmutuleanu
force-pushed
the
fix/container-sarif-replace-colon-within-location-uri
branch
2 times, most recently
from
e364a81
to
716ea88
Compare
| @@ -5,7 +5,7 @@ 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', | |||
| path: 'alpine', | |||
There was a problem hiding this comment.
I'm a bit confused, isn't this the input to the fix, and shouldn't that include the tag? doesn't it need to include the tag to get the below results? 🤔
There was a problem hiding this comment.
Yes, you are right, I've added 3 runs inside that test with different inputs each. I will split the test into 3 individual tests.
| uri: testResult.displayTargetFile || testResult.path, | ||
| uri: | ||
| testResult.displayTargetFile || | ||
| getArtifactLocationUriFromPath(testResult.path), |
There was a problem hiding this comment.
This path variable is weirdly named, I would think that an image name would rather be in displayTargetFile than in path....can we add a comment to explain that? 🤔
There was a problem hiding this comment.
The displayTargetFile is what we are passing to the --file flag (i.e. Dockerfile in our case)
The test result path seem to be the image reference used and with an optional project name appended in some cases (e.g. alpine, alpine:3.18.0, alpine/kubernetes-monitor, last one has the project name appended)
There was a problem hiding this comment.
I can add a comment to explain this
There was a problem hiding this comment.
Yeah I think that would be nice 😅
rmutuleanu
force-pushed
the
fix/container-sarif-replace-colon-within-location-uri
branch
2 times, most recently
from
351c74a
to
bb8c36c
Compare
| @@ -42,3 +42,15 @@ export function getLevel(vuln: AnnotatedIssue) { | |||
| return 'note'; | |||
| } | |||
| } | |||
|
|
|||
| function getArtifactLocationUri(targetFile: string, path: string): string { | |||
rmutuleanu
force-pushed
the
fix/container-sarif-replace-colon-within-location-uri
branch
from
bb8c36c
to
a0c29cf
Compare
rmutuleanu
force-pushed
the
fix/container-sarif-replace-colon-within-location-uri
branch
from
a0c29cf
to
20e6b97
Compare
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
rmutuleanu
force-pushed
the
fix/container-sarif-replace-colon-within-location-uri
branch
from
20e6b97
to
6b3e046
Compare
rmutuleanu
deleted the
fix/container-sarif-replace-colon-within-location-uri
branch
When using sarif as output and the Dockerfile is not specified, the
pathis used as the sarif results location uri (added with PR: #4649).But the
pathcan contain colon characters (e.g. between repo and tag) and GitHub Code Scanning seems to return an error in that case.What does this PR do?
Replaces the colon characters from the sarif results location uri with underscore characters.
Issue: LUM-257