Allow setting read and write bits on mutable files

pkg/filesystem/virtual/file_allocator.go

@@ -7,5 +7,5 @@ package virtual
 // Files returned by this interface should have a link count of 1, and
 // are opened using the provided share access mask.
 type FileAllocator interface {
-	NewFile(isExecutable bool, size uint64, shareAccess ShareMask) (NativeLeaf, Status)
+	NewFile(permissions Permissions, size uint64, shareAccess ShareMask) (NativeLeaf, Status)
 }

pkg/filesystem/virtual/handle_allocating_file_allocator.go

@@ -16,8 +16,8 @@ func NewHandleAllocatingFileAllocator(base FileAllocator, allocator StatefulHand
 	}
 }
 
-func (fa *handleAllocatingFileAllocator) NewFile(isExecutable bool, size uint64, shareAccess ShareMask) (NativeLeaf, Status) {
-	leaf, s := fa.base.NewFile(isExecutable, size, shareAccess)
+func (fa *handleAllocatingFileAllocator) NewFile(permissions Permissions, size uint64, shareAccess ShareMask) (NativeLeaf, Status) {
+	leaf, s := fa.base.NewFile(permissions, size, shareAccess)
 	if s != StatusOK {
 		return nil, s
 	}

pkg/filesystem/virtual/in_memory_prepopulated_directory.go

@@ -686,17 +686,18 @@ func (i *inMemoryPrepopulatedDirectory) VirtualOpenChild(ctx context.Context, na
 
 	// Create new file with attributes provided.
 	var respected AttributesMask
-	isExecutable := false
-	if permissions, ok := createAttributes.GetPermissions(); ok {
+	permissions, ok := createAttributes.GetPermissions()
+	if ok {
 		respected |= AttributesMaskPermissions
-		isExecutable = permissions&PermissionsExecute != 0
+	} else {
+		permissions = PermissionsRead | PermissionsWrite
 	}
 	size := uint64(0)
 	if sizeBytes, ok := createAttributes.GetSizeBytes(); ok {
 		respected |= AttributesMaskSizeBytes
 		size = sizeBytes
 	}
-	leaf, s := i.subtree.fileAllocator.NewFile(isExecutable, size, shareAccess)
+	leaf, s := i.subtree.fileAllocator.NewFile(permissions, size, shareAccess)
 	if s != StatusOK {
 		return nil, 0, ChangeInfo{}, s
 	}

pkg/filesystem/virtual/in_memory_prepopulated_directory_test.go

@@ -391,7 +391,7 @@ func TestInMemoryPrepopulatedDirectoryInstallHooks(t *testing.T) {
 
 	// Validate that the top-level directory uses both the new file
 	// allocator and error logger.
-	fileAllocator2.EXPECT().NewFile(false, uint64(0), virtual.ShareMaskWrite).
+	fileAllocator2.EXPECT().NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, uint64(0), virtual.ShareMaskWrite).
 		Return(nil, virtual.StatusErrIO)
 	var attr virtual.Attributes
 	_, _, _, s := d.VirtualOpenChild(
@@ -409,7 +409,7 @@ func TestInMemoryPrepopulatedDirectoryInstallHooks(t *testing.T) {
 	inMemoryPrepopulatedDirectoryExpectMkdir(ctrl, handleAllocator)
 	child, err := d.CreateAndEnterPrepopulatedDirectory(path.MustNewComponent("dir"))
 	require.NoError(t, err)
-	fileAllocator2.EXPECT().NewFile(false, uint64(0), virtual.ShareMaskWrite).
+	fileAllocator2.EXPECT().NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, uint64(0), virtual.ShareMaskWrite).
 		Return(nil, virtual.StatusErrIO)
 	_, _, _, s = child.VirtualOpenChild(
 		ctx,
@@ -551,7 +551,7 @@ func TestInMemoryPrepopulatedDirectoryVirtualOpenChildAllocationFailure(t *testi
 
 	fileAllocator := mock.NewMockFileAllocator(ctrl)
 	symlinkFactory := mock.NewMockSymlinkFactory(ctrl)
-	fileAllocator.EXPECT().NewFile(false, uint64(0), virtual.ShareMaskWrite).
+	fileAllocator.EXPECT().NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, uint64(0), virtual.ShareMaskWrite).
 		Return(nil, virtual.StatusErrIO)
 	errorLogger := mock.NewMockErrorLogger(ctrl)
 	handleAllocator := mock.NewMockStatefulHandleAllocator(ctrl)
@@ -609,7 +609,7 @@ func TestInMemoryPrepopulatedDirectoryVirtualOpenChildSuccess(t *testing.T) {
 	fileAllocator := mock.NewMockFileAllocator(ctrl)
 	symlinkFactory := mock.NewMockSymlinkFactory(ctrl)
 	child := mock.NewMockNativeLeaf(ctrl)
-	fileAllocator.EXPECT().NewFile(false, uint64(0), virtual.ShareMaskWrite).
+	fileAllocator.EXPECT().NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, uint64(0), virtual.ShareMaskWrite).
 		Return(child, virtual.StatusOK)
 	child.EXPECT().VirtualGetAttributes(
 		ctx,

pkg/filesystem/virtual/pool_backed_file_allocator.go

@@ -72,7 +72,7 @@ func NewPoolBackedFileAllocator(pool re_filesystem.FilePool, errorLogger util.Er
 	}
 }
 
-func (fa *poolBackedFileAllocator) NewFile(isExecutable bool, size uint64, shareAccess ShareMask) (NativeLeaf, Status) {
+func (fa *poolBackedFileAllocator) NewFile(permissions Permissions, size uint64, shareAccess ShareMask) (NativeLeaf, Status) {
 	file, err := fa.pool.NewFile()
 	if err != nil {
 		fa.errorLogger.Log(util.StatusWrapf(err, "Failed to create new file"))
@@ -89,7 +89,7 @@ func (fa *poolBackedFileAllocator) NewFile(isExecutable bool, size uint64, share
 		errorLogger: fa.errorLogger,
 
 		file:           file,
-		isExecutable:   isExecutable,
+		permissions:    permissions,
 		size:           size,
 		referenceCount: 1,
 		cachedDigest:   digest.BadDigest,
@@ -103,7 +103,7 @@ type fileBackedFile struct {
 
 	lock                     sync.RWMutex
 	file                     filesystem.FileReadWriter
-	isExecutable             bool
+	permissions              Permissions
 	size                     uint64
 	referenceCount           uint
 	writableDescriptorsCount uint
@@ -343,7 +343,7 @@ func (f *fileBackedFile) AppendOutputPathPersistencyDirectoryNode(directory *out
 		directory.Files = append(directory.Files, &remoteexecution.FileNode{
 			Name:         name.String(),
 			Digest:       f.cachedDigest.GetProto(),
-			IsExecutable: f.isExecutable,
+			IsExecutable: f.permissions&PermissionsExecute != 0,
 		})
 	}
 }
@@ -382,11 +382,7 @@ func (f *fileBackedFile) virtualGetAttributesUnlocked(attributes *Attributes) {
 // obtained while picking up the file's lock.
 func (f *fileBackedFile) virtualGetAttributesLocked(attributes *Attributes) {
 	attributes.SetChangeID(f.changeID)
-	permissions := PermissionsRead | PermissionsWrite
-	if f.isExecutable {
-		permissions |= PermissionsExecute
-	}
-	attributes.SetPermissions(permissions)
+	attributes.SetPermissions(f.permissions)
 	attributes.SetSizeBytes(f.size)
 }
 
@@ -435,6 +431,12 @@ func (f *fileBackedFile) VirtualOpenSelf(ctx context.Context, shareAccess ShareM
 		return StatusErrStale
 	}
 
+	// Permissions checking.
+	if (shareAccess&^ShareMaskRead != 0 && f.permissions&(PermissionsRead|PermissionsExecute) == 0) ||
+		(shareAccess&^ShareMaskWrite != 0 && f.permissions&PermissionsWrite == 0) {
+		return StatusErrAccess
+	}
+
 	// Handling of O_TRUNC.
 	if options.Truncate {
 		if s := f.virtualTruncate(0); s != StatusOK {
@@ -509,7 +511,7 @@ func (f *fileBackedFile) VirtualSetAttributes(ctx context.Context, in *Attribute
 		}
 	}
 	if permissions, ok := in.GetPermissions(); ok {
-		f.isExecutable = (permissions & PermissionsExecute) != 0
+		f.permissions = permissions
 		f.changeID++
 	}
 

pkg/filesystem/virtual/pool_backed_file_allocator_test.go

@@ -35,7 +35,7 @@ func TestPoolBackedFileAllocatorGetBazelOutputServiceStat(t *testing.T) {
 	errorLogger := mock.NewMockErrorLogger(ctrl)
 
 	f, s := virtual.NewPoolBackedFileAllocator(pool, errorLogger).
-		NewFile(false, 0, virtual.ShareMaskRead|virtual.ShareMaskWrite)
+		NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, 0, virtual.ShareMaskRead|virtual.ShareMaskWrite)
 	require.Equal(t, virtual.StatusOK, s)
 
 	underlyingFile.EXPECT().WriteAt([]byte("Hello"), int64(0)).Return(5, nil)
@@ -185,7 +185,7 @@ func TestPoolBackedFileAllocatorVirtualSeek(t *testing.T) {
 	errorLogger := mock.NewMockErrorLogger(ctrl)
 
 	f, s := virtual.NewPoolBackedFileAllocator(pool, errorLogger).
-		NewFile(false, 0, virtual.ShareMaskRead|virtual.ShareMaskWrite)
+		NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, 0, virtual.ShareMaskRead|virtual.ShareMaskWrite)
 	require.Equal(t, virtual.StatusOK, s)
 
 	// Grow the file.
@@ -253,7 +253,7 @@ func TestPoolBackedFileAllocatorVirtualOpenSelfStaleAfterUnlink(t *testing.T) {
 	errorLogger := mock.NewMockErrorLogger(ctrl)
 
 	f, s := virtual.NewPoolBackedFileAllocator(pool, errorLogger).
-		NewFile(false, 0, virtual.ShareMaskWrite)
+		NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, 0, virtual.ShareMaskWrite)
 	require.Equal(t, virtual.StatusOK, s)
 
 	f.VirtualClose(virtual.ShareMaskWrite)
@@ -278,7 +278,7 @@ func TestPoolBackedFileAllocatorVirtualOpenSelfStaleAfterClose(t *testing.T) {
 	errorLogger := mock.NewMockErrorLogger(ctrl)
 
 	f, s := virtual.NewPoolBackedFileAllocator(pool, errorLogger).
-		NewFile(false, 0, virtual.ShareMaskWrite)
+		NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, 0, virtual.ShareMaskWrite)
 	require.Equal(t, virtual.StatusOK, s)
 
 	f.Unlink()
@@ -299,7 +299,7 @@ func TestPoolBackedFileAllocatorVirtualRead(t *testing.T) {
 	errorLogger := mock.NewMockErrorLogger(ctrl)
 
 	f, s := virtual.NewPoolBackedFileAllocator(pool, errorLogger).
-		NewFile(false, 0, virtual.ShareMaskRead|virtual.ShareMaskWrite)
+		NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, 0, virtual.ShareMaskRead|virtual.ShareMaskWrite)
 	require.Equal(t, virtual.StatusOK, s)
 
 	// Let initial tests assume an empty file.
@@ -378,7 +378,7 @@ func TestPoolBackedFileAllocatorFUSETruncateFailure(t *testing.T) {
 	errorLogger.EXPECT().Log(testutil.EqStatus(t, status.Error(codes.Unavailable, "Failed to truncate file to length 42: Storage backends offline")))
 
 	f, s := virtual.NewPoolBackedFileAllocator(pool, errorLogger).
-		NewFile(false, 0, virtual.ShareMaskWrite)
+		NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, 0, virtual.ShareMaskWrite)
 	require.Equal(t, virtual.StatusOK, s)
 
 	require.Equal(t, virtual.StatusErrIO, f.VirtualSetAttributes(
@@ -406,7 +406,7 @@ func TestPoolBackedFileAllocatorVirtualWriteFailure(t *testing.T) {
 	errorLogger.EXPECT().Log(testutil.EqStatus(t, status.Error(codes.Unavailable, "Failed to write to file at offset 42: Storage backends offline")))
 
 	f, s := virtual.NewPoolBackedFileAllocator(pool, errorLogger).
-		NewFile(false, 0, virtual.ShareMaskWrite)
+		NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, 0, virtual.ShareMaskWrite)
 	require.Equal(t, virtual.StatusOK, s)
 	_, s = f.VirtualWrite(p[:], 42)
 	require.Equal(t, virtual.StatusErrIO, s)
@@ -424,7 +424,7 @@ func TestPoolBackedFileAllocatorUploadFile(t *testing.T) {
 	errorLogger := mock.NewMockErrorLogger(ctrl)
 
 	f, s := virtual.NewPoolBackedFileAllocator(pool, errorLogger).
-		NewFile(false, 0, virtual.ShareMaskWrite)
+		NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, 0, virtual.ShareMaskWrite)
 	require.Equal(t, virtual.StatusOK, s)
 
 	// Initialize the file with the contents "Hello".
@@ -570,7 +570,7 @@ func TestPoolBackedFileAllocatorVirtualClose(t *testing.T) {
 	errorLogger := mock.NewMockErrorLogger(ctrl)
 
 	f, s := virtual.NewPoolBackedFileAllocator(pool, errorLogger).
-		NewFile(false, 0, virtual.ShareMaskWrite)
+		NewFile(virtual.PermissionsRead|virtual.PermissionsWrite, 0, virtual.ShareMaskWrite)
 	require.Equal(t, virtual.StatusOK, s)
 
 	// Initially it should be opened exactly once. Open it a couple