diff --git a/internal/ufs/filesystem.go b/internal/ufs/filesystem.go index 3fa16824..e0f3ce87 100644 --- a/internal/ufs/filesystem.go +++ b/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. // diff --git a/internal/ufs/fs_unix.go b/internal/ufs/fs_unix.go index dff36c9b..019491e9 100644 --- a/internal/ufs/fs_unix.go +++ b/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 } diff --git a/internal/ufs/fs_unix_test.go b/internal/ufs/fs_unix_test.go index 9187b6ff..7b8c5c22 100644 --- a/internal/ufs/fs_unix_test.go +++ b/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) { diff --git a/internal/ufs/mkdir_unix.go b/internal/ufs/mkdir_unix.go index eb2942b3..79afb248 100644 --- a/internal/ufs/mkdir_unix.go +++ b/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 } diff --git a/server/filesystem/compress.go b/server/filesystem/compress.go index c36a003a..d6d3f056 100644 --- a/server/filesystem/compress.go +++ b/server/filesystem/compress.go @@ -369,14 +369,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 diff --git a/server/filesystem/filesystem.go b/server/filesystem/filesystem.go index 4eae7667..b275e40b 100644 --- a/server/filesystem/filesystem.go +++ b/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,6 +193,7 @@ 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 } @@ -192,7 +201,7 @@ func (fs *Filesystem) Write(p string, r io.Reader, newSize int64, mode ufs.FileM // 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