Fix docker cp Behavior With Symlinks

[pkg/archive] Update archive/copy path handling

  - Remove unused TarOptions.Name field.
  - Add new TarOptions.RebaseNames field.
  - Update some of the logic around path dir/base splitting.
  - Update some of the logic behind archive entry name rebasing.

[api/types] Add LinkTarget field to PathStat

[daemon] Fix stat, archive, extract of symlinks

  These operations *should* resolve symlinks that are in the path but if the
  resource itself is a symlink then it *should not* be resolved. This patch
  puts this logic into a common function `resolvePath` which resolves symlinks
  of the path's dir in scope of the container rootfs but does not resolve the
  final element of the path. Now archive, extract, and stat operations will
  return symlinks if the path is indeed a symlink.

[api/client] Update cp path hanling

[docs/reference/api] Update description of stat

  Add the linkTarget field to the header of the archive endpoint.
  Remove path field.

[integration-cli] Fix/Add cp symlink test cases

  Copying a symlink should do just that: copy the symlink NOT
  copy the target of the symlink. Also, the resulting file from
  the copy should have the name of the symlink NOT the name of
  the target file.

  Copying to a symlink should copy to the symlink target and not
  modify the symlink itself.

Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)

integration-cli/docker_api_containers_test.go

@@ -4,9 +4,11 @@ import (
 	"archive/tar"
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"io"
 	"net/http"
 	"net/http/httputil"
+	"net/url"
 	"os"
 	"strconv"
 	"strings"
@@ -1697,3 +1699,35 @@ func (s *DockerSuite) TestContainersApiCreateNoHostConfig118(c *check.C) {
 	c.Assert(err, check.IsNil)
 	c.Assert(status, check.Equals, http.StatusCreated)
 }
+
+// Ensure an error occurs when you have a container read-only rootfs but you
+// extract an archive to a symlink in a writable volume which points to a
+// directory outside of the volume.
+func (s *DockerSuite) TestPutContainerArchiveErrSymlinkInVolumeToReadOnlyRootfs(c *check.C) {
+	testRequires(c, SameHostDaemon) // Requires local volume mount bind.
+
+	testVol := getTestDir(c, "test-put-container-archive-err-symlink-in-volume-to-read-only-rootfs-")
+	defer os.RemoveAll(testVol)
+
+	makeTestContentInDir(c, testVol)
+
+	cID := makeTestContainer(c, testContainerOptions{
+		readOnly: true,
+		volumes:  defaultVolumes(testVol), // Our bind mount is at /vol2
+	})
+	defer deleteContainer(cID)
+
+	// Attempt to extract to a symlink in the volume which points to a
+	// directory outside the volume. This should cause an error because the
+	// rootfs is read-only.
+	query := make(url.Values, 1)
+	query.Set("path", "/vol2/symlinkToAbsDir")
+	urlPath := fmt.Sprintf("/v1.20/containers/%s/archive?%s", cID, query.Encode())
+
+	statusCode, body, err := sockRequest("PUT", urlPath, nil)
+	c.Assert(err, check.IsNil)
+
+	if !isCpCannotCopyReadOnly(fmt.Errorf(string(body))) {
+		c.Fatalf("expected ErrContainerRootfsReadonly error, but got %d: %s", statusCode, string(body))
+	}
+}

integration-cli/docker_cli_cp_from_container_test.go

@@ -130,6 +130,114 @@ func (s *DockerSuite) TestCpFromErrDstNotDir(c *check.C) {
 	}
 }
 
+// Check that copying from a container to a local symlink copies to the symlink
+// target and does not overwrite the local symlink itself.
+func (s *DockerSuite) TestCpFromSymlinkDestination(c *check.C) {
+	cID := makeTestContainer(c, testContainerOptions{addContent: true})
+	defer deleteContainer(cID)
+
+	tmpDir := getTestDir(c, "test-cp-from-err-dst-not-dir")
+	defer os.RemoveAll(tmpDir)
+
+	makeTestContentInDir(c, tmpDir)
+
+	// First, copy a file from the container to a symlink to a file. This
+	// should overwrite the symlink target contents with the source contents.
+	srcPath := containerCpPath(cID, "/file2")
+	dstPath := cpPath(tmpDir, "symlinkToFile1")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, dstPath, "file1"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The file should have the contents of "file2" now.
+	if err := fileContentEquals(c, cpPath(tmpDir, "file1"), "file2\n"); err != nil {
+		c.Fatal(err)
+	}
+
+	// Next, copy a file from the container to a symlink to a directory. This
+	// should copy the file into the symlink target directory.
+	dstPath = cpPath(tmpDir, "symlinkToDir1")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, dstPath, "dir1"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The file should have the contents of "file2" now.
+	if err := fileContentEquals(c, cpPath(tmpDir, "file2"), "file2\n"); err != nil {
+		c.Fatal(err)
+	}
+
+	// Next, copy a file from the container to a symlink to a file that does
+	// not exist (a broken symlink). This should create the target file with
+	// the contents of the source file.
+	dstPath = cpPath(tmpDir, "brokenSymlinkToFileX")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, dstPath, "fileX"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The file should have the contents of "file2" now.
+	if err := fileContentEquals(c, cpPath(tmpDir, "fileX"), "file2\n"); err != nil {
+		c.Fatal(err)
+	}
+
+	// Next, copy a directory from the container to a symlink to a local
+	// directory. This should copy the directory into the symlink target
+	// directory and not modify the symlink.
+	srcPath = containerCpPath(cID, "/dir2")
+	dstPath = cpPath(tmpDir, "symlinkToDir1")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, dstPath, "dir1"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The directory should now contain a copy of "dir2".
+	if err := fileContentEquals(c, cpPath(tmpDir, "dir1/dir2/file2-1"), "file2-1\n"); err != nil {
+		c.Fatal(err)
+	}
+
+	// Next, copy a directory from the container to a symlink to a local
+	// directory that does not exist (a broken symlink). This should create
+	// the target as a directory with the contents of the source directory. It
+	// should not modify the symlink.
+	dstPath = cpPath(tmpDir, "brokenSymlinkToDirX")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, dstPath, "dirX"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The "dirX" directory should now be a copy of "dir2".
+	if err := fileContentEquals(c, cpPath(tmpDir, "dirX/file2-1"), "file2-1\n"); err != nil {
+		c.Fatal(err)
+	}
+}
+
 // Possibilities are reduced to the remaining 10 cases:
 //
 //  case | srcIsDir | onlyDirContents | dstExists | dstIsDir | dstTrSep | action

integration-cli/docker_cli_cp_test.go

@@ -250,29 +250,185 @@ func (s *DockerSuite) TestCpAbsoluteSymlink(c *check.C) {
 		c.Fatal(err)
 	}
 
-	tmpname := filepath.Join(tmpdir, cpTestName)
+	tmpname := filepath.Join(tmpdir, "container_path")
 	defer os.RemoveAll(tmpdir)
 
 	path := path.Join("/", "container_path")
 
 	dockerCmd(c, "cp", cleanedContainerID+":"+path, tmpdir)
 
-	file, _ := os.Open(tmpname)
-	defer file.Close()
-
-	test, err := ioutil.ReadAll(file)
+	// We should have copied a symlink *NOT* the file itself!
+	linkTarget, err := os.Readlink(tmpname)
 	if err != nil {
 		c.Fatal(err)
 	}
 
-	if string(test) == cpHostContents {
-		c.Errorf("output matched host file -- absolute symlink can escape container rootfs")
+	if linkTarget != filepath.FromSlash(cpFullPath) {
+		c.Errorf("symlink target was %q, but expected: %q", linkTarget, cpFullPath)
+	}
+}
+
+// Check that symlinks to a directory behave as expected when copying one from
+// a container.
+func (s *DockerSuite) TestCpFromSymlinkToDirectory(c *check.C) {
+	out, exitCode := dockerCmd(c, "run", "-d", "busybox", "/bin/sh", "-c", "mkdir -p '"+cpTestPath+"' && echo -n '"+cpContainerContents+"' > "+cpFullPath+" && ln -s "+cpTestPathParent+" /dir_link")
+	if exitCode != 0 {
+		c.Fatal("failed to create a container", out)
 	}
 
-	if string(test) != cpContainerContents {
-		c.Errorf("output doesn't match the input for absolute symlink")
+	cleanedContainerID := strings.TrimSpace(out)
+
+	out, _ = dockerCmd(c, "wait", cleanedContainerID)
+	if strings.TrimSpace(out) != "0" {
+		c.Fatal("failed to set up container", out)
 	}
 
+	testDir, err := ioutil.TempDir("", "test-cp-from-symlink-to-dir-")
+	if err != nil {
+		c.Fatal(err)
+	}
+	defer os.RemoveAll(testDir)
+
+	// This copy command should copy the symlink, not the target, into the
+	// temporary directory.
+	dockerCmd(c, "cp", cleanedContainerID+":"+"/dir_link", testDir)
+
+	expectedPath := filepath.Join(testDir, "dir_link")
+	linkTarget, err := os.Readlink(expectedPath)
+	if err != nil {
+		c.Fatalf("unable to read symlink at %q: %v", expectedPath, err)
+	}
+
+	if linkTarget != filepath.FromSlash(cpTestPathParent) {
+		c.Errorf("symlink target was %q, but expected: %q", linkTarget, cpTestPathParent)
+	}
+
+	os.Remove(expectedPath)
+
+	// This copy command should resolve the symlink (note the trailing
+	// seperator), copying the target into the temporary directory.
+	dockerCmd(c, "cp", cleanedContainerID+":"+"/dir_link/", testDir)
+
+	// It *should not* have copied the directory using the target's name, but
+	// used the given name instead.
+	unexpectedPath := filepath.Join(testDir, cpTestPathParent)
+	if stat, err := os.Lstat(unexpectedPath); err == nil {
+		c.Fatalf("target name was copied: %q - %q", stat.Mode(), stat.Name())
+	}
+
+	// It *should* have copied the directory using the asked name "dir_link".
+	stat, err := os.Lstat(expectedPath)
+	if err != nil {
+		c.Fatalf("unable to stat resource at %q: %v", expectedPath, err)
+	}
+
+	if !stat.IsDir() {
+		c.Errorf("should have copied a directory but got %q instead", stat.Mode())
+	}
+}
+
+// Check that symlinks to a directory behave as expected when copying one to a
+// container.
+func (s *DockerSuite) TestCpToSymlinkToDirectory(c *check.C) {
+	testRequires(c, SameHostDaemon) // Requires local volume mount bind.
+
+	testVol, err := ioutil.TempDir("", "test-cp-to-symlink-to-dir-")
+	if err != nil {
+		c.Fatal(err)
+	}
+	defer os.RemoveAll(testVol)
+
+	// Create a test container with a local volume. We will test by copying
+	// to the volume path in the container which we can then verify locally.
+	out, exitCode := dockerCmd(c, "create", "-v", testVol+":/testVol", "busybox")
+	if exitCode != 0 {
+		c.Fatal("failed to create a container", out)
+	}
+
+	cleanedContainerID := strings.TrimSpace(out)
+
+	// Create a temp directory to hold a test file nested in a direcotry.
+	testDir, err := ioutil.TempDir("", "test-cp-to-symlink-to-dir-")
+	if err != nil {
+		c.Fatal(err)
+	}
+	defer os.RemoveAll(testDir)
+
+	// This file will be at "/testDir/some/path/test" and will be copied into
+	// the test volume later.
+	hostTestFilename := filepath.Join(testDir, cpFullPath)
+	if err := os.MkdirAll(filepath.Dir(hostTestFilename), os.FileMode(0700)); err != nil {
+		c.Fatal(err)
+	}
+	if err := ioutil.WriteFile(hostTestFilename, []byte(cpHostContents), os.FileMode(0600)); err != nil {
+		c.Fatal(err)
+	}
+
+	// Now create another temp directory to hold a symlink to the
+	// "/testDir/some" directory.
+	linkDir, err := ioutil.TempDir("", "test-cp-to-symlink-to-dir-")
+	if err != nil {
+		c.Fatal(err)
+	}
+	defer os.RemoveAll(linkDir)
+
+	// Then symlink "/linkDir/dir_link" to "/testdir/some".
+	linkTarget := filepath.Join(testDir, cpTestPathParent)
+	localLink := filepath.Join(linkDir, "dir_link")
+	if err := os.Symlink(linkTarget, localLink); err != nil {
+		c.Fatal(err)
+	}
+
+	// Now copy that symlink into the test volume in the container.
+	dockerCmd(c, "cp", localLink, cleanedContainerID+":/testVol")
+
+	// This copy command should have copied the symlink *not* the target.
+	expectedPath := filepath.Join(testVol, "dir_link")
+	actualLinkTarget, err := os.Readlink(expectedPath)
+	if err != nil {
+		c.Fatalf("unable to read symlink at %q: %v", expectedPath, err)
+	}
+
+	if actualLinkTarget != linkTarget {
+		c.Errorf("symlink target was %q, but expected: %q", actualLinkTarget, linkTarget)
+	}
+
+	// Good, now remove that copied link for the next test.
+	os.Remove(expectedPath)
+
+	// This copy command should resolve the symlink (note the trailing
+	// seperator), copying the target into the test volume directory in the
+	// container.
+	dockerCmd(c, "cp", localLink+"/", cleanedContainerID+":/testVol")
+
+	// It *should not* have copied the directory using the target's name, but
+	// used the given name instead.
+	unexpectedPath := filepath.Join(testVol, cpTestPathParent)
+	if stat, err := os.Lstat(unexpectedPath); err == nil {
+		c.Fatalf("target name was copied: %q - %q", stat.Mode(), stat.Name())
+	}
+
+	// It *should* have copied the directory using the asked name "dir_link".
+	stat, err := os.Lstat(expectedPath)
+	if err != nil {
+		c.Fatalf("unable to stat resource at %q: %v", expectedPath, err)
+	}
+
+	if !stat.IsDir() {
+		c.Errorf("should have copied a directory but got %q instead", stat.Mode())
+	}
+
+	// And this directory should contain the file copied from the host at the
+	// expected location: "/testVol/dir_link/path/test"
+	expectedFilepath := filepath.Join(testVol, "dir_link/path/test")
+	fileContents, err := ioutil.ReadFile(expectedFilepath)
+	if err != nil {
+		c.Fatal(err)
+	}
+
+	if string(fileContents) != cpHostContents {
+		c.Fatalf("file contains %q but expected %q", string(fileContents), cpHostContents)
+	}
 }
 
 // Test for #5619

integration-cli/docker_cli_cp_to_container_test.go

@@ -146,6 +146,118 @@ func (s *DockerSuite) TestCpToErrDstNotDir(c *check.C) {
 	}
 }
 
+// Check that copying from a local path to a symlink in a container copies to
+// the symlink target and does not overwrite the container symlink itself.
+func (s *DockerSuite) TestCpToSymlinkDestination(c *check.C) {
+	testRequires(c, SameHostDaemon) // Requires local volume mount bind.
+
+	testVol := getTestDir(c, "test-cp-to-symlink-destination-")
+	defer os.RemoveAll(testVol)
+
+	makeTestContentInDir(c, testVol)
+
+	cID := makeTestContainer(c, testContainerOptions{
+		volumes: defaultVolumes(testVol), // Our bind mount is at /vol2
+	})
+	defer deleteContainer(cID)
+
+	// First, copy a local file to a symlink to a file in the container. This
+	// should overwrite the symlink target contents with the source contents.
+	srcPath := cpPath(testVol, "file2")
+	dstPath := containerCpPath(cID, "/vol2/symlinkToFile1")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, cpPath(testVol, "symlinkToFile1"), "file1"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The file should have the contents of "file2" now.
+	if err := fileContentEquals(c, cpPath(testVol, "file1"), "file2\n"); err != nil {
+		c.Fatal(err)
+	}
+
+	// Next, copy a local file to a symlink to a directory in the container.
+	// This should copy the file into the symlink target directory.
+	dstPath = containerCpPath(cID, "/vol2/symlinkToDir1")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, cpPath(testVol, "symlinkToDir1"), "dir1"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The file should have the contents of "file2" now.
+	if err := fileContentEquals(c, cpPath(testVol, "file2"), "file2\n"); err != nil {
+		c.Fatal(err)
+	}
+
+	// Next, copy a file to a symlink to a file that does not exist (a broken
+	// symlink) in the container. This should create the target file with the
+	// contents of the source file.
+	dstPath = containerCpPath(cID, "/vol2/brokenSymlinkToFileX")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, cpPath(testVol, "brokenSymlinkToFileX"), "fileX"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The file should have the contents of "file2" now.
+	if err := fileContentEquals(c, cpPath(testVol, "fileX"), "file2\n"); err != nil {
+		c.Fatal(err)
+	}
+
+	// Next, copy a local directory to a symlink to a directory in the
+	// container. This should copy the directory into the symlink target
+	// directory and not modify the symlink.
+	srcPath = cpPath(testVol, "/dir2")
+	dstPath = containerCpPath(cID, "/vol2/symlinkToDir1")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, cpPath(testVol, "symlinkToDir1"), "dir1"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The directory should now contain a copy of "dir2".
+	if err := fileContentEquals(c, cpPath(testVol, "dir1/dir2/file2-1"), "file2-1\n"); err != nil {
+		c.Fatal(err)
+	}
+
+	// Next, copy a local directory to a symlink to a local directory that does
+	// not exist (a broken symlink) in the container. This should create the
+	// target as a directory with the contents of the source directory. It
+	// should not modify the symlink.
+	dstPath = containerCpPath(cID, "/vol2/brokenSymlinkToDirX")
+
+	if err := runDockerCp(c, srcPath, dstPath); err != nil {
+		c.Fatalf("unexpected error %T: %s", err, err)
+	}
+
+	// The symlink should not have been modified.
+	if err := symlinkTargetEquals(c, cpPath(testVol, "brokenSymlinkToDirX"), "dirX"); err != nil {
+		c.Fatal(err)
+	}
+
+	// The "dirX" directory should now be a copy of "dir2".
+	if err := fileContentEquals(c, cpPath(testVol, "dirX/file2-1"), "file2-1\n"); err != nil {
+		c.Fatal(err)
+	}
+}
+
 // Possibilities are reduced to the remaining 10 cases:
 //
 //  case | srcIsDir | onlyDirContents | dstExists | dstIsDir | dstTrSep | action

integration-cli/docker_cli_cp_utils.go

@@ -74,8 +74,11 @@ var defaultFileData = []fileData{
 	{ftRegular, "dir4/file3-1", "file4-1"},
 	{ftRegular, "dir4/file3-2", "file4-2"},
 	{ftDir, "dir5", ""},
-	{ftSymlink, "symlink1", "target1"},
-	{ftSymlink, "symlink2", "target2"},
+	{ftSymlink, "symlinkToFile1", "file1"},
+	{ftSymlink, "symlinkToDir1", "dir1"},
+	{ftSymlink, "brokenSymlinkToFileX", "fileX"},
+	{ftSymlink, "brokenSymlinkToDirX", "dirX"},
+	{ftSymlink, "symlinkToAbsDir", "/root"},
 }
 
 func defaultMkContentCommand() string {
@@ -268,6 +271,21 @@ func fileContentEquals(c *check.C, filename, contents string) (err error) {
 	return
 }
 
+func symlinkTargetEquals(c *check.C, symlink, expectedTarget string) (err error) {
+	c.Logf("checking that the symlink %q points to %q\n", symlink, expectedTarget)
+
+	actualTarget, err := os.Readlink(symlink)
+	if err != nil {
+		return err
+	}
+
+	if actualTarget != expectedTarget {
+		return fmt.Errorf("symlink target points to %q not %q", actualTarget, expectedTarget)
+	}
+
+	return nil
+}
+
 func containerStartOutputEquals(c *check.C, cID, contents string) (err error) {
 	c.Logf("checking that container %q start output contains %q\n", cID, contents)