fix: Directories created via panel are owned by root:root (#183)

* fix: Directories created via panel are owned by root:root

* make mkdirall hand back the dirs it created so we chown just those

Author: QuintenQVD0

internal/ufs/filesystem.go

@@ -74,13 +74,14 @@ type Filesystem interface {
 	Mkdir(name string, perm FileMode) error
 
 	// MkdirAll creates a directory named path, along with any necessary
-	// parents, and returns nil, or else returns an error.
-	//
-	// The permission bits perm (before umask) are used for all
-	// directories that MkdirAll creates.
-	// If path is already a directory, MkdirAll does nothing
-	// and returns nil.
-	MkdirAll(path string, perm FileMode) error
+	// parents, and returns the directories it created, or else returns an
+	// error.
+	//
+	// The returned directories are ordered from shallowest to deepest. The
+	// permission bits perm (before umask) are used for all directories that
+	// MkdirAll creates. If path is already a directory, MkdirAll does nothing
+	// and returns no created directories.
+	MkdirAll(path string, perm FileMode) ([]string, error)
 
 	// Open opens the named file for reading.
 	//

internal/ufs/fs_unix.go

@@ -207,17 +207,17 @@ func (fs *UnixFS) mkdirat(op string, dirfd int, name string, mode FileMode) erro
 }
 
 // MkdirAll creates a directory named path, along with any necessary
-// parents, and returns nil, or else returns an error.
+// parents, and returns the directories it created, or else returns an error.
 //
-// The permission bits perm (before umask) are used for all
-// directories that MkdirAll creates.
-// If path is already a directory, MkdirAll does nothing
-// and returns nil.
-func (fs *UnixFS) MkdirAll(name string, mode FileMode) error {
+// The returned directories are ordered from shallowest to deepest. The
+// permission bits perm (before umask) are used for all directories that
+// MkdirAll creates. If path is already a directory, MkdirAll does nothing and
+// returns no created directories
+func (fs *UnixFS) MkdirAll(name string, mode FileMode) ([]string, error) {
 	// Ensure name is somewhat clean before continuing.
 	name, err := fs.unsafePath(name)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	return fs.mkdirAll(name, mode)
 }
@@ -471,7 +471,7 @@ func (fs *UnixFS) Rename(oldpath, newpath string) error {
 		if !errors.As(err, &pathErr) {
 			return err
 		}
-		if err := fs.MkdirAll(pathErr.Path, 0o755); err != nil {
+		if _, err := fs.MkdirAll(pathErr.Path, 0o755); err != nil {
 			return err
 		}
 		newdirfd, newname, closeFd2, err = fs.safePath(newpath)
@@ -623,7 +623,7 @@ func (fs *UnixFS) TouchPath(path string) (int, string, func(), error, bool) {
 	if !errors.As(err, &pathErr) {
 		return dirfd, name, closeFd, err, false
 	}
-	if err := fs.MkdirAll(pathErr.Path, 0o755); err != nil {
+	if _, err := fs.MkdirAll(pathErr.Path, 0o755); err != nil {
 		return dirfd, name, closeFd, err, false
 	}
 

internal/ufs/fs_unix_test.go

@@ -164,7 +164,7 @@ func TestUnixFS(t *testing.T) {
 	}
 
 	// Create multiple nested directories.
-	if err := fs.MkdirAll("ima_directory/ima_directory/ima_directory/ima_directory", 0o755); err != nil {
+	if _, err := fs.MkdirAll("ima_directory/ima_directory/ima_directory/ima_directory", 0o755); err != nil {
 		t.Error(err)
 		return
 	}
@@ -174,7 +174,7 @@ func TestUnixFS(t *testing.T) {
 	}
 
 	// Test creating a directory under a symlink with a pre-existing directory.
-	if err := fs.MkdirAll("ima_bad_link/ima_directory/ima_bad_directory/ima_bad_directory", 0o755); err == nil {
+	if _, err := fs.MkdirAll("ima_bad_link/ima_directory/ima_bad_directory/ima_bad_directory", 0o755); err == nil {
 		t.Error("expected an error")
 		return
 	}
@@ -324,12 +324,58 @@ func TestUnixFS_MkdirAll(t *testing.T) {
 	}
 	defer fs.Cleanup()
 
-	if err := fs.MkdirAll("/a/bunch/of/directories", 0o755); err != nil {
-		t.Error(err)
-		return
-	}
+	t.Run("creates and reports every missing directory", func(t *testing.T) {
+		created, err := fs.MkdirAll("/a/bunch/of/directories", 0o755)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		want := []string{"a", "a/bunch", "a/bunch/of", "a/bunch/of/directories"}
+		if !slices.Equal(created, want) {
+			t.Errorf("created = %v, want %v", created, want)
+		}
+		// Sanity check that everything we reported actually exists on disk.
+		for _, dir := range want {
+			st, err := os.Lstat(filepath.Join(fs.Root, dir))
+			if err != nil {
+				t.Errorf("Lstat %q: %v", dir, err)
+				continue
+			}
+			if !st.IsDir() {
+				t.Errorf("%q is not a directory", dir)
+			}
+		}
+	})
+
+	t.Run("only reports the directories it creates", func(t *testing.T) {
+		if _, err := fs.MkdirAll("partial/exists", 0o755); err != nil {
+			t.Fatalf("seeding directories: %v", err)
+		}
+
+		created, err := fs.MkdirAll("partial/exists/and/more", 0o755)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		want := []string{"partial/exists/and", "partial/exists/and/more"}
+		if !slices.Equal(created, want) {
+			t.Errorf("created = %v, want %v", created, want)
+		}
+	})
 
-	// TODO: stat sanity check
+	t.Run("reports nothing when the directory already exists", func(t *testing.T) {
+		if _, err := fs.MkdirAll("already/here", 0o755); err != nil {
+			t.Fatalf("seeding directories: %v", err)
+		}
+
+		created, err := fs.MkdirAll("already/here", 0o755)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(created) != 0 {
+			t.Errorf("created = %v, want no directories", created)
+		}
+	})
 }
 
 func TestUnixFS_Open(t *testing.T) {

internal/ufs/mkdir_unix.go

@@ -11,7 +11,11 @@
 package ufs
 
 // mkdirAll is a recursive Mkdir implementation that properly handles symlinks.
-func (fs *UnixFS) mkdirAll(name string, mode FileMode) error {
+//
+// It returns the directories it created, ordered from shallowest to deepest, so
+// callers can act on exactly the paths that were new (for example to change
+// their ownership). Directories that already existed are not included.
+func (fs *UnixFS) mkdirAll(name string, mode FileMode) ([]string, error) {
 	// Fast path: if we can tell whether path is a directory or file, stop with success or error.
 	dir, err := fs.Lstat(name)
 	if err == nil {
@@ -20,13 +24,13 @@ func (fs *UnixFS) mkdirAll(name string, mode FileMode) error {
 			// to check instead.
 			dir, err = fs.Stat(name)
 			if err != nil {
-				return err
+				return nil, err
 			}
 		}
 		if dir.IsDir() {
-			return nil
+			return nil, nil
 		}
-		return &PathError{Op: "mkdir", Path: name, Err: ErrNotDirectory}
+		return nil, &PathError{Op: "mkdir", Path: name, Err: ErrNotDirectory}
 	}
 
 	// Slow path: make sure parent exists and then call Mkdir for path.
@@ -40,11 +44,12 @@ func (fs *UnixFS) mkdirAll(name string, mode FileMode) error {
 		j--
 	}
 
+	var created []string
 	if j > 1 {
 		// Create parent.
-		err = fs.mkdirAll(name[:j-1], mode)
+		created, err = fs.mkdirAll(name[:j-1], mode)
 		if err != nil {
-			return err
+			return created, err
 		}
 	}
 
@@ -55,9 +60,9 @@ func (fs *UnixFS) mkdirAll(name string, mode FileMode) error {
 		// double-checking that directory doesn't exist.
 		dir, err1 := fs.Lstat(name)
 		if err1 == nil && dir.IsDir() {
-			return nil
+			return created, nil
 		}
-		return err
+		return created, err
 	}
-	return nil
+	return append(created, name), nil
 }

server/filesystem/compress.go

@@ -375,14 +375,19 @@ func (fs *Filesystem) extractStream(ctx context.Context, opts extractStreamOptio
 
 	// Decompress and extract archive
 	return ex.Extract(ctx, opts.Reader, func(ctx context.Context, f archives.FileInfo) error {
-		if f.IsDir() {
-			return nil
-		}
 		p := filepath.Join(opts.Directory, f.NameInArchive)
-		// If it is ignored, just don't do anything with the file and skip over it.
+		// If it is ignored, just don't do anything with the entry and skip over it.
 		if err := fs.IsIgnored(p); err != nil {
 			return nil
 		}
+		// Create directories explicitly; an empty one has no file to create it
+		// implicitly and would otherwise be dropped during extraction.
+		if f.IsDir() {
+			if err := fs.mkdirAll(p, 0o755); err != nil {
+				return wrapError(err, opts.FileName)
+			}
+			return nil
+		}
 		r, err := f.Open()
 		if err != nil {
 			return err

server/filesystem/filesystem.go

@@ -160,6 +160,14 @@ func (fs *Filesystem) Write(p string, r io.Reader, newSize int64, mode ufs.FileM
 		return err
 	}
 
+	// Ensure the parent directories exist and are owned by the server user
+	// before creating the file. Touch would create any missing parents
+	// implicitly, but as the user Wings runs as; creating them here lets us
+	// chown the ones we add.
+	if err := fs.mkdirAll(filepath.Dir(p), 0o755); err != nil {
+		return err
+	}
+
 	// Touch the file and return the handle to it at this point. This will
 	// create or truncate the file, and create any necessary parent directories
 	// if they are missing.
@@ -185,14 +193,15 @@ func (fs *Filesystem) Write(p string, r io.Reader, newSize int64, mode ufs.FileM
 	if err := fs.chownFile(p); err != nil {
 		return err
 	}
+
 	// Return any remaining error.
 	return err
 }
 
 // CreateDirectory creates a new directory (name) at a specified path (p) for
 // the server.
 func (fs *Filesystem) CreateDirectory(name string, p string) error {
-	return fs.unixFS.MkdirAll(filepath.Join(p, name), 0o755)
+	return fs.mkdirAll(filepath.Join(p, name), 0o755)
 }
 
 func (fs *Filesystem) Rename(oldpath, newpath string) error {
@@ -213,6 +222,22 @@ func (fs *Filesystem) chownFile(name string) error {
 	return fs.unixFS.Lchown(name, uid, gid)
 }
 
+// mkdirAll creates the directory p along with any missing parents, chowning
+// every directory it creates to the server user so they are not left owned by
+// the user Wings runs as.
+func (fs *Filesystem) mkdirAll(p string, mode ufs.FileMode) error {
+	created, err := fs.unixFS.MkdirAll(p, mode)
+	if err != nil {
+		return err
+	}
+	for _, dir := range created {
+		if err := fs.chownFile(dir); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // Chown recursively iterates over a file or directory and sets the permissions on all of the
 // underlying files. Iterate over all of the files and directories. If it is a file just
 // go ahead and perform the chown operation. Otherwise dig deeper into the directory until