Add configuration options for FileMode (#183)

* Add configuration options for FileMode

Add two additional configuration values, and their corresponding default values:

* DirFileModeBeforeUmask - Dir FileMode is used on all directories created.  DefaultDirFileModeBeforeUmask is 0700.
* FileFileModeBeforeUmask - File FileMode is used on all files created, except for the "lock" file (managed by the Flock library).  DefaultFileFileModeBeforeUmask is 0600.

When using these bits of configuration, keep in mind these FileMode values are set BEFORE any umask rules are applied.  For example, if the user's umask is 022, setting DirFileFileModeBeforeUmask to 777 will result in directories with FileMode set to 755 (this umask prevents the write bit from being applied to group and world permissions).

* moving defer statements after checking for errors

use os.ModePerm const instead of os.FileMode(777)

* fix spelling/grammar

* skip these tests for Windows as they appear to break - Windows is less POSIX-y than it claims

* ignore "lock" file for default case too -- this was incorrectly passing before including this, as my local dev station has umask 022

.github/workflows/greetings.yml

@@ -16,8 +16,8 @@ jobs:
           and [Code of Conduct](https://github.com/prologic/bitcask/blob/master/CODE_OF_CONDUCT.md)
           documents.
           
-          if possible please also make sure your Bug Report or Feature Request is clearly defined
-          with either examples or a reprodicible case (_if a bug_).
+          If possible please also make sure your Bug Report or Feature Request is clearly defined
+          with either examples or a reproducible case (_if a bug_).
           
           Thank you 😃
         pr-message: |-

AUTHORS

@@ -4,6 +4,7 @@
 
 Alain Gilbert <alain.gilbert.15@gmail.com>
 Awn Umar <awn@spacetime.dev>
+Bryan Stenson <bryan@siliconvortex.com>
 Christian Muehlhaeuser <muesli@gmail.com>
 Ignacio Hagopian <jsign.uy@gmail.com>
 James Mills <prologic@shortcircuit.net.au>

bitcask.go

@@ -284,7 +284,7 @@ func (b *Bitcask) put(key, value []byte) (int64, int64, error) {
 
 		id := b.curr.FileID()
 
-		df, err := data.NewDatafile(b.path, id, true, b.config.MaxKeySize, b.config.MaxValueSize)
+		df, err := data.NewDatafile(b.path, id, true, b.config.MaxKeySize, b.config.MaxValueSize, b.config.FileFileModeBeforeUmask)
 		if err != nil {
 			return -1, 0, err
 		}
@@ -292,7 +292,7 @@ func (b *Bitcask) put(key, value []byte) (int64, int64, error) {
 		b.datafiles[id] = df
 
 		id = b.curr.FileID() + 1
-		curr, err := data.NewDatafile(b.path, id, false, b.config.MaxKeySize, b.config.MaxValueSize)
+		curr, err := data.NewDatafile(b.path, id, false, b.config.MaxKeySize, b.config.MaxValueSize, b.config.FileFileModeBeforeUmask)
 		if err != nil {
 			return -1, 0, err
 		}
@@ -307,7 +307,7 @@ func (b *Bitcask) Reopen() error {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 
-	datafiles, lastID, err := loadDatafiles(b.path, b.config.MaxKeySize, b.config.MaxValueSize)
+	datafiles, lastID, err := loadDatafiles(b.path, b.config.MaxKeySize, b.config.MaxValueSize, b.config.FileFileModeBeforeUmask)
 	if err != nil {
 		return err
 	}
@@ -316,7 +316,7 @@ func (b *Bitcask) Reopen() error {
 		return err
 	}
 
-	curr, err := data.NewDatafile(b.path, lastID, false, b.config.MaxKeySize, b.config.MaxValueSize)
+	curr, err := data.NewDatafile(b.path, lastID, false, b.config.MaxKeySize, b.config.MaxValueSize, b.config.FileFileModeBeforeUmask)
 	if err != nil {
 		return err
 	}
@@ -417,10 +417,6 @@ func Open(path string, options ...Option) (*Bitcask, error) {
 		err error
 	)
 
-	if err := os.MkdirAll(path, 0755); err != nil {
-		return nil, err
-	}
-
 	configPath := filepath.Join(path, "config.json")
 	if internal.Exists(configPath) {
 		cfg, err = config.Load(configPath)
@@ -431,6 +427,16 @@ func Open(path string, options ...Option) (*Bitcask, error) {
 		cfg = newDefaultConfig()
 	}
 
+	for _, opt := range options {
+		if err := opt(cfg); err != nil {
+			return nil, err
+		}
+	}
+
+	if err := os.MkdirAll(path, cfg.DirFileModeBeforeUmask); err != nil {
+		return nil, err
+	}
+
 	bitcask := &Bitcask{
 		Flock:   flock.New(filepath.Join(path, "lock")),
 		config:  cfg,
@@ -439,12 +445,6 @@ func Open(path string, options ...Option) (*Bitcask, error) {
 		indexer: index.NewIndexer(),
 	}
 
-	for _, opt := range options {
-		if err := opt(bitcask.config); err != nil {
-			return nil, err
-		}
-	}
-
 	locked, err := bitcask.Flock.TryLock()
 	if err != nil {
 		return nil, err
@@ -470,7 +470,7 @@ func Open(path string, options ...Option) (*Bitcask, error) {
 	return bitcask, nil
 }
 
-func loadDatafiles(path string, maxKeySize uint32, maxValueSize uint64) (datafiles map[int]data.Datafile, lastID int, err error) {
+func loadDatafiles(path string, maxKeySize uint32, maxValueSize uint64, fileModeBeforeUmask os.FileMode) (datafiles map[int]data.Datafile, lastID int, err error) {
 	fns, err := internal.GetDatafiles(path)
 	if err != nil {
 		return nil, 0, err
@@ -483,7 +483,7 @@ func loadDatafiles(path string, maxKeySize uint32, maxValueSize uint64) (datafil
 
 	datafiles = make(map[int]data.Datafile, len(ids))
 	for _, id := range ids {
-		datafiles[id], err = data.NewDatafile(path, id, true, maxKeySize, maxValueSize)
+		datafiles[id], err = data.NewDatafile(path, id, true, maxKeySize, maxValueSize, fileModeBeforeUmask)
 		if err != nil {
 			return
 		}

bitcask_test.go

@@ -723,6 +723,166 @@ func TestStatsError(t *testing.T) {
 	})
 }
 
+func TestDirFileModeBeforeUmask(t *testing.T) {
+	skipIfWindows(t)
+
+	assert := assert.New(t)
+
+	t.Run("Setup", func(t *testing.T) {
+		t.Run("Default DirFileModeBeforeUmask is 0700", func(t *testing.T) {
+			testdir, err := ioutil.TempDir("", "bitcask")
+			embeddedDir := filepath.Join(testdir, "cache")
+			assert.NoError(err)
+			defer os.RemoveAll(testdir)
+
+			defaultTestMode := os.FileMode(0700)
+
+			db, err := Open(embeddedDir)
+			assert.NoError(err)
+			defer db.Close()
+			err = filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error {
+				// skip the root directory
+				if path == testdir {
+					return nil
+				}
+				if info.IsDir() {
+					// perms for directory on disk are filtered through defaultTestMode, AND umask of user running test.
+					// this means the mkdir calls can only FURTHER restrict permissions, not grant more (preventing escalatation).
+					// to make this test OS agnostic, we'll skip using golang.org/x/sys/unix, inferring umask via XOR and AND NOT.
+
+					// create anotherDir with allPerms - to infer umask
+					anotherDir := filepath.Join(testdir, "temp")
+					err := os.Mkdir(anotherDir, os.ModePerm)
+					assert.NoError(err)
+					defer os.RemoveAll(anotherDir)
+
+					anotherStat, err := os.Stat(anotherDir)
+					assert.NoError(err)
+
+					// infer umask from anotherDir
+					umask := os.ModePerm ^ (anotherStat.Mode() & os.ModePerm)
+
+					assert.Equal(info.Mode()&os.ModePerm, defaultTestMode&^umask)
+				}
+				return nil
+			})
+			assert.NoError(err)
+		})
+
+		t.Run("Dir FileModeBeforeUmask is set via options for all subdirectories", func(t *testing.T) {
+			testdir, err := ioutil.TempDir("", "bitcask")
+			embeddedDir := filepath.Join(testdir, "cache")
+			assert.NoError(err)
+			defer os.RemoveAll(testdir)
+
+			testMode := os.FileMode(0713)
+
+			db, err := Open(embeddedDir, WithDirFileModeBeforeUmask(testMode))
+			assert.NoError(err)
+			defer db.Close()
+			err = filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error {
+				// skip the root directory
+				if path == testdir {
+					return nil
+				}
+				if info.IsDir() {
+					// create anotherDir with allPerms - to infer umask
+					anotherDir := filepath.Join(testdir, "temp")
+					err := os.Mkdir(anotherDir, os.ModePerm)
+					assert.NoError(err)
+					defer os.RemoveAll(anotherDir)
+
+					anotherStat, _ := os.Stat(anotherDir)
+
+					// infer umask from anotherDir
+					umask := os.ModePerm ^ (anotherStat.Mode() & os.ModePerm)
+
+					assert.Equal(info.Mode()&os.ModePerm, testMode&^umask)
+				}
+				return nil
+			})
+			assert.NoError(err)
+		})
+
+	})
+}
+
+func TestFileFileModeBeforeUmask(t *testing.T) {
+	skipIfWindows(t)
+
+	assert := assert.New(t)
+
+	t.Run("Setup", func(t *testing.T) {
+		t.Run("Default File FileModeBeforeUmask is 0600", func(t *testing.T) {
+			testdir, err := ioutil.TempDir("", "bitcask")
+			assert.NoError(err)
+			defer os.RemoveAll(testdir)
+
+			defaultTestMode := os.FileMode(0600)
+
+			db, err := Open(testdir)
+			assert.NoError(err)
+			defer db.Close()
+			err = filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error {
+				if !info.IsDir() {
+					// the lock file is set within Flock, so ignore it
+					if filepath.Base(path) == "lock" {
+						return nil
+					}
+					// create aFile with allPerms - to infer umask
+					aFilePath := filepath.Join(testdir, "temp")
+					_, err := os.OpenFile(aFilePath, os.O_CREATE, os.ModePerm)
+					assert.NoError(err)
+					defer os.RemoveAll(aFilePath)
+
+					fileStat, _ := os.Stat(aFilePath)
+
+					// infer umask from anotherDir
+					umask := os.ModePerm ^ (fileStat.Mode() & os.ModePerm)
+
+					assert.Equal(info.Mode()&os.ModePerm, defaultTestMode&^umask)
+				}
+				return nil
+			})
+			assert.NoError(err)
+		})
+
+		t.Run("File FileModeBeforeUmask is set via options for all files", func(t *testing.T) {
+			testdir, err := ioutil.TempDir("", "bitcask")
+			assert.NoError(err)
+			defer os.RemoveAll(testdir)
+
+			testMode := os.FileMode(0673)
+
+			db, err := Open(testdir, WithFileFileModeBeforeUmask(testMode))
+			assert.NoError(err)
+			defer db.Close()
+			err = filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error {
+				if !info.IsDir() {
+					// the lock file is set within Flock, so ignore it
+					if filepath.Base(path) == "lock" {
+						return nil
+					}
+					// create aFile with allPerms - to infer umask
+					aFilePath := filepath.Join(testdir, "temp")
+					_, err := os.OpenFile(aFilePath, os.O_CREATE, os.ModePerm)
+					assert.NoError(err)
+					defer os.RemoveAll(aFilePath)
+
+					fileStat, _ := os.Stat(aFilePath)
+
+					// infer umask from anotherDir
+					umask := os.ModePerm ^ (fileStat.Mode() & os.ModePerm)
+
+					assert.Equal(info.Mode()&os.ModePerm, testMode&^umask)
+				}
+				return nil
+			})
+			assert.NoError(err)
+		})
+	})
+}
+
 func TestMaxDatafileSize(t *testing.T) {
 	var (
 		db  *Bitcask

go.sum

@@ -236,6 +236,7 @@ github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s
 github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
 github.com/tidwall/redcon v1.3.2 h1:8INx/Nm3VSUbDUT16TH1rMgYQsbXNqy9xcX70edHXbo=
 github.com/tidwall/redcon v1.3.2/go.mod h1:bdYBm4rlcWpst2XMwKVzWDF9CoUxEbUmM7CQrKeOZas=
+github.com/tidwall/redcon v1.3.3 h1:BkvIOVPSMw7JnPlyjaSCckgEhvXCy3U5GgKkssveax4=
 github.com/tidwall/redcon v1.3.3/go.mod h1:bdYBm4rlcWpst2XMwKVzWDF9CoUxEbUmM7CQrKeOZas=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=

internal/config/config.go

@@ -3,15 +3,18 @@ package config
 import (
 	"encoding/json"
 	"io/ioutil"
+	"os"
 )
 
 // Config contains the bitcask configuration parameters
 type Config struct {
-	MaxDatafileSize int    `json:"max_datafile_size"`
-	MaxKeySize      uint32 `json:"max_key_size"`
-	MaxValueSize    uint64 `json:"max_value_size"`
-	Sync            bool   `json:"sync"`
-	AutoRecovery    bool   `json:"autorecovery"`
+	MaxDatafileSize         int    `json:"max_datafile_size"`
+	MaxKeySize              uint32 `json:"max_key_size"`
+	MaxValueSize            uint64 `json:"max_value_size"`
+	Sync                    bool   `json:"sync"`
+	AutoRecovery            bool   `json:"autorecovery"`
+	DirFileModeBeforeUmask  os.FileMode
+	FileFileModeBeforeUmask os.FileMode
 }
 
 // Load loads a configuration from the given path
@@ -38,10 +41,10 @@ func (c *Config) Save(path string) error {
 		return err
 	}
 
-	err = ioutil.WriteFile(path, data, 0600)
+	err = ioutil.WriteFile(path, data, c.FileFileModeBeforeUmask)
 	if err != nil {
 		return err
 	}
-	
+
 	return nil
 }

internal/data/datafile.go

@@ -48,7 +48,7 @@ type datafile struct {
 }
 
 // NewDatafile opens an existing datafile
-func NewDatafile(path string, id int, readonly bool, maxKeySize uint32, maxValueSize uint64) (Datafile, error) {
+func NewDatafile(path string, id int, readonly bool, maxKeySize uint32, maxValueSize uint64, fileMode os.FileMode) (Datafile, error) {
 	var (
 		r   *os.File
 		ra  *mmap.ReaderAt
@@ -59,7 +59,7 @@ func NewDatafile(path string, id int, readonly bool, maxKeySize uint32, maxValue
 	fn := filepath.Join(path, fmt.Sprintf(defaultDatafileFilename, id))
 
 	if !readonly {
-		w, err = os.OpenFile(fn, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0640)
+		w, err = os.OpenFile(fn, os.O_WRONLY|os.O_APPEND|os.O_CREATE, fileMode)
 		if err != nil {
 			return nil, err
 		}

options.go

@@ -1,8 +1,18 @@
 package bitcask
 
-import "github.com/prologic/bitcask/internal/config"
+import (
+	"os"
+
+	"github.com/prologic/bitcask/internal/config"
+)
 
 const (
+	// DefaultDirFileModeBeforeUmask is the default os.FileMode used when creating directories
+	DefaultDirFileModeBeforeUmask = os.FileMode(0700)
+
+	// DefaultFileFileModeBeforeUmask is the default os.FileMode used when creating files
+	DefaultFileFileModeBeforeUmask = os.FileMode(0600)
+
 	// DefaultMaxDatafileSize is the default maximum datafile size in bytes
 	DefaultMaxDatafileSize = 1 << 20 // 1MB
 
@@ -31,6 +41,22 @@ func WithAutoRecovery(enabled bool) Option {
 	}
 }
 
+// WithDirFileModeBeforeUmask sets the FileMode used for each new file created.
+func WithDirFileModeBeforeUmask(mode os.FileMode) Option {
+	return func(cfg *config.Config) error {
+		cfg.DirFileModeBeforeUmask = mode
+		return nil
+	}
+}
+
+// WithFileFileModeBeforeUmask sets the FileMode used for each new file created.
+func WithFileFileModeBeforeUmask(mode os.FileMode) Option {
+	return func(cfg *config.Config) error {
+		cfg.FileFileModeBeforeUmask = mode
+		return nil
+	}
+}
+
 // WithMaxDatafileSize sets the maximum datafile size option
 func WithMaxDatafileSize(size int) Option {
 	return func(cfg *config.Config) error {
@@ -66,9 +92,11 @@ func WithSync(sync bool) Option {
 
 func newDefaultConfig() *config.Config {
 	return &config.Config{
-		MaxDatafileSize: DefaultMaxDatafileSize,
-		MaxKeySize:      DefaultMaxKeySize,
-		MaxValueSize:    DefaultMaxValueSize,
-		Sync:            DefaultSync,
+		MaxDatafileSize:         DefaultMaxDatafileSize,
+		MaxKeySize:              DefaultMaxKeySize,
+		MaxValueSize:            DefaultMaxValueSize,
+		Sync:                    DefaultSync,
+		DirFileModeBeforeUmask:  DefaultDirFileModeBeforeUmask,
+		FileFileModeBeforeUmask: DefaultFileFileModeBeforeUmask,
 	}
 }