From 0ce8a470cac4cc347dbc0b767be9094f1ea68f26 Mon Sep 17 00:00:00 2001
From: Matthias Diester <matthias.diester@de.ibm.com>
Date: Fri, 25 Oct 2024 15:46:12 +0200
Subject: [PATCH] Fix issue with non-writable sub-directories

There are file system setups where there is a file in a directory, but
the directory does not have the write bit set (anymore). This kind of
setup is perfect fine, however, the bundle logic is unable to unpack it.

Fix unpack logic so that it is not setting the file permissions of
directories in the moment it creates the directory, but later when all
other files are unpacked already and no more write operations are
required.

Signed-off-by: Matthias Diester <matthias.diester@de.ibm.com>
---
 pkg/bundle/bundle.go      | 31 ++++++++++++++++++++++++++++++-
 pkg/bundle/bundle_test.go | 23 ++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go
index 1d14b3f9e4..65b39c58ab 100644
--- a/pkg/bundle/bundle.go
+++ b/pkg/bundle/bundle.go
@@ -231,12 +231,34 @@ func Pack(directory string) (io.ReadCloser, error) {
 // Unpack reads a tar stream and writes the content into the local file system
 // with all files and directories.
 func Unpack(in io.Reader, targetPath string) (*UnpackDetails, error) {
+	type chmod struct {
+		name string
+		mode os.FileMode
+	}
+
+	// Make sure the target path exists and is a directory
+	if stat, err := os.Stat(targetPath); err != nil {
+		if err := os.MkdirAll(targetPath, os.FileMode(0755)); err != nil {
+			return nil, err
+		}
+	} else if !stat.IsDir() {
+		return nil, fmt.Errorf("target %q exists, but it's not a directory", targetPath)
+	}
+
+	var chmods []chmod
 	var details = UnpackDetails{}
 	var tr = tar.NewReader(in)
 	for {
 		header, err := tr.Next()
 		switch {
 		case err == io.EOF:
+			// before leaving, make sure to set the file permissions to the ones specified in the tar stream
+			for _, chmod := range chmods {
+				if err := os.Chmod(chmod.name, chmod.mode); err != nil {
+					return nil, err
+				}
+			}
+
 			return &details, nil
 
 		case err != nil:
@@ -253,10 +275,17 @@ func Unpack(in io.Reader, targetPath string) (*UnpackDetails, error) {
 
 		switch header.Typeflag {
 		case tar.TypeDir:
-			if err := os.MkdirAll(target, fileMode(header)); err != nil {
+			// Skip the root directory, since it already exists
+			if target == targetPath {
+				continue
+			}
+
+			if err := os.MkdirAll(target, os.FileMode(0777)); err != nil {
 				return nil, err
 			}
 
+			chmods = append(chmods, chmod{name: target, mode: fileMode(header)})
+
 		case tar.TypeReg:
 			// Edge case in which that tarball did not have a directory entry
 			dir, _ := filepath.Split(target)
diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go
index cd980ae5cb..e1f48ff445 100644
--- a/pkg/bundle/bundle_test.go
+++ b/pkg/bundle/bundle_test.go
@@ -55,8 +55,8 @@ var _ = Describe("Bundle", func() {
 				Expect(r).ToNot(BeNil())
 
 				details, err := Unpack(r, tempDir)
-				Expect(details).ToNot(BeNil())
 				Expect(err).ToNot(HaveOccurred())
+				Expect(details).ToNot(BeNil())
 
 				Expect(filepath.Join(tempDir, "README.md")).To(BeAnExistingFile())
 				Expect(filepath.Join(tempDir, ".someToolDir", "config.json")).ToNot(BeAnExistingFile())
@@ -64,6 +64,27 @@ var _ = Describe("Bundle", func() {
 				Expect(filepath.Join(tempDir, "linktofile")).To(BeAnExistingFile())
 			})
 		})
+
+		It("should pack and unpack a directory including all files even if a sub-directory is non-writable", func() {
+			withTempDir(func(source string) {
+				// Setup a case where there are files in a directory, which is non-writable
+				Expect(os.Mkdir(filepath.Join(source, "some-dir"), os.FileMode(0777))).To(Succeed())
+				Expect(os.WriteFile(filepath.Join(source, "some-dir", "some-file"), []byte(`foobar`), os.FileMode(0644))).To(Succeed())
+				Expect(os.Chmod(filepath.Join(source, "some-dir"), os.FileMode(0555))).To(Succeed())
+
+				r, err := Pack(source)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(r).ToNot(BeNil())
+
+				withTempDir(func(target string) {
+					details, err := Unpack(r, target)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(details).ToNot(BeNil())
+
+					Expect(filepath.Join(target, "some-dir", "some-file")).To(BeAnExistingFile())
+				})
+			})
+		})
 	})
 
 	Context("packing/pushing and pulling/unpacking", func() {