From b84b795f238a4b089d07c22a67b1a53633132239 Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Tue, 5 Dec 2023 19:39:45 -0600 Subject: [PATCH] Relax file permissions check on existing directories (#13274) --- beacon-chain/db/filesystem/blob_test.go | 7 ------- io/file/fileutil.go | 15 ++++----------- io/file/fileutil_test.go | 8 -------- 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/beacon-chain/db/filesystem/blob_test.go b/beacon-chain/db/filesystem/blob_test.go index 6cc740568..347fd3635 100644 --- a/beacon-chain/db/filesystem/blob_test.go +++ b/beacon-chain/db/filesystem/blob_test.go @@ -2,7 +2,6 @@ package filesystem import ( "bytes" - "os" "path" "testing" @@ -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) } diff --git a/io/file/fileutil.go b/io/file/fileutil.go index fc98d4ddb..4555200ac 100644 --- a/io/file/fileutil.go +++ b/io/file/fileutil.go @@ -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) @@ -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) } diff --git a/io/file/fileutil_test.go b/io/file/fileutil_test.go index d1a7e7c48..8ee8f7bc3 100644 --- a/io/file/fileutil_test.go +++ b/io/file/fileutil_test.go @@ -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)