Skip to content

Commit

Permalink
Relax file permissions check on existing directories (#13274)
Browse files Browse the repository at this point in the history
  • Loading branch information
prestonvanloon authored Dec 6, 2023
1 parent f40b858 commit b84b795
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 26 deletions.
7 changes: 0 additions & 7 deletions beacon-chain/db/filesystem/blob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package filesystem

import (
"bytes"
"os"
"path"
"testing"

Expand Down Expand Up @@ -206,10 +205,4 @@ func TestBlobStorageDelete(t *testing.T) {
func TestNewBlobStorage(t *testing.T) {
_, err := NewBlobStorage(path.Join(t.TempDir(), "good"))
require.NoError(t, err)

// If directory already exists with improper permissions, expect a wrapped err message.
fp := path.Join(t.TempDir(), "bad")
require.NoError(t, os.Mkdir(fp, 0777))
_, err = NewBlobStorage(fp)
require.ErrorContains(t, "failed to create blob storage", err)
}
15 changes: 4 additions & 11 deletions io/file/fileutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,9 @@ func HandleBackupDir(dirPath string, permissionOverride bool) error {
return os.MkdirAll(expanded, params.BeaconIoConfig().ReadWriteExecutePermissions)
}

// MkdirAll takes in a path, expands it if necessary, and looks through the
// permissions of every directory along the path, ensuring we are not attempting
// to overwrite any existing permissions. Finally, creates the directory accordingly
// with standardized, Prysm project permissions. This is the static-analysis enforced
// MkdirAll takes in a path, expands it if necessary, and creates the directory accordingly
// with standardized, Prysm project permissions. If a directory already exists as this path,
// then the method returns without making any changes. This is the static-analysis enforced
// method for creating a directory programmatically in Prysm.
func MkdirAll(dirPath string) error {
expanded, err := ExpandPath(dirPath)
Expand All @@ -74,13 +73,7 @@ func MkdirAll(dirPath string) error {
return err
}
if exists {
info, err := os.Stat(expanded)
if err != nil {
return err
}
if info.Mode().Perm() != params.BeaconIoConfig().ReadWriteExecutePermissions {
return errors.New("dir already exists without proper 0700 permissions")
}
return nil
}
return os.MkdirAll(expanded, params.BeaconIoConfig().ReadWriteExecutePermissions)
}
Expand Down
8 changes: 0 additions & 8 deletions io/file/fileutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,6 @@ func TestPathExpansion(t *testing.T) {
}
}

func TestMkdirAll_AlreadyExists_WrongPermissions(t *testing.T) {
dirName := t.TempDir() + "somedir"
err := os.MkdirAll(dirName, os.ModePerm)
require.NoError(t, err)
err = file.MkdirAll(dirName)
assert.ErrorContains(t, "already exists without proper 0700 permissions", err)
}

func TestMkdirAll_AlreadyExists_Override(t *testing.T) {
dirName := t.TempDir() + "somedir"
err := os.MkdirAll(dirName, params.BeaconIoConfig().ReadWriteExecutePermissions)
Expand Down

0 comments on commit b84b795

Please sign in to comment.