Ensure backup restore location contains no files

Today we check it's an empty directory.  However, if Keysync has started, it'll
create empty directories for the clients.  So now this only verifies that there
are no files, instead of nothing.

backup/tar.go

@@ -83,32 +83,19 @@ func createTar(dir string) ([]byte, error) {
 // If Chown is true, set file ownership from the tarball.
 // This is intended to be only used with files from createTar.
 func extractTar(tarball []byte, chown bool, dirpath string, filesystem output.Filesystem) error {
-	// Open the destination directory and verify it's empty
-	dir, err := os.Open(dirpath)
+	_, err := os.Open(dirpath)
 	if os.IsNotExist(err) {
 		// The directory doesn't exist, so try to make it.
 		if err := os.MkdirAll(dirpath, 0755); err != nil {
 			return errors.Wrapf(err, "could not create secrets directory %s", dirpath)
 		}
-		dir, err = os.Open(dirpath)
-	}
-	if err != nil {
+	} else if err != nil {
 		return errors.Wrapf(err, "error opening secrets directory %s", dirpath)
 	}
 
-	info, err := dir.Stat()
-	if err != nil {
-		return errors.Wrapf(err, "could not stat directory %s", dirpath)
-	}
-	if !info.IsDir() {
-		return fmt.Errorf("secrets directory exists but is a file %s", dirpath)
-	}
-
-	// Check if the directory is empty
-	if list, err := dir.Readdir(-1); err != nil {
-		return errors.Wrapf(err, "could not read contents of secrets directory %s", dirpath)
-	} else if len(list) != 0 {
-		return fmt.Errorf("secrets directory exists but is nonempty (%d entries)", len(list))
+	// Don't risk overwriting any existing files:
+	if err := checkIfEmpty(dirpath); err != nil {
+		return err
 	}
 
 	// At this point, the directory exists and is non-empty, so let's unpack files there
@@ -147,3 +134,23 @@ func extractTar(tarball []byte, chown bool, dirpath string, filesystem output.Fi
 
 	return nil
 }
+
+// checkIfEmptyDir verifies a given path contains no files
+func checkIfEmpty(dir string) error {
+	var files []string
+	if err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if !info.IsDir() {
+			files = append(files, path)
+		}
+		return nil
+	}); err != nil {
+		return err
+	}
+	if len(files) > 0 {
+		return fmt.Errorf("non-empty directory %s: %q", dir, files)
+	}
+	return nil
+}

backup/tar_test.go

@@ -60,3 +60,30 @@ func TestCreateExtractTar(t *testing.T) {
 		testSha(t, test.sha2hash, file)
 	}
 }
+
+// TestCheckIfEmpty makes sure we detect files, which should help us avoid accidentally restoring
+// a backup when there's secrets in place.
+func TestCheckIfEmpty(t *testing.T) {
+	tmpdir, err := ioutil.TempDir("", "test-create-extract-tar")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpdir)
+
+	// Case 0: A non-existent path
+	assert.Error(t, checkIfEmpty(filepath.Join(tmpdir, "this-path-does-not-exist")))
+
+	// Case 1: An empty directory.
+	assert.NoError(t, checkIfEmpty(tmpdir))
+
+	// Case 2: Some directories nested
+	nested, err := ioutil.TempDir(tmpdir, "nested")
+	require.NoError(t, err)
+	assert.NoError(t, checkIfEmpty(tmpdir))
+
+	// Case 3: file in nested.  Should error since there's a file
+	myfile := filepath.Join(nested, "myfile")
+	assert.NoError(t, ioutil.WriteFile(myfile, []byte("hello world"), 0600))
+	assert.Error(t, checkIfEmpty(tmpdir))
+
+	// Case 4: Passed a file instead of a directory
+	assert.Error(t, checkIfEmpty(myfile))
+}