From 26855a2a9eb5534041b90b218683eb2eab09566b Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Mon, 28 Sep 2020 17:22:17 -0400 Subject: [PATCH] ignore apk xattr file checksum + remove log.Errorf error wraps (#192) Signed-off-by: Alex Goodman --- internal/file/zip_file_manifest.go | 2 +- internal/file/zip_file_traversal.go | 2 +- syft/cataloger/apkdb/parse_apk_db.go | 4 +- syft/cataloger/apkdb/parse_apk_db_test.go | 50 +++++++++++++++++++ .../apkdb/test-fixtures/extra-file-attributes | 22 ++++++++ syft/cataloger/common/generic_cataloger.go | 4 +- syft/cataloger/java/save_archive_to_tmp.go | 2 +- syft/pkg/apk_metadata.go | 6 ++- 8 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 syft/cataloger/apkdb/test-fixtures/extra-file-attributes diff --git a/internal/file/zip_file_manifest.go b/internal/file/zip_file_manifest.go index d479ad716..35a27f2d8 100644 --- a/internal/file/zip_file_manifest.go +++ b/internal/file/zip_file_manifest.go @@ -47,7 +47,7 @@ func NewZipFileManifest(archivePath string) (ZipFileManifest, error) { defer func() { err = zipReader.Close() if err != nil { - log.Errorf("unable to close zip archive (%s): %w", archivePath, err) + log.Errorf("unable to close zip archive (%s): %+v", archivePath, err) } }() diff --git a/internal/file/zip_file_traversal.go b/internal/file/zip_file_traversal.go index 7a016bd87..46f103ad0 100644 --- a/internal/file/zip_file_traversal.go +++ b/internal/file/zip_file_traversal.go @@ -43,7 +43,7 @@ func TraverseFilesInZip(archivePath string, visitor func(*zip.File) error, paths defer func() { err = zipReader.Close() if err != nil { - log.Errorf("unable to close zip archive (%s): %w", archivePath, err) + log.Errorf("unable to close zip archive (%s): %+v", archivePath, err) } }() diff --git a/syft/cataloger/apkdb/parse_apk_db.go b/syft/cataloger/apkdb/parse_apk_db.go index 999cff739..d11d34194 100644 --- a/syft/cataloger/apkdb/parse_apk_db.go +++ b/syft/cataloger/apkdb/parse_apk_db.go @@ -93,7 +93,7 @@ func parseApkDBEntry(reader io.Reader) (*pkg.ApkMetadata, error) { fileRecord = &files[len(files)-1] case "a": ownershipFields := strings.Split(value, ":") - if len(ownershipFields) != 3 { + if len(ownershipFields) < 3 { log.Errorf("unexpected APK ownership field: %q", value) continue } @@ -104,6 +104,8 @@ func parseApkDBEntry(reader io.Reader) (*pkg.ApkMetadata, error) { fileRecord.OwnerUID = ownershipFields[0] fileRecord.OwnerGUI = ownershipFields[1] fileRecord.Permissions = ownershipFields[2] + // note: there are more optional fields available that we are not capturing, e.g.: + // "0:0:755:Q1JaDEHQHBbizhEzoWK1YxuraNU/4=" case "Z": if fileRecord == nil { log.Errorf("checksum field with no parent record: %q", value) diff --git a/syft/cataloger/apkdb/parse_apk_db_test.go b/syft/cataloger/apkdb/parse_apk_db_test.go index 18068e1ee..3b22cc11d 100644 --- a/syft/cataloger/apkdb/parse_apk_db_test.go +++ b/syft/cataloger/apkdb/parse_apk_db_test.go @@ -10,6 +10,56 @@ import ( "github.com/anchore/syft/syft/pkg" ) +func TestExtraFileAttributes(t *testing.T) { + tests := []struct { + name string + expected pkg.ApkMetadata + }{ + { + name: "test extra file attributes (checksum) are ignored", + expected: pkg.ApkMetadata{ + Files: []pkg.ApkFileRecord{ + { + Path: "/usr/lib/jvm/java-1.8-openjdk/bin/policytool", + OwnerUID: "0", + OwnerGUI: "0", + Permissions: "755", + Checksum: "Q1M0C9qfC/+kdRiOodeihG2GMRtkE=", + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + file, err := os.Open("test-fixtures/extra-file-attributes") + if err != nil { + t.Fatal("Unable to read test-fixtures/extra-file-attributes: ", err) + } + defer func() { + err := file.Close() + if err != nil { + t.Fatal("closing file failed:", err) + } + }() + + reader := bufio.NewReader(file) + + entry, err := parseApkDBEntry(reader) + if err != nil { + t.Fatal("Unable to read file contents: ", err) + } + + if diff := deep.Equal(entry.Files, test.expected.Files); diff != nil { + for _, d := range diff { + t.Errorf("diff: %+v", d) + } + } + }) + } +} + func TestSinglePackage(t *testing.T) { tests := []struct { name string diff --git a/syft/cataloger/apkdb/test-fixtures/extra-file-attributes b/syft/cataloger/apkdb/test-fixtures/extra-file-attributes new file mode 100644 index 000000000..0c96d4e0e --- /dev/null +++ b/syft/cataloger/apkdb/test-fixtures/extra-file-attributes @@ -0,0 +1,22 @@ +P:openjdk8-jre +V:8.212.04-r0 +A:x86_64 +S:359029 +I:970752 +T:OpenJDK 8 Java Runtime +U:https://icedtea.classpath.org/ +L:custom +o:openjdk8 +m:Timo Teras +t:1556993127 +c:46badd1aabd3375ae4b41c7194bc448f74b2ea4a +D:java-cacerts nss so:libX11.so.6 so:libXcomposite.so.1 so:libXext.so.6 so:libXi.so.6 so:libXrender.so.1 so:libXtst.so.6 so:libasound.so.2 so:libc.musl-x86_64.so.1 so:libfreetype.so.6 so:libgcc_s.so.1 so:libgif.so.7 so:libjpeg.so.8 so:libpng16.so.16 so:libstdc++.so.6 so:openjdk8:libawt.so so:openjdk8:libjava.so so:openjdk8:libjli.so so:openjdk8:libjvm.so +p:so:openjdk8:libawt_xawt.so=0 so:openjdk8:libfontmanager.so=0 so:openjdk8:libjawt.so=0 so:openjdk8:libjsoundalsa.so=0 so:openjdk8:libsplashscreen.so=0 +F:usr +F:usr/lib +F:usr/lib/jvm +F:usr/lib/jvm/java-1.8-openjdk +F:usr/lib/jvm/java-1.8-openjdk/bin +R:policytool +a:0:0:755:Q1JaDEHQHBbizhEzoWK1YxuraNU/4= +Z:Q1M0C9qfC/+kdRiOodeihG2GMRtkE= diff --git a/syft/cataloger/common/generic_cataloger.go b/syft/cataloger/common/generic_cataloger.go index 0aacedf3a..69b2a7fef 100644 --- a/syft/cataloger/common/generic_cataloger.go +++ b/syft/cataloger/common/generic_cataloger.go @@ -51,7 +51,7 @@ func (a *GenericCataloger) SelectFiles(resolver scope.FileResolver) []file.Refer for path, parser := range a.pathParsers { files, err := resolver.FilesByPath(file.Path(path)) if err != nil { - log.Errorf("cataloger failed to select files by path: %w", err) + log.Errorf("cataloger failed to select files by path: %+v", err) } if files != nil { a.register(files, parser) @@ -88,7 +88,7 @@ func (a *GenericCataloger) Catalog(contents map[file.Reference]string, upstreamM entries, err := parser(string(reference.Path), strings.NewReader(content)) if err != nil { // TODO: should we fail? or only log? - log.Errorf("cataloger '%s' failed to parse entries (reference=%+v): %w", upstreamMatcher, reference, err) + log.Errorf("cataloger '%s' failed to parse entries (reference=%+v): %+v", upstreamMatcher, reference, err) continue } diff --git a/syft/cataloger/java/save_archive_to_tmp.go b/syft/cataloger/java/save_archive_to_tmp.go index 2400e5a7d..699bdf84c 100644 --- a/syft/cataloger/java/save_archive_to_tmp.go +++ b/syft/cataloger/java/save_archive_to_tmp.go @@ -19,7 +19,7 @@ func saveArchiveToTmp(reader io.Reader) (string, string, func(), error) { cleanupFn := func() { err = os.RemoveAll(tempDir) if err != nil { - log.Errorf("unable to cleanup jar tempdir: %w", err) + log.Errorf("unable to cleanup jar tempdir: %+v", err) } } diff --git a/syft/pkg/apk_metadata.go b/syft/pkg/apk_metadata.go index 1acbca244..a77f2f764 100644 --- a/syft/pkg/apk_metadata.go +++ b/syft/pkg/apk_metadata.go @@ -4,7 +4,11 @@ import ( "github.com/package-url/packageurl-go" ) -// ApkMetadata represents all captured data for a Alpine DB package entry. See https://wiki.alpinelinux.org/wiki/Apk_spec for more information. +// ApkMetadata represents all captured data for a Alpine DB package entry. +// See the following sources for more information: +// - https://wiki.alpinelinux.org/wiki/Apk_spec +// - https://git.alpinelinux.org/apk-tools/tree/src/package.c +// - https://git.alpinelinux.org/apk-tools/tree/src/database.c type ApkMetadata struct { Package string `mapstructure:"P" json:"package"` OriginPackage string `mapstructure:"o" json:"originPackage"`