From 32cc4967de14710dcbff7e1a50599f86f07a1c55 Mon Sep 17 00:00:00 2001
From: Owen Diehl <ow.diehl@gmail.com>
Date: Mon, 17 Jun 2024 12:00:02 -0700
Subject: [PATCH] fix: separates directory creation from permission checks
 (#13248)

(cherry picked from commit 1086783a1d8886f0e6888289975e771e18d800e6)
---
 pkg/storage/chunk/client/util/util.go         | 21 +++++++++++--
 pkg/storage/chunk/client/util/util_test.go    | 30 +++++++++++++++++++
 .../stores/shipper/bloomshipper/store.go      |  3 ++
 3 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 pkg/storage/chunk/client/util/util_test.go

diff --git a/pkg/storage/chunk/client/util/util.go b/pkg/storage/chunk/client/util/util.go
index 3485552c220fd..7b62b475caaf7 100644
--- a/pkg/storage/chunk/client/util/util.go
+++ b/pkg/storage/chunk/client/util/util.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 
 	ot "github.com/opentracing/opentracing-go"
@@ -67,17 +68,31 @@ func DoParallelQueries(
 
 // EnsureDirectory makes sure directory is there, if not creates it if not
 func EnsureDirectory(dir string) error {
+	return EnsureDirectoryWithDefaultPermissions(dir, 0o777)
+}
+
+func EnsureDirectoryWithDefaultPermissions(dir string, mode fs.FileMode) error {
 	info, err := os.Stat(dir)
 	if os.IsNotExist(err) {
-		return os.MkdirAll(dir, 0o777)
+		return os.MkdirAll(dir, mode)
 	} else if err == nil && !info.IsDir() {
 		return fmt.Errorf("not a directory: %s", dir)
-	} else if err == nil && info.Mode()&0700 != 0700 {
-		return fmt.Errorf("insufficient permissions: %s %s", dir, info.Mode())
 	}
 	return err
 }
 
+func RequirePermissions(path string, required fs.FileMode) error {
+	info, err := os.Stat(path)
+	if err != nil {
+		return err
+	}
+
+	if mode := info.Mode(); mode&required != required {
+		return fmt.Errorf("insufficient permissions for path %s: required %s but found %s", path, required.String(), mode.String())
+	}
+	return nil
+}
+
 // ReadCloserWithContextCancelFunc helps with cancelling the context when closing a ReadCloser.
 // NOTE: The consumer of ReadCloserWithContextCancelFunc should always call the Close method when it is done reading which otherwise could cause a resource leak.
 type ReadCloserWithContextCancelFunc struct {
diff --git a/pkg/storage/chunk/client/util/util_test.go b/pkg/storage/chunk/client/util/util_test.go
new file mode 100644
index 0000000000000..360194cefa160
--- /dev/null
+++ b/pkg/storage/chunk/client/util/util_test.go
@@ -0,0 +1,30 @@
+package util
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestEnsureDir(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Directory to be created by EnsureDir
+	dirPath := filepath.Join(tmpDir, "testdir")
+
+	// Ensure the directory does not exist before the test
+	if _, err := os.Stat(dirPath); !os.IsNotExist(err) {
+		t.Fatalf("Directory already exists: %v", err)
+	}
+
+	// create with default permissions
+	require.NoError(t, EnsureDirectoryWithDefaultPermissions(dirPath, 0o640))
+
+	// ensure the directory passes the permission check for more restrictive permissions
+	require.NoError(t, RequirePermissions(dirPath, 0o600))
+
+	// ensure the directory fails the permission check for less restrictive permissions
+	require.Error(t, RequirePermissions(dirPath, 0o660))
+}
diff --git a/pkg/storage/stores/shipper/bloomshipper/store.go b/pkg/storage/stores/shipper/bloomshipper/store.go
index 8b2ff3365d766..f2c77d7ac74e0 100644
--- a/pkg/storage/stores/shipper/bloomshipper/store.go
+++ b/pkg/storage/stores/shipper/bloomshipper/store.go
@@ -324,6 +324,9 @@ func NewBloomStore(
 		if err := util.EnsureDirectory(wd); err != nil {
 			return nil, errors.Wrapf(err, "failed to create working directory for bloom store: '%s'", wd)
 		}
+		if err := util.RequirePermissions(wd, 0o700); err != nil {
+			return nil, errors.Wrapf(err, "insufficient permissions on working directory for bloom store: '%s'", wd)
+		}
 	}
 
 	for _, periodicConfig := range periodicConfigs {