fix: explicit parents override implicit (#166)

This commits introduces a new flag for fsutil.Create called OverrideMode which updates the mode of existing entries. It is used to ensure that the explicit folder overrides the permissions of the implicit ones.

internal/deb/extract.go

@@ -247,11 +247,12 @@ func extractData(dataReader io.Reader, options *ExtractOptions) error {
 			}
 			// Create the entry itself.
 			createOptions := &fsutil.CreateOptions{
-				Path:        filepath.Join(options.TargetDir, targetPath),
-				Mode:        tarHeader.FileInfo().Mode(),
-				Data:        pathReader,
-				Link:        tarHeader.Linkname,
-				MakeParents: true,
+				Path:         filepath.Join(options.TargetDir, targetPath),
+				Mode:         tarHeader.FileInfo().Mode(),
+				Data:         pathReader,
+				Link:         tarHeader.Linkname,
+				MakeParents:  true,
+				OverrideMode: true,
 			}
 			err := options.Create(extractInfos, createOptions)
 			if err != nil {

internal/deb/extract_test.go

@@ -2,6 +2,8 @@ package deb_test
 
 import (
 	"bytes"
+	"os"
+	"path"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -17,7 +19,7 @@ type extractTest struct {
 	summary string
 	pkgdata []byte
 	options deb.ExtractOptions
-	hackopt func(o *deb.ExtractOptions)
+	hackopt func(c *C, o *deb.ExtractOptions)
 	result  map[string]string
 	// paths which the extractor did not create explicitly.
 	notCreated []string
@@ -98,7 +100,7 @@ var extractTests = []extractTest{{
 		"/dir/several/levels/deep/file": "file 0644 6bc26dff",
 		"/other-dir/":                   "dir 0755",
 	},
-	hackopt: func(o *deb.ExtractOptions) {
+	hackopt: func(c *C, o *deb.ExtractOptions) {
 		o.Create = nil
 	},
 }, {
@@ -352,6 +354,25 @@ var extractTests = []extractTest{{
 		},
 	},
 	error: `cannot extract from package "test-package": path /dir/ requested twice with diverging mode: 0777 != 0000`,
+}, {
+	summary: "Explicit extraction overrides existing file",
+	pkgdata: testutil.PackageData["test-package"],
+	options: deb.ExtractOptions{
+		Extract: map[string][]deb.ExtractInfo{
+			"/dir/": []deb.ExtractInfo{{
+				Path: "/dir/",
+				Mode: 0777,
+			}},
+		},
+	},
+	hackopt: func(c *C, o *deb.ExtractOptions) {
+		err := os.Mkdir(path.Join(o.TargetDir, "/dir"), 0666)
+		c.Assert(err, IsNil)
+	},
+	result: map[string]string{
+		"/dir/": "dir 0777",
+	},
+	notCreated: []string{},
 }}
 
 func (s *S) TestExtract(c *C) {
@@ -374,7 +395,7 @@ func (s *S) TestExtract(c *C) {
 		}
 
 		if test.hackopt != nil {
-			test.hackopt(&options)
+			test.hackopt(c, &options)
 		}
 
 		err := deb.Extract(bytes.NewBuffer(test.pkgdata), &options)

internal/fsutil/create.go

@@ -19,6 +19,9 @@ type CreateOptions struct {
 	// If MakeParents is true, missing parent directories of Path are
 	// created with permissions 0755.
 	MakeParents bool
+	// If OverrideMode is true and entry already exists, update the mode. Does
+	// not affect symlinks.
+	OverrideMode bool
 }
 
 type Entry struct {
@@ -65,9 +68,18 @@ func Create(options *CreateOptions) (*Entry, error) {
 	if err != nil {
 		return nil, err
 	}
+	mode := s.Mode()
+	if o.OverrideMode && mode != o.Mode && o.Mode&fs.ModeSymlink == 0 {
+		err := os.Chmod(o.Path, o.Mode)
+		if err != nil {
+			return nil, err
+		}
+		mode = o.Mode
+	}
+
 	entry := &Entry{
 		Path:   o.Path,
-		Mode:   s.Mode(),
+		Mode:   mode,
 		SHA256: hash,
 		Size:   rp.size,
 		Link:   o.Link,

internal/fsutil/create_test.go

@@ -93,6 +93,93 @@ var createTests = []createTest{{
 		// mode is not updated.
 		"/foo": "file 0666 d67e2e94",
 	},
+}, {
+	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",
+	},
+}, {
+	options: fsutil.CreateOptions{
+		Path:         "foo",
+		Mode:         0775,
+		Data:         bytes.NewBufferString("whatever"),
+		OverrideMode: true,
+	},
+	hackdir: func(c *C, dir string) {
+		err := os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666)
+		c.Assert(err, IsNil)
+	},
+	result: map[string]string{
+		// mode is updated.
+		"/foo": "file 0775 85738f8f",
+	},
+}, {
+	options: fsutil.CreateOptions{
+		Path: "foo",
+		Link: "./bar",
+		Mode: 0666 | fs.ModeSymlink,
+	},
+	hackdir: func(c *C, dir string) {
+		err := os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666)
+		c.Assert(err, IsNil)
+	},
+	result: map[string]string{
+		"/foo": "symlink ./bar",
+	},
+}, {
+	options: fsutil.CreateOptions{
+		Path:         "foo",
+		Link:         "./bar",
+		Mode:         0776 | fs.ModeSymlink,
+		OverrideMode: true,
+	},
+	hackdir: func(c *C, dir string) {
+		err := os.WriteFile(filepath.Join(dir, "bar"), []byte("data"), 0666)
+		c.Assert(err, IsNil)
+		err = os.WriteFile(filepath.Join(dir, "foo"), []byte("data"), 0666)
+		c.Assert(err, IsNil)
+	},
+	result: map[string]string{
+		"/foo": "symlink ./bar",
+		// mode is not updated.
+		"/bar": "file 0666 3a6eb079",
+	},
+}, {
+	options: fsutil.CreateOptions{
+		Path: "bar",
+		// Existing link with different target.
+		Link: "other",
+		Mode: 0666 | fs.ModeSymlink,
+	},
+	hackdir: func(c *C, dir string) {
+		err := os.Symlink("foo", filepath.Join(dir, "bar"))
+		c.Assert(err, IsNil)
+	},
+	result: map[string]string{
+		"/bar": "symlink other",
+	},
+}, {
+	options: fsutil.CreateOptions{
+		Path: "bar",
+		// Existing link with same target.
+		Link: "foo",
+		Mode: 0666 | fs.ModeSymlink,
+	},
+	hackdir: func(c *C, dir string) {
+		err := os.Symlink("foo", filepath.Join(dir, "bar"))
+		c.Assert(err, IsNil)
+	},
+	result: map[string]string{
+		"/bar": "symlink foo",
+	},
 }}
 
 func (s *S) TestCreate(c *C) {

internal/slicer/slicer_test.go

@@ -321,43 +321,59 @@ var slicerTests = []slicerTest{{
 }, {
 	summary: "Install two packages, explicit path has preference over implicit parent",
 	slices: []setup.SliceKey{
-		{"implicit-parent", "myslice"},
-		{"explicit-dir", "myslice"}},
+		{"a-implicit-parent", "myslice"},
+		{"b-explicit-dir", "myslice"},
+		{"c-implicit-parent", "myslice"}},
 	pkgs: []*testutil.TestPackage{{
-		Name: "implicit-parent",
+		Name: "a-implicit-parent",
 		Data: testutil.MustMakeDeb([]testutil.TarEntry{
 			testutil.Dir(0755, "./dir/"),
-			testutil.Reg(0644, "./dir/file", "random"),
+			testutil.Reg(0644, "./dir/file-1", "random"),
 		}),
 	}, {
-		Name: "explicit-dir",
+		Name: "b-explicit-dir",
 		Data: testutil.MustMakeDeb([]testutil.TarEntry{
 			testutil.Dir(01777, "./dir/"),
 		}),
+	}, {
+		Name: "c-implicit-parent",
+		Data: testutil.MustMakeDeb([]testutil.TarEntry{
+			testutil.Dir(0755, "./dir/"),
+			testutil.Reg(0644, "./dir/file-2", "random"),
+		}),
 	}},
 	release: map[string]string{
-		"slices/mydir/implicit-parent.yaml": `
-			package: implicit-parent
+		"slices/mydir/a-implicit-parent.yaml": `
+			package: a-implicit-parent
 			slices:
 				myslice:
 					contents:
-						/dir/file:
+						/dir/file-1:
 		`,
-		"slices/mydir/explicit-dir.yaml": `
-			package: explicit-dir
+		"slices/mydir/b-explicit-dir.yaml": `
+			package: b-explicit-dir
 			slices:
 				myslice:
 					contents:
 						/dir/:
 		`,
+		"slices/mydir/c-implicit-parent.yaml": `
+			package: c-implicit-parent
+			slices:
+				myslice:
+					contents:
+						/dir/file-2:
+		`,
 	},
 	filesystem: map[string]string{
-		"/dir/":     "dir 01777",
-		"/dir/file": "file 0644 a441b15f",
+		"/dir/":       "dir 01777",
+		"/dir/file-1": "file 0644 a441b15f",
+		"/dir/file-2": "file 0644 a441b15f",
 	},
 	manifestPaths: map[string]string{
-		"/dir/":     "dir 01777 {explicit-dir_myslice}",
-		"/dir/file": "file 0644 a441b15f {implicit-parent_myslice}",
+		"/dir/":       "dir 01777 {b-explicit-dir_myslice}",
+		"/dir/file-1": "file 0644 a441b15f {a-implicit-parent_myslice}",
+		"/dir/file-2": "file 0644 a441b15f {c-implicit-parent_myslice}",
 	},
 }, {
 	summary: "Valid same file in two slices in different packages",