From b04c4e261696518e429bd95b608870fecd595bdf Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 17 Oct 2023 13:03:41 -0400 Subject: [PATCH 1/3] Add workaround for macOS archive xattrs This commit introduces a workaround for an issue with .tar archives created with libarchive on Darwin/macOS. When encountering a file that has extended attributes, such as the com.apple.provenance attribute, it will add a corresponding AppleDouble file with the prefix "._" in the same directory. Because these files have the same filename otherwise, the buf tool will see these when they correspond to proto files and subsequently fail to parse them as protobuf IDL. Due to the fact that libarchive is used by default with the version of tar that ships with macOS, and the provenance extended attribute is set by macOS under many conditions when SIP is enabled, archives with these files are likely to occur on macOS. .zip archives created by libarchive do not seem to have this issue. However, .zip archives created by Archive Utility.app have a very similar behavior when encountering extended attributes: it will place AppleDouble files in a separate directory tree under the MACOSX directory, but also with the "._" prefix. Since this can be handled by the same logic, this behavior is extended to ZIP files as well. Closes #2387. --- .../storage/storagearchive/storagearchive.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/private/pkg/storage/storagearchive/storagearchive.go b/private/pkg/storage/storagearchive/storagearchive.go index 3f78a2b1d5..26c8e1be98 100644 --- a/private/pkg/storage/storagearchive/storagearchive.go +++ b/private/pkg/storage/storagearchive/storagearchive.go @@ -21,7 +21,9 @@ import ( "errors" "fmt" "io" + "io/fs" "math" + "strings" "github.com/bufbuild/buf/private/pkg/normalpath" "github.com/bufbuild/buf/private/pkg/storage" @@ -109,6 +111,9 @@ func Untar( if tarHeader.Size < 0 { return fmt.Errorf("invalid size for tar file %s: %d", tarHeader.Name, tarHeader.Size) } + if isAppleExtendedAttributesFile(tarHeader.FileInfo()) { + continue + } path, ok, err := unmapArchivePath(tarHeader.Name, mapper, stripComponentCount) if err != nil { return err @@ -210,6 +215,9 @@ func Unzip( if !ok { continue } + if isAppleExtendedAttributesFile(zipFile.FileInfo()) { + continue + } if zipFile.FileInfo().Mode().IsRegular() { if err := copyZipFile(ctx, writeBucket, zipFile, path); err != nil { return err @@ -219,6 +227,18 @@ func Unzip( return nil } +func isAppleExtendedAttributesFile(fileInfo fs.FileInfo) bool { + // On macOS, .tar archives created with libarchive will contain additional + // files with a prefix of "._" if there are files with extended attributes + // and copyfile is enabled. + // Archive Utility.app has a similar behavior when creating .zip archives, + // except they are placed under a separate MACOSX directory tree. + // Here, both are handled by just ignoring all files with a "._" prefix. + // This is a reasonable compromise because protobuf IDL files with a "._" + // prefix are very unlikely to occur. + return strings.HasPrefix(fileInfo.Name(), "._") +} + func copyZipFile( ctx context.Context, writeBucket storage.WriteBucket, From f5839836b53b252e0fa15658c0a70b3ccfc7f8e9 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 17 Oct 2023 13:54:07 -0400 Subject: [PATCH 2/3] Update code comment with better clarification --- private/pkg/storage/storagearchive/storagearchive.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/private/pkg/storage/storagearchive/storagearchive.go b/private/pkg/storage/storagearchive/storagearchive.go index 26c8e1be98..509b93619b 100644 --- a/private/pkg/storage/storagearchive/storagearchive.go +++ b/private/pkg/storage/storagearchive/storagearchive.go @@ -234,8 +234,10 @@ func isAppleExtendedAttributesFile(fileInfo fs.FileInfo) bool { // Archive Utility.app has a similar behavior when creating .zip archives, // except they are placed under a separate MACOSX directory tree. // Here, both are handled by just ignoring all files with a "._" prefix. - // This is a reasonable compromise because protobuf IDL files with a "._" - // prefix are very unlikely to occur. + // This is a reasonable compromise because files that live in a Module + // (.proto files, configuration files such as buf.yaml, README files) are + // almost never prefixed with ._, and fixing this issue in this manner + // outweighs the slight incorrectness. return strings.HasPrefix(fileInfo.Name(), "._") } From cd265d6197c19f6bc7727c334ef800e7644c19c6 Mon Sep 17 00:00:00 2001 From: John Chadwick Date: Tue, 17 Oct 2023 13:56:24 -0400 Subject: [PATCH 3/3] Add changelog entry --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eae97d0e40..511e22c3a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ ## [Unreleased] -- No changes yet. +- Fix issue where `buf build` and other commands may fail when handling certain + archives created on macOS that contain files with extended attributes. ## [v1.27.1] - 2023-10-16