rewrite podman-cp

* Add a new `pkg/copy` to centralize all container-copy related code.

* The new code is based on Buildah's `copier` package.

* The compat `/archive` endpoints use the new `copy` package.

* Update docs and an several new tests.

* Includes many fixes, most notably, the look-up of volumes and mounts.

Breaking changes:

 * Podman is now expecting that container-destination paths exist.
   Before, Podman created the paths if needed.  Docker does not do
   that and I believe Podman should not either as it's a recipe for
   masking errors.  These errors may be user induced (e.g., a path
   typo), or internal typos (e.g., when the destination may be a
   mistakenly unmounted volume).  Let's keep the magic low for such
   a security sensitive feature.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

cmd/podman/containers/cp.go

@@ -3,10 +3,7 @@ package containers
 import (
 	"github.com/containers/podman/v2/cmd/podman/common"
 	"github.com/containers/podman/v2/cmd/podman/registry"
-	"github.com/containers/podman/v2/pkg/cgroups"
 	"github.com/containers/podman/v2/pkg/domain/entities"
-	"github.com/containers/podman/v2/pkg/rootless"
-	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -43,7 +40,7 @@ var (
 func cpFlags(cmd *cobra.Command) {
 	flags := cmd.Flags()
 	flags.BoolVar(&cpOpts.Extract, "extract", false, "Extract the tar file into the destination directory.")
-	flags.BoolVar(&cpOpts.Pause, "pause", copyPause(), "Pause the container while copying")
+	flags.BoolVar(&cpOpts.Pause, "pause", true, "Pause the container while copying")
 }
 
 func init() {
@@ -62,17 +59,5 @@ func init() {
 }
 
 func cp(cmd *cobra.Command, args []string) error {
-	_, err := registry.ContainerEngine().ContainerCp(registry.GetContext(), args[0], args[1], cpOpts)
-	return err
-}
-
-func copyPause() bool {
-	if rootless.IsRootless() {
-		cgroupv2, _ := cgroups.IsCgroup2UnifiedMode()
-		if !cgroupv2 {
-			logrus.Debugf("defaulting to pause==false on rootless cp in cgroupv1 systems")
-			return false
-		}
-	}
-	return true
+	return registry.ContainerEngine().ContainerCp(registry.GetContext(), args[0], args[1], cpOpts)
 }

docs/source/markdown/podman-cp.1.md

@@ -9,12 +9,12 @@ podman\-cp - Copy files/folders between a container and the local filesystem
 **podman container cp** [*options*] [*container*:]*src_path* [*container*:]*dest_path*
 
 ## DESCRIPTION
-Copies the contents of **src_path** to the **dest_path**. You can copy from the container's filesystem to the local machine or the reverse, from the local filesystem to the container.
-If - is specified for either the SRC_PATH or DEST_PATH, you can also stream a tar archive from STDIN or to STDOUT.
+Copy the contents of **src_path** to the **dest_path**. You can copy from the container's filesystem to the local machine or the reverse, from the local filesystem to the container.
+If `-` is specified for either the SRC_PATH or DEST_PATH, you can also stream a tar archive from STDIN or to STDOUT.
 
 The CONTAINER can be a running or stopped container. The **src_path** or **dest_path** can be a file or directory.
 
-The **podman cp** command assumes container paths are relative to the container's / (root) directory.
+The **podman cp** command assumes container paths are relative to the container's root directory (i.e., `/`).
 
 This means supplying the initial forward slash is optional;
 
@@ -27,9 +27,7 @@ Assuming a path separator of /, a first argument of **src_path** and second argu
 
 **src_path** specifies a file
   - **dest_path** does not exist
-	- the file is saved to a file created at **dest_path**
-  - **dest_path** does not exist and ends with /
-	- Error condition: the destination directory must exist.
+    - the file is saved to a file created at **dest_path** (note that parent directory must exist)
   - **dest_path** exists and is a file
     - the destination is overwritten with the source file's contents
   - **dest_path** exists and is a directory
@@ -41,9 +39,9 @@ Assuming a path separator of /, a first argument of **src_path** and second argu
   - **dest_path** exists and is a file
     - Error condition: cannot copy a directory to a file
   - **dest_path** exists and is a directory
-	- **src_path** ends with /
+    - **src_path** ends with `/`
       - the source directory is copied into this directory
-	- **src_path** ends with /. (that is: slash followed by dot)
+    - **src_path** ends with `/.` (i.e., slash followed by dot)
       - the content of the source directory is copied into this directory
 
 The command requires **src_path** and **dest_path** to exist according to the above rules.
@@ -57,11 +55,13 @@ You can also use : when specifying paths to a **src_path** or **dest_path** on a
 If you use a : in a local machine path, you must be explicit with a relative or absolute path, for example:
 	`/path/to/file:name.txt` or `./file:name.txt`
 
+Using `-` as the *src_path* streams the contents of STDIN as a tar archive. The command extracts the content of the tar to the *DEST_PATH* in the container. In this case, *dest_path* must specify a directory. Using `-` as the *dest_path* streams the contents of the resource (can be a directory) as a tar archive to STDOUT.
+
 ## OPTIONS
 
 #### **--extract**
 
-Extract the tar file into the destination directory. If the destination directory is not provided, extract the tar file into the root directory.
+If the source is a tar archive, extract it to the provided destination (must be a directory).  If the source is not a tar archive, follow the above rules.
 
 #### **--pause**
 

libpod/container.go

@@ -267,6 +267,11 @@ func (c *Container) Config() *ContainerConfig {
 	return returnConfig
 }
 
+// Runtime returns the container's Runtime.
+func (c *Container) Runtime() *Runtime {
+	return c.runtime
+}
+
 // Spec returns the container's OCI runtime spec
 // The spec returned is the one used to create the container. The running
 // spec may differ slightly as mounts are added based on the image

libpod/events/config.go

@@ -121,6 +121,8 @@ const (
 	Cleanup Status = "cleanup"
 	// Commit ...
 	Commit Status = "commit"
+	// Copy ...
+	Copy Status = "copy"
 	// Create ...
 	Create Status = "create"
 	// Exec ...

pkg/api/handlers/compat/containers_archive.go

@@ -5,24 +5,16 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"path/filepath"
-	"strings"
-
-	"github.com/containers/buildah/copier"
-	"github.com/containers/buildah/pkg/chrootuser"
-	"github.com/containers/podman/v2/libpod"
-	"github.com/containers/podman/v2/libpod/define"
-	"github.com/containers/podman/v2/pkg/api/handlers/utils"
-	"github.com/containers/storage/pkg/idtools"
-	"github.com/opencontainers/runtime-spec/specs-go"
-
 	"net/http"
 	"os"
 	"time"
 
+	"github.com/containers/podman/v2/libpod"
+	"github.com/containers/podman/v2/libpod/define"
+	"github.com/containers/podman/v2/pkg/api/handlers/utils"
+	"github.com/containers/podman/v2/pkg/copy"
 	"github.com/gorilla/schema"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
 func Archive(w http.ResponseWriter, r *http.Request) {
@@ -32,14 +24,14 @@ func Archive(w http.ResponseWriter, r *http.Request) {
 	switch r.Method {
 	case http.MethodPut:
 		handlePut(w, r, decoder, runtime)
-	case http.MethodGet, http.MethodHead:
-		handleHeadOrGet(w, r, decoder, runtime)
+	case http.MethodHead, http.MethodGet:
+		handleHeadAndGet(w, r, decoder, runtime)
 	default:
-		utils.Error(w, fmt.Sprintf("not implemented, method: %v", r.Method), http.StatusNotImplemented, errors.New(fmt.Sprintf("not implemented, method: %v", r.Method)))
+		utils.Error(w, fmt.Sprintf("unsupported method: %v", r.Method), http.StatusNotImplemented, errors.New(fmt.Sprintf("unsupported method: %v", r.Method)))
 	}
 }
 
-func handleHeadOrGet(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder, runtime *libpod.Runtime) {
+func handleHeadAndGet(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder, runtime *libpod.Runtime) {
 	query := struct {
 		Path string `schema:"path"`
 	}{}
@@ -66,84 +58,82 @@ func handleHeadOrGet(w http.ResponseWriter, r *http.Request, decoder *schema.Dec
 		return
 	}
 
-	mountPoint, err := ctr.Mount()
+	source, err := copy.CopyItemForContainer(ctr, query.Path, true, true)
+	defer source.CleanUp()
 	if err != nil {
-		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "failed to mount the container"))
+		utils.Error(w, "Not found.", http.StatusNotFound, errors.Wrapf(err, "error stating container path %q", query.Path))
 		return
 	}
 
-	defer func() {
-		if err := ctr.Unmount(false); err != nil {
-			logrus.Warnf("failed to unmount container %s: %q", containerName, err)
-		}
-	}()
-
-	opts := copier.StatOptions{}
-
-	mountPoint, path, err := fixUpMountPointAndPath(runtime, ctr, mountPoint, query.Path)
+	// NOTE: Docker always sets the header.
+	info, err := source.Stat()
 	if err != nil {
-		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
+		utils.Error(w, "Not found.", http.StatusNotFound, errors.Wrapf(err, "error stating container path %q", query.Path))
 		return
 	}
-
-	stats, err := copier.Stat(mountPoint, "", opts, []string{filepath.Join(mountPoint, path)})
+	statHeader, err := fileInfoToDockerStats(info)
 	if err != nil {
-		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "failed to get stats about file"))
+		utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
 		return
 	}
-
-	if len(stats) <= 0 || len(stats[0].Globbed) <= 0 {
-		errs := make([]string, 0, len(stats))
-
-		for _, stat := range stats {
-			if stat.Error != "" {
-				errs = append(errs, stat.Error)
-			}
-		}
-
-		utils.Error(w, "Not found.", http.StatusNotFound, fmt.Errorf("file doesn't exist (errs: %q)", strings.Join(errs, ";")))
-
-		return
-	}
-
-	statHeader, err := statsToHeader(stats[0].Results[stats[0].Globbed[0]])
-	if err != nil {
-		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, err)
-		return
-	}
-
 	w.Header().Add("X-Docker-Container-Path-Stat", statHeader)
 
-	if r.Method == http.MethodGet {
-		idMappingOpts, err := ctr.IDMappings()
-		if err != nil {
-			utils.Error(w, "Not found.", http.StatusInternalServerError,
-				errors.Wrapf(err, "error getting IDMappingOptions"))
+	// Our work is done when the user is interested in the header only.
+	if r.Method == http.MethodHead {
+		w.WriteHeader(http.StatusOK)
 		return
 	}
 
-		destOwner := idtools.IDPair{UID: os.Getuid(), GID: os.Getgid()}
-
-		opts := copier.GetOptions{
-			UIDMap:             idMappingOpts.UIDMap,
-			GIDMap:             idMappingOpts.GIDMap,
-			ChownDirs:          &destOwner,
-			ChownFiles:         &destOwner,
-			KeepDirectoryNames: true,
-		}
-
-		w.WriteHeader(http.StatusOK)
-
-		err = copier.Get(mountPoint, "", opts, []string{filepath.Join(mountPoint, path)}, w)
+	// Alright, the users wants data from the container.
+	destination, err := copy.CopyItemForWriter(w)
 	if err != nil {
-			logrus.Error(errors.Wrapf(err, "failed to copy from the %s container path %s", containerName, query.Path))
+		utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
 		return
 	}
-	} else {
+
 	w.WriteHeader(http.StatusOK)
+	if err := copy.Copy(&source, &destination, false); err != nil {
+		utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
+		return
 	}
 }
 
+func fileInfoToDockerStats(info *copy.FileInfo) (string, error) {
+	dockerStats := struct {
+		Name       string      `json:"name"`
+		Size       int64       `json:"size"`
+		Mode       os.FileMode `json:"mode"`
+		ModTime    time.Time   `json:"mtime"`
+		LinkTarget string      `json:"linkTarget"`
+	}{
+		Name:       info.Name,
+		Size:       info.Size,
+		Mode:       info.Mode,
+		ModTime:    info.ModTime,
+		LinkTarget: info.LinkTarget,
+	}
+
+	jsonBytes, err := json.Marshal(&dockerStats)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to serialize file stats")
+	}
+
+	buff := bytes.NewBuffer(make([]byte, 0, 128))
+	base64encoder := base64.NewEncoder(base64.StdEncoding, buff)
+
+	_, err = base64encoder.Write(jsonBytes)
+	if err != nil {
+		return "", err
+	}
+
+	err = base64encoder.Close()
+	if err != nil {
+		return "", err
+	}
+
+	return buff.String(), nil
+}
+
 func handlePut(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder, runtime *libpod.Runtime) {
 	query := struct {
 		Path string `schema:"path"`
@@ -166,214 +156,23 @@ func handlePut(w http.ResponseWriter, r *http.Request, decoder *schema.Decoder,
 		return
 	}
 
-	mountPoint, err := ctr.Mount()
-	if err != nil {
-		utils.Error(w, "Something went wrong", http.StatusInternalServerError, errors.Wrapf(err, "failed to mount the %s container", ctrName))
-		return
-	}
-
-	defer func() {
-		if err := ctr.Unmount(false); err != nil {
-			logrus.Warnf("failed to unmount container %s", ctrName)
-		}
-	}()
-
-	user, err := getUser(mountPoint, ctr.User())
+	destination, err := copy.CopyItemForContainer(ctr, query.Path, true, false)
+	defer destination.CleanUp()
 	if err != nil {
 		utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
 		return
 	}
 
-	idMappingOpts, err := ctr.IDMappings()
-	if err != nil {
-		utils.Error(w, "Something went wrong", http.StatusInternalServerError, errors.Wrapf(err, "error getting IDMappingOptions"))
-		return
-	}
-
-	destOwner := idtools.IDPair{UID: int(user.UID), GID: int(user.GID)}
-
-	opts := copier.PutOptions{
-		UIDMap:     idMappingOpts.UIDMap,
-		GIDMap:     idMappingOpts.GIDMap,
-		ChownDirs:  &destOwner,
-		ChownFiles: &destOwner,
-	}
-
-	mountPoint, path, err := fixUpMountPointAndPath(runtime, ctr, mountPoint, query.Path)
+	source, err := copy.CopyItemForReader(r.Body)
+	defer source.CleanUp()
 	if err != nil {
 		utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
 		return
 	}
 
 	w.WriteHeader(http.StatusOK)
-
-	err = copier.Put(mountPoint, filepath.Join(mountPoint, path), opts, r.Body)
-	if err != nil {
-		logrus.Error(errors.Wrapf(err, "failed to copy to the %s container path %s", ctrName, query.Path))
+	if err := copy.Copy(&source, &destination, false); err != nil {
+		utils.Error(w, "Something went wrong", http.StatusInternalServerError, err)
 		return
 	}
 }
-
-func statsToHeader(stats *copier.StatForItem) (string, error) {
-	statsDTO := struct {
-		Name       string      `json:"name"`
-		Size       int64       `json:"size"`
-		Mode       os.FileMode `json:"mode"`
-		ModTime    time.Time   `json:"mtime"`
-		LinkTarget string      `json:"linkTarget"`
-	}{
-		Name:       filepath.Base(stats.Name),
-		Size:       stats.Size,
-		Mode:       stats.Mode,
-		ModTime:    stats.ModTime,
-		LinkTarget: stats.ImmediateTarget,
-	}
-
-	jsonBytes, err := json.Marshal(&statsDTO)
-	if err != nil {
-		return "", errors.Wrap(err, "failed to serialize file stats")
-	}
-
-	buff := bytes.NewBuffer(make([]byte, 0, 128))
-	base64encoder := base64.NewEncoder(base64.StdEncoding, buff)
-
-	_, err = base64encoder.Write(jsonBytes)
-	if err != nil {
-		return "", err
-	}
-
-	err = base64encoder.Close()
-	if err != nil {
-		return "", err
-	}
-
-	return buff.String(), nil
-}
-
-// the utility functions below are copied from abi/cp.go
-
-func getUser(mountPoint string, userspec string) (specs.User, error) {
-	uid, gid, _, err := chrootuser.GetUser(mountPoint, userspec)
-	u := specs.User{
-		UID:      uid,
-		GID:      gid,
-		Username: userspec,
-	}
-
-	if !strings.Contains(userspec, ":") {
-		groups, err2 := chrootuser.GetAdditionalGroupsForUser(mountPoint, uint64(u.UID))
-		if err2 != nil {
-			if errors.Cause(err2) != chrootuser.ErrNoSuchUser && err == nil {
-				err = err2
-			}
-		} else {
-			u.AdditionalGids = groups
-		}
-	}
-
-	return u, err
-}
-
-func fixUpMountPointAndPath(runtime *libpod.Runtime, ctr *libpod.Container, mountPoint, ctrPath string) (string, string, error) {
-	if !filepath.IsAbs(ctrPath) {
-		endsWithSep := strings.HasSuffix(ctrPath, string(filepath.Separator))
-		ctrPath = filepath.Join(ctr.WorkingDir(), ctrPath)
-
-		if endsWithSep {
-			ctrPath = ctrPath + string(filepath.Separator)
-		}
-	}
-	if isVol, volDestName, volName := isVolumeDestName(ctrPath, ctr); isVol { //nolint(gocritic)
-		newMountPoint, path, err := pathWithVolumeMount(runtime, volDestName, volName, ctrPath)
-		if err != nil {
-			return "", "", errors.Wrapf(err, "error getting source path from volume %s", volDestName)
-		}
-
-		mountPoint = newMountPoint
-		ctrPath = path
-	} else if isBindMount, mount := isBindMountDestName(ctrPath, ctr); isBindMount { //nolint(gocritic)
-		newMountPoint, path := pathWithBindMountSource(mount, ctrPath)
-		mountPoint = newMountPoint
-		ctrPath = path
-	}
-
-	return mountPoint, ctrPath, nil
-}
-
-func isVolumeDestName(path string, ctr *libpod.Container) (bool, string, string) {
-	separator := string(os.PathSeparator)
-
-	if filepath.IsAbs(path) {
-		path = strings.TrimPrefix(path, separator)
-	}
-
-	if path == "" {
-		return false, "", ""
-	}
-
-	for _, vol := range ctr.Config().NamedVolumes {
-		volNamePath := strings.TrimPrefix(vol.Dest, separator)
-		if matchVolumePath(path, volNamePath) {
-			return true, vol.Dest, vol.Name
-		}
-	}
-
-	return false, "", ""
-}
-
-func pathWithVolumeMount(runtime *libpod.Runtime, volDestName, volName, path string) (string, string, error) {
-	destVolume, err := runtime.GetVolume(volName)
-	if err != nil {
-		return "", "", errors.Wrapf(err, "error getting volume destination %s", volName)
-	}
-
-	if !filepath.IsAbs(path) {
-		path = filepath.Join(string(os.PathSeparator), path)
-	}
-
-	return destVolume.MountPoint(), strings.TrimPrefix(path, volDestName), err
-}
-
-func isBindMountDestName(path string, ctr *libpod.Container) (bool, specs.Mount) {
-	separator := string(os.PathSeparator)
-
-	if filepath.IsAbs(path) {
-		path = strings.TrimPrefix(path, string(os.PathSeparator))
-	}
-
-	if path == "" {
-		return false, specs.Mount{}
-	}
-
-	for _, m := range ctr.Config().Spec.Mounts {
-		if m.Type != "bind" {
-			continue
-		}
-
-		mDest := strings.TrimPrefix(m.Destination, separator)
-		if matchVolumePath(path, mDest) {
-			return true, m
-		}
-	}
-
-	return false, specs.Mount{}
-}
-
-func matchVolumePath(path, target string) bool {
-	pathStr := filepath.Clean(path)
-	target = filepath.Clean(target)
-
-	for len(pathStr) > len(target) && strings.Contains(pathStr, string(os.PathSeparator)) {
-		pathStr = pathStr[:strings.LastIndex(pathStr, string(os.PathSeparator))]
-	}
-
-	return pathStr == target
-}
-
-func pathWithBindMountSource(m specs.Mount, path string) (string, string) {
-	if !filepath.IsAbs(path) {
-		path = filepath.Join(string(os.PathSeparator), path)
-	}
-
-	return m.Source, strings.TrimPrefix(path, m.Destination)
-}

pkg/copy/copy.go

@@ -0,0 +1,188 @@
+package copy
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+
+	buildahCopiah "github.com/containers/buildah/copier"
+	"github.com/containers/storage/pkg/archive"
+	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/pkg/errors"
+)
+
+// ********************************* NOTE *************************************
+//
+// Most security bugs are caused by attackers playing around with symlinks
+// trying to escape from the container onto the host and/or trick into data
+// corruption on the host.  Hence, file operations on containers (including
+// *stat) should always be handled by `github.com/containers/buildah/copier`
+// which makes sure to evaluate files in a chroot'ed environment.
+//
+// Please make sure to add verbose comments when changing code to make the
+// lives of future readers easier.
+//
+// ****************************************************************************
+
+// Copy the source item to destination.  Use extract to untar the source if
+// it's a tar archive.
+func Copy(source *CopyItem, destination *CopyItem, extract bool) error {
+	// First, do the man-page dance.  See podman-cp(1) for details.
+	if err := enforceCopyRules(source, destination); err != nil {
+		return err
+	}
+
+	// Destination is a stream (e.g., stdout or an http body).
+	if destination.info.IsStream {
+		// Source is a stream (e.g., stdin or an http body).
+		if source.info.IsStream {
+			_, err := io.Copy(destination.writer, source.reader)
+			return err
+		}
+		root, glob, err := source.buildahGlobs()
+		if err != nil {
+			return err
+		}
+		return buildahCopiah.Get(root, "", source.getOptions(), []string{glob}, destination.writer)
+	}
+
+	// Destination is either a file or a directory.
+	if source.info.IsStream {
+		return buildahCopiah.Put(destination.root, destination.resolved, source.putOptions(), source.reader)
+	}
+
+	tarOptions := &archive.TarOptions{
+		Compression: archive.Uncompressed,
+		CopyPass:    true,
+	}
+
+	root := destination.root
+	dir := destination.resolved
+	if !source.info.IsDir {
+		// When copying a file, make sure to rename the
+		// destination base path.
+		nameMap := make(map[string]string)
+		nameMap[filepath.Base(source.resolved)] = filepath.Base(destination.resolved)
+		tarOptions.RebaseNames = nameMap
+		dir = filepath.Dir(dir)
+	}
+
+	var tarReader io.ReadCloser
+	if extract && archive.IsArchivePath(source.resolved) {
+		if !destination.info.IsDir {
+			return errors.Errorf("cannot extract archive %q to file %q", source.original, destination.original)
+		}
+
+		reader, err := os.Open(source.resolved)
+		if err != nil {
+			return err
+		}
+		defer reader.Close()
+
+		// The stream from stdin may be compressed (e.g., via gzip).
+		decompressedStream, err := archive.DecompressStream(reader)
+		if err != nil {
+			return err
+		}
+
+		defer decompressedStream.Close()
+		tarReader = decompressedStream
+	} else {
+		reader, err := archive.TarWithOptions(source.resolved, tarOptions)
+		if err != nil {
+			return err
+		}
+		defer reader.Close()
+		tarReader = reader
+	}
+
+	return buildahCopiah.Put(root, dir, source.putOptions(), tarReader)
+}
+
+// enforceCopyRules enforces the rules for copying from a source to a
+// destination as mentioned in the podman-cp(1) man page.  Please refer to the
+// man page and/or the inline comments for further details.  Note that source
+// and destination are passed by reference and the their data may be changed.
+func enforceCopyRules(source, destination *CopyItem) error {
+	if source.statError != nil {
+		return source.statError
+	}
+
+	// We can copy everything to a stream.
+	if destination.info.IsStream {
+		return nil
+	}
+
+	// Source is a *stream*.
+	if source.info.IsStream {
+		if !(destination.info.IsDir || destination.info.IsStream) {
+			return errors.New("destination must be a directory or stream when copying from a stream")
+		}
+		return nil
+	}
+
+	// Source is a *directory*.
+	if source.info.IsDir {
+		if destination.statError != nil {
+			// It's okay if the destination does not exist.  We
+			// made sure before that it's parent exists, so it
+			// would be created while copying.
+			if os.IsNotExist(destination.statError) {
+				return nil
+			}
+			// Could be a permission error.
+			return destination.statError
+		}
+
+		// If the destination exists and is not a directory, we have a
+		// problem.
+		if !destination.info.IsDir {
+			return errors.Errorf("cannot copy directory %q to file %q", source.original, destination.original)
+		}
+
+		// If the destination exists and is a directory, we need to
+		// append the source base directory to it.  This makes sure
+		// that copying "/foo/bar" "/tmp" will copy to "/tmp/bar" (and
+		// not "/tmp").
+		newDestination, err := securejoin.SecureJoin(destination.resolved, filepath.Base(source.resolved))
+		if err != nil {
+			return err
+		}
+		destination.resolved = newDestination
+		return nil
+	}
+
+	// Source is a *file*.
+	if destination.statError != nil {
+		// It's okay if the destination does not exist, unless it ends
+		// with "/".
+		if !os.IsNotExist(destination.statError) {
+			return destination.statError
+		} else if strings.HasSuffix(destination.resolved, "/") {
+			// Note: this is practically unreachable code as the
+			// existence of parent directories is enforced early
+			// on.  It's left here as an extra security net.
+			return errors.Errorf("destination directory %q must exist (trailing %q)", destination.original, "/")
+		}
+		// Does not exist and does not end with "/".
+		return nil
+	}
+
+	// If the destination is a file, we're good.  We will overwrite the
+	// contents while copying.
+	if !destination.info.IsDir {
+		return nil
+	}
+
+	// If the destination exists and is a directory, we need to append the
+	// source base directory to it.  This makes sure that copying
+	// "/foo/bar" "/tmp" will copy to "/tmp/bar" (and not "/tmp").
+	newDestination, err := securejoin.SecureJoin(destination.resolved, filepath.Base(source.resolved))
+	if err != nil {
+		return err
+	}
+
+	destination.resolved = newDestination
+	return nil
+}

pkg/copy/item.go

@@ -0,0 +1,601 @@
+package copy
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"time"
+
+	buildahCopiah "github.com/containers/buildah/copier"
+	"github.com/containers/buildah/pkg/chrootuser"
+	"github.com/containers/buildah/util"
+	"github.com/containers/podman/v2/libpod"
+	"github.com/containers/podman/v2/libpod/define"
+	"github.com/containers/podman/v2/pkg/cgroups"
+	"github.com/containers/podman/v2/pkg/rootless"
+	"github.com/containers/storage"
+	"github.com/containers/storage/pkg/archive"
+	"github.com/containers/storage/pkg/idtools"
+	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+)
+
+// ********************************* NOTE *************************************
+//
+// Most security bugs are caused by attackers playing around with symlinks
+// trying to escape from the container onto the host and/or trick into data
+// corruption on the host.  Hence, file operations on containers (including
+// *stat) should always be handled by `github.com/containers/buildah/copier`
+// which makes sure to evaluate files in a chroot'ed environment.
+//
+// Please make sure to add verbose comments when changing code to make the
+// lives of future readers easier.
+//
+// ****************************************************************************
+
+var (
+	_stdin  = os.Stdin.Name()
+	_stdout = os.Stdout.Name()
+)
+
+// CopyItem is the source or destination of a copy operation.  Use the
+// CopyItemFrom* functions to create one for the specific source/destination
+// item.
+type CopyItem struct {
+	// The original path provided by the caller.  Useful in error messages.
+	original string
+	// The resolved path on the host or container.  Maybe altered at
+	// multiple stages when copying.
+	resolved string
+	// The root for copying data in a chroot'ed environment.
+	root string
+
+	// IDPair of the resolved path.
+	idPair *idtools.IDPair
+	// Storage ID mappings.
+	idMappings *storage.IDMappingOptions
+
+	// Internal FileInfo.  We really don't want users to mess with a
+	// CopyItem but only plug and play with it.
+	info FileInfo
+	// Error when creating the upper FileInfo.  Some errors are non-fatal,
+	// for instance, when a destination *base* path does not exist.
+	statError error
+
+	writer io.Writer
+	reader io.Reader
+
+	// Needed to clean up resources (e.g., unmount a container).
+	cleanUpFuncs []deferFunc
+}
+
+// deferFunc allows for returning functions that must be deferred at call sites.
+type deferFunc func()
+
+// FileInfo describes a file or directory and is returned by
+// (*CopyItem).Stat().
+type FileInfo struct {
+	Name       string      `json:"name"`
+	Size       int64       `json:"size"`
+	Mode       os.FileMode `json:"mode"`
+	ModTime    time.Time   `json:"mtime"`
+	IsDir      bool        `json:"isDir"`
+	IsStream   bool        `json:"isStream"`
+	LinkTarget string      `json:"linkTarget"`
+}
+
+// Stat returns the FileInfo.
+func (item *CopyItem) Stat() (*FileInfo, error) {
+	return &item.info, item.statError
+}
+
+// CleanUp releases resources such as the container mounts.  It *must* be
+// called even in case of errors.
+func (item *CopyItem) CleanUp() {
+	for _, f := range item.cleanUpFuncs {
+		f()
+	}
+}
+
+// CopyItemForWriter returns a CopyItem for the specified io.WriteCloser.  Note
+// that the returned item can only act as a copy destination.
+func CopyItemForWriter(writer io.Writer) (item CopyItem, _ error) {
+	item.writer = writer
+	item.info.IsStream = true
+	return item, nil
+}
+
+// CopyItemForReader returns a CopyItem for the specified io.ReaderCloser.  Note
+// that the returned item can only act as a copy source.
+//
+// Note that the specified reader will be auto-decompressed if needed.
+func CopyItemForReader(reader io.Reader) (item CopyItem, _ error) {
+	item.info.IsStream = true
+	decompressed, err := archive.DecompressStream(reader)
+	if err != nil {
+		return item, err
+	}
+	item.reader = decompressed
+	item.cleanUpFuncs = append(item.cleanUpFuncs, func() {
+		if err := decompressed.Close(); err != nil {
+			logrus.Errorf("Error closing decompressed reader of copy item: %v", err)
+		}
+	})
+	return item, nil
+}
+
+// CopyItemForHost creates a CopyItem for the specified host path.  It's a
+// destination by default.  Use isSource to set it as a destination.
+//
+// Note that callers *must* call (CopyItem).CleanUp(), even in case of errors.
+func CopyItemForHost(hostPath string, isSource bool) (item CopyItem, _ error) {
+	if hostPath == "-" {
+		if isSource {
+			hostPath = _stdin
+		} else {
+			hostPath = _stdout
+		}
+	}
+
+	if hostPath == _stdin {
+		return CopyItemForReader(os.Stdin)
+	}
+
+	if hostPath == _stdout {
+		return CopyItemForWriter(os.Stdout)
+	}
+
+	// Now do the dance for the host data.
+	resolvedHostPath, err := filepath.Abs(hostPath)
+	if err != nil {
+		return item, err
+	}
+
+	resolvedHostPath = preserveBasePath(hostPath, resolvedHostPath)
+	item.original = hostPath
+	item.resolved = resolvedHostPath
+	item.root = "/"
+
+	statInfo, statError := os.Stat(resolvedHostPath)
+	item.statError = statError
+
+	// It exists, we're done.
+	if statError == nil {
+		item.info.Name = statInfo.Name()
+		item.info.Size = statInfo.Size()
+		item.info.Mode = statInfo.Mode()
+		item.info.ModTime = statInfo.ModTime()
+		item.info.IsDir = statInfo.IsDir()
+		item.info.LinkTarget = resolvedHostPath
+		return item, nil
+	}
+
+	// The source must exist, but let's try to give some human-friendly
+	// errors.
+	if isSource {
+		if os.IsNotExist(item.statError) {
+			return item, errors.Wrapf(os.ErrNotExist, "%q could not be found on the host", hostPath)
+		}
+		return item, item.statError // could be a permission error
+	}
+
+	// If we're a destination, we need to make sure that the parent
+	// directory exists.
+	parent := filepath.Dir(resolvedHostPath)
+	if _, err := os.Stat(parent); err != nil {
+		if os.IsNotExist(err) {
+			return item, errors.Wrapf(os.ErrNotExist, "%q could not be found on the host", parent)
+		}
+		return item, err
+	}
+
+	return item, nil
+}
+
+// CopyItemForContainer creates a CopyItem for the specified path on the
+// container.  It's a destination by default.  Use isSource to set it as a
+// destination.  Note that the container path may resolve to a path outside of
+// the container's mount point if the path hits a volume or mount on the
+// container.
+//
+// Note that callers *must* call (CopyItem).CleanUp(), even in case of errors.
+func CopyItemForContainer(container *libpod.Container, containerPath string, pause bool, isSource bool) (item CopyItem, _ error) {
+	// Mount and pause the container.
+	containerMountPoint, err := item.mountAndPauseContainer(container, pause)
+	if err != nil {
+		return item, err
+	}
+
+	// Make sure that "/" copies the *contents* of the mount point and not
+	// the directory.
+	if containerPath == "/" {
+		containerPath += "/."
+	}
+
+	// Now resolve the container's path.  It may hit a volume, it may hit a
+	// bind mount, it may be relative.
+	resolvedRoot, resolvedContainerPath, err := resolveContainerPaths(container, containerMountPoint, containerPath)
+	if err != nil {
+		return item, err
+	}
+	resolvedContainerPath = preserveBasePath(containerPath, resolvedContainerPath)
+
+	idMappings, idPair, err := getIDMappingsAndPair(container, containerMountPoint)
+	if err != nil {
+		return item, err
+	}
+
+	item.original = containerPath
+	item.resolved = resolvedContainerPath
+	item.root = resolvedRoot
+	item.idMappings = idMappings
+	item.idPair = idPair
+
+	statInfo, statError := secureStat(resolvedRoot, resolvedContainerPath)
+	item.statError = statError
+
+	// It exists, we're done.
+	if statError == nil {
+		item.info.IsDir = statInfo.IsDir
+		item.info.Name = filepath.Base(statInfo.Name)
+		item.info.Size = statInfo.Size
+		item.info.Mode = statInfo.Mode
+		item.info.ModTime = statInfo.ModTime
+		item.info.IsDir = statInfo.IsDir
+		item.info.LinkTarget = resolvedContainerPath
+		return item, nil
+	}
+
+	// The source must exist, but let's try to give some human-friendly
+	// errors.
+	if isSource {
+		if os.IsNotExist(statError) {
+			return item, errors.Wrapf(os.ErrNotExist, "%q could not be found on container %s (resolved to %q)", containerPath, container.ID(), resolvedContainerPath)
+		}
+		return item, item.statError // could be a permission error
+	}
+
+	// If we're a destination, we need to make sure that the parent
+	// directory exists.
+	parent := filepath.Dir(resolvedContainerPath)
+	if _, err := secureStat(resolvedRoot, parent); err != nil {
+		if os.IsNotExist(err) {
+			return item, errors.Wrapf(os.ErrNotExist, "%q could not be found on container %s (resolved to %q)", containerPath, container.ID(), resolvedContainerPath)
+		}
+		return item, err
+	}
+
+	return item, nil
+}
+
+// putOptions returns PUT options for buildah's copier package.
+func (item *CopyItem) putOptions() buildahCopiah.PutOptions {
+	options := buildahCopiah.PutOptions{}
+	if item.idMappings != nil {
+		options.UIDMap = item.idMappings.UIDMap
+		options.GIDMap = item.idMappings.GIDMap
+	}
+	if item.idPair != nil {
+		options.ChownDirs = item.idPair
+		options.ChownFiles = item.idPair
+	}
+	return options
+}
+
+// getOptions returns GET options for buildah's copier package.
+func (item *CopyItem) getOptions() buildahCopiah.GetOptions {
+	options := buildahCopiah.GetOptions{}
+	if item.idMappings != nil {
+		options.UIDMap = item.idMappings.UIDMap
+		options.GIDMap = item.idMappings.GIDMap
+	}
+	if item.idPair != nil {
+		options.ChownDirs = item.idPair
+		options.ChownFiles = item.idPair
+	}
+	return options
+
+}
+
+// mount and pause the container.  Also set the item's cleanUpFuncs.  Those
+// *must* be invoked by callers, even in case of errors.
+func (item *CopyItem) mountAndPauseContainer(container *libpod.Container, pause bool) (string, error) {
+	// Make sure to pause and unpause the container.  We cannot pause on
+	// cgroupsv1 as rootless user, in which case we turn off pausing.
+	if pause && rootless.IsRootless() {
+		cgroupv2, _ := cgroups.IsCgroup2UnifiedMode()
+		if !cgroupv2 {
+			logrus.Debugf("Cannot pause container for copying as a rootless user on cgroupsv1: default to not pause")
+			pause = false
+		}
+	}
+
+	// Mount and unmount the container.
+	mountPoint, err := container.Mount()
+	if err != nil {
+		return "", err
+	}
+
+	item.cleanUpFuncs = append(item.cleanUpFuncs, func() {
+		if err := container.Unmount(false); err != nil {
+			logrus.Errorf("Error unmounting container after copy operation: %v", err)
+		}
+	})
+
+	// Pause and unpause the container.
+	if pause {
+		if err := container.Pause(); err != nil {
+			// Ignore errors when the container isn't running.  No
+			// need to pause.
+			if errors.Cause(err) != define.ErrCtrStateInvalid {
+				return "", err
+			}
+		} else {
+			item.cleanUpFuncs = append(item.cleanUpFuncs, func() {
+				if err := container.Unpause(); err != nil {
+					logrus.Errorf("Error unpausing container after copy operation: %v", err)
+				}
+			})
+		}
+	}
+
+	return mountPoint, nil
+}
+
+// buildahGlobs returns the root, dir and glob used in buildah's copier
+// package.
+//
+// Note that dir is always empty.
+func (item *CopyItem) buildahGlobs() (root string, glob string, err error) {
+	root = item.root
+
+	// If the root and the resolved path are equal, then dir must be empty
+	// and the glob must be ".".
+	if filepath.Clean(root) == filepath.Clean(item.resolved) {
+		glob = "."
+		return
+	}
+
+	glob, err = filepath.Rel(root, item.resolved)
+	return
+}
+
+// preserveBasePath makes sure that the original base path (e.g., "/" or "./")
+// is preserved.  The filepath API among tends to clean up a bit too much but
+// we *must* preserve this data by all means.
+func preserveBasePath(original, resolved string) string {
+	// Handle "/"
+	if strings.HasSuffix(original, "/") {
+		if !strings.HasSuffix(resolved, "/") {
+			resolved += "/"
+		}
+		return resolved
+	}
+
+	// Handle "/."
+	if strings.HasSuffix(original, "/.") {
+		if strings.HasSuffix(resolved, "/") { // could be root!
+			resolved += "."
+		} else if !strings.HasSuffix(resolved, "/.") {
+			resolved += "/."
+		}
+		return resolved
+	}
+
+	return resolved
+}
+
+// secureStat extracts file info for path in a chroot'ed environment in root.
+func secureStat(root string, path string) (*buildahCopiah.StatForItem, error) {
+	var glob string
+	var err error
+
+	// If root and path are equal, then dir must be empty and the glob must
+	// be ".".
+	if filepath.Clean(root) == filepath.Clean(path) {
+		glob = "."
+	} else {
+		glob, err = filepath.Rel(root, path)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	globStats, err := buildahCopiah.Stat(root, "", buildahCopiah.StatOptions{}, []string{glob})
+	if err != nil {
+		return nil, err
+	}
+
+	if len(globStats) != 1 {
+		return nil, errors.Errorf("internal libpod error: secureStat: expected 1 item but got %d", len(globStats))
+	}
+
+	stat, exists := globStats[0].Results[glob] // only one glob passed, so that's okay
+	if !exists {
+		return stat, os.ErrNotExist
+	}
+
+	var statErr error
+	if stat.Error != "" {
+		statErr = errors.New(stat.Error)
+	}
+	return stat, statErr
+}
+
+// resolveContainerPaths resolves the container's mount point and the container
+// path as specified by the user.  Both may resolve to paths outside of the
+// container's mount point when the container path hits a volume or bind mount.
+//
+// NOTE: We must take volumes and bind mounts into account as, regrettably, we
+// can copy to/from stopped containers.  In that case, the volumes and bind
+// mounts are not present.  For running containers, the runtime (e.g., runc or
+// crun) takes care of these mounts.  For stopped ones, we need to do quite
+// some dance, as done below.
+func resolveContainerPaths(container *libpod.Container, mountPoint string, containerPath string) (string, string, error) {
+	// Let's first make sure we have a path relative to the mount point.
+	pathRelativeToContainerMountPoint := containerPath
+	if !filepath.IsAbs(containerPath) {
+		// If the containerPath is not absolute, it's relative to the
+		// container's working dir.  To be extra careful, let's first
+		// join the working dir with "/", and the add the containerPath
+		// to it.
+		pathRelativeToContainerMountPoint = filepath.Join(filepath.Join("/", container.WorkingDir()), containerPath)
+	}
+	// NOTE: the secure join makes sure that we follow symlinks.  This way,
+	// we catch scenarios where the container path symlinks to a volume or
+	// bind mount.
+	resolvedPathOnTheContainerMountPoint, err := securejoin.SecureJoin(mountPoint, pathRelativeToContainerMountPoint)
+	if err != nil {
+		return "", "", err
+	}
+	pathRelativeToContainerMountPoint = strings.TrimPrefix(pathRelativeToContainerMountPoint, mountPoint)
+	pathRelativeToContainerMountPoint = filepath.Join("/", pathRelativeToContainerMountPoint)
+
+	// Now we have an "absolute container Path" but not yet resolved on the
+	// host (e.g., "/foo/bar/file.txt").  As mentioned above, we need to
+	// check if "/foo/bar/file.txt" is on a volume or bind mount.  To do
+	// that, we need to walk *down* the paths to the root.  Assuming
+	// volume-1 is mounted to "/foo" and volume-2 is mounted to "/foo/bar",
+	// we must select "/foo/bar".  Once selected, we need to rebase the
+	// remainder (i.e, "/file.txt") on the volume's mount point on the
+	// host.  Same applies to bind mounts.
+
+	searchPath := pathRelativeToContainerMountPoint
+	for {
+		volume, err := findVolume(container, searchPath)
+		if err != nil {
+			return "", "", err
+		}
+		if volume != nil {
+			logrus.Debugf("Container path %q resolved to volume %q on path %q", containerPath, volume.Name(), searchPath)
+			// We found a matching volume for searchPath.  We now
+			// need to first find the relative path of our input
+			// path to the searchPath, and then join it with the
+			// volume's mount point.
+			pathRelativeToVolume := strings.TrimPrefix(pathRelativeToContainerMountPoint, searchPath)
+			absolutePathOnTheVolumeMount, err := securejoin.SecureJoin(volume.MountPoint(), pathRelativeToVolume)
+			if err != nil {
+				return "", "", err
+			}
+			return volume.MountPoint(), absolutePathOnTheVolumeMount, nil
+		}
+
+		if mount := findBindMount(container, searchPath); mount != nil {
+			logrus.Debugf("Container path %q resolved to bind mount %q:%q on path %q", containerPath, mount.Source, mount.Destination, searchPath)
+			// We found a matching bind mount for searchPath.  We
+			// now need to first find the relative path of our
+			// input path to the searchPath, and then join it with
+			// the source of the bind mount.
+			pathRelativeToBindMount := strings.TrimPrefix(pathRelativeToContainerMountPoint, searchPath)
+			absolutePathOnTheBindMount, err := securejoin.SecureJoin(mount.Source, pathRelativeToBindMount)
+			if err != nil {
+				return "", "", err
+			}
+			return mount.Source, absolutePathOnTheBindMount, nil
+
+		}
+
+		if searchPath == "/" {
+			// Cannot go beyond "/", so we're done.
+			break
+		}
+
+		// Walk *down* the path (e.g., "/foo/bar/x" -> "/foo/bar").
+		searchPath = filepath.Dir(searchPath)
+	}
+
+	// No volume, no bind mount but just a normal path on the container.
+	return mountPoint, resolvedPathOnTheContainerMountPoint, nil
+}
+
+// findVolume checks if the specified container path matches a volume inside
+// the container.  It returns a matching volume or nil.
+func findVolume(c *libpod.Container, containerPath string) (*libpod.Volume, error) {
+	runtime := c.Runtime()
+	cleanedContainerPath := filepath.Clean(containerPath)
+	for _, vol := range c.Config().NamedVolumes {
+		if cleanedContainerPath == filepath.Clean(vol.Dest) {
+			return runtime.GetVolume(vol.Name)
+		}
+	}
+	return nil, nil
+}
+
+// findBindMount checks if the specified container path matches a bind mount
+// inside the container.  It returns a matching mount or nil.
+func findBindMount(c *libpod.Container, containerPath string) *specs.Mount {
+	cleanedPath := filepath.Clean(containerPath)
+	for _, m := range c.Config().Spec.Mounts {
+		if m.Type != "bind" {
+			continue
+		}
+		if cleanedPath == filepath.Clean(m.Destination) {
+			mount := m
+			return &mount
+		}
+	}
+	return nil
+}
+
+// getIDMappingsAndPair returns the ID mappings for the container and the host
+// ID pair.
+func getIDMappingsAndPair(container *libpod.Container, containerMount string) (*storage.IDMappingOptions, *idtools.IDPair, error) {
+	user, err := getContainerUser(container, containerMount)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	idMappingOpts, err := container.IDMappings()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	hostUID, hostGID, err := util.GetHostIDs(idtoolsToRuntimeSpec(idMappingOpts.UIDMap), idtoolsToRuntimeSpec(idMappingOpts.GIDMap), user.UID, user.GID)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	idPair := idtools.IDPair{UID: int(hostUID), GID: int(hostGID)}
+	return &idMappingOpts, &idPair, nil
+}
+
+// getContainerUser returns the specs.User of the container.
+func getContainerUser(container *libpod.Container, mountPoint string) (specs.User, error) {
+	userspec := container.Config().User
+
+	uid, gid, _, err := chrootuser.GetUser(mountPoint, userspec)
+	u := specs.User{
+		UID:      uid,
+		GID:      gid,
+		Username: userspec,
+	}
+
+	if !strings.Contains(userspec, ":") {
+		groups, err2 := chrootuser.GetAdditionalGroupsForUser(mountPoint, uint64(u.UID))
+		if err2 != nil {
+			if errors.Cause(err2) != chrootuser.ErrNoSuchUser && err == nil {
+				err = err2
+			}
+		} else {
+			u.AdditionalGids = groups
+		}
+	}
+
+	return u, err
+}
+
+// idtoolsToRuntimeSpec converts idtools ID mapping to the one of the runtime spec.
+func idtoolsToRuntimeSpec(idMaps []idtools.IDMap) (convertedIDMap []specs.LinuxIDMapping) {
+	for _, idmap := range idMaps {
+		tempIDMap := specs.LinuxIDMapping{
+			ContainerID: uint32(idmap.ContainerID),
+			HostID:      uint32(idmap.HostID),
+			Size:        uint32(idmap.Size),
+		}
+		convertedIDMap = append(convertedIDMap, tempIDMap)
+	}
+	return convertedIDMap
+}

pkg/domain/entities/containers.go

@@ -403,16 +403,14 @@ type ContainerPortReport struct {
 	Ports []ocicni.PortMapping
 }
 
-// ContainerCpOptions describes input options for cp
+// ContainerCpOptions describes input options for cp.
 type ContainerCpOptions struct {
+	// Pause the container while copying.
 	Pause bool
+	// Extract the tarfile into the destination directory.
 	Extract bool
 }
 
-// ContainerCpReport describes the output from a cp operation
-type ContainerCpReport struct {
-}
-
 // ContainerStatsOptions describes input options for getting
 // stats on containers
 type ContainerStatsOptions struct {

pkg/domain/entities/engine_container.go

@@ -16,7 +16,7 @@ type ContainerEngine interface {
 	ContainerCheckpoint(ctx context.Context, namesOrIds []string, options CheckpointOptions) ([]*CheckpointReport, error)
 	ContainerCleanup(ctx context.Context, namesOrIds []string, options ContainerCleanupOptions) ([]*ContainerCleanupReport, error)
 	ContainerCommit(ctx context.Context, nameOrID string, options CommitOptions) (*CommitReport, error)
-	ContainerCp(ctx context.Context, source, dest string, options ContainerCpOptions) (*ContainerCpReport, error)
+	ContainerCp(ctx context.Context, source, dest string, options ContainerCpOptions) error
 	ContainerCreate(ctx context.Context, s *specgen.SpecGenerator) (*ContainerCreateReport, error)
 	ContainerDiff(ctx context.Context, nameOrID string, options DiffOptions) (*DiffReport, error)
 	ContainerExec(ctx context.Context, nameOrID string, options ExecOptions, streams define.AttachStreams) (int, error)

pkg/domain/infra/abi/cp.go

@@ -1,195 +1,70 @@
 package abi
 
 import (
-	"archive/tar"
 	"context"
-	"fmt"
-	"io"
-	"os"
-	"path/filepath"
 	"strings"
 
-	"github.com/containers/buildah/pkg/chrootuser"
-	"github.com/containers/buildah/util"
 	"github.com/containers/podman/v2/libpod"
-	"github.com/containers/podman/v2/libpod/define"
+	"github.com/containers/podman/v2/pkg/copy"
 	"github.com/containers/podman/v2/pkg/domain/entities"
-	"github.com/containers/storage"
-	"github.com/containers/storage/pkg/chrootarchive"
-	"github.com/containers/storage/pkg/idtools"
-	securejoin "github.com/cyphar/filepath-securejoin"
-	"github.com/docker/docker/pkg/archive"
-	"github.com/opencontainers/go-digest"
-	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 
-func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string, options entities.ContainerCpOptions) (*entities.ContainerCpReport, error) {
-	extract := options.Extract
-
+func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string, options entities.ContainerCpOptions) error {
 	srcCtr, srcPath := parsePath(ic.Libpod, source)
 	destCtr, destPath := parsePath(ic.Libpod, dest)
 
-	if (srcCtr == nil && destCtr == nil) || (srcCtr != nil && destCtr != nil) {
-		return nil, errors.Errorf("invalid arguments %s, %s you must use just one container", source, dest)
+	if srcCtr != nil && destCtr != nil {
+		return errors.Errorf("invalid arguments %q, %q: you must use just one container", source, dest)
+	}
+	if srcCtr == nil && destCtr == nil {
+		return errors.Errorf("invalid arguments %q, %q: you must specify one container", source, dest)
 	}
-
 	if len(srcPath) == 0 || len(destPath) == 0 {
-		return nil, errors.Errorf("invalid arguments %s, %s you must specify paths", source, dest)
-	}
-	ctr := srcCtr
-	isFromHostToCtr := ctr == nil
-	if isFromHostToCtr {
-		ctr = destCtr
+		return errors.Errorf("invalid arguments %q, %q: you must specify paths", source, dest)
 	}
 
-	mountPoint, err := ctr.Mount()
+	var sourceItem, destinationItem copy.CopyItem
+	var err error
+	// Copy from the container to the host.
+	if srcCtr != nil {
+		sourceItem, err = copy.CopyItemForContainer(srcCtr, srcPath, options.Pause, true)
+		defer sourceItem.CleanUp()
 		if err != nil {
-		return nil, err
-	}
-	defer func() {
-		if err := ctr.Unmount(false); err != nil {
-			logrus.Errorf("unable to umount container '%s': %q", ctr.ID(), err)
-		}
-	}()
-
-	if options.Pause {
-		if err := ctr.Pause(); err != nil {
-			// An invalid state error is fine.
-			// The container isn't running or is already paused.
-			// TODO: We can potentially start the container while
-			// the copy is running, which still allows a race where
-			// malicious code could mess with the symlink.
-			if errors.Cause(err) != define.ErrCtrStateInvalid {
-				return nil, err
+			return err
 		}
 	} else {
-			// Only add the defer if we actually paused
-			defer func() {
-				if err := ctr.Unpause(); err != nil {
-					logrus.Errorf("Error unpausing container after copying: %v", err)
-				}
-			}()
+		sourceItem, err = copy.CopyItemForHost(srcPath, true)
+		if err != nil {
+			return err
 		}
 	}
 
-	user, err := getUser(mountPoint, ctr.User())
+	if destCtr != nil {
+		destinationItem, err = copy.CopyItemForContainer(destCtr, destPath, options.Pause, false)
+		defer destinationItem.CleanUp()
 		if err != nil {
-		return nil, err
-	}
-	idMappingOpts, err := ctr.IDMappings()
-	if err != nil {
-		return nil, errors.Wrapf(err, "error getting IDMappingOptions")
-	}
-	destOwner := idtools.IDPair{UID: int(user.UID), GID: int(user.GID)}
-	hostUID, hostGID, err := util.GetHostIDs(convertIDMap(idMappingOpts.UIDMap), convertIDMap(idMappingOpts.GIDMap), user.UID, user.GID)
-	if err != nil {
-		return nil, err
-	}
-
-	hostOwner := idtools.IDPair{UID: int(hostUID), GID: int(hostGID)}
-
-	if isFromHostToCtr {
-		if isVol, volDestName, volName := isVolumeDestName(destPath, ctr); isVol { //nolint(gocritic)
-			path, err := pathWithVolumeMount(ic.Libpod, volDestName, volName, destPath)
-			if err != nil {
-				return nil, errors.Wrapf(err, "error getting destination path from volume %s", volDestName)
-			}
-			destPath = path
-		} else if isBindMount, mount := isBindMountDestName(destPath, ctr); isBindMount { //nolint(gocritic)
-			path, err := pathWithBindMountSource(mount, destPath)
-			if err != nil {
-				return nil, errors.Wrapf(err, "error getting destination path from bind mount %s", mount.Destination)
-			}
-			destPath = path
-		} else if filepath.IsAbs(destPath) { //nolint(gocritic)
-			cleanedPath, err := securejoin.SecureJoin(mountPoint, destPath)
-			if err != nil {
-				return nil, err
-			}
-			destPath = cleanedPath
-		} else { //nolint(gocritic)
-			ctrWorkDir, err := securejoin.SecureJoin(mountPoint, ctr.WorkingDir())
-			if err != nil {
-				return nil, err
-			}
-			if err = idtools.MkdirAllAndChownNew(ctrWorkDir, 0755, hostOwner); err != nil {
-				return nil, err
-			}
-			cleanedPath, err := securejoin.SecureJoin(mountPoint, filepath.Join(ctr.WorkingDir(), destPath))
-			if err != nil {
-				return nil, err
-			}
-			destPath = cleanedPath
+			return err
 		}
 	} else {
-		destOwner = idtools.IDPair{UID: os.Getuid(), GID: os.Getgid()}
-		if isVol, volDestName, volName := isVolumeDestName(srcPath, ctr); isVol { //nolint(gocritic)
-			path, err := pathWithVolumeMount(ic.Libpod, volDestName, volName, srcPath)
+		destinationItem, err = copy.CopyItemForHost(destPath, false)
+		defer destinationItem.CleanUp()
 		if err != nil {
-				return nil, errors.Wrapf(err, "error getting source path from volume %s", volDestName)
-			}
-			srcPath = path
-		} else if isBindMount, mount := isBindMountDestName(srcPath, ctr); isBindMount { //nolint(gocritic)
-			path, err := pathWithBindMountSource(mount, srcPath)
-			if err != nil {
-				return nil, errors.Wrapf(err, "error getting source path from bind mount %s", mount.Destination)
-			}
-			srcPath = path
-		} else if filepath.IsAbs(srcPath) { //nolint(gocritic)
-			cleanedPath, err := securejoin.SecureJoin(mountPoint, srcPath)
-			if err != nil {
-				return nil, err
-			}
-			srcPath = cleanedPath
-		} else { //nolint(gocritic)
-			cleanedPath, err := securejoin.SecureJoin(mountPoint, filepath.Join(ctr.WorkingDir(), srcPath))
-			if err != nil {
-				return nil, err
-			}
-			srcPath = cleanedPath
+			return err
 		}
 	}
 
-	if !filepath.IsAbs(destPath) {
-		dir, err := os.Getwd()
-		if err != nil {
-			return nil, errors.Wrapf(err, "err getting current working directory")
-		}
-		destPath = filepath.Join(dir, destPath)
-	}
-
-	if source == "-" {
-		srcPath = os.Stdin.Name()
-		extract = true
-	}
-	err = containerCopy(srcPath, destPath, source, dest, idMappingOpts, &destOwner, extract, isFromHostToCtr)
-	return &entities.ContainerCpReport{}, err
-}
-
-func getUser(mountPoint string, userspec string) (specs.User, error) {
-	uid, gid, _, err := chrootuser.GetUser(mountPoint, userspec)
-	u := specs.User{
-		UID:      uid,
-		GID:      gid,
-		Username: userspec,
-	}
-	if !strings.Contains(userspec, ":") {
-		groups, err2 := chrootuser.GetAdditionalGroupsForUser(mountPoint, uint64(u.UID))
-		if err2 != nil {
-			if errors.Cause(err2) != chrootuser.ErrNoSuchUser && err == nil {
-				err = err2
-			}
-		} else {
-			u.AdditionalGids = groups
-		}
-
-	}
-	return u, err
+	// Copy from the host to the container.
+	return copy.Copy(&sourceItem, &destinationItem, options.Extract)
 }
 
 func parsePath(runtime *libpod.Runtime, path string) (*libpod.Container, string) {
+	if len(path) == 0 {
+		return nil, ""
+	}
+	if path[0] == '.' || path[0] == '/' { // A path cannot point to a container.
+		return nil, path
+	}
 	pathArr := strings.SplitN(path, ":", 2)
 	if len(pathArr) == 2 {
 		ctr, err := runtime.LookupContainer(pathArr[0])
@@ -199,247 +74,3 @@ func parsePath(runtime *libpod.Runtime, path string) (*libpod.Container, string)
 	}
 	return nil, path
 }
-
-func evalSymlinks(path string) (string, error) {
-	if path == os.Stdin.Name() {
-		return path, nil
-	}
-	return filepath.EvalSymlinks(path)
-}
-
-func getPathInfo(path string) (string, os.FileInfo, error) {
-	path, err := evalSymlinks(path)
-	if err != nil {
-		return "", nil, errors.Wrapf(err, "error evaluating symlinks %q", path)
-	}
-	srcfi, err := os.Stat(path)
-	if err != nil {
-		return "", nil, err
-	}
-	return path, srcfi, nil
-}
-
-func containerCopy(srcPath, destPath, src, dest string, idMappingOpts storage.IDMappingOptions, chownOpts *idtools.IDPair, extract, isFromHostToCtr bool) error {
-	srcPath, err := evalSymlinks(srcPath)
-	if err != nil {
-		return errors.Wrapf(err, "error evaluating symlinks %q", srcPath)
-	}
-
-	srcPath, srcfi, err := getPathInfo(srcPath)
-	if err != nil {
-		return err
-	}
-
-	filename := filepath.Base(destPath)
-	if filename == "-" && !isFromHostToCtr {
-		err := streamFileToStdout(srcPath, srcfi)
-		if err != nil {
-			return errors.Wrapf(err, "error streaming source file %s to Stdout", srcPath)
-		}
-		return nil
-	}
-
-	destdir := destPath
-	if !srcfi.IsDir() {
-		destdir = filepath.Dir(destPath)
-	}
-	_, err = os.Stat(destdir)
-	if err != nil && !os.IsNotExist(err) {
-		return err
-	}
-	destDirIsExist := err == nil
-	if err = os.MkdirAll(destdir, 0755); err != nil {
-		return err
-	}
-
-	// return functions for copying items
-	copyFileWithTar := chrootarchive.CopyFileWithTarAndChown(chownOpts, digest.Canonical.Digester().Hash(), idMappingOpts.UIDMap, idMappingOpts.GIDMap)
-	copyWithTar := chrootarchive.CopyWithTarAndChown(chownOpts, digest.Canonical.Digester().Hash(), idMappingOpts.UIDMap, idMappingOpts.GIDMap)
-	untarPath := chrootarchive.UntarPathAndChown(chownOpts, digest.Canonical.Digester().Hash(), idMappingOpts.UIDMap, idMappingOpts.GIDMap)
-
-	if srcfi.IsDir() {
-		logrus.Debugf("copying %q to %q", srcPath+string(os.PathSeparator)+"*", dest+string(os.PathSeparator)+"*")
-		if destDirIsExist && !strings.HasSuffix(src, fmt.Sprintf("%s.", string(os.PathSeparator))) {
-			srcPathBase := filepath.Base(srcPath)
-			if !isFromHostToCtr {
-				pathArr := strings.SplitN(src, ":", 2)
-				if len(pathArr) != 2 {
-					return errors.Errorf("invalid arguments %s, you must specify source path", src)
-				}
-				if pathArr[1] == "/" {
-					// If `srcPath` is the root directory of the container,
-					// `srcPath` will be `.../${sha256_ID}/merged/`, so do not join it
-					srcPathBase = ""
-				}
-			}
-			destPath = filepath.Join(destPath, srcPathBase)
-		}
-		if err = copyWithTar(srcPath, destPath); err != nil {
-			return errors.Wrapf(err, "error copying %q to %q", srcPath, dest)
-		}
-		return nil
-	}
-
-	if extract {
-		// We're extracting an archive into the destination directory.
-		logrus.Debugf("extracting contents of %q into %q", srcPath, destPath)
-		if err = untarPath(srcPath, destPath); err != nil {
-			return errors.Wrapf(err, "error extracting %q into %q", srcPath, destPath)
-		}
-		return nil
-	}
-
-	destfi, err := os.Stat(destPath)
-	if err != nil {
-		if !os.IsNotExist(err) || strings.HasSuffix(dest, string(os.PathSeparator)) {
-			return err
-		}
-	}
-	if destfi != nil && destfi.IsDir() {
-		destPath = filepath.Join(destPath, filepath.Base(srcPath))
-	}
-
-	// Copy the file, preserving attributes.
-	logrus.Debugf("copying %q to %q", srcPath, destPath)
-	if err = copyFileWithTar(srcPath, destPath); err != nil {
-		return errors.Wrapf(err, "error copying %q to %q", srcPath, destPath)
-	}
-	return nil
-}
-
-func convertIDMap(idMaps []idtools.IDMap) (convertedIDMap []specs.LinuxIDMapping) {
-	for _, idmap := range idMaps {
-		tempIDMap := specs.LinuxIDMapping{
-			ContainerID: uint32(idmap.ContainerID),
-			HostID:      uint32(idmap.HostID),
-			Size:        uint32(idmap.Size),
-		}
-		convertedIDMap = append(convertedIDMap, tempIDMap)
-	}
-	return convertedIDMap
-}
-
-func streamFileToStdout(srcPath string, srcfi os.FileInfo) error {
-	if srcfi.IsDir() {
-		tw := tar.NewWriter(os.Stdout)
-		err := filepath.Walk(srcPath, func(path string, info os.FileInfo, err error) error {
-			if err != nil || !info.Mode().IsRegular() || path == srcPath {
-				return err
-			}
-			hdr, err := tar.FileInfoHeader(info, "")
-			if err != nil {
-				return err
-			}
-
-			if err = tw.WriteHeader(hdr); err != nil {
-				return err
-			}
-			fh, err := os.Open(path)
-			if err != nil {
-				return err
-			}
-			defer fh.Close()
-
-			_, err = io.Copy(tw, fh)
-			return err
-		})
-		if err != nil {
-			return errors.Wrapf(err, "error streaming directory %s to Stdout", srcPath)
-		}
-		return nil
-	}
-
-	file, err := os.Open(srcPath)
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-	if !archive.IsArchivePath(srcPath) {
-		tw := tar.NewWriter(os.Stdout)
-		hdr, err := tar.FileInfoHeader(srcfi, "")
-		if err != nil {
-			return err
-		}
-		err = tw.WriteHeader(hdr)
-		if err != nil {
-			return err
-		}
-		_, err = io.Copy(tw, file)
-		if err != nil {
-			return errors.Wrapf(err, "error streaming archive %s to Stdout", srcPath)
-		}
-		return nil
-	}
-
-	_, err = io.Copy(os.Stdout, file)
-	if err != nil {
-		return errors.Wrapf(err, "error streaming file to Stdout")
-	}
-	return nil
-}
-
-func isVolumeDestName(path string, ctr *libpod.Container) (bool, string, string) {
-	separator := string(os.PathSeparator)
-	if filepath.IsAbs(path) {
-		path = strings.TrimPrefix(path, separator)
-	}
-	if path == "" {
-		return false, "", ""
-	}
-	for _, vol := range ctr.Config().NamedVolumes {
-		volNamePath := strings.TrimPrefix(vol.Dest, separator)
-		if matchVolumePath(path, volNamePath) {
-			return true, vol.Dest, vol.Name
-		}
-	}
-	return false, "", ""
-}
-
-// if SRCPATH or DESTPATH is from volume mount's destination -v or --mount type=volume, generates the path with volume mount point
-func pathWithVolumeMount(runtime *libpod.Runtime, volDestName, volName, path string) (string, error) {
-	destVolume, err := runtime.GetVolume(volName)
-	if err != nil {
-		return "", errors.Wrapf(err, "error getting volume destination %s", volName)
-	}
-	if !filepath.IsAbs(path) {
-		path = filepath.Join(string(os.PathSeparator), path)
-	}
-	path, err = securejoin.SecureJoin(destVolume.MountPoint(), strings.TrimPrefix(path, volDestName))
-	return path, err
-}
-
-func isBindMountDestName(path string, ctr *libpod.Container) (bool, specs.Mount) {
-	separator := string(os.PathSeparator)
-	if filepath.IsAbs(path) {
-		path = strings.TrimPrefix(path, string(os.PathSeparator))
-	}
-	if path == "" {
-		return false, specs.Mount{}
-	}
-	for _, m := range ctr.Config().Spec.Mounts {
-		if m.Type != "bind" {
-			continue
-		}
-		mDest := strings.TrimPrefix(m.Destination, separator)
-		if matchVolumePath(path, mDest) {
-			return true, m
-		}
-	}
-	return false, specs.Mount{}
-}
-
-func matchVolumePath(path, target string) bool {
-	pathStr := filepath.Clean(path)
-	target = filepath.Clean(target)
-	for len(pathStr) > len(target) && strings.Contains(pathStr, string(os.PathSeparator)) {
-		pathStr = pathStr[:strings.LastIndex(pathStr, string(os.PathSeparator))]
-	}
-	return pathStr == target
-}
-
-func pathWithBindMountSource(m specs.Mount, path string) (string, error) {
-	if !filepath.IsAbs(path) {
-		path = filepath.Join(string(os.PathSeparator), path)
-	}
-	return securejoin.SecureJoin(m.Source, strings.TrimPrefix(path, m.Destination))
-}

pkg/domain/infra/tunnel/containers.go

@@ -731,8 +731,8 @@ func (ic *ContainerEngine) ContainerPort(ctx context.Context, nameOrID string, o
 	return reports, nil
 }
 
-func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string, options entities.ContainerCpOptions) (*entities.ContainerCpReport, error) {
-	return nil, errors.New("not implemented")
+func (ic *ContainerEngine) ContainerCp(ctx context.Context, source, dest string, options entities.ContainerCpOptions) error {
+	return errors.New("not implemented")
 }
 
 // Shutdown Libpod engine

test/e2e/cp_test.go

@@ -4,14 +4,18 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"os/user"
 	"path/filepath"
-	"strings"
 
 	. "github.com/containers/podman/v2/test/utils"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
 
+// NOTE: Only smoke tests.  The system tests (i.e., "./test/system/*") take
+// care of function and regression tests.  Please consider adding system tests
+// rather than e2e tests.  System tests are used in RHEL gating.
+
 var _ = Describe("Podman cp", func() {
 	var (
 		tempdir    string
@@ -37,240 +41,108 @@ var _ = Describe("Podman cp", func() {
 
 	})
 
+	// Copy a file to the container, then back to the host and make sure
+	// that the contents match.
 	It("podman cp file", func() {
-		srcPath := filepath.Join(podmanTest.RunRoot, "cp_test.txt")
-		dstPath := filepath.Join(podmanTest.RunRoot, "cp_from_container")
-		fromHostToContainer := []byte("copy from host to container")
+		srcFile, err := ioutil.TempFile("", "")
+		Expect(err).To(BeNil())
+		defer srcFile.Close()
+		defer os.Remove(srcFile.Name())
 
-		session := podmanTest.Podman([]string{"create", ALPINE, "cat", "foo"})
+		originalContent := []byte("podman cp file test")
+		err = ioutil.WriteFile(srcFile.Name(), originalContent, 0644)
+		Expect(err).To(BeNil())
+
+		// Create a container. NOTE that container mustn't be running for copying.
+		session := podmanTest.Podman([]string{"create", ALPINE})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		name := session.OutputToString()
 
-		err := ioutil.WriteFile(srcPath, fromHostToContainer, 0644)
-		Expect(err).To(BeNil())
+		// Copy TO the container.
 
-		session = podmanTest.Podman([]string{"cp", srcPath, name + ":foo/"})
+		// Cannot copy to a non-existent path (note the trailing "/").
+		session = podmanTest.Podman([]string{"cp", srcFile.Name(), name + ":foo/"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).To(ExitWithError())
 
-		session = podmanTest.Podman([]string{"cp", srcPath, name + ":foo"})
+		// The file will now be created (and written to).
+		session = podmanTest.Podman([]string{"cp", srcFile.Name(), name + ":foo"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		session = podmanTest.Podman([]string{"cp", name + ":foo", dstPath})
+		// Copy FROM the container.
+
+		destFile, err := ioutil.TempFile("", "")
+		Expect(err).To(BeNil())
+		defer destFile.Close()
+		defer os.Remove(destFile.Name())
+
+		session = podmanTest.Podman([]string{"cp", name + ":foo", destFile.Name()})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
 		session = podmanTest.Podman([]string{"start", name})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
+
+		// Now make sure the content matches.
+		roundtripContent, err := ioutil.ReadFile(destFile.Name())
+		Expect(err).To(BeNil())
+		Expect(roundtripContent).To(Equal(originalContent))
 	})
 
-	It("podman cp file to dir", func() {
-		name := "testctr"
-		setup := podmanTest.RunTopContainer(name)
-		setup.WaitWithDefaultTimeout()
-		Expect(setup.ExitCode()).To(Equal(0))
-
-		srcPath := "/tmp/cp_test.txt"
-		fromHostToContainer := []byte("copy from host to container directory")
-		err := ioutil.WriteFile(srcPath, fromHostToContainer, 0644)
-		Expect(err).To(BeNil())
-
-		session := podmanTest.Podman([]string{"exec", name, "mkdir", "foodir"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		session = podmanTest.Podman([]string{"cp", srcPath, name + ":foodir/"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		session = podmanTest.Podman([]string{"exec", name, "ls", "foodir/cp_test.txt"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		os.Remove("/tmp/cp_test.txt")
-	})
-
-	It("podman cp dir to dir", func() {
-		testDirPath := filepath.Join(podmanTest.RunRoot, "TestDir1")
-
-		session := podmanTest.Podman([]string{"create", ALPINE, "ls", "/foodir"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		name := session.OutputToString()
-
-		err := os.Mkdir(testDirPath, 0755)
-		Expect(err).To(BeNil())
-		defer os.RemoveAll(testDirPath)
-
-		session = podmanTest.Podman([]string{"cp", testDirPath, name + ":/foodir"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		session = podmanTest.Podman([]string{"cp", testDirPath, name + ":/foodir"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		testctr := "testctr"
-		setup := podmanTest.RunTopContainer(testctr)
-		setup.WaitWithDefaultTimeout()
-		Expect(setup.ExitCode()).To(Equal(0))
-
-		session = podmanTest.Podman([]string{"exec", testctr, "mkdir", "foo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		session = podmanTest.Podman([]string{"cp", testDirPath + "/.", testctr + ":/foo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		session = podmanTest.Podman([]string{"exec", testctr, "ls", "foo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		Expect(len(session.OutputToString())).To(Equal(0))
-
-		session = podmanTest.Podman([]string{"cp", testctr + ":/foo/.", testDirPath})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		cmd := exec.Command("ls", testDirPath)
-		res, err := cmd.Output()
-		Expect(err).To(BeNil())
-		Expect(len(res)).To(Equal(0))
-	})
-
-	It("podman cp stdin/stdout", func() {
-		SkipIfRemote("FIXME: podman-remote cp not implemented yet")
-		session := podmanTest.Podman([]string{"create", ALPINE, "ls", "foo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		name := session.OutputToString()
-
-		testDirPath := filepath.Join(podmanTest.RunRoot, "TestDir2")
-		err := os.Mkdir(testDirPath, 0755)
-		Expect(err).To(BeNil())
-		defer os.RemoveAll(testDirPath)
-		cmd := exec.Command("tar", "-zcvf", "file.tar.gz", testDirPath)
-		_, err = cmd.Output()
-		Expect(err).To(BeNil())
-
-		data, err := ioutil.ReadFile("foo.tar.gz")
-		reader := strings.NewReader(string(data))
-		cmd.Stdin = reader
-		session = podmanTest.Podman([]string{"cp", "-", name + ":/foo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		session = podmanTest.Podman([]string{"cp", "file.tar.gz", name + ":/foo.tar.gz"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		session = podmanTest.Podman([]string{"cp", name + ":/foo.tar.gz", "-"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		os.Remove("file.tar.gz")
-	})
-
-	It("podman cp tar", func() {
-		testctr := "testctr"
-		setup := podmanTest.RunTopContainer(testctr)
-		setup.WaitWithDefaultTimeout()
-		Expect(setup.ExitCode()).To(Equal(0))
-
-		session := podmanTest.Podman([]string{"exec", testctr, "mkdir", "foo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		path, err := os.Getwd()
-		Expect(err).To(BeNil())
-		testDirPath := filepath.Join(path, "TestDir3")
-		err = os.Mkdir(testDirPath, 0777)
-		Expect(err).To(BeNil())
-		defer os.RemoveAll(testDirPath)
-		cmd := exec.Command("tar", "-cvf", "file.tar", testDirPath)
-		_, err = cmd.Output()
-		Expect(err).To(BeNil())
-
-		session = podmanTest.Podman([]string{"cp", "file.tar", "testctr:/foo/"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		session = podmanTest.Podman([]string{"exec", testctr, "ls", "-l", "foo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		Expect(session.OutputToString()).To(ContainSubstring("file.tar"))
-
-		os.Remove("file.tar")
-	})
-
-	It("podman cp tar --extract", func() {
-		testctr := "testctr"
-		setup := podmanTest.RunTopContainer(testctr)
-		setup.WaitWithDefaultTimeout()
-		Expect(setup.ExitCode()).To(Equal(0))
-
-		session := podmanTest.Podman([]string{"exec", testctr, "mkdir", "/foo"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		path, err := os.Getwd()
-		Expect(err).To(BeNil())
-		testDirPath := filepath.Join(path, "TestDir4")
-		err = os.Mkdir(testDirPath, 0777)
-		Expect(err).To(BeNil())
-		defer os.RemoveAll(testDirPath)
-		f, err := os.Create(filepath.Join(testDirPath, "a.txt"))
-		Expect(err).To(BeNil())
-		_, err = f.Write([]byte("Hello World!!!\n"))
-		f.Close()
-		cmd := exec.Command("tar", "-cvf", "file.tar", "TestDir4")
-		exec.Command("tar", "-cvf", "/home/mvasek/file.tar", testDirPath)
-		_, err = cmd.Output()
-		Expect(err).To(BeNil())
-		defer os.Remove("file.tar")
-
-		session = podmanTest.Podman([]string{"cp", "--extract", "file.tar", "testctr:/foo/"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		session = podmanTest.Podman([]string{"exec", testctr, "cat", "/foo/TestDir4/a.txt"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-		Expect(session.OutputToString()).To(ContainSubstring("Hello World!!!"))
-	})
-
+	// Create a symlink in the container, use it as a copy destination and
+	// make sure that the link and the resolved path are accessible and
+	// give the right content.
 	It("podman cp symlink", func() {
+		srcFile, err := ioutil.TempFile("", "")
+		Expect(err).To(BeNil())
+		defer srcFile.Close()
+		defer os.Remove(srcFile.Name())
+
+		originalContent := []byte("podman cp symlink test")
+		err = ioutil.WriteFile(srcFile.Name(), originalContent, 0644)
+		Expect(err).To(BeNil())
+
 		session := podmanTest.Podman([]string{"run", "-d", ALPINE, "top"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		name := session.OutputToString()
 
-		srcPath := filepath.Join(podmanTest.RunRoot, "cp_test.txt")
-		fromHostToContainer := []byte("copy from host to container")
-		err := ioutil.WriteFile(srcPath, fromHostToContainer, 0644)
-		Expect(err).To(BeNil())
-
 		session = podmanTest.Podman([]string{"exec", name, "ln", "-s", "/tmp", "/test"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		session = podmanTest.Podman([]string{"cp", "--pause=false", srcPath, name + ":/test"})
+		session = podmanTest.Podman([]string{"cp", "--pause=false", srcFile.Name(), name + ":/test"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		_, err = os.Stat("/tmp/cp_test.txt")
-		Expect(err).To(Not(BeNil()))
-
-		session = podmanTest.Podman([]string{"exec", name, "ln", "-s", "/tmp/nonesuch", "/test1"})
+		session = podmanTest.Podman([]string{"exec", name, "cat", "/tmp/" + filepath.Base(srcFile.Name())})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(ContainSubstring(string(originalContent)))
 
-		session = podmanTest.Podman([]string{"cp", "--pause=false", srcPath, name + ":/test1/"})
+		session = podmanTest.Podman([]string{"exec", name, "cat", "/test/" + filepath.Base(srcFile.Name())})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(ExitWithError())
-
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(ContainSubstring(string(originalContent)))
 	})
+
+	// Copy a file to a volume in the container.  The tricky part is that
+	// containers mustn't be running for copying, so Podman has to do some
+	// intense Yoga and 1) detect volume paths on the container, 2) resolve
+	// the path to the volume's mount point on the host, and 3) copy the
+	// data to the volume and not the container.
 	It("podman cp volume", func() {
+		srcFile, err := ioutil.TempFile("", "")
+		Expect(err).To(BeNil())
+		defer srcFile.Close()
+		defer os.Remove(srcFile.Name())
+
+		originalContent := []byte("podman cp volume")
+		err = ioutil.WriteFile(srcFile.Name(), originalContent, 0644)
+		Expect(err).To(BeNil())
 		session := podmanTest.Podman([]string{"volume", "create", "data"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
@@ -279,23 +151,31 @@ var _ = Describe("Podman cp", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		err = ioutil.WriteFile("cp_vol", []byte("copy to the volume"), 0644)
-		if err != nil {
-			os.Exit(1)
-		}
-		session = podmanTest.Podman([]string{"cp", "cp_vol", "container1" + ":/data/cp_vol1"})
+		session = podmanTest.Podman([]string{"cp", srcFile.Name(), "container1" + ":/data/file.txt"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		session = podmanTest.Podman([]string{"cp", "container1" + ":/data/cp_vol1", "cp_vol2"})
+		// Now get the volume's mount point, read the file and make
+		// sure the contents match.
+		session = podmanTest.Podman([]string{"volume", "inspect", "data", "--format", "{{.Mountpoint}}"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		os.Remove("cp_vol")
-		os.Remove("cp_vol2")
+		volumeMountPoint := session.OutputToString()
+		copiedContent, err := ioutil.ReadFile(filepath.Join(volumeMountPoint, "file.txt"))
+		Expect(err).To(BeNil())
+		Expect(copiedContent).To(Equal(originalContent))
 	})
 
+	// Create another user in the container, let them create a file, copy
+	// it to the host and back to the container and make sure that we can
+	// access it, and (roughly) the right users own it.
 	It("podman cp from ctr chown ", func() {
+		srcFile, err := ioutil.TempFile("", "")
+		Expect(err).To(BeNil())
+		defer srcFile.Close()
+		defer os.Remove(srcFile.Name())
+
 		setup := podmanTest.RunTopContainer("testctr")
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
@@ -308,17 +188,19 @@ var _ = Describe("Podman cp", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		session = podmanTest.Podman([]string{"cp", "--pause=false", "testctr:/tmp/testfile", "testfile1"})
+		session = podmanTest.Podman([]string{"cp", "--pause=false", "testctr:/tmp/testfile", srcFile.Name()})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// owner of the file copied to local machine is not testuser
-		cmd := exec.Command("ls", "-l", "testfile1")
+		u, err := user.Current()
+		Expect(err).To(BeNil())
+		cmd := exec.Command("ls", "-l", srcFile.Name())
 		cmdRet, err := cmd.Output()
 		Expect(err).To(BeNil())
-		Expect(strings.Contains(string(cmdRet), "testuser")).To(BeFalse())
+		Expect(string(cmdRet)).To(ContainSubstring(u.Username))
 
-		session = podmanTest.Podman([]string{"cp", "--pause=false", "testfile1", "testctr:testfile2"})
+		session = podmanTest.Podman([]string{"cp", "--pause=false", srcFile.Name(), "testctr:testfile2"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
@@ -327,45 +209,35 @@ var _ = Describe("Podman cp", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(session.OutputToString()).To(ContainSubstring("root"))
-
-		os.Remove("testfile1")
 	})
+
+	// Copy the root dir "/" of a container to the host.
 	It("podman cp the root directory from the ctr to an existing directory on the host ", func() {
-		imgName := "test-cp-root-dir:latest"
-		DockerfileName := "Dockerfile.test-cp-root-dir"
-		ctrName := "test-container-cp-root"
-
-		session := podmanTest.Podman([]string{"build", "-f", "build/" + DockerfileName, "-t", imgName, "build/"})
+		container := "copyroottohost"
+		session := podmanTest.RunTopContainer(container)
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		testDirPath := filepath.Join(podmanTest.RunRoot, "TestDirForCp")
-
-		session = podmanTest.Podman([]string{"create", "--name", ctrName, imgName, "dummy"})
+		session = podmanTest.Podman([]string{"exec", container, "touch", "/dummy.txt"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		err := os.Mkdir(testDirPath, 0755)
-		Expect(err).To(BeNil())
-		defer os.RemoveAll(testDirPath)
-
-		// Copy the root directory of the container to an existing directory
-		session = podmanTest.Podman([]string{"cp", ctrName + ":/", testDirPath})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
-
-		// The file should be in the directory,
-		// not one layer too much of the directory called merged
-		checkFile := filepath.Join(testDirPath, DockerfileName)
-		_, err = os.Stat(checkFile)
+		tmpDir, err := ioutil.TempDir("", "")
 		Expect(err).To(BeNil())
 
-		session = podmanTest.Podman([]string{"container", "rm", ctrName})
+		session = podmanTest.Podman([]string{"cp", container + ":/", tmpDir})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		session = podmanTest.Podman([]string{"rmi", "-f", imgName})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).To(Equal(0))
+		cmd := exec.Command("ls", "-la", tmpDir)
+		output, err := cmd.Output()
+		lsOutput := string(output)
+		Expect(err).To(BeNil())
+		Expect(lsOutput).To(ContainSubstring("dummy.txt"))
+		Expect(lsOutput).To(ContainSubstring("tmp"))
+		Expect(lsOutput).To(ContainSubstring("etc"))
+		Expect(lsOutput).To(ContainSubstring("var"))
+		Expect(lsOutput).To(ContainSubstring("bin"))
+		Expect(lsOutput).To(ContainSubstring("usr"))
 	})
 })

test/system/065-cp.bats

@@ -7,6 +7,290 @@
 
 load helpers
 
+@test "podman cp file from host to container" {
+    skip_if_remote "podman-remote does not yet handle cp"
+
+    srcdir=$PODMAN_TMPDIR/cp-test-file-host-to-ctr
+    mkdir -p $srcdir
+    local -a randomcontent=(
+        random-0-$(random_string 10)
+        random-1-$(random_string 15)
+        random-2-$(random_string 20)
+    )
+    echo "${randomcontent[0]}" > $srcdir/hostfile0
+    echo "${randomcontent[1]}" > $srcdir/hostfile1
+    echo "${randomcontent[2]}" > $srcdir/hostfile2
+
+    run_podman run -d --name cpcontainer --workdir=/srv $IMAGE sleep infinity
+    run_podman exec cpcontainer mkdir /srv/subdir
+
+    # format is: <id> | <destination arg to cp> | <full dest path> | <test name>
+    # where:
+    #    id        is 0-2, one of the random strings/files
+    #    dest arg  is the right-hand argument to 'podman cp' (may be implicit)
+    #    dest path is the full explicit path we expect to see
+    #    test name is a short description of what we're testing here
+    tests="
+0 | /                    | /hostfile0            | copy to root
+0 | /anotherbase.txt     | /anotherbase.txt      | copy to root, new name
+0 | /tmp                 | /tmp/hostfile0        | copy to /tmp
+1 | /tmp/                | /tmp/hostfile1        | copy to /tmp/
+2 | /tmp/.               | /tmp/hostfile2        | copy to /tmp/.
+0 | /tmp/hostfile2       | /tmp/hostfile2        | overwrite previous copy
+0 | /tmp/anotherbase.txt | /tmp/anotherbase.txt  | copy to /tmp, new name
+0 | .                    | /srv/hostfile0        | copy to workdir (rel path), new name
+1 | ./                   | /srv/hostfile1        | copy to workdir (rel path), new name
+0 | anotherbase.txt      | /srv/anotherbase.txt  | copy to workdir (rel path), new name
+0 | subdir               | /srv/subdir/hostfile0 | copy to workdir/subdir
+"
+
+    # Copy one of the files into container, exec+cat, confirm the file
+    # is there and matches what we expect
+    while read id dest dest_fullname description; do
+        run_podman cp $srcdir/hostfile$id cpcontainer:$dest
+        run_podman exec cpcontainer cat $dest_fullname
+        is "$output" "${randomcontent[$id]}" "$description (cp -> ctr:$dest)"
+    done < <(parse_table "$tests")
+
+    # Host path does not exist.
+    run_podman 125 cp $srcdir/IdoNotExist cpcontainer:/tmp
+    is "$output" 'Error: ".*/IdoNotExist" could not be found on the host' \
+       "copy nonexistent host path"
+
+    # Container path does not exist.  Notice that the error message shows how
+    # the specified container is resolved.
+    run_podman 125 cp $srcdir/hostfile0 cpcontainer:/IdoNotExist/
+    is "$output" 'Error: "/IdoNotExist/" could not be found on container.*(resolved to .*/IdoNotExist.*' \
+       "copy into nonexistent path in container"
+
+    run_podman rm -f cpcontainer
+}
+
+
+@test "podman cp --extract=true tar archive to container" {
+    skip_if_remote "podman-remote does not yet handle cp"
+
+    # Create tempfile with random name and content
+    dirname=cp-test-extract
+    srcdir=$PODMAN_TMPDIR/$dirname
+    mkdir -p $srcdir
+    rand_filename=$(random_string 20)
+    rand_content=$(random_string 50)
+    echo $rand_content > $srcdir/$rand_filename
+    chmod 644 $srcdir/$rand_filename
+
+    # Now tar it up!
+    tar_file=$PODMAN_TMPDIR/archive.tar.gz
+    tar -C $PODMAN_TMPDIR -zvcf $tar_file $dirname
+
+    run_podman run -d --name cpcontainer $IMAGE sleep infinity
+
+    # First just copy without extracting the archive.
+    run_podman cp $tar_file cpcontainer:/tmp
+    # Now remove the archive which will also test if it exists and is a file.
+    # To save expensive exec'ing, create a file for the next tests.
+    run_podman exec cpcontainer sh -c "rm /tmp/archive.tar.gz; touch /tmp/file.txt"
+
+    # Now copy with extracting the archive. NOTE that Podman should
+    # auto-decompress the file if needed.
+    run_podman cp --extract=true $tar_file cpcontainer:/tmp
+    run_podman exec cpcontainer cat /tmp/$dirname/$rand_filename
+    is "$output" "$rand_content"
+
+    # Test extract on non archive.
+    run_podman cp --extract=true $srcdir/$rand_filename cpcontainer:/foo.txt
+
+    # Cannot extract an archive to a file!
+    run_podman 125 cp --extract=true $tar_file cpcontainer:/tmp/file.txt
+    is "$output" 'Error: cannot extract archive .* to file "/tmp/file.txt"'
+
+    run_podman rm -f cpcontainer
+}
+
+
+@test "podman cp file from container to host" {
+    skip_if_remote "podman-remote does not yet handle cp"
+
+    srcdir=$PODMAN_TMPDIR/cp-test-file-ctr-to-host
+    mkdir -p $srcdir
+
+    # Create 3 files with random content in the container.
+    local -a randomcontent=(
+        random-0-$(random_string 10)
+        random-1-$(random_string 15)
+        random-2-$(random_string 20)
+    )
+    run_podman run -d --name cpcontainer --workdir=/srv $IMAGE sleep infinity
+    run_podman exec cpcontainer sh -c "echo ${randomcontent[0]} > /tmp/containerfile"
+    run_podman exec cpcontainer sh -c "echo ${randomcontent[1]} > /srv/containerfile1"
+    run_podman exec cpcontainer sh -c "mkdir /srv/subdir; echo ${randomcontent[2]} > /srv/subdir/containerfile2"
+
+    # format is: <id> | <source arg to cp> | <destination arg (appended to $srcdir) to cp> | <full dest path (appended to $srcdir)> | <test name>
+    tests="
+0 | /tmp/containerfile    |          | /containerfile  | copy to srcdir/
+0 | /tmp/containerfile    | /        | /containerfile  | copy to srcdir/
+0 | /tmp/containerfile    | /.       | /containerfile  | copy to srcdir/.
+0 | /tmp/containerfile    | /newfile | /newfile        | copy to srcdir/newfile
+1 | containerfile1        | /        | /containerfile1 | copy from workdir (rel path) to srcdir
+2 | subdir/containerfile2 | /        | /containerfile2 | copy from workdir/subdir (rel path) to srcdir
+"
+
+    # Copy one of the files to the host, cat, confirm the file
+    # is there and matches what we expect
+    while read id src dest dest_fullname description; do
+        # dest may be "''" for empty table cells
+        if [[ $dest == "''" ]];then
+            unset dest
+        fi
+        run_podman cp cpcontainer:$src "$srcdir$dest"
+        run cat $srcdir$dest_fullname
+        is "$output" "${randomcontent[$id]}" "$description (cp ctr:$src to \$srcdir$dest)"
+        rm $srcdir/$dest_fullname
+    done < <(parse_table "$tests")
+
+    run_podman rm -f cpcontainer
+}
+
+
+@test "podman cp dir from host to container" {
+    skip_if_remote "podman-remote does not yet handle cp"
+
+    dirname=dir-test
+    srcdir=$PODMAN_TMPDIR/$dirname
+    mkdir -p $srcdir
+    local -a randomcontent=(
+        random-0-$(random_string 10)
+        random-1-$(random_string 15)
+    )
+    echo "${randomcontent[0]}" > $srcdir/hostfile0
+    echo "${randomcontent[1]}" > $srcdir/hostfile1
+
+    run_podman run -d --name cpcontainer --workdir=/srv $IMAGE sleep infinity
+    run_podman exec cpcontainer mkdir /srv/subdir
+
+    # format is: <source arg to cp (appended to srcdir)> | <destination arg to cp> | <full dest path> | <test name>
+    tests="
+    | /        | /dir-test             | copy to root
+ /  | /tmp     | /tmp/dir-test         | copy to tmp
+ /. | /usr/    | /usr/                 | copy contents of dir to usr/
+    | .        | /srv/dir-test         | copy to workdir (rel path)
+    | subdir/. | /srv/subdir/dir-test | copy to workdir subdir (rel path)
+"
+
+    while read src dest dest_fullname description; do
+        # src may be "''" for empty table cells
+        if [[ $src == "''" ]];then
+            unset src
+        fi
+        run_podman cp $srcdir$src cpcontainer:$dest
+        run_podman exec cpcontainer ls $dest_fullname
+        run_podman exec cpcontainer cat $dest_fullname/hostfile0
+        is "$output" "${randomcontent[0]}" "$description (cp -> ctr:$dest)"
+        run_podman exec cpcontainer cat $dest_fullname/hostfile1
+        is "$output" "${randomcontent[1]}" "$description (cp -> ctr:$dest)"
+    done < <(parse_table "$tests")
+
+    run_podman rm -f cpcontainer
+}
+
+
+@test "podman cp dir from container to host" {
+    skip_if_remote "podman-remote does not yet handle cp"
+
+    srcdir=$PODMAN_TMPDIR/dir-test
+    mkdir -p $srcdir
+
+    run_podman run -d --name cpcontainer --workdir=/srv $IMAGE sleep infinity
+    run_podman exec cpcontainer sh -c 'mkdir /srv/subdir; echo "This first file is on the container" > /srv/subdir/containerfile1'
+    run_podman exec cpcontainer sh -c 'echo "This second file is on the container as well" > /srv/subdir/containerfile2'
+
+    run_podman cp cpcontainer:/srv $srcdir
+    run cat $srcdir/srv/subdir/containerfile1
+    is "$output" "This first file is on the container"
+    run cat $srcdir/srv/subdir/containerfile2
+    is "$output" "This second file is on the container as well"
+    rm -rf $srcdir/srv/subdir
+
+    run_podman cp cpcontainer:/srv/. $srcdir
+    run ls $srcdir/subdir
+    run cat $srcdir/subdir/containerfile1
+    is "$output" "This first file is on the container"
+    run cat $srcdir/subdir/containerfile2
+    is "$output" "This second file is on the container as well"
+    rm -rf $srcdir/subdir
+
+    run_podman cp cpcontainer:/srv/subdir/. $srcdir
+    run cat $srcdir/containerfile1
+    is "$output" "This first file is on the container"
+    run cat $srcdir/containerfile2
+    is "$output" "This second file is on the container as well"
+
+    run_podman rm -f cpcontainer
+}
+
+
+@test "podman cp file from host to container volume" {
+    skip_if_remote "podman-remote does not yet handle cp"
+
+    srcdir=$PODMAN_TMPDIR/cp-test-volume
+    mkdir -p $srcdir
+    echo "This file should be in volume2" > $srcdir/hostfile
+    volume1=$(random_string 20)
+    volume2=$(random_string 20)
+
+    run_podman volume create $volume1
+    run_podman volume inspect $volume1 --format "{{.Mountpoint}}"
+    volume1_mount="$output"
+    run_podman volume create $volume2
+    run_podman volume inspect $volume2 --format "{{.Mountpoint}}"
+    volume2_mount="$output"
+
+    # Create a container using the volume.  Note that copying on not-running
+    # containers is allowed, so Podman has to analyze the container paths and
+    # check if they are hitting a volume, and eventually resolve to the path on
+    # the *host*.
+    # This test is extra tricky, as volume2 is mounted into a sub-directory of
+    # volume1.  Podman must copy the file into volume2 and not volume1.
+    run_podman create --name cpcontainer -v $volume1:/tmp/volume -v $volume2:/tmp/volume/sub-volume $IMAGE
+
+    run_podman cp $srcdir/hostfile cpcontainer:/tmp/volume/sub-volume
+
+    run cat $volume2_mount/hostfile
+    is "$output" "This file should be in volume2"
+
+    # Volume 1 must be empty.
+    run ls $volume1_mount
+    is "$output" ""
+
+    run_podman rm -f cpcontainer
+    run_podman volume rm $volume1 $volume2
+}
+
+
+@test "podman cp file from host to container mount" {
+    skip_if_remote "podman-remote does not yet handle cp"
+
+    srcdir=$PODMAN_TMPDIR/cp-test-mount-src
+    mountdir=$PODMAN_TMPDIR/cp-test-mount
+    mkdir -p $srcdir $mountdir
+    echo "This file should be in the mount" > $srcdir/hostfile
+
+    volume=$(random_string 20)
+    run_podman volume create $volume
+
+    # Make it a bit more complex and put the mount on a volume.
+    run_podman create --name cpcontainer -v $volume:/tmp/volume -v $mountdir:/tmp/volume/mount $IMAGE
+
+    run_podman cp $srcdir/hostfile cpcontainer:/tmp/volume/mount
+
+    run cat $mountdir/hostfile
+    is "$output" "This file should be in the mount"
+
+    run_podman rm -f cpcontainer
+    run_podman volume rm $volume
+}
+
+
 # Create two random-name random-content files in /tmp in the container
 # podman-cp them into the host using '/tmp/*', i.e. asking podman to
 # perform wildcard expansion in the container. We should get both
@@ -51,8 +335,7 @@ load helpers
     run_podman 125 cp 'cpcontainer:/tmp/*' $dstdir/
 
     # FIXME: this might not be the exactly correct error message
-    is "$output" ".*error evaluating symlinks.*lstat.*no such file or dir" \
-       "Expected error from copying invalid symlink"
+    is "$output" 'Error: "/tmp/\*" could not be found on container.*'
 
     # make sure there are no files in dstdir
     is "$(/bin/ls -1 $dstdir)" "" "incorrectly copied symlink from host"
@@ -78,8 +361,7 @@ load helpers
                sh -c "ln -s $srcdir/hostfile file1;ln -s file\* copyme"
     run_podman 125 cp cpcontainer:copyme $dstdir
 
-    is "$output" ".*error evaluating symlinks.*lstat.*no such file or dir" \
-       "Expected error from copying invalid symlink"
+    is "$output" 'Error: "copyme*" could not be found on container.*'
 
     # make sure there are no files in dstdir
     is "$(/bin/ls -1 $dstdir)" "" "incorrectly copied symlink from host"
@@ -101,8 +383,7 @@ load helpers
                sh -c "ln -s $srcdir/hostfile /tmp/\*"
     run_podman 125 cp 'cpcontainer:/tmp/*' $dstdir
 
-    is "$output" ".*error evaluating symlinks.*lstat.*no such file or dir" \
-       "Expected error from copying invalid symlink"
+    is "$output" 'Error: "/tmp/\*" could not be found on container.*'
 
     # dstdir must be empty
     is "$(/bin/ls -1 $dstdir)" "" "incorrectly copied symlink from host"
@@ -110,8 +391,6 @@ load helpers
     run_podman rm cpcontainer
 }
 
-###############################################################################
-# cp INTO container
 
 # THIS IS EXTREMELY WEIRD. Podman expands symlinks in weird ways.
 @test "podman cp into container: weird symlink expansion" {
@@ -148,7 +427,7 @@ load helpers
     is "$output" "" "output from podman cp 1"
 
     run_podman 125 cp --pause=false $srcdir/$rand_filename2 cpcontainer:/tmp/d2/x/
-    is "$output" ".*stat.* no such file or directory" "cp will not create nonexistent destination directory"
+    is "$output" 'Error: "/tmp/d2/x/" could not be found on container.*' "cp will not create nonexistent destination directory"
 
     run_podman cp --pause=false $srcdir/$rand_filename3 cpcontainer:/tmp/d3/x
     is "$output" "" "output from podman cp 3"
@@ -160,6 +439,7 @@ load helpers
     run_podman exec cpcontainer cat /tmp/nonesuch1
     is "$output" "$rand_content1" "cp creates destination file"
 
+
     # cp into nonexistent directory should not mkdir nonesuch2 directory
     run_podman 1 exec cpcontainer test -e /tmp/nonesuch2
 
@@ -168,8 +448,6 @@ load helpers
     is "$output" "$rand_content3" "cp creates file named x"
 
     run_podman rm -f cpcontainer
-
-
 }
 
 
@@ -212,6 +490,103 @@ load helpers
 }
 
 
+@test "podman cp from stdin to container" {
+    skip_if_remote "podman-remote does not yet handle cp"
+
+    # Create tempfile with random name and content
+    srcdir=$PODMAN_TMPDIR/cp-test-stdin
+    mkdir -p $srcdir
+    rand_filename=$(random_string 20)
+    rand_content=$(random_string 50)
+    echo $rand_content > $srcdir/$rand_filename
+    chmod 644 $srcdir/$rand_filename
+
+    # Now tar it up!
+    tar_file=$PODMAN_TMPDIR/archive.tar.gz
+    tar -zvcf $tar_file $srcdir
+
+    run_podman run -d --name cpcontainer $IMAGE sleep infinity
+
+    # NOTE: podman is supposed to auto-detect the gzip compression and
+    # decompress automatically.
+    #
+    # "-" will evaluate to "/dev/stdin" when used a source.
+    run_podman cp - cpcontainer:/tmp < $tar_file
+    run_podman exec cpcontainer cat /tmp/$srcdir/$rand_filename
+    is "$output" "$rand_content"
+    run_podman exec cpcontainer rm -rf /tmp/$srcdir
+
+    # Now for "/dev/stdin".
+    run_podman cp /dev/stdin cpcontainer:/tmp < $tar_file
+    run_podman exec cpcontainer cat /tmp/$srcdir/$rand_filename
+    is "$output" "$rand_content"
+
+    # Error checks below ...
+
+    # Input stream must be a (compressed) tar archive.
+    run_podman 125 cp - cpcontainer:/tmp < $srcdir/$rand_filename
+    is "$output" "Error:.*: error reading tar stream.*" "input stream must be a (compressed) tar archive"
+
+    # Destination must be a directory (on an existing file).
+    run_podman exec cpcontainer touch /tmp/file.txt
+    run_podman 125 cp /dev/stdin cpcontainer:/tmp/file.txt < $tar_file
+    is "$output" 'Error: destination must be a directory or stream when copying from a stream'
+
+    # Destination must be a directory (on an absent path).
+    run_podman 125 cp /dev/stdin cpcontainer:/tmp/IdoNotExist < $tar_file
+    is "$output" 'Error: destination must be a directory or stream when copying from a stream'
+
+    run_podman rm -f cpcontainer
+}
+
+
+@test "podman cp from container to stdout" {
+    skip_if_remote "podman-remote does not yet handle cp"
+
+    srcdir=$PODMAN_TMPDIR/cp-test-stdout
+    mkdir -p $srcdir
+    rand_content=$(random_string 50)
+
+    run_podman run -d --name cpcontainer $IMAGE sleep infinity
+
+    run_podman exec cpcontainer sh -c "echo '$rand_content' > /tmp/file.txt"
+    run_podman exec cpcontainer touch /tmp/empty.txt
+
+    # Copying from stdout will always compress.  So let's copy the previously
+    # created file from the container via stdout, untar the archive and make
+    # sure the file exists with the expected content.
+    #
+    # NOTE that we can't use run_podman because that uses the BATS 'run'
+    # function which redirects stdout and stderr. Here we need to guarantee
+    # that podman's stdout is a pipe, not any other form of redirection.
+
+    # Copy file.
+    $PODMAN cp cpcontainer:/tmp/file.txt - > $srcdir/stdout.tar
+    if [ $? -ne 0 ]; then
+        die "Command failed: podman cp ... - | cat"
+    fi
+
+    tar xvf $srcdir/stdout.tar -C $srcdir
+    run cat $srcdir/file.txt
+    is "$output" "$rand_content"
+    run 1 ls $srcfir/empty.txt
+    rm -f $srcdir/*
+
+    # Copy directory.
+    $PODMAN cp cpcontainer:/tmp - > $srcdir/stdout.tar
+    if [ $? -ne 0 ]; then
+        die "Command failed: podman cp ... - | cat : $output"
+    fi
+
+    tar xvf $srcdir/stdout.tar -C $srcdir
+    run cat $srcdir/file.txt
+    is "$output" "$rand_content"
+    run cat $srcdir/empty.txt
+    is "$output" ""
+
+    run_podman rm -f cpcontainer
+}
+
 function teardown() {
     # In case any test fails, clean up the container we left behind
     run_podman rm -f cpcontainer