From d541caa52b65897fdd9629da17d04e38d756c1ea Mon Sep 17 00:00:00 2001 From: nielash Date: Sat, 6 Sep 2025 05:09:22 -0400 Subject: [PATCH] local: fix rmdir "Access is denied" on windows - fixes #8363 Before this change, Rmdir (and other commands that rely on Rmdir) would fail with "Access is denied" on Windows, if the directory had FILE_ATTRIBUTE_READONLY. This could happen if, for example, an empty folder had a custom icon added via Windows Explorer's interface (Properties => Customize => Change Icon...). However, Microsoft docs indicate that "This attribute is not honored on directories." https://learn.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants#file_attribute_readonly Accordingly, this created an odd situation where such directories were removable (by their owner) via File Explorer and the rd command, but not via rclone. An upstream issue has been open since 2018, but has not yet resulted in a fix. https://github.com/golang/go/issues/26295 This change gets around the issue by doing os.Chmod on the dir and then retrying os.Remove. If the dir is not empty, this will still fail with "The directory is not empty." A bisync user confirmed that it fixed their issue in https://forum.rclone.org/t/bisync-leaving-empty-directories-on-unc-path-1-or-local-filesystem-path-2-on-directory-renames/52456/4?u=nielash It is likely also a fix for #8019, although @ncw is correct that Purge would be a more efficient solution in that particular scenario. --- backend/local/local.go | 9 ++++- backend/local/local_internal_windows_test.go | 40 ++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 backend/local/local_internal_windows_test.go diff --git a/backend/local/local.go b/backend/local/local.go index 3f4ee58fd..b4462d8ed 100644 --- a/backend/local/local.go +++ b/backend/local/local.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + iofs "io/fs" "os" "path" "path/filepath" @@ -841,7 +842,13 @@ func (f *Fs) Rmdir(ctx context.Context, dir string) error { } else if !fi.IsDir() { return fs.ErrorIsFile } - return os.Remove(localPath) + err := os.Remove(localPath) + if runtime.GOOS == "windows" && errors.Is(err, iofs.ErrPermission) { // https://github.com/golang/go/issues/26295 + if os.Chmod(localPath, 0o600) == nil { + err = os.Remove(localPath) + } + } + return err } // Precision of the file system diff --git a/backend/local/local_internal_windows_test.go b/backend/local/local_internal_windows_test.go new file mode 100644 index 000000000..6379dd436 --- /dev/null +++ b/backend/local/local_internal_windows_test.go @@ -0,0 +1,40 @@ +//go:build windows + +package local + +import ( + "context" + "path/filepath" + "runtime" + "syscall" + "testing" + + "github.com/rclone/rclone/fs/operations" + "github.com/rclone/rclone/fstest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestRmdirWindows tests that FILE_ATTRIBUTE_READONLY does not block Rmdir on windows. +// Microsoft docs indicate that "This attribute is not honored on directories." +// See https://learn.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants#file_attribute_readonly +// and https://github.com/golang/go/issues/26295 +func TestRmdirWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skipf("windows only") + } + r := fstest.NewRun(t) + defer r.Finalise() + + err := operations.Mkdir(context.Background(), r.Flocal, "testdir") + require.NoError(t, err) + + ptr, err := syscall.UTF16PtrFromString(filepath.Join(r.Flocal.Root(), "testdir")) + require.NoError(t, err) + + err = syscall.SetFileAttributes(ptr, uint32(syscall.FILE_ATTRIBUTE_DIRECTORY+syscall.FILE_ATTRIBUTE_READONLY)) + require.NoError(t, err) + + err = operations.Rmdir(context.Background(), r.Flocal, "testdir") + assert.NoError(t, err) +}