chezmoi unmanaged on macOS Sonoma halts on ~/Library/Application Support/com.apple.LaunchServicesTemplateApp.dv

[halostatue]

Describe the bug

$ chezmoi unmanaged
chezmoi: lstat /Users/halostatue/Library/Application Support/com.apple.LaunchServicesTemplateApp.dv: operation not permitted

The output is the same with --verbose.

I can no longer run chezmoi unmanaged or chezmoi unmanaged ~ or similar because of the file permissions (finder tells me that I have "unknown permissions" and do not see any way of changing those, even as root).

This fails only on macOS Sonoma with webapp support.

Expected behavior

chezmoi unmanaged should output an warning about the inaccessible file (like eza, see below), but otherwise continue.

$ cd ~/Library/Application\ Support
$ eza -lah
[./com.apple.LaunchServicesTemplateApp.dv: Operation not permitted (os error 1)]
Permissions Size User       Date Modified Name
.rw-r--r--    30 halostatue 28 Aug  2023  .cached.db
.rw-r--r--@ 6.1k halostatue 21 Feb 13:49  .DS_Store
drwx------     - halostatue 23 Nov  2023  1Password
drwx------@    - halostatue 23 Dec  2021  AddressBook
…

Additional context

RIDER-93883 and IDEA-334412 show this happening elsewhere.

I will be able to investigate this later, but I suspect that this is a matter of not returning from unmanaged if lstat errors happen, but reporting the error on stderr in realtime.

Other commands are also likely affected by this or similar issues (chezmoi add --recursive), but the correct behaviour will be different depending on the commands involved.

[halostatue]

@twpayne Not quite sure where to fix this, because this look like it's breaking out in vfs.Walk and is unable to reenter because of the use of dirEntry.Info() in walk/5. Because that return is breaking out of for … range dirEntries, there's no way to continue without this.

This may be where WalkDir and/or WalkDirSlash are required, because at least for unmanaged, the dirEntry.Info() appears to be mostly unnecessary, but it would require a different approach.

1. Would you like a parallel ticket opened for twpayne/go-vfs
2. Should we implement WalkDir and shift chezmoi unmanaged to using WalkDir or should we look at some other way to do this?

A simplistic fix would be:

diff --git c/walk.go i/walk.go
index 1075f1225194..d6c090e1ac66 100644
--- c/walk.go
+++ i/walk.go
@@ -6,10 +6,11 @@
 import (
 	"errors"
 	"io/fs"
 	"path/filepath"
 	"sort"
+	"syscall"
 )
 
 // SkipDir is fs.SkipDir.
 var SkipDir = fs.SkipDir
 
@@ -42,15 +43,20 @@ func walk(fileSystem LstatReadDirer, path string, walkFn filepath.WalkFunc, info
 		return err
 	}
 	sort.Sort(dirEntriesByName(dirEntries))
 	for _, dirEntry := range dirEntries {
 		name := dirEntry.Name()
+
 		if name == "." || name == ".." {
 			continue
 		}
 		info, err := dirEntry.Info()
 		if err != nil {
+			if errors.Is(err, syscall.EPERM) {
+				continue
+			}
+
 			return err
 		}
 		if err := walk(fileSystem, filepath.Join(path, dirEntry.Name()), walkFn, info, nil); err != nil {
 			return err
 		}

[twpayne]

Thank you for this.

I think there are bugs in both twpayne/go-vfs and in chezmoi:

1. The bug in twpayne/go-vfs is that walk/5 should call walkFn when it encounters any error so that the caller can decide how to handle the error. In this case, walk/5 should call walkFn but doesnt.
2. Within chezmoi, the -k/--keep-going flag should instruct chezmoi to keep going in this case.

No need for a separate issue in twpayne/go-vfs, but I will request your review on the PR :)

[halostatue]

Unfortunately, this is not resolved.

$ chezmoi unmanaged --keep-going
chezmoi: /Users/halostatue/Library/Application Support/com.apple.LaunchServicesTemplateApp.dv: lstat /Users/halostatue/Library/Application Support/com.apple.LaunchServicesTemplateApp.dv: operation not permittedpanic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x18 pc=0x101265458]

goroutine 1 [running]:
github.com/twpayne/chezmoi/v2/internal/cmd.(*Config).runUnmanagedCmd.func1({0x140005b8e60, 0x50}, {0x0, 0x0}, {0x0?, 0x0?})
	/home/runner/work/chezmoi/chezmoi/internal/cmd/unmanagedcmd.go:76 +0x348
github.com/twpayne/chezmoi/v2/internal/chezmoi.Walk.func1({0x140005b8e60?, 0x140005787b0?}, {0x0?, 0x0?}, {0x0?, 0x0?})
	/home/runner/work/chezmoi/chezmoi/internal/chezmoi/system.go:157 +0x38
github.com/twpayne/go-vfs/v5.walk({0x1293e54e0, 0x10244caa0}, {0x140005b8e60, 0x50}, 0x14000809688, {0x0, 0x0}, {0x0?, 0x0?})
	/home/runner/go/pkg/mod/github.com/twpayne/go-vfs/v5@v5.0.2/walk.go:33 +0x80
github.com/twpayne/go-vfs/v5.walk({0x1293e54e0, 0x10244caa0}, {0x14000477ce0, 0x29}, 0x14000809688, {0x101928040, 0x1400053ec30}, {0x0?, 0x0?})
	/home/runner/go/pkg/mod/github.com/twpayne/go-vfs/v5@v5.0.2/walk.go:65 +0x338
github.com/twpayne/go-vfs/v5.walk({0x1293e54e0, 0x10244caa0}, {0x14000907188, 0x15}, 0x14000809688, {0x101928040, 0x1400053e5b0}, {0x0?, 0x0?})
	/home/runner/go/pkg/mod/github.com/twpayne/go-vfs/v5@v5.0.2/walk.go:65 +0x338
github.com/twpayne/go-vfs/v5.walk({0x1293e54e0, 0x10244caa0}, {0x1400004a035, 0xd}, 0x14000809688, {0x101928040, 0x14000713ad0}, {0x0?, 0x0?})
	/home/runner/go/pkg/mod/github.com/twpayne/go-vfs/v5@v5.0.2/walk.go:65 +0x338
github.com/twpayne/go-vfs/v5.Walk({0x1293e54e0, 0x10244caa0}, {0x1400004a035, 0xd}, 0x14000809688)
	/home/runner/go/pkg/mod/github.com/twpayne/go-vfs/v5@v5.0.2/walk.go:76 +0x68
github.com/twpayne/chezmoi/v2/internal/chezmoi.Walk({0x10192dcb0?, 0x1400041af40?}, {0x1400004a035, 0xd}, 0x1018ab140?)
	/home/runner/work/chezmoi/chezmoi/internal/chezmoi/system.go:159 +0x78
github.com/twpayne/chezmoi/v2/internal/cmd.(*Config).runUnmanagedCmd(0x140001ad008, 0x14000342908?, {0x140003150b0?, 0x0?, 0x0?}, 0x140001a2c00)
	/home/runner/work/chezmoi/chezmoi/internal/cmd/unmanagedcmd.go:93 +0x324
github.com/twpayne/chezmoi/v2/internal/cmd.(*Config).newUnmanagedCmd.(*Config).makeRunEWithSourceState.func1(0x14000342908, {0x140003150b0, 0x0, 0x1})
	/home/runner/work/chezmoi/chezmoi/internal/cmd/config.go:1532 +0x88
github.com/spf13/cobra.(*Command).execute(0x14000342908, {0x14000315090, 0x1, 0x1})
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:983 +0x840
github.com/spf13/cobra.(*Command).ExecuteC(0x14000164308)
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x344
github.com/spf13/cobra.(*Command).Execute(...)
	/home/runner/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/twpayne/chezmoi/v2/internal/cmd.(*Config).execute(0x1400079fde8?, {0x1400003a340, 0x2, 0x2})
	/home/runner/work/chezmoi/chezmoi/internal/cmd/config.go:1201 +0x70
github.com/twpayne/chezmoi/v2/internal/cmd.runMain({{0x1015d3648, 0x6}, {0x1015d48e0, 0x28}, {0x1015d45d0, 0x14}, {0x1015d44f0, 0xa}}, {0x1400003a340, 0x2, ...})
	/home/runner/work/chezmoi/chezmoi/internal/cmd/cmd.go:279 +0x17c
github.com/twpayne/chezmoi/v2/internal/cmd.Main({{0x1015d3648, 0x6}, {0x1015d48e0, 0x28}, {0x1015d45d0, 0x14}, {0x1015d44f0, 0xa}}, {0x1400003a340, 0x2, ...})
	/home/runner/work/chezmoi/chezmoi/internal/cmd/cmd.go:105 +0x5c
main.main()
	/home/runner/work/chezmoi/chezmoi/main.go:24 +0xd8

Beyond the crash, having to always remember chezmoi unmanaged -k on macOS 14.3 or higher is suboptimal, because that directory is not going away (and I don't consider adding the file to .chezmoiignore to be a proper solution, either).

Printing a warning about files/directories where EPERM (errno 1, "Operation not permitted") happens is fine, but for that case, chezmoi / go-vfs should probably be turning that into a continue operation.

[halostatue]

The crash happens at internal/cmd/unmanagedcmd.go:76 because the passed fileInfo is null. This case needs to be handled, but it is insufficient.

chezmoi/internal/cmd/unmanagedcmd.go

Lines 76 to 89 in 5bb87f1

	if fileInfo.IsDir() {
	switch {
	case !managed:
	return fs.SkipDir
	case ignored:
	return fs.SkipDir
	case sourceStateEntry != nil:
	if external, ok := sourceStateEntry.Origin().(*chezmoi.External); ok {
	if external.Type == chezmoi.ExternalTypeGitRepo {
	return fs.SkipDir
	}
	}
	}
	}

After returning from the passed walkFunc, go-vfs/walk.go:58 will crash because it tries to access the very same info null pointer.

                        switch err := walkFn(path, info, err); {
                        case errors.Is(err, fs.SkipDir) && info.IsDir():
                                // Do nothing.
                        case err != nil:
                                return err
                        }

The latter can be made to work because it still has access to dirEntry (from go-vfs/walk.go:50) that has dirEntry.IsDir() for checking this.

I suspect that for chezmoi unmanaged, this would be a good opportunity use the (as yet unimplemented) WalkDir function since it never asks for fs.FileInfo, but it will still report an error (Errno.EPERM) on the specific file mentioned.

[halostatue]

Nice. There is one small thing:

chezmoi: /Users/halostatue/Library/Application Support/com.apple.LaunchServicesTemplateApp.dv: lstat /Users/halostatue/Library/Application Support/com.apple.LaunchServicesTemplateApp.dv: operation not permitted.CFUserTextEncoding

I think that this is more general where some error outputs don't have newlines; I can open a new issue for this.

The output should be:

chezmoi: /Users/halostatue/Library/Application Support/com.apple.LaunchServicesTemplateApp.dv: lstat /Users/halostatue/Library/Application Support/com.apple.LaunchServicesTemplateApp.dv: operation not permitted.
CFUserTextEncoding

[twpayne]

I can open a new issue for this.

Please do open a new issue for this. chezmoi already attempts to tidy up error messages, but clearly it can do better here.

[twpayne]

Actually, I think #3627 catches all of them.