Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(report): update gitlab template to populate operating_system value #7735

Merged
merged 5 commits into from
Oct 24, 2024
Merged

feat(report): update gitlab template to populate operating_system value #7735

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb88ee8
feat(report): update gitlab template to populate operating_system value
aarongoldenthal Oct 14, 2024
a35ddbc
feat(report): update gitlab template with default operating_system value
aarongoldenthal Oct 14, 2024
2536016
feat(report): update gitlab template to avoid using regex
aarongoldenthal Oct 18, 2024
4a5ad14
feat(report): update gitlab template to check for os packages to dete…
aarongoldenthal Oct 22, 2024
7c7c62a
feat(report): add test with gitlab template and unknown os and image
aarongoldenthal Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion contrib/gitlab.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
{{- range . }}
{{- $target := .Target }}
{{- $image := $target | regexFind "[^\\s]+" }}
{{- $os := "Unknown" }}
{{- if contains "(" $target -}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am worried that filepath might contain (:

trivy/pkg/scanner/langpkg/scan.go

Line 52 in 56dbe1f

Target: targetName(app.Type, app.FilePath),

Can Class field be used to detect OS package vulnerabilities?

trivy/pkg/scanner/ospkg/scan.go

Lines 40 to 41 in 56dbe1f

Target: fmt.Sprintf("%s (%s %s)", target.Name, target.OS.Family, target.OS.Name),
Class: types.ClassOSPkg,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My goal here was to replicate the data from GitLab's container scanning tool using Trivy directly. Within GitLab, the format is unique to container scanning (i.e. container images in a registry). In that use case, there's shouldn't be parentheses in the .Target value as they're invalid. If it's being used in other cases with file paths then parentheses could be an issue, but then spaces could also be an issue with line 31

{{- $image := $target | regexFind "[^\\s]+" }}`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Class question, GitLab actually displays the results, not just looking to identify OS packages, for example:
image

Copy link
Contributor

@DmitriyLewen DmitriyLewen Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's being used in other cases with file paths then parentheses could be an issue, but then spaces could also be an issue with line 31

Hm.. you are right.
We need to think about language-specific files from container.

I think we can check all targets to find os and image and use them for all targets.

Something like this:

  {{- $image := "Unknown" -}}
  {{- $os := "Unknown" -}}
  {{- range . }}
    {{- if eq .Class "os-pkgs" -}}
      {{- $target := .Target }}
        {{- $image = $target | regexFind "[^\\s]+" }}
        {{- $os = $target | splitList "(" | last | trimSuffix ")" }}
    {{- end }}
  {{- end }}
  "vulnerabilities": [
  {{- $t_first := true }}
  {{- range . }}
  {{- $target := .Target }}
    {{- range .Vulnerabilities -}}

wdyt?

test:

FROM alpine

COPY log4j-core-2.17.0.jar /te(st)/log4j-core-2.17.0.jar

Before:

➜ trivy -q image test:7735 -f template -t @./contrib/gitlab.tpl | grep "operating_system" -A 1
        "operating_system": "alpine 3.20.3",
        "image": "test:7735"
--
        "operating_system": "alpine 3.20.3",
        "image": "test:7735"
--
        "operating_system": "Unknown",
        "image": "Java"

After:

➜  trivy -q image test:7735 -f template -t @./contrib/gitlab.tpl | grep "operating_system" -A 1
        "operating_system": "alpine 3.20.3",
        "image": "test:7735"
--
        "operating_system": "alpine 3.20.3",
        "image": "test:7735"
--
        "operating_system": "alpine 3.20.3",
        "image": "test:7735"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DmitriyLewen I like it. I did a quick test and it is slightly different than what GitLab output, but I think it's more logical. By default they run with --vuln-type os, which I had forgotten, but after overriding they output "operating_system": "Unknown" for non-OS packages. There are some cases, for example I can think of some npm package examples, that have language-specific installations (installing binaries), so I think the OS is useful data.

At the moment there's one test for this template, which I had updated, but what are your thoughts on tests for the "Unknown" cases?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment there's one test for this template, which I had updated, but what are your thoughts on tests for the "Unknown" cases?

Hm... You can add test for testdata/fixtures/images/vulnimage.tar.gz (use low severity to skip a few vulns).

➜ trivy -q image  -f template -t @./contrib/gitlab.tpl  --input ./integration/testdata/fixtures/images/vulnimage.tar.gz --severity LOW | grep "operating_system" -A 1
        "operating_system": "alpine 3.7.1",
        "image": "./integration/testdata/fixtures/images/vulnimage.tar.gz"
--
        "operating_system": "alpine 3.7.1",
        "image": "./integration/testdata/fixtures/images/vulnimage.tar.gz"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually thinking the other direction - an image, maybe something FROM scratch, that would report "operating_system": "Unknown", just to test the defaults. I'm not sure it's critical, but didn't see an image in the existing testcases that looked like it would work. I think the image/os values are already covered in the test for integration/testdata/alpine-310.gitlab.golden.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have provided vulnimage.tar.gz to show that vulnerabilities for language packages now contain the correct operating_system and image values.

Actual behavior:

➜  trivy -q image  -f template -t @./contrib/gitlab.tpl  --input ./integration/testdata/fixtures/images/vulnimage.tar.gz --severity LOW | grep "operating_system" -A 1
        "operating_system": "Unknown",
        "image": "./integration/testdata/fixtures/images/vulnimage.tar.gz"
--
        "operating_system": "Unknown",
        "image": "rust-app/Cargo.lock"

that would report "operating_system": "Unknown", just to test the defaults

We can use fs mode (use one dir from repo to check this. In this case OS is not detected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test and the actions are all passing in my fork, so from my perspective I think this is ready.

{{- $os = regexReplaceAll ".+\\((.+)\\)" $target "${1}" }}
DmitriyLewen marked this conversation as resolved.
Show resolved Hide resolved
{{- end }}
{{- range .Vulnerabilities -}}
{{- if $t_first -}}
{{- $t_first = false -}}
Expand Down Expand Up @@ -65,7 +69,7 @@
"version": "{{ .InstalledVersion }}"
},
{{- /* TODO: No mapping available - https://github.com/aquasecurity/trivy/issues/332 */}}
"operating_system": "Unknown",
"operating_system": "{{ $os }}",
"image": "{{ $image }}"
},
"identifiers": [
Expand Down
8 changes: 4 additions & 4 deletions integration/testdata/alpine-310.gitlab.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
},
"version": "1.1.1c-r0"
},
"operating_system": "Unknown",
"operating_system": "alpine 3.10.2",
"image": "testdata/fixtures/images/alpine-310.tar.gz"
},
"identifiers": [
Expand Down Expand Up @@ -104,7 +104,7 @@
},
"version": "1.1.1c-r0"
},
"operating_system": "Unknown",
"operating_system": "alpine 3.10.2",
"image": "testdata/fixtures/images/alpine-310.tar.gz"
},
"identifiers": [
Expand Down Expand Up @@ -191,7 +191,7 @@
},
"version": "1.1.1c-r0"
},
"operating_system": "Unknown",
"operating_system": "alpine 3.10.2",
"image": "testdata/fixtures/images/alpine-310.tar.gz"
},
"identifiers": [
Expand Down Expand Up @@ -258,7 +258,7 @@
},
"version": "1.1.1c-r0"
},
"operating_system": "Unknown",
"operating_system": "alpine 3.10.2",
"image": "testdata/fixtures/images/alpine-310.tar.gz"
},
"identifiers": [
Expand Down