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.

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

scan image with --list-all-pkgs flag has missing os installed files with in the report #5857

Closed
chen-keinan opened this issue Jan 2, 2024 · 5 comments · Fixed by #6849
Closed
Labels
bug priority/backlog Higher priority than priority/awaiting-more-evidence.
Milestone

Comments

@chen-keinan
Copy link
Contributor

when running a simple scan:

trivy image nginx --list-all-pkgs --format json

OS packages installed files are detected from each layer and apply it to map

Each layer has different installed files on packages.
Since the map key is the same var/lib/dpkg/status/type:ospkg for os packages then it get overwritten by the different layers

@chen-keinan chen-keinan added priority/backlog Higher priority than priority/awaiting-more-evidence. bug labels Jan 2, 2024
@knqyf263
Copy link
Collaborator

knqyf263 commented Jan 3, 2024

I'm still debugging this issue, but it looks like the installed files need to be handled similarly to licenses since dpkg has separate files for installed files, unlike apk and rpm.

// Extract dpkg licenses
// The license information is not stored in the dpkg database and in a separate file,
// so we have to merge the license information into the package.
dpkgLicenses := make(map[string][]string)
mergedLayer.Licenses = lo.Reject(mergedLayer.Licenses, func(license ftypes.LicenseFile, _ int) bool {
if license.Type != ftypes.LicenseTypeDpkg {
return false
}
// e.g.
// "adduser" => {"GPL-2"}
// "openssl" => {"MIT", "BSD"}
dpkgLicenses[license.PkgName] = lo.Map(license.Findings, func(finding ftypes.LicenseFinding, _ int) string {
return finding.Name
})
// Remove this license in the merged result as it is merged into the package information.
return true
})
if len(mergedLayer.Licenses) == 0 {
mergedLayer.Licenses = nil
}

@knqyf263
Copy link
Collaborator

knqyf263 commented May 8, 2024

I'm sorry I forgot to look into it.
@DmitriyLewen Could you take a look?

@DmitriyLewen
Copy link
Contributor

@knqyf263 I have researched this problem and have not found better solution than the one you suggested.
I created #6682 with a fix.

@knqyf263
Copy link
Collaborator

@DmitriyLewen Do you think we can polish the existing PR from Chen? Or should we create a new PR?

@DmitriyLewen
Copy link
Contributor

DmitriyLewen commented May 15, 2024

Chen and my PR are different.
Chen updated analyzers for Node package and Python packaging (though I didn’t understand why).
I also used different logic for applier (I created common map similar to the one for secrets).
Therefore, it was easier for me to create new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
Archived in project
3 participants