sftp: enforce file ACLs on rename targets (#11770)

* sftp: check delete permission on an existing rename target

renameEntry authorized only the source path and the target directory,
so renaming over a file with a more restrictive ACL destroyed it even
though Remove on the same file was denied.

* sftp: deny a rename that would strand a carved-out ACL

A rename moves data without moving its path-based ACL: renaming an
ancestor directory left a more specific read-only entry behind, so the
relocated file fell under a permissive ancestor ACL and became
writable. Deny the rename when a configured ACL under the source would
grant less write/delete access than the relocated path would get.

* sftp: cover inherited and destination-side ACLs on rename

The rename ACL check compared only configured paths under the source,
so it missed restrictions inherited from ancestors above the source and
deeper grants configured under the destination. It now compares the
moved root plus every configured path under either tree mapped to its
source counterpart, and skips paths whose entries do not actually move.
The target delete check no longer depends on an existence lookup, so a
protected target created during a rename cannot be overwritten; a no-op
rename skips it. Root bypasses the comparison like every other check.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* sftp: fail closed on rename ACL lookups and restore new-name renames

- checkRenameKeepsAcl returns lookup errors instead of treating them as a missing entry, so a transient filer failure cannot skip the ACL comparison; it also compares read and list gains, not only write and delete, and evaluates each pair with the moved entry's own IsDirectory.

- The target delete check now distinguishes an existing target (still requires delete) from a fresh name (no delete needed) while still enforcing a configured ACL on the target name itself, so a concurrent create cannot slip a protected target under the rename.

* filer: add fail_if_target_exists option to AtomicRenameEntry

Callers that authorize a rename while the target is absent cannot see a
target created in the gap before the move runs. The flag makes the filer
reject the rename when the target exists, checked under the rename path
locks that CreateEntry on the same filer also takes.

* sftp: guard rename against concurrent targets and file-child ACL gains

A rename onto a name that inherits its directory's ACL still needs no
delete access, but now asks the filer to fail if a target appears before
the move, so a concurrently created file cannot be overwritten without
delete permission. Moved directories are also compared in the file ACL
shape, where list no longer counts as read, so their file children cannot
gain read access through the relocation.

---------

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

other/java/client/src/main/proto/filer.proto

@@ -527,6 +527,10 @@ message AtomicRenameEntryRequest {
     string new_directory = 3;
     string new_name = 4;
     repeated int32 signatures = 5;
+    // When true, the rename fails with ErrEntryAlreadyExists if the target
+    // already exists. Lets a caller that authorized the rename without delete
+    // access on an absent target stay safe if one is created concurrently.
+    bool fail_if_target_exists = 6;
 }
 
 message AtomicRenameEntryResponse {

weed/pb/filer.proto

@@ -535,6 +535,10 @@ message AtomicRenameEntryRequest {
     string new_directory = 3;
     string new_name = 4;
     repeated int32 signatures = 5;
+    // When true, the rename fails with ErrEntryAlreadyExists if the target
+    // already exists. Lets a caller that authorized the rename without delete
+    // access on an absent target stay safe if one is created concurrently.
+    bool fail_if_target_exists = 6;
 }
 
 message AtomicRenameEntryResponse {

weed/pb/filer_pb/filer.pb.go

@@ -2921,14 +2921,18 @@ func (x *DeleteEntryResponse) GetMetadataEvent() *SubscribeMetadataResponse {
 }
 
 type AtomicRenameEntryRequest struct {
-	state         protoimpl.MessageState `protogen:"open.v1"`
-	OldDirectory  string                 `protobuf:"bytes,1,opt,name=old_directory,json=oldDirectory,proto3" json:"old_directory,omitempty"`
-	OldName       string                 `protobuf:"bytes,2,opt,name=old_name,json=oldName,proto3" json:"old_name,omitempty"`
-	NewDirectory  string                 `protobuf:"bytes,3,opt,name=new_directory,json=newDirectory,proto3" json:"new_directory,omitempty"`
-	NewName       string                 `protobuf:"bytes,4,opt,name=new_name,json=newName,proto3" json:"new_name,omitempty"`
-	Signatures    []int32                `protobuf:"varint,5,rep,packed,name=signatures,proto3" json:"signatures,omitempty"`
-	unknownFields protoimpl.UnknownFields
-	sizeCache     protoimpl.SizeCache
+	state        protoimpl.MessageState `protogen:"open.v1"`
+	OldDirectory string                 `protobuf:"bytes,1,opt,name=old_directory,json=oldDirectory,proto3" json:"old_directory,omitempty"`
+	OldName      string                 `protobuf:"bytes,2,opt,name=old_name,json=oldName,proto3" json:"old_name,omitempty"`
+	NewDirectory string                 `protobuf:"bytes,3,opt,name=new_directory,json=newDirectory,proto3" json:"new_directory,omitempty"`
+	NewName      string                 `protobuf:"bytes,4,opt,name=new_name,json=newName,proto3" json:"new_name,omitempty"`
+	Signatures   []int32                `protobuf:"varint,5,rep,packed,name=signatures,proto3" json:"signatures,omitempty"`
+	// When true, the rename fails with ErrEntryAlreadyExists if the target
+	// already exists. Lets a caller that authorized the rename without delete
+	// access on an absent target stay safe if one is created concurrently.
+	FailIfTargetExists bool `protobuf:"varint,6,opt,name=fail_if_target_exists,json=failIfTargetExists,proto3" json:"fail_if_target_exists,omitempty"`
+	unknownFields      protoimpl.UnknownFields
+	sizeCache          protoimpl.SizeCache
 }
 
 func (x *AtomicRenameEntryRequest) Reset() {
@@ -2996,6 +3000,13 @@ func (x *AtomicRenameEntryRequest) GetSignatures() []int32 {
 	return nil
 }
 
+func (x *AtomicRenameEntryRequest) GetFailIfTargetExists() bool {
+	if x != nil {
+		return x.FailIfTargetExists
+	}
+	return false
+}
+
 type AtomicRenameEntryResponse struct {
 	state         protoimpl.MessageState `protogen:"open.v1"`
 	unknownFields protoimpl.UnknownFields
@@ -7544,7 +7555,7 @@ const file_filer_proto_rawDesc = "" +
 	" \x01(\bR\x10keepRemoteObject\"w\n" +
 	"\x13DeleteEntryResponse\x12\x14\n" +
 	"\x05error\x18\x01 \x01(\tR\x05error\x12J\n" +
-	"\x0emetadata_event\x18\x02 \x01(\v2#.filer_pb.SubscribeMetadataResponseR\rmetadataEvent\"\xba\x01\n" +
+	"\x0emetadata_event\x18\x02 \x01(\v2#.filer_pb.SubscribeMetadataResponseR\rmetadataEvent\"\xed\x01\n" +
 	"\x18AtomicRenameEntryRequest\x12#\n" +
 	"\rold_directory\x18\x01 \x01(\tR\foldDirectory\x12\x19\n" +
 	"\bold_name\x18\x02 \x01(\tR\aoldName\x12#\n" +
@@ -7552,7 +7563,8 @@ const file_filer_proto_rawDesc = "" +
 	"\bnew_name\x18\x04 \x01(\tR\anewName\x12\x1e\n" +
 	"\n" +
 	"signatures\x18\x05 \x03(\x05R\n" +
-	"signatures\"\x1b\n" +
+	"signatures\x121\n" +
+	"\x15fail_if_target_exists\x18\x06 \x01(\bR\x12failIfTargetExists\"\x1b\n" +
 	"\x19AtomicRenameEntryResponse\"\xba\x01\n" +
 	"\x18StreamRenameEntryRequest\x12#\n" +
 	"\rold_directory\x18\x01 \x01(\tR\foldDirectory\x12\x19\n" +

weed/server/filer_grpc_server_rename.go

@@ -61,6 +61,18 @@ func (fs *FilerServer) AtomicRenameEntry(ctx context.Context, req *filer_pb.Atom
 		return nil, fmt.Errorf("%s/%s not found: %v", req.OldDirectory, req.OldName, err)
 	}
 
+	// The check runs under the rename path locks, which CreateEntry on the same
+	// filer also takes, so a target created concurrently cannot slip through.
+	if req.FailIfTargetExists && newParent.Child(req.NewName) != oldParent.Child(req.OldName) {
+		if _, findErr := fs.filer.FindEntry(ctx, newParent.Child(req.NewName)); findErr == nil {
+			fs.filer.RollbackTransaction(ctx)
+			return nil, fmt.Errorf("%s/%s: %w", req.NewDirectory, req.NewName, filer_pb.ErrEntryAlreadyExists)
+		} else if findErr != filer_pb.ErrNotFound {
+			fs.filer.RollbackTransaction(ctx)
+			return nil, findErr
+		}
+	}
+
 	var metadataEvents []metadataEvent
 	var pendingChunkDeletes []*filer_pb.FileChunk
 	var pendingRemoteCopies []*pendingRemoteCopy

weed/sftpd/sftp_filer.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"crypto/md5"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -263,17 +264,148 @@ func (fs *SftpServer) renameEntry(absPath, absTarget string) error {
 	if err := fs.checkFilePermission(targetDir, "write"); err != nil {
 		return err
 	}
+	requireAbsentTarget := false
+	if path.Clean(absPath) != path.Clean(absTarget) {
+		var err error
+		requireAbsentTarget, err = fs.checkRenameTargetDelete(absTarget, targetDir)
+		if err != nil {
+			return err
+		}
+	}
+	if err := fs.checkRenameKeepsAcl(absPath, absTarget); err != nil {
+		return err
+	}
 	oldDir, oldName := util.FullPath(absPath).DirAndName()
 	newDir, newName := util.FullPath(absTarget).DirAndName()
 	return fs.callWithClient(false, func(ctx context.Context, client filer_pb.SeaweedFilerClient) error {
 		_, err := client.AtomicRenameEntry(ctx, &filer_pb.AtomicRenameEntryRequest{
 			OldDirectory: oldDir, OldName: oldName,
 			NewDirectory: newDir, NewName: newName,
+			// A target the user may not delete must stay absent: the filer
+			// checks it again under the rename locks, where a create cannot
+			// slip in.
+			FailIfTargetExists: requireAbsentTarget,
 		})
 		return err
 	})
 }
 
+// checkRenameTargetDelete enforces delete access where a rename can destroy an
+// entry. Overwriting an existing target deletes it, so it needs delete access
+// on the target itself. A name that does not exist yet needs no delete check
+// unless a configured ACL restricts that name itself: enforcing such an ACL
+// even while the entry is absent keeps a concurrent create from slipping a
+// protected target under the rename. When the target name simply inherits its
+// directory's ACL and the user could not delete an entry there, the rename is
+// still allowed for a new name but the caller is told to require the target to
+// stay absent, so the filer rejects the overwrite of one created in the gap.
+// Lookup failures other than not-found fail closed.
+func (fs *SftpServer) checkRenameTargetDelete(absTarget, targetDir string) (requireAbsentTarget bool, err error) {
+	if _, err := fs.getEntry(absTarget); err == nil {
+		return false, fs.checkFilePermission(absTarget, "delete")
+	} else if !errors.Is(err, os.ErrNotExist) {
+		return false, err
+	}
+	if fs.user == nil || fs.user.Username == "root" || fs.user.Uid == 0 {
+		return false, nil
+	}
+	if matchingAclPrefix(fs.user, absTarget) != matchingAclPrefix(fs.user, targetDir) {
+		if !HasExplicitPermission(fs.user, absTarget, PermDelete, false) {
+			glog.V(0).Infof("permission denied for user %s: rename target %s carries a delete-protected ACL", fs.user.Username, absTarget)
+			return false, os.ErrPermission
+		}
+		return false, nil
+	}
+	if err := fs.checkFilePermission(absTarget, "delete"); err != nil {
+		if errors.Is(err, os.ErrPermission) {
+			return true, nil
+		}
+		return false, err
+	}
+	return false, nil
+}
+
+// matchingAclPrefix returns the longest configured ACL path that applies to p,
+// or "" when no configured path covers it.
+func matchingAclPrefix(user *user.User, p string) string {
+	best := ""
+	for aclPath := range user.Permissions {
+		cleaned := path.Clean(aclPath)
+		if pathWithin(cleaned, p) && len(cleaned) > len(best) {
+			best = cleaned
+		}
+	}
+	return best
+}
+
+// checkRenameKeepsAcl denies a rename that would move entries to locations
+// where ACLs grant the user read, list, write, or delete access the source
+// side lacked. It compares the moved root and every configured ACL path under
+// either the source or the target (mapped back to the source) so both
+// inherited restrictions and deeper destination grants are caught.
+func (fs *SftpServer) checkRenameKeepsAcl(absPath, absTarget string) error {
+	if fs.user == nil || fs.user.Username == "root" || fs.user.Uid == 0 {
+		return nil
+	}
+	type aclPair struct {
+		src, dst string
+		isDir    bool
+	}
+	srcEntry, err := fs.getEntry(absPath)
+	if err != nil {
+		return err
+	}
+	pairs := []aclPair{{absPath, absTarget, srcEntry.IsDirectory}}
+	seen := map[string]bool{absPath: true}
+	for p := range fs.user.Permissions {
+		cleaned := path.Clean(p)
+		var src string
+		switch {
+		case pathWithin(absPath, cleaned):
+			src = cleaned
+		case pathWithin(absTarget, cleaned):
+			src = path.Join(absPath, strings.TrimPrefix(cleaned, absTarget))
+		default:
+			continue
+		}
+		if seen[src] {
+			continue
+		}
+		seen[src] = true
+		// A rule only protects an entry that actually moves; a path reserved
+		// for a missing child must not block the rename. A lookup error is not
+		// proof that the child is missing, so only os.ErrNotExist may skip.
+		entry, err := fs.getEntry(src)
+		if err != nil {
+			if errors.Is(err, os.ErrNotExist) {
+				continue
+			}
+			return err
+		}
+		relocated := path.Join(absTarget, strings.TrimPrefix(src, absPath))
+		pairs = append(pairs, aclPair{src, relocated, entry.IsDirectory})
+	}
+	for _, pair := range pairs {
+		// A moved directory carries file children whose ACLs resolve with
+		// isDirectory=false, where list no longer counts as read. Comparing
+		// only the directory shape would miss children gaining read, so a
+		// directory is checked in the file shape too.
+		shapes := []bool{pair.isDir}
+		if pair.isDir {
+			shapes = append(shapes, false)
+		}
+		for _, isDir := range shapes {
+			for _, perm := range []string{PermRead, PermList, PermWrite, PermDelete} {
+				if !HasExplicitPermission(fs.user, pair.src, perm, isDir) && HasExplicitPermission(fs.user, pair.dst, perm, isDir) {
+					glog.V(0).Infof("permission denied for user %s: rename of %s to %s would drop the ACL on %s", fs.user.Username, absPath, absTarget, pair.src)
+					return os.ErrPermission
+				}
+			}
+		}
+	}
+	return nil
+}
+
 func (fs *SftpServer) setFileStatWithRequest(absPath string, r *sftp.Request) error {
 	if err := fs.checkFilePermission(absPath, "write"); err != nil {
 		return err

weed/sftpd/sftp_permissions_test.go

@@ -2,6 +2,7 @@ package sftpd
 
 import (
 	"context"
+	"errors"
 	"net"
 	"os"
 	"testing"
@@ -136,6 +137,96 @@ func TestNewFileWriterChecksTargetFile(t *testing.T) {
 	}
 }
 
+// renameEntry must honor a more specific ACL on an existing target: renaming
+// over a read-only file deletes it, so it requires delete permission on the
+// target file itself, not only write permission on the target directory.
+func TestRenameEntryChecksTargetFile(t *testing.T) {
+	lis, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	filer := &permissionTestFiler{entries: map[string]*filer_pb.Entry{
+		"/home/alice":            {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/own.txt":    {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+		"/home/alice/ok.txt":     {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+		"/home/alice/locked.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+	}}
+	grpcServer := grpc.NewServer()
+	filer_pb.RegisterSeaweedFilerServer(grpcServer, filer)
+	go func() { _ = grpcServer.Serve(lis) }()
+	t.Cleanup(grpcServer.Stop)
+
+	_, port, _ := net.SplitHostPort(lis.Addr().String())
+	fs := &SftpServer{
+		filerAddr:      pb.ServerAddress("127.0.0.1:0." + port),
+		grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+		user: &user.User{
+			Username: "alice", Uid: 1000, Gid: 1000, HomeDir: "/home/alice",
+			Permissions: map[string][]string{
+				"/home/alice":            {"read", "write", "list", "delete", "rename"},
+				"/home/alice/locked.txt": {"read", "list"},
+			},
+		},
+	}
+
+	renameDenied := func(src, dst string) bool {
+		return errors.Is(fs.renameEntry(src, dst), os.ErrPermission)
+	}
+
+	if !renameDenied("/home/alice/own.txt", "/home/alice/locked.txt") {
+		t.Error("expected rename over a read-only file to be denied")
+	}
+	if err := fs.renameEntry("/home/alice/own.txt", "/home/alice/ok.txt"); err != nil {
+		t.Errorf("expected rename over a writable file to succeed, got %v", err)
+	}
+	if err := fs.renameEntry("/home/alice/ok.txt", "/home/alice/new.txt"); err != nil {
+		t.Errorf("expected rename onto a new path in a writable dir to succeed, got %v", err)
+	}
+	if err := fs.renameEntry("/home/alice/new.txt", "/home/alice/new.txt"); err != nil {
+		t.Errorf("expected a no-op rename to skip the target delete check, got %v", err)
+	}
+}
+
+// Renaming a directory must not strand a more specific ACL configured under
+// it: after /sub moves to /sub2, /sub/ro.txt no longer matches its read-only
+// ACL, so the move is denied unless the destination grants no new access.
+func TestRenameEntryKeepsNestedAcl(t *testing.T) {
+	lis, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	filer := &permissionTestFiler{entries: map[string]*filer_pb.Entry{
+		"/home/alice":            {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/sub":        {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/sub/ro.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+	}}
+	grpcServer := grpc.NewServer()
+	filer_pb.RegisterSeaweedFilerServer(grpcServer, filer)
+	go func() { _ = grpcServer.Serve(lis) }()
+	t.Cleanup(grpcServer.Stop)
+
+	_, port, _ := net.SplitHostPort(lis.Addr().String())
+	fs := &SftpServer{
+		filerAddr:      pb.ServerAddress("127.0.0.1:0." + port),
+		grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+		user: &user.User{
+			Username: "alice", Uid: 1000, Gid: 1000, HomeDir: "/home/alice",
+			Permissions: map[string][]string{
+				"/home/alice":            {"read", "write", "list", "delete", "rename"},
+				"/home/alice/sub/ro.txt": {"read", "list"},
+			},
+		},
+	}
+
+	if err := fs.renameEntry("/home/alice/sub", "/home/alice/sub2"); !errors.Is(err, os.ErrPermission) {
+		t.Errorf("expected rename that strands the /sub/ro.txt ACL to be denied, got %v", err)
+	}
+	delete(fs.user.Permissions, "/home/alice/sub/ro.txt")
+	if err := fs.renameEntry("/home/alice/sub", "/home/alice/sub2"); err != nil {
+		t.Errorf("expected rename without a nested carve-out to succeed, got %v", err)
+	}
+}
+
 func TestCheckFilePermissionMissingAncestorsTerminates(t *testing.T) {
 	lis, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
@@ -160,12 +251,304 @@ func TestCheckFilePermissionMissingAncestorsTerminates(t *testing.T) {
 
 type permissionTestFiler struct {
 	filer_pb.UnimplementedSeaweedFilerServer
-	entries map[string]*filer_pb.Entry
+	entries   map[string]*filer_pb.Entry
+	lookupErr map[string]error
+	// createBeforeRename inserts an entry at the rename target inside the RPC,
+	// simulating a file a concurrent connection creates after the checks ran.
+	createBeforeRename map[string]*filer_pb.Entry
 }
 
 func (f *permissionTestFiler) LookupDirectoryEntry(_ context.Context, req *filer_pb.LookupDirectoryEntryRequest) (*filer_pb.LookupDirectoryEntryResponse, error) {
-	if entry, ok := f.entries[req.Directory+"/"+req.Name]; ok {
+	key := req.Directory + "/" + req.Name
+	if err, ok := f.lookupErr[key]; ok {
+		return nil, err
+	}
+	if entry, ok := f.entries[key]; ok {
 		return &filer_pb.LookupDirectoryEntryResponse{Entry: entry}, nil
 	}
 	return nil, status.Error(codes.NotFound, "no entry is found")
 }
+
+func (f *permissionTestFiler) AtomicRenameEntry(_ context.Context, req *filer_pb.AtomicRenameEntryRequest) (*filer_pb.AtomicRenameEntryResponse, error) {
+	oldKey := req.OldDirectory + "/" + req.OldName
+	entry, ok := f.entries[oldKey]
+	if !ok {
+		return nil, status.Error(codes.NotFound, "no entry is found")
+	}
+	newKey := req.NewDirectory + "/" + req.NewName
+	if created, ok := f.createBeforeRename[newKey]; ok {
+		f.entries[newKey] = created
+	}
+	if req.FailIfTargetExists {
+		if _, exists := f.entries[newKey]; exists && newKey != oldKey {
+			return nil, status.Error(codes.AlreadyExists, "entry already exists")
+		}
+	}
+	delete(f.entries, oldKey)
+	f.entries[newKey] = entry
+	return &filer_pb.AtomicRenameEntryResponse{}, nil
+}
+
+// checkRenameKeepsAcl also compares the moved root (inherited restrictions) and
+// ACL paths configured under the destination (deeper grants), while configured
+// paths without a real entry do not block the rename.
+func TestRenameEntryKeepsInheritedAndDestinationAcl(t *testing.T) {
+	lis, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	filer := &permissionTestFiler{entries: map[string]*filer_pb.Entry{
+		"/home/alice":                   {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/restricted":        {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/restricted/sub":    {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/sub":               {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/sub/locked":        {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/sub/locked/secret": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+		"/home/alice/sub3":              {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+	}}
+	grpcServer := grpc.NewServer()
+	filer_pb.RegisterSeaweedFilerServer(grpcServer, filer)
+	go func() { _ = grpcServer.Serve(lis) }()
+	t.Cleanup(grpcServer.Stop)
+
+	_, port, _ := net.SplitHostPort(lis.Addr().String())
+	fs := &SftpServer{
+		filerAddr:      pb.ServerAddress("127.0.0.1:0." + port),
+		grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+		user: &user.User{
+			Username: "alice", Uid: 1000, Gid: 1000, HomeDir: "/home/alice",
+			Permissions: map[string][]string{
+				"/home/alice":                    {"read", "write", "list", "delete", "rename"},
+				"/home/alice/restricted":         {"read", "list", "rename"},
+				"/home/alice/sub/locked":         {"read", "list"},
+				"/home/alice/sub2/locked":        {"read", "list"},
+				"/home/alice/sub2/locked/secret": {"read", "write", "list", "delete"},
+				"/home/alice/sub3/ghost.txt":     {"read", "list"},
+			},
+		},
+	}
+
+	if err := fs.renameEntry("/home/alice/restricted/sub", "/home/alice/sub5"); !errors.Is(err, os.ErrPermission) {
+		t.Errorf("expected rename escaping the inherited restricted ACL to be denied, got %v", err)
+	}
+	if err := fs.renameEntry("/home/alice/sub", "/home/alice/sub2"); !errors.Is(err, os.ErrPermission) {
+		t.Errorf("expected rename into a deeper destination grant to be denied, got %v", err)
+	}
+	if err := fs.renameEntry("/home/alice/sub3", "/home/alice/sub4"); err != nil {
+		t.Errorf("expected rename with a missing reserved ACL child to succeed, got %v", err)
+	}
+}
+
+// A rename onto a name that does not exist needs no delete access unless a
+// configured ACL restricts that name itself. Without the scoped check a user
+// holding write+rename but not delete could no longer move files to new names.
+func TestRenameEntryNewNameNeedsNoDelete(t *testing.T) {
+	lis, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	filer := &permissionTestFiler{entries: map[string]*filer_pb.Entry{
+		"/home/alice":               {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/own.txt":       {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+		"/home/alice/existing.txt":  {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+		"/home/alice/removable.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+	}}
+	grpcServer := grpc.NewServer()
+	filer_pb.RegisterSeaweedFilerServer(grpcServer, filer)
+	go func() { _ = grpcServer.Serve(lis) }()
+	t.Cleanup(grpcServer.Stop)
+
+	_, port, _ := net.SplitHostPort(lis.Addr().String())
+	fs := &SftpServer{
+		filerAddr:      pb.ServerAddress("127.0.0.1:0." + port),
+		grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+		user: &user.User{
+			Username: "alice", Uid: 1000, Gid: 1000, HomeDir: "/home/alice",
+			Permissions: map[string][]string{
+				"/home/alice":               {"read", "write", "list", "rename"},
+				"/home/alice/reserved.txt":  {"read", "list"},
+				"/home/alice/managed.txt":   {"read", "write", "list", "delete"},
+				"/home/alice/removable.txt": {"read", "write", "list", "delete", "rename"},
+			},
+		},
+	}
+
+	if err := fs.renameEntry("/home/alice/own.txt", "/home/alice/new.txt"); err != nil {
+		t.Errorf("expected rename to a new name without delete access to succeed, got %v", err)
+	}
+	if err := fs.renameEntry("/home/alice/new.txt", "/home/alice/existing.txt"); !errors.Is(err, os.ErrPermission) {
+		t.Errorf("expected rename over an existing file without delete access to be denied, got %v", err)
+	}
+	if err := fs.renameEntry("/home/alice/new.txt", "/home/alice/reserved.txt"); !errors.Is(err, os.ErrPermission) {
+		t.Errorf("expected rename onto a delete-protected configured name to be denied, got %v", err)
+	}
+	if err := fs.renameEntry("/home/alice/removable.txt", "/home/alice/managed.txt"); err != nil {
+		t.Errorf("expected rename onto a configured name that grants delete to succeed, got %v", err)
+	}
+}
+
+// checkRenameKeepsAcl fails closed: a lookup error on a configured ACL path is
+// not proof that the entry is missing, so it must stop the rename instead of
+// skipping the comparison.
+func TestRenameEntryFailsClosedOnLookupError(t *testing.T) {
+	lis, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	filer := &permissionTestFiler{
+		entries: map[string]*filer_pb.Entry{
+			"/home/alice":            {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+			"/home/alice/sub":        {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+			"/home/alice/sub/ro.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+		},
+		lookupErr: map[string]error{
+			"/home/alice/sub/ro.txt": status.Error(codes.Unavailable, "filer unavailable"),
+		},
+	}
+	grpcServer := grpc.NewServer()
+	filer_pb.RegisterSeaweedFilerServer(grpcServer, filer)
+	go func() { _ = grpcServer.Serve(lis) }()
+	t.Cleanup(grpcServer.Stop)
+
+	_, port, _ := net.SplitHostPort(lis.Addr().String())
+	fs := &SftpServer{
+		filerAddr:      pb.ServerAddress("127.0.0.1:0." + port),
+		grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+		user: &user.User{
+			Username: "alice", Uid: 1000, Gid: 1000, HomeDir: "/home/alice",
+			Permissions: map[string][]string{
+				"/home/alice":            {"read", "write", "list", "delete", "rename"},
+				"/home/alice/sub/ro.txt": {"read", "list"},
+			},
+		},
+	}
+
+	err = fs.renameEntry("/home/alice/sub", "/home/alice/sub2")
+	if err == nil || errors.Is(err, os.ErrPermission) {
+		t.Errorf("expected rename to fail closed on a lookup error, got %v", err)
+	}
+	delete(filer.lookupErr, "/home/alice/sub/ro.txt")
+	if err := fs.renameEntry("/home/alice/sub", "/home/alice/sub2"); !errors.Is(err, os.ErrPermission) {
+		t.Errorf("expected the stranded ACL check to deny the rename once the lookup succeeds, got %v", err)
+	}
+}
+
+// A rename must not grant access the source side lacked, including read and
+// directory listing: moving a subtree whose ACL withholds read/list into a
+// readable destination would expose its contents.
+func TestRenameEntryKeepsReadAndListAcl(t *testing.T) {
+	lis, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	filer := &permissionTestFiler{entries: map[string]*filer_pb.Entry{
+		"/home/alice":              {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/noread":       {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/noread/f.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+	}}
+	grpcServer := grpc.NewServer()
+	filer_pb.RegisterSeaweedFilerServer(grpcServer, filer)
+	go func() { _ = grpcServer.Serve(lis) }()
+	t.Cleanup(grpcServer.Stop)
+
+	_, port, _ := net.SplitHostPort(lis.Addr().String())
+	fs := &SftpServer{
+		filerAddr:      pb.ServerAddress("127.0.0.1:0." + port),
+		grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+		user: &user.User{
+			Username: "alice", Uid: 1000, Gid: 1000, HomeDir: "/home/alice",
+			Permissions: map[string][]string{
+				"/home/alice":        {"read", "write", "list", "delete", "rename"},
+				"/home/alice/noread": {"write", "delete", "rename"},
+			},
+		},
+	}
+
+	if err := fs.renameEntry("/home/alice/noread", "/home/alice/visible"); !errors.Is(err, os.ErrPermission) {
+		t.Errorf("expected rename that grants read/list on the moved directory to be denied, got %v", err)
+	}
+}
+
+// A moved directory carries file children whose ACLs resolve with
+// isDirectory=false, where list does not count as read. Moving a directory
+// that grants list but not read into a readable destination must be denied:
+// its file children would become readable even though the directory check
+// passes.
+func TestRenameEntryKeepsFileChildAcl(t *testing.T) {
+	lis, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	filer := &permissionTestFiler{entries: map[string]*filer_pb.Entry{
+		"/home/alice":                    {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/private":            {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+		"/home/alice/private/secret.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+	}}
+	grpcServer := grpc.NewServer()
+	filer_pb.RegisterSeaweedFilerServer(grpcServer, filer)
+	go func() { _ = grpcServer.Serve(lis) }()
+	t.Cleanup(grpcServer.Stop)
+
+	_, port, _ := net.SplitHostPort(lis.Addr().String())
+	fs := &SftpServer{
+		filerAddr:      pb.ServerAddress("127.0.0.1:0." + port),
+		grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+		user: &user.User{
+			Username: "alice", Uid: 1000, Gid: 1000, HomeDir: "/home/alice",
+			Permissions: map[string][]string{
+				"/home/alice":         {"read", "write", "list", "delete", "rename"},
+				"/home/alice/private": {"list", "write", "delete", "rename"},
+			},
+		},
+	}
+
+	if err := fs.renameEntry("/home/alice/private", "/home/alice/public"); !errors.Is(err, os.ErrPermission) {
+		t.Errorf("expected rename that grants read on file children to be denied, got %v", err)
+	}
+	fs.user.Permissions["/home/alice/public"] = []string{"list", "write", "delete", "rename"}
+	if err := fs.renameEntry("/home/alice/private", "/home/alice/public"); err != nil {
+		t.Errorf("expected rename onto an equally restricted ACL to succeed, got %v", err)
+	}
+}
+
+// A rename onto a new name needs no delete access, but a target created in the
+// gap between the check and the move must not be silently overwritten: the
+// filer is asked to fail the rename when the target appears.
+func TestRenameEntryRejectsConcurrentTarget(t *testing.T) {
+	lis, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatalf("listen: %v", err)
+	}
+	filer := &permissionTestFiler{
+		entries: map[string]*filer_pb.Entry{
+			"/home/alice":         {IsDirectory: true, Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0755}},
+			"/home/alice/own.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+		},
+		createBeforeRename: map[string]*filer_pb.Entry{
+			"/home/alice/taken.txt": {Attributes: &filer_pb.FuseAttributes{Uid: 1000, FileMode: 0644}},
+		},
+	}
+	grpcServer := grpc.NewServer()
+	filer_pb.RegisterSeaweedFilerServer(grpcServer, filer)
+	go func() { _ = grpcServer.Serve(lis) }()
+	t.Cleanup(grpcServer.Stop)
+
+	_, port, _ := net.SplitHostPort(lis.Addr().String())
+	fs := &SftpServer{
+		filerAddr:      pb.ServerAddress("127.0.0.1:0." + port),
+		grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+		user: &user.User{
+			Username: "alice", Uid: 1000, Gid: 1000, HomeDir: "/home/alice",
+			Permissions: map[string][]string{
+				"/home/alice": {"read", "write", "list", "rename"},
+			},
+		},
+	}
+
+	if err := fs.renameEntry("/home/alice/own.txt", "/home/alice/free.txt"); err != nil {
+		t.Errorf("expected rename to a name that stays absent to succeed, got %v", err)
+	}
+	if err := fs.renameEntry("/home/alice/free.txt", "/home/alice/taken.txt"); err == nil {
+		t.Error("expected rename to fail when the target appears concurrently and the user lacks delete access")
+	}
+}