libs/os: EnsureDir now returns IO errors and checks file type (#5852)

Fixes #5839.

CHANGELOG_PENDING.md

@@ -15,6 +15,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 - P2P Protocol
 
 - Go API
+  - [libs/os] `EnsureDir` now propagates IO errors and checks the file type (@erikgrinaker)
 
 - Blockchain Protocol
 

libs/os/os.go

@@ -43,12 +43,19 @@ func Exit(s string) {
 	os.Exit(1)
 }
 
+// EnsureDir ensures the given directory exists, creating it if necessary.
+// Errors if the path already exists as a non-directory.
 func EnsureDir(dir string, mode os.FileMode) error {
-	if _, err := os.Stat(dir); os.IsNotExist(err) {
-		err := os.MkdirAll(dir, mode)
-		if err != nil {
-			return fmt.Errorf("could not create directory %v: %w", dir, err)
-		}
+	info, err := os.Stat(dir)
+	if err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("failed to stat %q: %w", dir, err)
+	}
+	if info != nil && !info.IsDir() {
+		return fmt.Errorf("path %q already exists as a non-directory", dir)
+	}
+	err = os.MkdirAll(dir, mode)
+	if err != nil {
+		return fmt.Errorf("could not create directory %q: %w", dir, err)
 	}
 	return nil
 }

libs/os/os_test.go

@@ -5,7 +5,10 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestCopyFile(t *testing.T) {
@@ -35,3 +38,36 @@ func TestCopyFile(t *testing.T) {
 	}
 	os.Remove(copyfile)
 }
+
+func TestEnsureDir(t *testing.T) {
+	tmp, err := ioutil.TempDir("", "ensure-dir")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmp)
+
+	// Should be possible to create a new directory.
+	err = EnsureDir(filepath.Join(tmp, "dir"), 0755)
+	require.NoError(t, err)
+	require.DirExists(t, filepath.Join(tmp, "dir"))
+
+	// Should succeed on existing directory.
+	err = EnsureDir(filepath.Join(tmp, "dir"), 0755)
+	require.NoError(t, err)
+
+	// Should fail on file.
+	err = ioutil.WriteFile(filepath.Join(tmp, "file"), []byte{}, 0644)
+	require.NoError(t, err)
+	err = EnsureDir(filepath.Join(tmp, "file"), 0755)
+	require.Error(t, err)
+
+	// Should allow symlink to dir.
+	err = os.Symlink(filepath.Join(tmp, "dir"), filepath.Join(tmp, "linkdir"))
+	require.NoError(t, err)
+	err = EnsureDir(filepath.Join(tmp, "linkdir"), 0755)
+	require.NoError(t, err)
+
+	// Should error on symlink to file.
+	err = os.Symlink(filepath.Join(tmp, "file"), filepath.Join(tmp, "linkfile"))
+	require.NoError(t, err)
+	err = EnsureDir(filepath.Join(tmp, "linkfile"), 0755)
+	require.Error(t, err)
+}