diff --git a/packages/orchestrator/pkg/template/build/commands/copy.go b/packages/orchestrator/pkg/template/build/commands/copy.go index 828d6d0018..f1b9446ce9 100644 --- a/packages/orchestrator/pkg/template/build/commands/copy.go +++ b/packages/orchestrator/pkg/template/build/commands/copy.go @@ -57,7 +57,9 @@ var copyScriptTemplate = txtTemplate.Must(txtTemplate.New("copy-script-template" // 3) Extracts it (still in the /tmp directory) // 4) Moves the extracted files to the target path in the sandbox // - If the source is a file, it creates the parent directories and moves the file -// - If the source is a directory, it moves all its contents to the target directory +// - If the source is a directory, it merges its contents into the target +// directory (Docker COPY semantics: existing directories are merged into, +// existing files are overwritten) // Note: The temporary files in the /tmp directory are cleaned up automatically on sandbox restart // because the /tmp is mounted as a tmpfs and deleted on restart. diff --git a/packages/orchestrator/pkg/template/build/commands/copy_script.sh b/packages/orchestrator/pkg/template/build/commands/copy_script.sh index 3697c6eb44..dd414ed65c 100644 --- a/packages/orchestrator/pkg/template/build/commands/copy_script.sh +++ b/packages/orchestrator/pkg/template/build/commands/copy_script.sh @@ -1,5 +1,7 @@ #!/bin/bash +set -o pipefail + targetPath="{{ .TargetPath }}" sourcePath="{{ .SourcePath }}" owner="{{ .Owner }}" @@ -28,11 +30,11 @@ fi cd "$sourceFolder" || exit 1 -# Get the first entry (file, directory, or symlink) -entry=$(ls -A | head -n 1) +# Get the entry (file, directory, or symlink) named by the source path +entry="$(basename "$sourcePath")" -if [ -z "$entry" ]; then - echo "Error: sourceFolder is empty" +if [ ! -e "$entry" ] && [ ! -L "$entry" ]; then + echo "Error: source path does not exist: $sourcePath" exit 1 fi @@ -54,14 +56,24 @@ elif [ -f "$entry" ]; then mkdir -p "$(dirname "$targetPath")" mv "$entry" "$targetPath" elif [ -d "$entry" ]; then - # It's a directory – apply ownership/permissions recursively, then move contents + # It's a directory – apply ownership/permissions recursively, then merge + # its contents into the target (Docker COPY semantics: existing directories + # are merged into, existing files overwritten – mv can't merge into + # non-empty directories, so copy and remove the source instead) chown -R "$owner" "$entry" if [ -n "$permissions" ]; then chmod -R "$permissions" "$entry" fi mkdir -p "$targetPath" - # Move all contents including hidden files - find "$entry" -mindepth 1 -maxdepth 1 -exec mv {} "$targetPath/" \; + # Merge via tar, matching Docker's tar-based COPY: unlike cp, it replaces + # destination file symlinks instead of writing through them, follows + # destination directory symlinks (usrmerge, e.g. /lib -> usr/lib), and + # keeps the metadata of the target directory and other existing dirs + (cd "$entry" && tar -cf - .) | tar -xf - -C "$targetPath" --keep-directory-symlink --no-overwrite-dir || exit 1 + # Restore write permissions so cleanup works even when a read-only + # permissions argument was applied and we are not running as root + chmod -R u+rwx "$entry" + rm -rf "$entry" else echo "Error: entry is neither file, directory, nor symlink" exit 1 diff --git a/packages/orchestrator/pkg/template/build/commands/copy_test.go b/packages/orchestrator/pkg/template/build/commands/copy_test.go index 7a0864c405..e3d211d13c 100644 --- a/packages/orchestrator/pkg/template/build/commands/copy_test.go +++ b/packages/orchestrator/pkg/template/build/commands/copy_test.go @@ -9,6 +9,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -92,6 +93,12 @@ func renderTemplate(t *testing.T, data copyScriptData) string { // createFilesAndDirs creates files, directories, and symlinks from a map // Values: "file", "dir", "symlink" func createFilesAndDirs(t *testing.T, baseDir string, paths map[string]string) { + t.Helper() + createFilesAndDirsWithContent(t, baseDir, paths, "dummy") +} + +// createFilesAndDirsWithContent is createFilesAndDirs with custom file content +func createFilesAndDirsWithContent(t *testing.T, baseDir string, paths map[string]string, content string) { t.Helper() for path, entryType := range paths { fullPath := filepath.Join(baseDir, path) @@ -103,7 +110,7 @@ func createFilesAndDirs(t *testing.T, baseDir string, paths map[string]string) { // Ensure parent dir exists dir := filepath.Dir(fullPath) require.NoError(t, os.MkdirAll(dir, 0o755)) - require.NoError(t, os.WriteFile(fullPath, []byte("dummy"), 0o644)) + require.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) case "symlink": // Create symlink target outside the tree dir := filepath.Dir(fullPath) @@ -112,6 +119,13 @@ func createFilesAndDirs(t *testing.T, baseDir string, paths map[string]string) { require.NoError(t, os.WriteFile(targetFile, []byte("symlink target"), 0o644)) require.NoError(t, os.Symlink(targetFile, fullPath)) default: + // "symlink:" creates a symlink pointing at the given path + if target, ok := strings.CutPrefix(entryType, "symlink:"); ok { + require.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + require.NoError(t, os.Symlink(target, fullPath)) + + continue + } t.Fatalf("Unknown entry type: %s", entryType) } } @@ -132,6 +146,11 @@ func verifyFilesAndDirs(t *testing.T, baseDir string, paths map[string]string) { info, err := os.Lstat(fullPath) require.NoError(t, err, "Symlink %s should exist", path) assert.Equal(t, os.ModeSymlink, info.Mode()&os.ModeSymlink, "%s should be a symlink", path) + case "regular": + // A regular file, NOT a symlink pointing at one + info, err := os.Lstat(fullPath) + require.NoError(t, err, "File %s should exist", path) + assert.True(t, info.Mode().IsRegular(), "%s should be a regular file, got %s", path, info.Mode()) default: t.Fatalf("Unknown entry type: %s", entryType) } @@ -148,6 +167,10 @@ type testCase struct { // Example: {"app/": "dir", "app/main.js": "file", "link": "symlink"} files map[string]string + // Setup: paths pre-created in the target before the copy runs, + // to simulate copying over an existing filesystem tree + preexistingTargetPaths map[string]string + // Input: the path within the extracted files to copy from // Examples: "." (root), "app/" (subdirectory), "src/main.js" (specific file) copyFrom string @@ -168,6 +191,15 @@ type testCase struct { // Verification: what paths to check in the target with their types expectedPaths map[string]string + + // Verification: paths that must NOT exist in the target + absentPaths []string + + // Verification: file contents to check in the target + expectedContents map[string]string + + // Verification: octal permissions to check in the target + expectedPerms map[string]string } func TestParseCopyArgs(t *testing.T) { @@ -623,6 +655,178 @@ func TestCopyScriptBehavior(t *testing.T) { //nolint:paralleltest // no idea why "deep.txt": "file", }, }, + { + name: "merge_directory_into_existing_tree", + description: "COPY rootfs/etc /etc: merge into a target directory that already has content", + files: map[string]string{ + "etc/motd": "file", + "etc/profile.d/test-env.sh": "file", + }, + copyFrom: "etc/", + copyTo: "etc/", + preexistingTargetPaths: map[string]string{ + "etc/profile.d/existing.sh": "file", + }, + shouldSucceed: true, + expectedPaths: map[string]string{ + "etc/motd": "file", + "etc/profile.d/test-env.sh": "file", + "etc/profile.d/existing.sh": "file", + }, + }, + { + name: "merge_root_directory_into_existing_root", + description: "COPY rootfs/ /: every top-level dir already exists in the target root", + files: map[string]string{ + "rootfs/etc/profile.d/test-env.sh": "file", + "rootfs/usr/local/bin/tool": "file", + }, + copyFrom: "rootfs/", + copyTo: ".", + preexistingTargetPaths: map[string]string{ + "etc/profile.d/00-existing.sh": "file", + "usr/local/bin/existing-tool": "file", + }, + shouldSucceed: true, + expectedPaths: map[string]string{ + "etc/profile.d/test-env.sh": "file", + "etc/profile.d/00-existing.sh": "file", + "usr/local/bin/tool": "file", + "usr/local/bin/existing-tool": "file", + }, + }, + { + name: "overwrite_existing_file_in_target", + description: "Files that already exist in the target are overwritten", + files: map[string]string{ + "etc/motd": "file", + }, + copyFrom: "etc/", + copyTo: "etc/", + preexistingTargetPaths: map[string]string{ + "etc/motd": "file", + }, + shouldSucceed: true, + expectedPaths: map[string]string{ + "etc/motd": "file", + }, + expectedContents: map[string]string{ + "etc/motd": "dummy", + }, + }, + { + name: "preserve_target_directory_metadata", + description: "Only directory contents are copied; the existing target directory keeps its own permissions", + files: map[string]string{ + "etc/motd": "file", + }, + copyFrom: "etc/", + copyTo: "etc/", + // 700 must apply to the copied contents, not the existing target dir + permissions: "700", + preexistingTargetPaths: map[string]string{ + "etc/": "dir", + }, + shouldSucceed: true, + expectedPaths: map[string]string{ + "etc/motd": "file", + }, + expectedPerms: map[string]string{ + "etc": "755", + }, + }, + { + name: "directory_with_readonly_permissions", + description: "Read-only permissions on the copied tree do not break source cleanup", + files: map[string]string{ + "app/config.json": "file", + }, + copyFrom: "app/", + copyTo: "dest/", + permissions: "500", + shouldSucceed: true, + expectedPaths: map[string]string{ + "dest/config.json": "file", + }, + }, + { + name: "replace_target_symlink_to_file", + description: "A source file replaces a destination symlink instead of writing through it", + files: map[string]string{ + "etc/resolv.conf": "file", + }, + copyFrom: "etc/", + copyTo: "etc/", + preexistingTargetPaths: map[string]string{ + // resolv.conf points outside the target tree (as on Ubuntu images) + "../real/resolv.conf": "file", + "etc/resolv.conf": "symlink:../../real/resolv.conf", + }, + shouldSucceed: true, + expectedPaths: map[string]string{ + "etc/resolv.conf": "regular", + }, + expectedContents: map[string]string{ + "etc/resolv.conf": "dummy", + // The symlink's old target must not have been written through + "../real/resolv.conf": "preexisting", + }, + }, + { + name: "follow_target_symlink_to_directory", + description: "A source directory merges through a destination directory symlink (usrmerge layout)", + files: map[string]string{ + "lib/mylib.so": "file", + }, + copyFrom: ".", + copyTo: ".", + preexistingTargetPaths: map[string]string{ + "usr/lib/existing.so": "file", + "lib": "symlink:usr/lib", + }, + shouldSucceed: true, + expectedPaths: map[string]string{ + "lib": "symlink", + "usr/lib/mylib.so": "file", + "usr/lib/existing.so": "file", + }, + }, + { + name: "copy_directory_that_is_not_first_alphabetically", + description: "The entry named by the source path is copied, not the first entry in its parent", + files: map[string]string{ + "project/components/Button.tsx": "file", + "project/utils/helpers.ts": "file", + }, + copyFrom: "project/utils/", + copyTo: "out/", + shouldSucceed: true, + expectedPaths: map[string]string{ + "out/helpers.ts": "file", + }, + absentPaths: []string{ + "out/Button.tsx", + "out/components", + }, + }, + { + name: "copy_file_that_is_not_first_alphabetically", + description: "The file named by the source path is copied, not the first entry in its parent", + files: map[string]string{ + "assets/logo.png": "file", + "config.json": "file", + }, + copyFrom: "config.json", + copyTo: "dest/config.json", + shouldSucceed: true, + expectedPaths: map[string]string{ + "dest/config.json": "file", + }, + absentPaths: []string{ + "dest/logo.png", + "dest/config.json/logo.png", + }, + }, { name: "deeply_nested_folder", description: "Deeply nested folder should be copied correctly", @@ -656,6 +860,11 @@ func TestCopyScriptBehavior(t *testing.T) { //nolint:paralleltest // no idea why verifyFilesAndDirs(t, unpackDir, tc.files) } + // Pre-populate the target to simulate an existing filesystem tree + if len(tc.preexistingTargetPaths) > 0 { + createFilesAndDirsWithContent(t, targetBaseDir, tc.preexistingTargetPaths, "preexisting") + } + // Internal: construct SourcePath (sbxUnpackPath + user's copyFrom path) // This mimics how copy.go constructs the path: filepath.Join(sbxUnpackPath, sourcePath) sourcePath := filepath.Join(unpackDir, tc.copyFrom) @@ -701,6 +910,27 @@ func TestCopyScriptBehavior(t *testing.T) { //nolint:paralleltest // no idea why verifyFilesAndDirs(t, targetBaseDir, tc.expectedPaths) } + // Verify paths that must not exist in the target + for _, path := range tc.absentPaths { + assert.NoFileExists(t, filepath.Join(targetBaseDir, path), "Path %s should not exist", path) + assert.NoDirExists(t, filepath.Join(targetBaseDir, path), "Path %s should not exist", path) + } + + // Verify file contents + for path, expectedContent := range tc.expectedContents { + content, err := os.ReadFile(filepath.Join(targetBaseDir, path)) + require.NoError(t, err, "Failed to read file %s", path) + assert.Equal(t, expectedContent, string(content), "File %s content mismatch", path) + } + + // Verify permissions of specific paths + for path, permStr := range tc.expectedPerms { + perms := getFilePermissions(t, filepath.Join(targetBaseDir, path)) + expectedPerms := os.FileMode(0) + fmt.Sscanf(permStr, "%o", &expectedPerms) + assert.Equal(t, expectedPerms, perms, "Path %s should have %s permissions", path, permStr) + } + // Special verification for permissions tests if tc.permissions != "" && tc.shouldSucceed { for path, entryType := range tc.expectedPaths {