diff --git a/cmd/podman/containers/cp_remote.go b/cmd/podman/containers/cp_remote.go index 80b1e7cbfc..8e834fd5b5 100644 --- a/cmd/podman/containers/cp_remote.go +++ b/cmd/podman/containers/cp_remote.go @@ -14,6 +14,7 @@ import ( "github.com/containers/podman/v4/cmd/podman/registry" "github.com/containers/podman/v4/pkg/copy" "github.com/containers/podman/v4/pkg/domain/entities" + "github.com/containers/storage/pkg/archive" "github.com/spf13/cobra" ) @@ -25,7 +26,7 @@ func cp(cmd *cobra.Command, args []string) error { } if len(sourceContainerStr) > 0 && len(destContainerStr) > 0 { - return errors.New("copying between containers is not supported with podman-remote") + return copyBetweenContainersRemote(sourceContainerStr, sourcePath, destContainerStr, destPath) } else if len(sourceContainerStr) > 0 { return copyFromContainerRemote(sourceContainerStr, sourcePath, destPath) } @@ -35,6 +36,12 @@ func cp(cmd *cobra.Command, args []string) error { // copyFromContainerRemote copies from the containerPath on the container to hostPath. func copyFromContainerRemote(container string, containerPath string, hostPath string) error { + // Validate: /dev/stdout is not a valid destination + // Only "-" gets special treatment as stdout + if hostPath == "/dev/stdout" { + return fmt.Errorf("invalid destination: %q must be a directory or a regular file", hostPath) + } + isStdout := hostPath == "-" if isStdout { hostPath = os.Stdout.Name() @@ -68,8 +75,34 @@ func copyFromContainerRemote(container string, containerPath string, hostPath st return err } + // Validate: when copying a directory, destination must be a directory or non-existent + if containerInfo.IsDir { + if hostInfo, err := os.Stat(hostPath); err == nil { + // Destination exists, check if it's a directory + if !hostInfo.IsDir() { + return errors.New("destination must be a directory when copying a directory") + } + } + } + // Extract tar to destination - if err := extractTar(reader, hostPath, containerInfo.IsDir); err != nil { + // When copying a directory to a non-existent destination, we need to strip + // the source directory name from tar entries. For example, when copying + // /srv to /newdir, the tar contains "srv/subdir/file" but we want to extract + // to "/newdir/subdir/file" not "/newdir/srv/subdir/file". + stripComponents := 0 + if containerInfo.IsDir { + // Check if destination exists + if _, err := os.Stat(hostPath); os.IsNotExist(err) { + // Destination doesn't exist, strip the source directory name + // unless we're copying contents only (path ends with /.) + if !strings.HasSuffix(containerPath, "/.") { + stripComponents = 1 + } + } + } + + if err := extractTar(reader, hostPath, containerInfo.IsDir, stripComponents, cpOpts.OverwriteDirNonDir); err != nil { return err } @@ -78,15 +111,42 @@ func copyFromContainerRemote(container string, containerPath string, hostPath st // copyToContainerRemote copies from hostPath to the containerPath on the container. func copyToContainerRemote(container string, containerPath string, hostPath string) error { - isStdin := hostPath == "-" + isStdin := hostPath == "-" || hostPath == "/dev/stdin" || hostPath == os.Stdin.Name() + var stdinFile string if isStdin { - hostPath = os.Stdin.Name() + // Copy from stdin to a temporary file to validate it's a tar archive + // This provides proper client-side error reporting + tmpFile, err := os.CreateTemp("", "podman-cp-") + if err != nil { + return err + } + defer os.Remove(tmpFile.Name()) + + _, err = io.Copy(tmpFile, os.Stdin) + if err != nil { + tmpFile.Close() + return err + } + if err = tmpFile.Close(); err != nil { + return err + } + + if !archive.IsArchivePath(tmpFile.Name()) { + return errors.New("source must be a (compressed) tar archive when copying from stdin") + } + + stdinFile = tmpFile.Name() + hostPath = stdinFile } - // Get info about the host path - hostInfo, err := os.Stat(hostPath) - if err != nil { - return fmt.Errorf("%q could not be found on the host: %w", hostPath, err) + // Get info about the host path (skip stat for stdin) + var hostInfo os.FileInfo + var err error + if !isStdin { + hostInfo, err = os.Stat(hostPath) + if err != nil { + return fmt.Errorf("%q could not be found on the host: %w", hostPath, err) + } } // Get info about the container destination path @@ -98,23 +158,34 @@ func copyToContainerRemote(container string, containerPath string, hostPath stri var containerBaseName string if err != nil { - // Container path doesn't exist + // Container path doesn't exist (or is a broken symlink) // If path has trailing /, it must be a directory (error if it doesn't exist) if strings.HasSuffix(containerPath, "/") { return fmt.Errorf("%q could not be found on container %s: %w", containerPath, container, err) } - containerExists = false - // If we're copying contents only (source ends with /.), use the dest path directly - // The server will create it as a directory - if strings.HasSuffix(hostPath, "/.") { - targetPath = containerPath - containerResolvedToParentDir = false + // If containerInfo is not nil, it's a symlink even if target doesn't exist + if containerInfo != nil && containerInfo.LinkTarget != "" { + // Broken symlink - treat like a file and use the symlink target + containerExists = true + containerIsDir = false + targetPath = filepath.Dir(containerInfo.LinkTarget) + containerBaseName = filepath.Base(containerInfo.LinkTarget) } else { - // Otherwise, use parent directory and rename - containerResolvedToParentDir = true - targetPath = filepath.Dir(containerPath) - containerBaseName = filepath.Base(containerPath) + // Path truly doesn't exist + containerExists = false + + // When copying from stdin or copying contents only (source ends with /.), + // use the dest path directly - the server will create it as a directory + if isStdin || strings.HasSuffix(hostPath, "/.") { + targetPath = containerPath + containerResolvedToParentDir = false + } else { + // Otherwise, use parent directory and rename + containerResolvedToParentDir = true + targetPath = filepath.Dir(containerPath) + containerBaseName = filepath.Base(containerPath) + } } } else { containerExists = true @@ -130,10 +201,17 @@ func copyToContainerRemote(container string, containerPath string, hostPath stri } // Validate: can't copy directory to a file - if hostInfo.IsDir() && containerExists && !containerIsDir { + if !isStdin && hostInfo.IsDir() && containerExists && !containerIsDir { return errors.New("destination must be a directory when copying a directory") } + // When copying from stdin, destination must exist and be a directory + if isStdin { + if !containerExists || !containerIsDir { + return errors.New("destination must be a directory when copying from stdin") + } + } + reader, writer := io.Pipe() // Create tar archive from host path in a goroutine @@ -141,7 +219,14 @@ func copyToContainerRemote(container string, containerPath string, hostPath stri go func() { defer writer.Close() if isStdin { - _, tarErr = io.Copy(writer, os.Stdin) + // Read from the temp file we created + f, err := os.Open(stdinFile) + if err != nil { + tarErr = err + return + } + defer f.Close() + _, tarErr = io.Copy(writer, f) } else { tarErr = createTar(hostPath, writer) } @@ -151,12 +236,13 @@ func copyToContainerRemote(container string, containerPath string, hostPath stri defer reader.Close() copyOptions := entities.CopyOptions{ - Chown: chown, + Chown: chown, + NoOverwriteDirNonDir: !cpOpts.OverwriteDirNonDir, } // If we're copying to a non-existent path or file-to-file, use Rename - // But NOT when copying contents only (hostPath ends with /.) - if ((!hostInfo.IsDir() && !containerIsDir) || containerResolvedToParentDir) && !strings.HasSuffix(hostPath, "/.") { + // But NOT when copying from stdin or when copying contents only (hostPath ends with /.) + if !isStdin && ((!hostInfo.IsDir() && !containerIsDir) || containerResolvedToParentDir) && !strings.HasSuffix(hostPath, "/.") { copyOptions.Rename = map[string]string{filepath.Base(hostPath): containerBaseName} } @@ -172,6 +258,109 @@ func copyToContainerRemote(container string, containerPath string, hostPath stri return tarErr } +// copyBetweenContainersRemote copies from source container to destination container. +func copyBetweenContainersRemote(sourceContainer string, sourcePath string, destContainer string, destPath string) error { + // Get the file info from the source container + sourceInfo, err := registry.ContainerEngine().ContainerStat(registry.GetContext(), sourceContainer, sourcePath) + if err != nil { + return fmt.Errorf("%q could not be found on container %s: %w", sourcePath, sourceContainer, err) + } + + // Get info about the destination container path + destInfo, err := registry.ContainerEngine().ContainerStat(registry.GetContext(), destContainer, destPath) + var destExists bool + var destIsDir bool + var destResolvedToParentDir bool + var targetPath string + var destBaseName string + + if err != nil { + // Destination path doesn't exist (or is a broken symlink) + // If path has trailing /, it must be a directory (error if it doesn't exist) + if strings.HasSuffix(destPath, "/") { + return fmt.Errorf("%q could not be found on container %s: %w", destPath, destContainer, err) + } + + // If destInfo is not nil, it's a symlink even if target doesn't exist + if destInfo != nil && destInfo.LinkTarget != "" { + // Broken symlink - treat like a file and use the symlink target + destExists = true + destIsDir = false + targetPath = filepath.Dir(destInfo.LinkTarget) + destBaseName = filepath.Base(destInfo.LinkTarget) + } else { + // Path truly doesn't exist + destExists = false + + // If we're copying contents only (source ends with /.), use the dest path directly + if strings.HasSuffix(sourcePath, "/.") { + targetPath = destPath + destResolvedToParentDir = false + } else { + // Otherwise, use parent directory and rename + destResolvedToParentDir = true + targetPath = filepath.Dir(destPath) + destBaseName = filepath.Base(destPath) + } + } + } else { + destExists = true + destIsDir = destInfo.IsDir + if destIsDir { + // Destination is a directory - extract into it + targetPath = destPath + } else { + // Destination is a file - use parent directory + targetPath = filepath.Dir(destInfo.LinkTarget) + destBaseName = filepath.Base(destInfo.LinkTarget) + } + } + + // Validate: can't copy directory to a file + if sourceInfo.IsDir && destExists && !destIsDir { + return errors.New("destination must be a directory when copying a directory") + } + + reader, writer := io.Pipe() + + // Copy from source container in a goroutine + var copyFromErr error + go func() { + defer writer.Close() + copyFunc, err := registry.ContainerEngine().ContainerCopyToArchive(registry.GetContext(), sourceContainer, sourceInfo.LinkTarget, writer) + if err != nil { + copyFromErr = err + return + } + copyFromErr = copyFunc() + }() + + // Copy to destination container + defer reader.Close() + + copyOptions := entities.CopyOptions{ + Chown: chown, + NoOverwriteDirNonDir: !cpOpts.OverwriteDirNonDir, + } + + // If we're copying to a non-existent path or file-to-file, use Rename + // But NOT when copying contents only (sourcePath ends with /.) + if ((!sourceInfo.IsDir && !destIsDir) || destResolvedToParentDir) && !strings.HasSuffix(sourcePath, "/.") { + copyOptions.Rename = map[string]string{filepath.Base(sourceInfo.LinkTarget): destBaseName} + } + + copyFunc, err := registry.ContainerEngine().ContainerCopyFromArchive(registry.GetContext(), destContainer, targetPath, reader, copyOptions) + if err != nil { + return err + } + + if err := copyFunc(); err != nil { + return err + } + + return copyFromErr +} + // createTar creates a tar archive from the specified path func createTar(sourcePath string, writer io.Writer) error { tw := tar.NewWriter(writer) @@ -233,7 +422,7 @@ func createTar(sourcePath string, writer io.Writer) error { } // extractTar extracts a tar archive to the specified destination -func extractTar(reader io.Reader, destPath string, isDir bool) error { +func extractTar(reader io.Reader, destPath string, isDir bool, stripComponents int, overwrite bool) error { tr := tar.NewReader(reader) // Check if destination exists @@ -250,16 +439,43 @@ func extractTar(reader io.Reader, destPath string, isDir bool) error { return err } + // Strip leading path components if requested + name := header.Name + if stripComponents > 0 { + parts := strings.Split(filepath.Clean(name), string(filepath.Separator)) + if len(parts) > stripComponents { + name = filepath.Join(parts[stripComponents:]...) + } else { + // Skip entries that would be completely stripped + continue + } + } + var target string // If dest doesn't exist and we're extracting a single file, use dest as the filename if !destExists && !isDir && header.Typeflag == tar.TypeReg { target = destPath } else if destIsDir { // Dest is a directory, extract into it - target = filepath.Join(destPath, header.Name) + target = filepath.Join(destPath, name) } else { // Dest exists but isn't a directory, or we're extracting a directory - target = filepath.Join(destPath, header.Name) + target = filepath.Join(destPath, name) + } + + // Check if target exists and handle overwrite + if targetInfo, err := os.Lstat(target); err == nil { + targetIsDir := targetInfo.IsDir() + // If types don't match (file vs directory) + if (header.Typeflag == tar.TypeDir && !targetIsDir) || (header.Typeflag == tar.TypeReg && targetIsDir) { + if !overwrite { + return fmt.Errorf("error creating %q: file exists", filepath.Join("/", name)) + } + // Remove the existing path to allow overwrite + if err := os.RemoveAll(target); err != nil { + return err + } + } } switch header.Typeflag {