From fca7c6449abe6c2a1f693d826cba680ea15793dc Mon Sep 17 00:00:00 2001
From: Erik Grinaker <erik@interchain.berlin>
Date: Mon, 4 Jan 2021 15:30:38 +0100
Subject: [PATCH] libs/os: EnsureDir now returns IO errors and checks file type
 (#5852)

Fixes #5839.
---
 CHANGELOG_PENDING.md |  1 +
 libs/os/os.go        | 17 ++++++++++++-----
 libs/os/os_test.go   | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md
index b2480c1cf..13b74e0e1 100644
--- a/CHANGELOG_PENDING.md
+++ b/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
 
diff --git a/libs/os/os.go b/libs/os/os.go
index ea24a42f6..8776e3a0c 100644
--- a/libs/os/os.go
+++ b/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
 }
diff --git a/libs/os/os_test.go b/libs/os/os_test.go
index 9c80f1f5a..c912465c5 100644
--- a/libs/os/os_test.go
+++ b/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)
+}