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

Fixes #5839.

CHANGELOG_PENDING.md

@@ -33,8 +33,8 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
   - [libs/bits] \#5720 Validate `BitArray` in `FromProto`, which now returns an error (@melekes)
   - [proto/p2p] Renamed `DefaultNodeInfo` and `DefaultNodeInfoOther` to `NodeInfo` and `NodeInfoOther` (@erikgrinaker)
   - [proto/p2p] Rename `NodeInfo.default_node_id` to `node_id` (@erikgrinaker)
-
-- [libs/os] Kill() and {Must,}{Read,Write}File() functions have been removed. (@alessio)
+  - [libs/os] `EnsureDir` now propagates IO errors and checks the file type (@erikgrinaker)
+  - [libs/os] Kill() and {Must,}{Read,Write}File() functions have been removed. (@alessio)
 
 - Blockchain Protocol
 

libs/os/os.go

@@ -33,12 +33,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

@@ -6,10 +6,12 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"syscall"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
 	tmos "github.com/tendermint/tendermint/libs/os"
 )
 
@@ -67,6 +69,39 @@ func TestTrapSignal(t *testing.T) {
 	t.Fatal("this error should not be triggered")
 }
 
+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 = tmos.EnsureDir(filepath.Join(tmp, "dir"), 0755)
+	require.NoError(t, err)
+	require.DirExists(t, filepath.Join(tmp, "dir"))
+
+	// Should succeed on existing directory.
+	err = tmos.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 = tmos.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 = tmos.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 = tmos.EnsureDir(filepath.Join(tmp, "linkfile"), 0755)
+	require.Error(t, err)
+}
+
 type mockLogger struct{}
 
 func (ml mockLogger) Info(msg string, keyvals ...interface{}) {}