From a89b1bed85d6b48bb03c9b719849d66dff11b046 Mon Sep 17 00:00:00 2001 From: kolaente Date: Fri, 30 Jan 2026 11:23:39 +0100 Subject: [PATCH] feat(doctor): add detailed file diagnostics for local storage (#2179) When using local file storage, the doctor command now reports: - Whether the files directory exists - Directory permissions (octal mode) - Directory owner and group with uid/gid (Unix) - Ownership mismatch warning if Vikunja runs as a different user - Total number of stored files and their combined size --------- Co-authored-by: Claude --- pkg/doctor/files.go | 87 +++++++++++++++++++++++++++++++++++++ pkg/doctor/files_unix.go | 81 ++++++++++++++++++++++++++++++++++ pkg/doctor/files_windows.go | 12 +++++ 3 files changed, 180 insertions(+) diff --git a/pkg/doctor/files.go b/pkg/doctor/files.go index 7e1f43533..9f286335e 100644 --- a/pkg/doctor/files.go +++ b/pkg/doctor/files.go @@ -18,6 +18,9 @@ package doctor import ( "fmt" + "io/fs" + "os" + "path/filepath" "code.vikunja.io/api/pkg/config" "code.vikunja.io/api/pkg/files" @@ -75,6 +78,34 @@ func checkLocalStorage() []CheckResult { }, } + // Check if the directory exists + info, err := os.Stat(basePath) + if err != nil { + results = append(results, CheckResult{ + Name: "Directory exists", + Passed: false, + Error: err.Error(), + }) + // If the directory doesn't exist, skip the remaining checks + return results + } + + results = append(results, CheckResult{ + Name: "Directory exists", + Passed: true, + Value: "yes", + }) + + // Directory permissions (octal mode) + results = append(results, CheckResult{ + Name: "Directory permissions", + Passed: true, + Value: fmt.Sprintf("%04o", info.Mode().Perm()), + }) + + // Directory ownership (platform-specific) + results = append(results, checkDirectoryOwnership(info)...) + // Check writable using the existing ValidateFileStorage function if err := files.ValidateFileStorage(); err != nil { results = append(results, CheckResult{ @@ -93,9 +124,65 @@ func checkLocalStorage() []CheckResult { // Check disk space (platform-specific) results = append(results, checkDiskSpace(basePath)) + // Count files and total size in the directory + results = append(results, checkFileStats(basePath)) + return results } +func checkFileStats(basePath string) CheckResult { + var totalFiles int + var totalSize int64 + + err := filepath.WalkDir(basePath, func(_ string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() { + totalFiles++ + info, err := d.Info() + if err != nil { + return err + } + totalSize += info.Size() + } + return nil + }) + + if err != nil { + return CheckResult{ + Name: "Stored files", + Passed: false, + Error: fmt.Sprintf("error scanning directory: %s", err.Error()), + } + } + + return CheckResult{ + Name: "Stored files", + Passed: true, + Value: fmt.Sprintf("%d files, %s total", totalFiles, formatBytes(totalSize)), + } +} + +func formatBytes(b int64) string { + const ( + kb = 1024 + mb = kb * 1024 + gb = mb * 1024 + ) + + switch { + case b >= gb: + return fmt.Sprintf("%.1f GB", float64(b)/float64(gb)) + case b >= mb: + return fmt.Sprintf("%.1f MB", float64(b)/float64(mb)) + case b >= kb: + return fmt.Sprintf("%.1f KB", float64(b)/float64(kb)) + default: + return fmt.Sprintf("%d B", b) + } +} + func checkS3Storage() []CheckResult { endpoint := config.FilesS3Endpoint.GetString() bucket := config.FilesS3Bucket.GetString() diff --git a/pkg/doctor/files_unix.go b/pkg/doctor/files_unix.go index d7a4b0062..3f0e9a785 100644 --- a/pkg/doctor/files_unix.go +++ b/pkg/doctor/files_unix.go @@ -20,6 +20,10 @@ package doctor import ( "fmt" + "os" + "os/user" + "strconv" + "syscall" "golang.org/x/sys/unix" ) @@ -44,3 +48,80 @@ func checkDiskSpace(path string) CheckResult { Value: fmt.Sprintf("%.1f GB available", availableGB), } } + +// isGroupMember reports whether the current process belongs to the given +// group, checking both the primary gid and all supplementary groups. +func isGroupMember(gid int) bool { + if os.Getgid() == gid { + return true + } + + groups, err := os.Getgroups() + if err != nil { + return false + } + for _, g := range groups { + if g == gid { + return true + } + } + return false +} + +func checkDirectoryOwnership(info os.FileInfo) []CheckResult { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return []CheckResult{ + { + Name: "Directory owner", + Passed: true, + Value: "unable to determine ownership", + }, + } + } + + uid := stat.Uid + gid := stat.Gid + + ownerName := strconv.FormatUint(uint64(uid), 10) + groupName := strconv.FormatUint(uint64(gid), 10) + + if u, err := user.LookupId(ownerName); err == nil { + ownerName = u.Username + } + if g, err := user.LookupGroupId(groupName); err == nil { + groupName = g.Name + } + + currentUID := os.Getuid() + + results := []CheckResult{ + { + Name: "Directory owner", + Passed: true, + Value: fmt.Sprintf("%s:%s (uid=%d, gid=%d)", ownerName, groupName, uid, gid), + }, + } + + if currentUID != 0 && currentUID != int(uid) { + results = append(results, CheckResult{ + Name: "Ownership match", + Passed: false, + Error: fmt.Sprintf( + "directory owned by uid %d but Vikunja runs as uid %d", + uid, currentUID, + ), + }) + } else if currentUID != 0 && !isGroupMember(int(gid)) { + results = append(results, CheckResult{ + Name: "Ownership match", + Passed: false, + Error: fmt.Sprintf( + "directory owned by gid %d but Vikunja process is not a member of that group", + gid, + ), + }) + } + + return results +} diff --git a/pkg/doctor/files_windows.go b/pkg/doctor/files_windows.go index ce954944a..0a51f505c 100644 --- a/pkg/doctor/files_windows.go +++ b/pkg/doctor/files_windows.go @@ -18,6 +18,8 @@ package doctor +import "os" + func checkDiskSpace(_ string) CheckResult { return CheckResult{ Name: "Disk space", @@ -25,3 +27,13 @@ func checkDiskSpace(_ string) CheckResult { Value: "check not available on Windows", } } + +func checkDirectoryOwnership(_ os.FileInfo) []CheckResult { + return []CheckResult{ + { + Name: "Directory owner", + Passed: true, + Value: "ownership check not available on Windows", + }, + } +}