Handle failed decompressed files

When untarring a tarball, attempt to create files initially with the
umask file permissions and delete the file if it fails to copy
correctly. This prevents a paritally written file from remaining on disk
with permissions that didn't include the umask.

Adds a unit test and a invalid tarball (created by truncating a large
file generated from /dev/urandom), which fails to decompress but errors
partially written.

Author: allisonlarson

decompress_tar_test.go

@@ -6,6 +6,7 @@ package getter
 import (
 	"archive/tar"
 	"bytes"
+	"errors"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -184,3 +185,29 @@ func TestDecompressTarPermissions(t *testing.T) {
 	expected["directory/setuid"] = masked
 	testDecompressorPermissions(t, d, input, expected, os.FileMode(060000000))
 }
+
+func TestDecompressTarPermissionsFailed(t *testing.T) {
+	d := new(TarDecompressor)
+	input := "./test-fixtures/decompress-tar/bad.tar"
+
+	td := t.TempDir()
+
+	// Destination is always joining result so that we have a new path
+	dst := filepath.Join(td, "subdir", "result")
+
+	err := d.Decompress(dst, input, true, os.FileMode(0))
+	if err == nil {
+		t.Fatalf("expected error when decompressing bad tar file but got none")
+	}
+
+	expectedDst := filepath.Join(dst, "directory/setuid2")
+	// Attempt to get file information
+	_, err = os.Stat(expectedDst)
+
+	if !errors.Is(err, os.ErrNotExist) {
+		if err != nil {
+			t.Fatalf("unexpected error when checking for file '%s': %s", expectedDst, err)
+		}
+		t.Fatalf("expected file '%s' to not exist", expectedDst)
+	}
+}

get_file_copy.go

@@ -35,7 +35,7 @@ func Copy(ctx context.Context, dst io.Writer, src io.Reader) (int64, error) {
 
 // copyReader copies from an io.Reader into a file, using umask to create the dst file
 func copyReader(dst string, src io.Reader, fmode, umask os.FileMode, fileSizeLimit int64) error {
-	dstF, err := os.OpenFile(dst, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fmode)
+	dstF, err := os.OpenFile(dst, os.O_RDWR|os.O_CREATE|os.O_TRUNC, mode(fmode, umask))
 	if err != nil {
 		return err
 	}
@@ -47,6 +47,8 @@ func copyReader(dst string, src io.Reader, fmode, umask os.FileMode, fileSizeLim
 
 	_, err = io.Copy(dstF, src)
 	if err != nil {
+		// Remove the file in case of partial write
+		_ = os.Remove(dst)
 		return err
 	}
 
@@ -74,14 +76,16 @@ func copyFile(ctx context.Context, dst, src string, disableSymlinks bool, fmode,
 	}
 	defer func() { _ = srcF.Close() }()
 
-	dstF, err := os.OpenFile(dst, os.O_RDWR|os.O_CREATE|os.O_TRUNC, fmode)
+	dstF, err := os.OpenFile(dst, os.O_RDWR|os.O_CREATE|os.O_TRUNC, mode(fmode, umask))
 	if err != nil {
 		return 0, err
 	}
 	defer func() { _ = dstF.Close() }()
 
 	count, err := Copy(ctx, dstF, srcF)
 	if err != nil {
+		// Remove the file in case of partial write
+		_ = os.Remove(dst)
 		return 0, err
 	}