fix(orchestrator): implement Docker COPY merge semantics in template builds (#3283)

## Problem

The template build `COPY` command silently dropped files whenever a
copied directory already existed non-empty at the destination:

- `COPY rootfs/ /` dropped **everything** (every top-level dir like
`etc`, `usr` already exists in `/`), leaving files only in the `/tmp`
unpack dir
- `COPY rootfs/etc /etc` copied `/etc/motd` but silently dropped
`/etc/profile.d/test-env.sh`, because `/etc/profile.d` already exists on
the Ubuntu image

Root cause: `copy_script.sh` moved directory contents with `find -exec
mv`, but `mv` cannot merge into a non-empty directory and `find -exec
\;` exits 0 even when the command fails. A second bug made the script
operate on the alphabetically-first entry of the source's parent
directory instead of the entry named by the source path.

## Fix

Merge directory contents with `cp -a` (existing directories merged,
existing files overwritten — Docker COPY semantics), select the entry by
`basename` of the source path, and fail the build step on copy errors
instead of swallowing them. Added regression tests covering merging into
existing trees, overwriting existing files, and correct entry selection;
verified they reproduce the bugs against the old script and pass (with
`-race`) against the fix, with golangci-lint v2.11.4 clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Author: 3 people

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.

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

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:<target>" 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 {