Merge pull request #1629 from dmcgowan/remove-archive-umask

archive: remove umask, replace with explicit chmod after create

archive/tar.go

@@ -88,8 +88,6 @@ const (
 // See https://github.com/opencontainers/image-spec/blob/master/layer.md#applying-changesets
 func Apply(ctx context.Context, root string, r io.Reader) (int64, error) {
 	root = filepath.Clean(root)
-	fn := prepareApply()
-	defer fn()
 
 	var (
 		tr   = tar.NewReader(r)
@@ -445,13 +443,13 @@ func createTarFile(ctx context.Context, path, extractDir string, hdr *tar.Header
 		// Create directory unless it exists as a directory already.
 		// In that case we just want to merge the two
 		if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) {
-			if err := os.Mkdir(path, hdrInfo.Mode()); err != nil {
+			if err := mkdir(path, hdrInfo.Mode()); err != nil {
 				return err
 			}
 		}
 
 	case tar.TypeReg, tar.TypeRegA:
-		file, err := openFile(path, os.O_CREATE|os.O_WRONLY, hdrInfo.Mode())
+		file, err := openFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, hdrInfo.Mode())
 		if err != nil {
 			return err
 		}

archive/tar_unix.go

@@ -43,20 +43,28 @@ func open(p string) (*os.File, error) {
 }
 
 func openFile(name string, flag int, perm os.FileMode) (*os.File, error) {
-	return os.OpenFile(name, flag, perm)
+	f, err := os.OpenFile(name, flag, perm)
+	if err != nil {
+		return nil, err
+	}
+	// Call chmod to avoid permission mask
+	if err := os.Chmod(name, perm); err != nil {
+		return nil, err
+	}
+	return f, err
 }
 
 func mkdirAll(path string, perm os.FileMode) error {
 	return os.MkdirAll(path, perm)
 }
 
-func prepareApply() func() {
-	// Unset unmask before doing an apply operation,
-	// restore unmask when complete
-	oldmask := unix.Umask(0)
-	return func() {
-		unix.Umask(oldmask)
+func mkdir(path string, perm os.FileMode) error {
+	if err := os.Mkdir(path, perm); err != nil {
+		return err
 	}
+	// Only final created directory gets explicit permission
+	// call to avoid permission mask
+	return os.Chmod(path, perm)
 }
 
 func skipFile(*tar.Header) bool {

archive/tar_windows.go

@@ -56,9 +56,8 @@ func mkdirAll(path string, perm os.FileMode) error {
 	return sys.MkdirAll(path, perm)
 }
 
-func prepareApply() func() {
-	// No umask or filesystem changes needed before apply
-	return func() {}
+func mkdir(path string, perm os.FileMode) error {
+	return os.Mkdir(path, perm)
 }
 
 func skipFile(hdr *tar.Header) bool {

fs/fstest/file.go

@@ -50,7 +50,10 @@ func RemoveAll(name string) Applier {
 func CreateDir(name string, perm os.FileMode) Applier {
 	return applyFn(func(root string) error {
 		fullPath := filepath.Join(root, name)
-		return os.MkdirAll(fullPath, perm)
+		if err := os.MkdirAll(fullPath, perm); err != nil {
+			return err
+		}
+		return os.Chmod(fullPath, perm)
 	})
 }
 
@@ -75,6 +78,13 @@ func Chtime(name string, t time.Time) Applier {
 	})
 }
 
+// Chmod returns a file applier which changes the file permission
+func Chmod(name string, perm os.FileMode) Applier {
+	return applyFn(func(root string) error {
+		return os.Chmod(filepath.Join(root, name), perm)
+	})
+}
+
 // Symlink returns a file applier which creates a symbolic link
 func Symlink(oldname, newname string) Applier {
 	return applyFn(func(root string) error {

fs/fstest/testsuite.go

@@ -17,8 +17,7 @@ type TestApplier interface {
 func FSSuite(t *testing.T, a TestApplier) {
 	t.Run("Basic", makeTest(t, a, basicTest))
 	t.Run("Deletion", makeTest(t, a, deletionTest))
-	// TODO: Add hard section, run if command line arg or function arg set to true
-	// Hard tests
+	t.Run("Update", makeTest(t, a, updateTest))
 	t.Run("HardlinkUnmodified", makeTest(t, a, hardlinkUnmodified))
 	t.Run("HardlinkBeforeUnmodified", makeTest(t, a, hardlinkBeforeUnmodified))
 	t.Run("HardlinkBeforeModified", makeTest(t, a, hardlinkBeforeModified))
@@ -125,6 +124,24 @@ var (
 		),
 	}
 
+	// updateTest covers file updates for content and permission
+	updateTest = []Applier{
+		Apply(
+			CreateDir("/d1", 0755),
+			CreateDir("/d2", 0700),
+			CreateFile("/d1/f1", []byte("something..."), 0644),
+			CreateFile("/d1/f2", []byte("else..."), 0644),
+			CreateFile("/d1/f3", []byte("entirely..."), 0644),
+		),
+		Apply(
+			CreateFile("/d1/f1", []byte("file content of a different length"), 0664),
+			Remove("/d1/f3"),
+			CreateFile("/d1/f3", []byte("updated content"), 0664),
+			Chmod("/d1/f2", 0766),
+			Chmod("/d2", 0777),
+		),
+	}
+
 	hardlinkUnmodified = []Applier{
 		baseApplier,
 		Apply(