Clarify permissions check naming and comment, add tests

client/filejsonstore/filejsonstore.go

@@ -71,7 +71,7 @@ func NewFileJSONStore(baseDir string) (*FileJSONStore, error) {
 				baseDir)
 		}
 		// Verify file mode is not too permissive.
-		if err = fsutil.EnsurePermission(fi, dirCreateMode); err != nil {
+		if err = fsutil.EnsureMaxPermissions(fi, dirCreateMode); err != nil {
 			return nil, ErrTooPermissive
 		}
 	}
@@ -104,7 +104,7 @@ func (f *FileJSONStore) GetMeta() (map[string]json.RawMessage, error) {
 		if err != nil {
 			return nil, err
 		}
-		if err = fsutil.EnsurePermission(info, fileCreateMode); err != nil {
+		if err = fsutil.EnsureMaxPermissions(info, fileCreateMode); err != nil {
 			return nil, ErrTooPermissive
 		}
 		b, err := os.ReadFile(p)

internal/fsutil/fsutil.go

@@ -3,13 +3,10 @@
 package fsutil
 
 import (
-	"errors"
 	"os"
 	"path/filepath"
 )
 
-var ErrPermission = errors.New("unexpected permission")
-
 // IsMetaFile tests wheter a DirEntry appears to be a metaddata file or not.
 func IsMetaFile(e os.DirEntry) (bool, error) {
 	if e.IsDir() || filepath.Ext(e.Name()) != ".json" {

internal/fsutil/perm.go

@@ -4,18 +4,26 @@
 package fsutil
 
 import (
-	"io/fs"
+	"fmt"
 	"os"
 )
 
-// EnsurePemissions tests the provided file info to make sure the
-// permission bits matches the provided.
-func EnsurePermission(fi os.FileInfo, perm os.FileMode) error {
-	// Clear all bits which are note related to the permission.
-	mode := fi.Mode() & fs.ModePerm
-	mask := ^perm
-	if (mode & mask) != 0 {
-		return ErrPermission
+// EnsureMaxPermissions tests the provided file info, returning an error if the
+// file's permission bits contain excess permissions not set in maxPerms.
+//
+// For example, a file with permissions -rw------- will successfully validate
+// with maxPerms -rw-r--r-- or -rw-rw-r--, but will not validate with maxPerms
+// -r-------- (due to excess --w------- permission) or --w------- (due to
+// excess -r-------- permission).
+//
+// Only permission bits of the file modes are considered.
+func EnsureMaxPermissions(fi os.FileInfo, maxPerms os.FileMode) error {
+	gotPerm := fi.Mode().Perm()
+	forbiddenPerms := (^maxPerms).Perm()
+	excessPerms := gotPerm & forbiddenPerms
+
+	if excessPerms != 0 {
+		return fmt.Errorf("permission bits for file %v failed validation: want at most %v, got %v with excess perms %v", fi.Name(), maxPerms.Perm(), gotPerm, excessPerms)
 	}
 
 	return nil

internal/fsutil/perm_test.go

@@ -0,0 +1,71 @@
+//go:build !windows
+// +build !windows
+
+package fsutil
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestEnsureMaxPermissions(t *testing.T) {
+	tmp := t.TempDir()
+	p := filepath.Join(tmp, "file.txt")
+
+	// Start with 0644 and change using os.Chmod so umask doesn't interfere.
+	err := os.WriteFile(p, []byte(`AAA`), 0644)
+	assert.NoError(t, err)
+
+	// Check matching (1)
+	err = os.Chmod(p, 0464)
+	assert.NoError(t, err)
+	fi, err := os.Stat(p)
+	assert.NoError(t, err)
+	err = EnsureMaxPermissions(fi, os.FileMode(0464))
+	assert.NoError(t, err)
+
+	// Check matching (2)
+	err = os.Chmod(p, 0642)
+	assert.NoError(t, err)
+	fi, err = os.Stat(p)
+	assert.NoError(t, err)
+	err = EnsureMaxPermissions(fi, os.FileMode(0642))
+	assert.NoError(t, err)
+
+	// Check matching with file mode bits
+	err = os.Chmod(p, 0444)
+	assert.NoError(t, err)
+	fi, err = os.Stat(p)
+	assert.NoError(t, err)
+	err = EnsureMaxPermissions(fi, os.ModeSymlink|os.ModeAppend|os.FileMode(0444))
+	assert.NoError(t, err)
+
+	// Check not matching (1)
+	err = os.Chmod(p, 0444)
+	assert.NoError(t, err)
+	fi, err = os.Stat(p)
+	assert.NoError(t, err)
+	err = EnsureMaxPermissions(fi, os.FileMode(0400))
+	assert.Error(t, err)
+
+	// Check not matching (2)
+	err = os.Chmod(p, 0444)
+	assert.NoError(t, err)
+	fi, err = os.Stat(p)
+	assert.NoError(t, err)
+	err = EnsureMaxPermissions(fi, os.FileMode(0222))
+	assert.Error(t, err)
+	fmt.Println(err)
+
+	// Check matching due to more restrictive perms on file
+	err = os.Chmod(p, 0444)
+	assert.NoError(t, err)
+	fi, err = os.Stat(p)
+	assert.NoError(t, err)
+	err = EnsureMaxPermissions(fi, os.FileMode(0666))
+	assert.NoError(t, err)
+}

internal/fsutil/perm_windows.go

@@ -4,11 +4,11 @@ import (
 	"os"
 )
 
-// EnsurePemissions tests the provided file info to make sure the
+// EnsureMaxPermissions tests the provided file info to make sure the
 // permission bits matches the provided.
 // On Windows system the permission bits are not really compatible with
 // UNIX-like permission bits.
 // Currently this method will always return nil.
-func EnsurePermission(fi os.FileInfo, perm os.FileMode) error {
+func EnsureMaxPermissions(fi os.FileInfo, perm os.FileMode) error {
 	return nil
 }