Skip to content

Commit

Permalink
fix: separates directory creation from permission checks (#13248)
Browse files Browse the repository at this point in the history
  • Loading branch information
owen-d authored Jun 17, 2024
1 parent 5e560f9 commit 1086783
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 3 deletions.
21 changes: 18 additions & 3 deletions pkg/storage/chunk/client/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"io"
"io/fs"
"os"

ot "github.com/opentracing/opentracing-go"
Expand Down Expand Up @@ -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 {
Expand Down
30 changes: 30 additions & 0 deletions pkg/storage/chunk/client/util/util_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
3 changes: 3 additions & 0 deletions pkg/storage/stores/shipper/bloomshipper/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 1086783

Please sign in to comment.