fix: parent directory permissions

internal/deb/extract.go

@@ -6,6 +6,7 @@ import (
 	"compress/gzip"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"sort"
@@ -159,6 +160,13 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error {
 		}
 	}
 
+	// When creating a file we will iterate through its parent directories and
+	// create them with the permissions defined in the tarball.
+	//
+	// The assumption is that the tar entries of the parent directories appear
+	// before the entry for the file itself. This is the case for .deb files but
+	// not for all tarballs.
+	tarDirMode := make(map[string]fs.FileMode)
 	tarReader := tar.NewReader(dataReader)
 	for {
 		tarHeader, err := tarReader.Next()
@@ -180,6 +188,9 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error {
 		}
 
 		sourceIsDir := sourcePath[len(sourcePath)-1] == '/'
+		if sourceIsDir {
+			tarDirMode[sourcePath] = tarHeader.FileInfo().Mode()
+		}
 
 		//debugf("Extracting header: %#v", tarHeader)
 
@@ -189,23 +200,10 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error {
 			delete(pendingPaths, globPath)
 		} else {
 			extractInfos, ok = options.Extract[sourcePath]
-			if ok {
-				delete(pendingPaths, sourcePath)
-			} else {
-				// Base directory for extracted content. Relevant mainly to preserve
-				// the metadata, since the extracted content itself will also create
-				// any missing directories unaccounted for in the options.
-				createOptions := &fsutil.CreateOptions{
-					Path:        filepath.Join(options.TargetDir, sourcePath),
-					Mode:        tarHeader.FileInfo().Mode(),
-					MakeParents: true,
-				}
-				err := options.Create(nil, createOptions)
-				if err != nil {
-					return err
-				}
+			if !ok {
 				continue
 			}
+			delete(pendingPaths, sourcePath)
 		}
 
 		var contentCache []byte
@@ -228,17 +226,43 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error {
 			if contentIsCached {
 				pathReader = bytes.NewReader(contentCache)
 			}
-			var targetPath string
+			var relPath string
 			if globPath == "" {
-				targetPath = filepath.Join(options.TargetDir, extractInfo.Path)
+				relPath = extractInfo.Path
 			} else {
-				targetPath = filepath.Join(options.TargetDir, sourcePath)
+				relPath = sourcePath
 			}
 			if extractInfo.Mode != 0 {
 				tarHeader.Mode = int64(extractInfo.Mode)
 			}
+			// Create the parent directories using the permissions from the tarball.
+			parents := parentDirs(relPath)
+			for _, path := range parents {
+				if path == "/" {
+					continue
+				}
+				mode, ok := tarDirMode[path]
+				if !ok {
+					continue
+				}
+				delete(tarDirMode, path)
+
+				createOptions := &fsutil.CreateOptions{
+					Path:        filepath.Join(options.TargetDir, path),
+					Mode:        mode,
+					MakeParents: true,
+					// We dont want this implicit parent directory to overwrite
+					// an existing directory.
+					OverrideMode: false,
+				}
+				err := options.Create(nil, createOptions)
+				if err != nil {
+					return err
+				}
+			}
+			// Create the entry itself.
 			createOptions := &fsutil.CreateOptions{
-				Path:        targetPath,
+				Path:        filepath.Join(options.TargetDir, relPath),
 				Mode:        tarHeader.FileInfo().Mode(),
 				Data:        pathReader,
 				Link:        tarHeader.Linkname,
@@ -269,3 +293,16 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error {
 
 	return nil
 }
+
+func parentDirs(path string) []string {
+	path = filepath.Clean(path)
+	parents := make([]string, strings.Count(path, "/"))
+	count := 0
+	for i, c := range path {
+		if c == '/' {
+			parents[count] = path[:i+1]
+			count++
+		}
+	}
+	return parents
+}

internal/deb/extract_test.go

@@ -300,8 +300,7 @@ var extractTests = []extractTest{{
 		},
 	},
 	result: map[string]string{
-		"/dir/":       "dir 0755",
-		"/other-dir/": "dir 0755",
+		"/dir/": "dir 0755",
 	},
 	notCreated: []string{},
 }, {
@@ -319,6 +318,25 @@ var extractTests = []extractTest{{
 		},
 	},
 	error: `cannot extract from package "test-package": no content at /dir/missing-file`,
+}, {
+	summary: "Extract non-ASCII path and preserve parent directories permissions",
+	pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{
+		testutil.Dir(0755, "./"),
+		testutil.Dir(0766, "./日本/"),
+		testutil.Reg(0644, "./日本/語", "whatever"),
+	}),
+	options: deb.ExtractOptions{
+		Extract: map[string][]deb.ExtractInfo{
+			"/日本/語": []deb.ExtractInfo{{
+				Path: "/日本/語",
+			}},
+		},
+	},
+	result: map[string]string{
+		"/日本/":  "dir 0766",
+		"/日本/語": "file 0644 85738f8f",
+	},
+	notCreated: []string{},
 }}
 
 func (s *S) TestExtract(c *C) {

internal/fsutil/create.go

@@ -16,6 +16,8 @@ type CreateOptions struct {
 	Mode fs.FileMode
 	Data io.Reader
 	Link string
+	// If OverrideMode is true and a directory exists, its mode will be modified.
+	OverrideMode bool
 	// If MakeParents is true, missing parent directories of Path are
 	// created with permissions 0755.
 	MakeParents bool
@@ -74,6 +76,9 @@ func createDir(o *CreateOptions) error {
 	debugf("Creating directory: %s (mode %#o)", o.Path, o.Mode)
 	err := os.Mkdir(o.Path, o.Mode)
 	if os.IsExist(err) {
+		if !o.OverrideMode {
+			return nil
+		}
 		err = os.Chmod(o.Path, o.Mode)
 	}
 	return err

internal/fsutil/create_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"io/fs"
+	"os"
 	"path/filepath"
 	"strings"
 	"syscall"
@@ -16,6 +17,7 @@ import (
 
 type createTest struct {
 	options fsutil.CreateOptions
+	hackdir func(c *C, dir string)
 	result  map[string]string
 	error   string
 }
@@ -66,6 +68,32 @@ var createTests = []createTest{{
 		Mode: fs.ModeDir | 0775,
 	},
 	error: `.*: no such file or directory`,
+}, {
+	options: fsutil.CreateOptions{
+		Path:         "foo",
+		Mode:         fs.ModeDir | 0775,
+		OverrideMode: false,
+	},
+	hackdir: func(c *C, dir string) {
+		c.Assert(os.Mkdir(filepath.Join(dir, "foo/"), fs.ModeDir|0765), IsNil)
+	},
+	result: map[string]string{
+		// mode is not updated.
+		"/foo/": "dir 0765",
+	},
+}, {
+	options: fsutil.CreateOptions{
+		Path:         "foo",
+		Mode:         fs.ModeDir | 0775,
+		OverrideMode: true,
+	},
+	hackdir: func(c *C, dir string) {
+		c.Assert(os.Mkdir(filepath.Join(dir, "foo/"), fs.ModeDir|0765), IsNil)
+	},
+	result: map[string]string{
+		// mode is updated.
+		"/foo/": "dir 0775",
+	},
 }}
 
 func (s *S) TestCreate(c *C) {
@@ -81,6 +109,9 @@ func (s *S) TestCreate(c *C) {
 		}
 		c.Logf("Options: %v", test.options)
 		dir := c.MkDir()
+		if test.hackdir != nil {
+			test.hackdir(c, dir)
+		}
 		options := test.options
 		options.Path = filepath.Join(dir, options.Path)
 		entry, err := fsutil.Create(&options)

internal/slicer/slicer_test.go

@@ -33,7 +33,6 @@ type slicerTest struct {
 	// TODO:
 	// The results of the report do not conform to the planned implementation
 	// yet. Namely:
-	// * Parent directories of {text} files are reported even though they should not.
 	// * We do not track removed directories or changes done in Starlark.
 	report map[string]string
 	error  string
@@ -191,6 +190,32 @@ var slicerTests = []slicerTest{{
 	report: map[string]string{
 		"/parent/new/": "dir 0755 {test-package_myslice}",
 	},
+}, {
+	summary: "Create new file using glob and preserve parent directory permissions",
+	slices:  []setup.SliceKey{{"test-package", "myslice"}},
+	pkgs: map[string][]byte{
+		"test-package": testutil.PackageData["test-package"],
+	},
+	release: map[string]string{
+		"slices/mydir/test-package.yaml": `
+			package: test-package
+			slices:
+				myslice:
+					contents:
+						# Note the missing /parent/ and /parent/permissions/ here.
+						/parent/**:
+		`,
+	},
+	filesystem: map[string]string{
+		"/parent/":                 "dir 01777", // This is the magic.
+		"/parent/permissions/":     "dir 0764",  // This is the magic.
+		"/parent/permissions/file": "file 0755 722c14b3",
+	},
+	report: map[string]string{
+		"/parent/":                 "dir 01777 {test-package_myslice}",
+		"/parent/permissions/":     "dir 0764 {test-package_myslice}",
+		"/parent/permissions/file": "file 0755 722c14b3 {test-package_myslice}",
+	},
 }, {
 	summary: "Conditional architecture",
 	arch:    "amd64",
@@ -323,8 +348,7 @@ var slicerTests = []slicerTest{{
 		`,
 	},
 	filesystem: map[string]string{
-		// TODO this is the wrong mode for the directory, it should be 01777.
-		"/dir/":     "dir 0755",
+		"/dir/":     "dir 01777",
 		"/dir/file": "file 0644 a441b15f",
 	},
 	report: map[string]string{
@@ -362,8 +386,7 @@ var slicerTests = []slicerTest{{
 		`,
 	},
 	filesystem: map[string]string{
-		// TODO this is the wrong mode for the directory, it should be 01777.
-		"/dir/":     "dir 0755",
+		"/dir/":     "dir 01777",
 		"/dir/file": "file 0644 a441b15f",
 	},
 	report: map[string]string{