Apply directory metadata only after the directory was populated

archive.go

@@ -182,7 +182,10 @@ loop:
 				a.last = c
 				break loop
 			}
-			a.dir = filepath.Dir(a.dir)
+			// path.Dir, not filepath.Dir: node names are slash-separated
+			// (built with path.Join below) and have to stay that way on
+			// Windows too, where filepath.Dir would rewrite the separators.
+			a.dir = path.Dir(a.dir)
 		case nil:
 			return nil, nil
 

cmd/desync/untar.go

@@ -53,7 +53,7 @@ the output can be set to GNU tar, either an archive or STDOUT with '-'.
 	return cmd
 }
 
-func runUntar(ctx context.Context, opt untarOptions, args []string) error {
+func runUntar(ctx context.Context, opt untarOptions, args []string) (err error) {
 	if err := opt.cmdStoreOptions.validate(); err != nil {
 		return err
 	}
@@ -65,14 +65,17 @@ func runUntar(ctx context.Context, opt untarOptions, args []string) error {
 	target := args[1]
 
 	// Prepare output
-	var (
-		fs  desync.FilesystemWriter
-		err error
-	)
+	var fs desync.FilesystemWriter
 	switch opt.outFormat {
 	case "disk": // Local filesystem
 		lfs := desync.NewLocalFS(target, opt.LocalFSOptions)
-		defer lfs.Close()
+		// Closing applies any directory metadata that is still outstanding,
+		// which is the case when the extraction failed part-way through.
+		defer func() {
+			if cerr := lfs.Close(); cerr != nil && err == nil {
+				err = cerr
+			}
+		}()
 		fs = lfs
 	case "gnu-tar": // GNU tar, either file or STDOUT
 		var w *os.File

filesystem.go

@@ -20,6 +20,16 @@ type FilesystemWriter interface {
 	CreateDevice(n NodeDevice) error
 }
 
+// FilesystemFinalizer is implemented by filesystems that defer part of their
+// work, such as directory metadata that can only be applied once a directory
+// has been populated, until the whole archive has been written. UnTar calls
+// Finalize when it reaches the end of the archive. It is not called when the
+// archive can't be read to the end, so implementations that leave the target
+// in an intermediate state until then should apply what they can on Close().
+type FilesystemFinalizer interface {
+	Finalize() error
+}
+
 // FilesystemReader is an interface for source filesystem to be used during
 // tar operations. Next() is expected to return files and directories in a
 // consistent and stable order and return io.EOF when no further files are available.

localfs.go

@@ -5,6 +5,8 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"slices"
+	"strings"
 	"sync"
 	"time"
 )
@@ -28,6 +30,12 @@ type LocalFS struct {
 	wroot    *os.Root
 	wErr     error
 	rootReal string
+
+	// Directories whose metadata (owner, xattrs, permissions and timestamps)
+	// hasn't been applied yet because they may still be populated. Held in
+	// the order they were created, i.e. a directory is always preceded by its
+	// parent.
+	pending []NodeDirectory
 }
 
 // LocalFSOptions influence the behavior of the filesystem when reading from or writing to it.
@@ -48,6 +56,7 @@ type LocalFSOptions struct {
 
 var _ FilesystemWriter = &LocalFS{}
 var _ FilesystemReader = &LocalFS{}
+var _ FilesystemFinalizer = &LocalFS{}
 
 // writeRoot lazily creates the extraction root directory and opens an os.Root
 // handle anchored to it. Every write/metadata operation goes through the
@@ -72,42 +81,122 @@ func (fs *LocalFS) writeRoot() (*os.Root, error) {
 	return fs.wroot, fs.wErr
 }
 
-// Close releases the os.Root handle used for writing. It is safe to call even
-// if no write operation was ever performed.
+// Close applies any outstanding directory metadata on a best-effort basis and
+// releases the os.Root handle used for writing. It is safe to call even if no
+// write operation was ever performed. On a successful untar the metadata has
+// normally been applied by Finalize() already.
 func (fs *LocalFS) Close() error {
+	err := fs.Finalize()
 	if fs.wroot != nil {
-		return fs.wroot.Close()
+		if cerr := fs.wroot.Close(); err == nil {
+			err = cerr
+		}
+	}
+	return err
+}
+
+// Finalize applies the deferred metadata of all directories that are still
+// pending, deepest first. It's called at the end of an untar operation, once
+// no further entries can be written. Safe to call more than once.
+func (fs *LocalFS) Finalize() error {
+	var err error
+	for _, n := range slices.Backward(fs.pending) {
+		if e := fs.applyDirMetadata(n); e != nil && err == nil {
+			err = e
+		}
+	}
+	fs.pending = nil
+	return err
+}
+
+// contains reports whether name refers to something inside the directory dir.
+// Both are clean slash-separated paths relative to the extraction root, with
+// "." being the root itself.
+func contains(dir, name string) bool {
+	return dir == "." || strings.HasPrefix(name, dir+"/")
+}
+
+// completeDirs applies the deferred metadata of every pending directory that
+// doesn't contain name, deepest first. Those directories are complete since
+// entries arrive depth-first.
+func (fs *LocalFS) completeDirs(name string) error {
+	for len(fs.pending) > 0 {
+		n := fs.pending[len(fs.pending)-1]
+		if contains(n.Name, name) {
+			return nil
+		}
+		// Only drop it once it's done, so a failure here can still be retried
+		// by the best-effort Finalize() in Close().
+		if err := fs.applyDirMetadata(n); err != nil {
+			return err
+		}
+		fs.pending = fs.pending[:len(fs.pending)-1]
 	}
 	return nil
 }
 
+// applyDirMetadata sets owner, xattrs, permissions and timestamps of a
+// directory that has been fully populated.
+func (fs *LocalFS) applyDirMetadata(n NodeDirectory) error {
+	r, err := fs.writeRoot()
+	if err != nil {
+		return err
+	}
+	if err := fs.SetDirPermissions(n); err != nil {
+		return err
+	}
+	if n.MTime == time.Unix(0, 0) {
+		return nil
+	}
+	return r.Chtimes(n.Name, n.MTime, n.MTime)
+}
+
 func (fs *LocalFS) CreateDir(n NodeDirectory) error {
 	r, err := fs.writeRoot()
 	if err != nil {
 		return err
 	}
 
+	// Everything that isn't an ancestor of this new directory is complete now.
+	if err := fs.completeDirs(n.Name); err != nil {
+		return err
+	}
+
 	// Let's see if there is a dir with the same name already
+	var (
+		created  bool
+		existing os.FileMode
+	)
 	if info, err := r.Lstat(n.Name); err == nil {
 		if !info.IsDir() {
 			return fmt.Errorf("%s exists and is not a directory", n.Name)
 		}
+		existing = info.Mode().Perm()
 	} else {
 		// Stat error'ed out, presumably because the dir doesn't exist. Create it.
 		// (n.Name == "." is the extraction root itself, which already exists.)
-		if err := r.Mkdir(n.Name, 0777); err != nil {
+		// The mode from the archive is the upper bound of what the directory
+		// gets here, so its contents are never written into a directory more
+		// permissive than the archive says. The exact mode, including any
+		// setuid/setgid/sticky bits, is applied by prepareDirWrite below and
+		// again by applyDirMetadata once the directory is complete.
+		mode := os.FileMode(0777)
+		if !fs.opts.NoSamePermissions {
+			mode = n.Mode.Perm() | 0700
+		}
+		if err := r.Mkdir(n.Name, mode); err != nil {
 			return fmt.Errorf("%s: %w", n.Name, err)
 		}
+		created = true
 	}
 
-	if err := fs.SetDirPermissions(n); err != nil {
-		return err
-	}
+	fs.prepareDirWrite(r, n, existing, created)
 
-	if n.MTime == time.Unix(0, 0) {
-		return nil
-	}
-	return r.Chtimes(n.Name, n.MTime, n.MTime)
+	// The remaining metadata is applied once the directory is complete. Doing
+	// it now would not only prevent writing into a read-only directory, it'd
+	// also see the timestamps overwritten by those very writes.
+	fs.pending = append(fs.pending, n)
+	return nil
 }
 
 func (fs *LocalFS) CreateFile(n NodeFile) error {
@@ -116,6 +205,10 @@ func (fs *LocalFS) CreateFile(n NodeFile) error {
 		return err
 	}
 
+	if err := fs.completeDirs(n.Name); err != nil {
+		return err
+	}
+
 	if err := r.RemoveAll(n.Name); err != nil && !os.IsNotExist(err) {
 		return fmt.Errorf("%s: %w", n.Name, err)
 	}
@@ -144,6 +237,10 @@ func (fs *LocalFS) CreateSymlink(n NodeSymlink) error {
 		return err
 	}
 
+	if err := fs.completeDirs(n.Name); err != nil {
+		return err
+	}
+
 	if err := r.Remove(n.Name); err != nil && !os.IsNotExist(err) {
 		return fmt.Errorf("%s: %w", n.Name, err)
 	}