From a5aebfe4b0cd16d9e0d7f412209c19d2467de0a3 Mon Sep 17 00:00:00 2001 From: Dawei Wei Date: Tue, 11 Nov 2025 23:37:13 +0000 Subject: [PATCH] Fix Windows copy operations with protected system files at mount roots Fixes copying from Windows container mount roots by detecting and skipping protected system files ('System Volume Information' and 'WcSandboxState') that cause 'Access is denied' errors. Changes: - Added platformCopy wrapper for Windows copy operations - Implemented manual directory enumeration to skip protected files - Enabled test case testCopyRelativeParents and testCopyParentsMissingDirectory on Windows The fix is Windows-specific and only activates when copying from mount roots where protected files exist. All other copy operations use the standard path. Issue: Resolves #6635 [v0.26] WCOW: Fix COPY --parents tests Signed-off-by: Dawei Wei --- .../dockerfile/dockerfile_parents_test.go | 102 +++++++++++- solver/llbsolver/file/backend.go | 2 +- solver/llbsolver/file/backend_unix.go | 6 + solver/llbsolver/file/backend_windows.go | 149 ++++++++++++++++++ 4 files changed, 252 insertions(+), 7 deletions(-) diff --git a/frontend/dockerfile/dockerfile_parents_test.go b/frontend/dockerfile/dockerfile_parents_test.go index a1d5dc2874db..f43202ce8336 100644 --- a/frontend/dockerfile/dockerfile_parents_test.go +++ b/frontend/dockerfile/dockerfile_parents_test.go @@ -76,10 +76,10 @@ COPY --parents foo1/foo2/ba* . } func testCopyRelativeParents(t *testing.T, sb integration.Sandbox) { - integration.SkipOnPlatform(t, "windows") f := getFrontend(t, sb) - dockerfile := []byte(` + dockerfile := []byte(integration.UnixOrWindows( + ` FROM alpine AS base WORKDIR /test RUN < a\b\c\d\foo" +RUN cmd /C "echo. > a\b\c\d\e\bay" +RUN cmd /C "echo. > a\b2\c\d\e\bar" +RUN cmd /C "echo. > a\b\c2\d\e\baz" +RUN cmd /C "echo. > a\b\c2\d\e2\baz" + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS middle +COPY --from=base --parents /test/a/b/./c/d /out/ +RUN if not exist \out\c\d\e exit /b 1 +RUN if not exist \out\c\d\foo exit /b 1 +RUN if exist \out\a exit /b 1 +RUN if exist \out\e exit /b 1 + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS end +COPY --from=base --parents /test/a/b/c/d/. /out/ +RUN if not exist \out\test\a\b\c\d\e exit /b 1 +RUN if not exist \out\test\a\b\c\d\foo exit /b 1 + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS start +COPY --from=base --parents ./test/a/b/c/d /out/ +RUN if not exist \out\test\a\b\c\d\e exit /b 1 +RUN if not exist \out\test\a\b\c\d\foo exit /b 1 + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS double +COPY --from=base --parents /test/a/./b/./c /out/ +RUN if not exist \out\b\c\d\e exit /b 1 +RUN if not exist \out\b\c\d\foo exit /b 1 + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS wildcard +COPY --from=base --parents /test/a/./*/c /out/ +RUN if not exist \out\b\c\d\e exit /b 1 +RUN if not exist \out\b2\c\d\e\bar exit /b 1 + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS doublewildcard +COPY --from=base --parents /test/a/b*/./c/**/e /out/ +RUN if not exist \out\c\d\e exit /b 1 +RUN if not exist \out\c\d\e\bay exit /b 1 +RUN if not exist \out\c\d\e\bar exit /b 1 + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS doubleinputs +COPY --from=base --parents /test/a/b/c*/./d/**/baz /test/a/b*/./c/**/bar /out/ +RUN if not exist \out\d\e\baz exit /b 1 +RUN if exist \out\d\e\bay exit /b 1 +RUN if not exist \out\d\e2\baz exit /b 1 +RUN if not exist \out\c\d\e\bar exit /b 1 +`, + )) dir := integration.Tmpdir( t, @@ -182,10 +236,10 @@ eot } func testCopyParentsMissingDirectory(t *testing.T, sb integration.Sandbox) { - integration.SkipOnPlatform(t, "windows") f := getFrontend(t, sb) - dockerfile := []byte(` + dockerfile := []byte(integration.UnixOrWindows( + ` FROM alpine AS base WORKDIR /test RUN < a\b\c\d\foo" +RUN cmd /C "echo. > a\b\c\d\e\bay" + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS normal +COPY --from=base --parents /test/a/b/c/d /out/ +RUN if not exist \out\test\a\b\c\d\e exit /b 1 +RUN if not exist \out\test\a\b\c\d\e\bay exit /b 1 +RUN if exist \out\e exit /b 1 +RUN if exist \out\a exit /b 1 + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS withpivot +COPY --from=base --parents /test/a/b/./c/d /out/ +RUN if not exist \out\c\d\e exit /b 1 +RUN if not exist \out\c\d\foo exit /b 1 +RUN if exist \out\a exit /b 1 +RUN if exist \out\e exit /b 1 + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS nonexistentfile +COPY --from=base --parents /test/nonexistent-file /out/ + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS wildcard-nonexistent +COPY --from=base --parents /test/a/b2*/c /out/ +RUN if not exist \out exit /b 1 +RUN if exist \out\a exit /b 1 + +FROM mcr.microsoft.com/windows/nanoserver:ltsc2022 AS wildcard-afterpivot +COPY --from=base --parents /test/a/b/./c2* /out/ +RUN if not exist \out exit /b 1 +RUN if exist \out\a exit /b 1 +RUN if exist \out\c exit /b 1 +`, + )) dir := integration.Tmpdir( t, diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go index c4a512ffba9d..3e380d53b5c1 100644 --- a/solver/llbsolver/file/backend.go +++ b/solver/llbsolver/file/backend.go @@ -261,7 +261,7 @@ func docopy(ctx context.Context, src, dest string, action *pb.FileActionCopy, u continue } } - if err := copy.Copy(ctx, src, s, dest, destPath, opt...); err != nil { + if err := platformCopy(ctx, src, s, dest, destPath, opt...); err != nil { return errors.WithStack(err) } } diff --git a/solver/llbsolver/file/backend_unix.go b/solver/llbsolver/file/backend_unix.go index 7014f73433b3..92fb233fa892 100644 --- a/solver/llbsolver/file/backend_unix.go +++ b/solver/llbsolver/file/backend_unix.go @@ -3,6 +3,8 @@ package file import ( + "context" + "github.com/moby/sys/user" "github.com/pkg/errors" copy "github.com/tonistiigi/fsutil/copy" @@ -41,3 +43,7 @@ func mapUserToChowner(user *copy.User, idmap *user.IdentityMapping) (copy.Chowne return &u, nil }, nil } + +func platformCopy(ctx context.Context, srcRoot string, src string, destRoot string, dest string, opt ...copy.Opt) error { + return copy.Copy(ctx, srcRoot, src, destRoot, dest, opt...) +} diff --git a/solver/llbsolver/file/backend_windows.go b/solver/llbsolver/file/backend_windows.go index d35080be083c..88e3c98b9b8b 100644 --- a/solver/llbsolver/file/backend_windows.go +++ b/solver/llbsolver/file/backend_windows.go @@ -1,11 +1,24 @@ package file import ( + "context" + "os" + "path/filepath" + "strings" + + "github.com/containerd/continuity/fs" "github.com/moby/buildkit/util/windows" "github.com/moby/sys/user" copy "github.com/tonistiigi/fsutil/copy" ) +// windowsProtectedFiles contains Windows system files/folders that must be skipped +// during copy operations to avoid "Access denied" errors. +var windowsProtectedFiles = map[string]struct{}{ + "system volume information": {}, + "wcsandboxstate": {}, +} + func mapUserToChowner(user *copy.User, _ *user.IdentityMapping) (copy.Chowner, error) { if user == nil || user.SID == "" { return func(old *copy.User) (*copy.User, error) { @@ -21,3 +34,139 @@ func mapUserToChowner(user *copy.User, _ *user.IdentityMapping) (copy.Chowner, e return user, nil }, nil } + +// platformCopy wraps copy.Copy to handle Windows protected system folders. +// On Windows, container snapshots mounted to the host filesystem include protected folders +// ("System Volume Information" and "WcSandboxState") at the mount root, which cause "Access is denied" +// errors. When copying from the mount root, we manually enumerate and skip these folders. +func platformCopy(ctx context.Context, srcRoot string, src string, destRoot string, dest string, opt ...copy.Opt) error { + // Resolve the source path to check if we're copying from the mount root + srcPath, err := fs.RootPath(srcRoot, src) + if err != nil { + return err + } + + // Check if copying from mount root where protected folders exist + if filepath.Clean(srcPath) == filepath.Clean(srcRoot) { + // Check if any protected files exist (indicates a Windows mount root) + for protectedFile := range windowsProtectedFiles { + protectedPath := filepath.Join(srcRoot, protectedFile) + if _, err := os.Stat(protectedPath); err == nil { + // Use manual enumeration to skip protected folders + return copyByEnumeratingChildren(ctx, srcRoot, src, destRoot, dest, opt...) + } + } + } + // Normal case - use standard copy + return copy.Copy(ctx, srcRoot, src, destRoot, dest, opt...) +} + +// copyByEnumeratingChildren manually enumerates the root directory and copies each child, +// skipping Windows protected files. This is necessary because copy.Copy calls os.Lstat +// before checking exclude patterns, which causes "Access denied" errors. +// +// When IncludePatterns are present, this function adjusts them to be relative to each +// child being copied. For example, if copying from "/" with pattern "test/a/b/c/d", +// when copying the "test" child, the pattern is adjusted to "a/b/c/d". +func copyByEnumeratingChildren(ctx context.Context, srcRoot string, src string, destRoot string, dest string, opt ...copy.Opt) error { + // Extract CopyInfo to access IncludePatterns that control which files to copy + ci := copy.CopyInfo{} + for _, o := range opt { + o(&ci) + } + + // Resolve the actual filesystem path we're copying from + srcPath, err := fs.RootPath(srcRoot, src) + if err != nil { + return err + } + + // Enumerate all entries at the root level (before os.Lstat can fail on protected files) + entries, err := os.ReadDir(srcPath) + if err != nil { + return err + } + + // Resolve destination path + destPath, err := fs.RootPath(destRoot, dest) + if err != nil { + return err + } + + // Create the destination directory with same permissions as source + srcInfo, err := os.Lstat(srcPath) + if err != nil { + return err + } + if srcInfo.IsDir() { + if err := os.MkdirAll(destPath, srcInfo.Mode()); err != nil && !os.IsExist(err) { + return err + } + } + + // Process each child entry individually + for _, entry := range entries { + name := entry.Name() + + // Skip protected files that would cause "Access denied" errors + if _, isProtected := windowsProtectedFiles[strings.ToLower(name)]; isProtected { + continue + } + + // Build source and destination paths for this child + // Handle special case where src is root ("/", ".", or "") + childSrc := filepath.Join(src, name) + if src == "/" || src == "." || src == "" { + childSrc = "/" + name + } + childDest := filepath.Join(dest, name) + + // Adjust patterns to be relative to this child's path + // E.g., pattern "test/a/b" becomes "a/b" when copying child "test" + adjustedIncludePatterns := adjustIncludePatternsForChild(ci.IncludePatterns, name) + + // Determine whether to copy this child based on patterns + var childOpts []copy.Opt + if len(adjustedIncludePatterns) > 0 { + // Patterns match this child - use adjusted patterns + childCi := ci + childCi.IncludePatterns = adjustedIncludePatterns + childOpts = append(childOpts, copy.WithCopyInfo(childCi)) + } else if len(ci.IncludePatterns) == 0 { + // No filtering needed - copy everything + childOpts = opt + } else { + // Patterns specified but don't match this child - skip it + continue + } + + // Recursively copy this child using standard copy.Copy + // (safe now because we've already enumerated past the protected files) + if err := copy.Copy(ctx, srcRoot, childSrc, destRoot, childDest, childOpts...); err != nil { + return err + } + } + + return nil +} + +// adjustIncludePatternsForChild adjusts include patterns to be relative to a child directory. +// When copying from "/" with pattern "test/a/b/c" and processing child "test", +// the pattern is adjusted to "a/b/c" so it matches correctly under the new source root. +func adjustIncludePatternsForChild(patterns []string, childName string) []string { + var adjusted []string + prefix := childName + "/" + + for _, pattern := range patterns { + // Remove the child name prefix from the pattern + // Example: "test/a/b/c" → "a/b/c" for child "test" + if adjustedPattern, ok := strings.CutPrefix(pattern, prefix); ok { + adjusted = append(adjusted, adjustedPattern) + } else if pattern == childName { + // Pattern exactly matches this child, include everything under it + adjusted = append(adjusted, "**") + } + } + + return adjusted +}