diff --git a/internal/tools/boundary_test.go b/internal/tools/boundary_test.go index 5c801537..d60476b2 100644 --- a/internal/tools/boundary_test.go +++ b/internal/tools/boundary_test.go @@ -205,9 +205,6 @@ func TestCheckHardlink_NormalFile(t *testing.T) { } func TestCheckHardlink_HardlinkedFileBlocked(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("hardlinks behave differently on Windows") - } dir := t.TempDir() original := filepath.Join(dir, "original.txt") if err := os.WriteFile(original, []byte("data"), 0644); err != nil { diff --git a/internal/tools/filesystem.go b/internal/tools/filesystem.go index 10d0b849..536d9723 100644 --- a/internal/tools/filesystem.go +++ b/internal/tools/filesystem.go @@ -7,7 +7,6 @@ import ( "os" "path/filepath" "strings" - "syscall" "github.com/nextlevelbuilder/goclaw/internal/bootstrap" "github.com/nextlevelbuilder/goclaw/internal/sandbox" @@ -428,49 +427,3 @@ func resolveThroughExistingAncestors(target string) (string, error) { return filepath.Clean(target), nil } -// hasMutableSymlinkParent checks if any component of the resolved path is a symlink -// whose parent directory is writable by the current process. A writable parent means -// the symlink could be replaced between path resolution and actual file operation -// (TOCTOU symlink rebind attack). -func hasMutableSymlinkParent(path string) bool { - clean := filepath.Clean(path) - components := strings.Split(clean, string(filepath.Separator)) - current := string(filepath.Separator) - for _, comp := range components { - if comp == "" { - continue - } - current = filepath.Join(current, comp) - info, err := os.Lstat(current) - if err != nil { - break // non-existent — stop checking - } - if info.Mode()&os.ModeSymlink != 0 { - // Symlink found — check if its parent dir is writable - parentDir := filepath.Dir(current) - if syscall.Access(parentDir, 0x2 /* W_OK */) == nil { - return true - } - } - } - return false -} - -// checkHardlink rejects regular files with nlink > 1 (hardlink attack prevention). -// Directories naturally have nlink > 1 and are exempt. -func checkHardlink(path string) error { - info, err := os.Lstat(path) - if err != nil { - return nil // non-existent files are OK — will fail at read/write - } - if info.IsDir() { - return nil - } - if stat, ok := info.Sys().(*syscall.Stat_t); ok { - if stat.Nlink > 1 { - slog.Warn("security.hardlink_rejected", "path", path, "nlink", stat.Nlink) - return fmt.Errorf("access denied: hardlinked file not allowed") - } - } - return nil -} diff --git a/internal/tools/filesystem_unix.go b/internal/tools/filesystem_unix.go new file mode 100644 index 00000000..50c52118 --- /dev/null +++ b/internal/tools/filesystem_unix.go @@ -0,0 +1,61 @@ +//go:build !windows + +package tools + +import ( + "fmt" + "log/slog" + "os" + "path/filepath" + "syscall" +) + +// hasMutableSymlinkParent checks if any component of the resolved path is a symlink +// whose parent directory is writable by the current process. A writable parent means +// the symlink could be replaced between path resolution and actual file operation +// (TOCTOU symlink rebind attack). +func hasMutableSymlinkParent(path string) bool { + clean := filepath.Clean(path) + + // Traverse from leaf up to root to find all path components. + var components []string + curr := clean + for { + components = append([]string{curr}, components...) + parent := filepath.Dir(curr) + if parent == curr { + break + } + curr = parent + } + + for _, p := range components { + info, err := os.Lstat(p) + if err != nil { + break // non-existent — stop checking + } + if info.Mode()&os.ModeSymlink != 0 { + // Symlink found — check if its parent dir is writable + if syscall.Access(filepath.Dir(p), 0x2 /* W_OK */) == nil { + return true + } + } + } + return false +} + +// checkHardlink rejects regular files with nlink > 1 (hardlink attack prevention). +// Directories naturally have nlink > 1 and are exempt. +func checkHardlink(path string) error { + info, err := os.Lstat(path) + if err != nil || info.IsDir() { + return nil + } + if stat, ok := info.Sys().(*syscall.Stat_t); ok { + if stat.Nlink > 1 { + slog.Warn("security.hardlink_rejected", "path", path, "nlink", stat.Nlink) + return fmt.Errorf("access denied: hardlinked file not allowed") + } + } + return nil +} diff --git a/internal/tools/filesystem_windows.go b/internal/tools/filesystem_windows.go new file mode 100644 index 00000000..18cbb2de --- /dev/null +++ b/internal/tools/filesystem_windows.go @@ -0,0 +1,105 @@ +//go:build windows + +package tools + +import ( + "fmt" + "log/slog" + "os" + "path/filepath" + "syscall" +) + +// hasMutableSymlinkParent checks if any component of the resolved path is a symlink +// whose parent directory is writable by the current process. A writable parent means +// the symlink could be replaced between path resolution and actual file operation +// (TOCTOU symlink rebind attack). +func hasMutableSymlinkParent(path string) bool { + clean := filepath.Clean(path) + + // Traverse from leaf up to root to find all path components. + var components []string + curr := clean + for { + components = append([]string{curr}, components...) + parent := filepath.Dir(curr) + if parent == curr { + break + } + curr = parent + } + + for _, p := range components { + info, err := os.Lstat(p) + if err != nil { + break // non-existent — stop checking + } + if info.Mode()&os.ModeSymlink != 0 { + // Symlink found — check if its parent dir is writable + if isDirWritable(filepath.Dir(p)) { + return true + } + } + } + return false +} + +// isDirWritable checks if a directory is writable by attempting to open it with write access. +// On Windows, this is more robust than temporary file creation and avoids artifact leakage. +func isDirWritable(dir string) bool { + dirUTF16, err := syscall.UTF16PtrFromString(dir) + if err != nil { + return false + } + // FILE_FLAG_BACKUP_SEMANTICS is required to open a handle to a directory. + h, err := syscall.CreateFile( + dirUTF16, + syscall.GENERIC_WRITE, + syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE|syscall.FILE_SHARE_DELETE, + nil, + syscall.OPEN_EXISTING, + syscall.FILE_FLAG_BACKUP_SEMANTICS, + 0) + if err != nil { + return false + } + syscall.CloseHandle(h) + return true +} + +// checkHardlink rejects regular files with NumberOfLinks > 1 (hardlink attack prevention). +func checkHardlink(path string) error { + info, err := os.Lstat(path) + if err != nil || info.IsDir() { + return nil + } + + pathUTF16, err := syscall.UTF16PtrFromString(path) + if err != nil { + return nil + } + + h, err := syscall.CreateFile( + pathUTF16, + syscall.GENERIC_READ, + syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE|syscall.FILE_SHARE_DELETE, + nil, + syscall.OPEN_EXISTING, + syscall.FILE_FLAG_BACKUP_SEMANTICS, + 0) + if err != nil { + return nil + } + defer syscall.CloseHandle(h) + + var fi syscall.ByHandleFileInformation + if err := syscall.GetFileInformationByHandle(h, &fi); err != nil { + return nil + } + + if fi.NumberOfLinks > 1 { + slog.Warn("security.hardlink_rejected", "path", path, "nlink", fi.NumberOfLinks) + return fmt.Errorf("access denied: hardlinked file not allowed") + } + return nil +}