feat(echo): Add Echo Support #528
Conversation
|
|
…rity#501) Co-authored-by: Teppei Fukuda <[email protected]>
…ecurity#503) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…e elsa ids for aqua storage,remove them before saving into oss db (aquasecurity#484) Co-authored-by: DmitriyLewen <[email protected]>
…ty#526) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: DmitriyLewen <[email protected]>
pkg/vulnsrc/echo/echo.go
Outdated
| echoDir = "echo" | ||
| distroName = "echo" |
There was a problem hiding this comment.
I think we can use distroName as dir name to avoid creating two constants with the same values.
pkg/vulnsrc/echo/echo.go
Outdated
| if err := json.NewDecoder(f).Decode(&advisory); err != nil { | ||
| return eb.With("file_path", filePath).Wrapf(err, "json decode error") | ||
| } | ||
| advisoryMap[pkgName] = advisory |
There was a problem hiding this comment.
You'll still check the advisory in another cycle later.
Perhaps we can save package here?
There was a problem hiding this comment.
@DmitriyLewen
I'm not sure I understand, you mean to save the package vulnerabilities separately for each package?
There was a problem hiding this comment.
Never mind. I looked at my idea again and I think your solution is good.
pkg/vulnsrc/echo/echo.go
Outdated
| } | ||
| } | ||
|
|
||
| ids := strings.Fields(cveID) |
There was a problem hiding this comment.
can there be several CVEs here?
There was a problem hiding this comment.
Nope, only one. Fixed.
There was a problem hiding this comment.
Can you also add test for Get fucntion?
pkg/vulnsrc/echo/echo.go
Outdated
| if err != nil { | ||
| return eb.With("file_path", filePath).Wrapf(err, "failed to open file") | ||
| } | ||
| defer f.Close() |
There was a problem hiding this comment.
I suggest small refactoring to avoid using defer in loop:
diff --git a/pkg/vulnsrc/echo/echo.go b/pkg/vulnsrc/echo/echo.go
index c103ac3..ddb109b 100644
--- a/pkg/vulnsrc/echo/echo.go
+++ b/pkg/vulnsrc/echo/echo.go
@@ -63,17 +63,9 @@ func (vs VulnSrc) Update(dir string) error {
continue
}
- filePath := filepath.Join(rootDir, entry.Name())
- f, err := os.Open(filePath)
+ pkgName, advisory, err := vs.readAdvisory(rootDir, entry.Name())
if err != nil {
- return eb.With("file_path", filePath).Wrapf(err, "failed to open file")
- }
- defer f.Close()
-
- pkgName := strings.TrimSuffix(entry.Name(), filepath.Ext(entry.Name()))
- var advisory Advisory
- if err := json.NewDecoder(f).Decode(&advisory); err != nil {
- return eb.With("file_path", filePath).Wrapf(err, "json decode error")
+ return eb.With("file_name", entry.Name()).Wrapf(err, "failed to read advisory")
}
advisoryMap[pkgName] = advisory
}
@@ -85,6 +77,23 @@ func (vs VulnSrc) Update(dir string) error {
return nil
}
+func (vs VulnSrc) readAdvisory(rootDir, fileName string) (string, Advisory, error) {
+ filePath := filepath.Join(rootDir, fileName)
+ f, err := os.Open(filePath)
+ if err != nil {
+ return "", Advisory{}, oops.Wrapf(err, "failed to open file")
+ }
+ defer f.Close()
+
+ pkgName := strings.TrimSuffix(fileName, filepath.Ext(fileName))
+ var advisory Advisory
+ if err := json.NewDecoder(f).Decode(&advisory); err != nil {
+ return "", Advisory{}, oops.Wrapf(err, "json decode error")
+ }
+
+ return pkgName, advisory, nil
+}
+
func (vs VulnSrc) save(advisoryMap map[string]Advisory) error {
err := vs.dbc.BatchUpdate(func(tx *bolt.Tx) error {
if err := vs.dbc.PutDataSource(tx, distroName, source); err != nil {
|
@DmitriyLewen I went through all of the comments |
pkg/vulnsrc/echo/echo_test.go
Outdated
| } | ||
| } | ||
|
|
||
| func Test_readPackageAdvisory(t *testing.T) { |
There was a problem hiding this comment.
I think this is too much :)
There was a problem hiding this comment.
Haha okay, I removed it.
|
and one more ask: To show that new trivy-db contains |
|
@DmitriyLewen I see now that the vuln-list-update pr was reverted, so vuln-list doesn't contain the Echo advisory |
|
workflow should take |
|
@DmitriyLewen It's running. Should I re-open the vuln-list-update pr? |
You may need to add credentials for DockerHub and ECR or remove the steps to login and download them (i do the similar for trivy-java-db - aquasecurity/trivy-java-db@c200179).
Don't need it now, we will write about it later |
|
I ran the workflow |
|
@orizerah I found the following case: Does it mean that all version are affected? |
|
@DmitriyLewen Umm no, it means it's fixed in all versions |
|
hmm.. this is a bit weird, but okay. |
In that case, we don't need to insert the record, right? This advisory will not detect any vulnerabilities. |
|
We can remove these rows from our published advisory instead. That way there is no need for special use cases in trivy-db. |
That would be great. |
|
@knqyf263 Done |
|
@DmitriyLewen Would you check if this PR works once vuln-list is updated? |
|
I just ran |
|
I can confirm that trivy-db build from new |
Adding support for Echo Images to Trivy
Vuln List Pull Request: vuln-list-update
Relevant Discussion: aquasecurity/trivy#8834