Merge pull request #15793 from giuseppe/fix-volume-subpath-lookup

libpod: fix lookup for subpath in volumes

libpod/container_internal_common.go

@@ -531,7 +531,7 @@ func (c *Container) isWorkDirSymlink(resolvedPath string) bool {
 		}
 		if resolvedSymlink != "" {
 			_, resolvedSymlinkWorkdir, err := c.resolvePath(c.state.Mountpoint, resolvedSymlink)
-			if isPathOnVolume(c, resolvedSymlinkWorkdir) || isPathOnBindMount(c, resolvedSymlinkWorkdir) {
+			if isPathOnVolume(c, resolvedSymlinkWorkdir) || isPathOnMount(c, resolvedSymlinkWorkdir) {
 				// Resolved symlink exists on external volume or mount
 				return true
 			}
@@ -564,7 +564,7 @@ func (c *Container) resolveWorkDir() error {
 	// If the specified workdir is a subdir of a volume or mount,
 	// we don't need to do anything.  The runtime is taking care of
 	// that.
-	if isPathOnVolume(c, workdir) || isPathOnBindMount(c, workdir) {
+	if isPathOnVolume(c, workdir) || isPathOnMount(c, workdir) {
 		logrus.Debugf("Workdir %q resolved to a volume or mount", workdir)
 		return nil
 	}

libpod/container_path_resolution.go

@@ -119,15 +119,29 @@ func findVolume(c *Container, containerPath string) (*Volume, error) {
 	return nil, nil
 }
 
+// isSubDir checks whether path is a subdirectory of root.
+func isSubDir(path, root string) bool {
+	// check if the specified container path is below a bind mount.
+	rel, err := filepath.Rel(root, path)
+	if err != nil {
+		return false
+	}
+	return rel != ".." && !strings.HasPrefix(rel, "../")
+}
+
 // isPathOnVolume returns true if the specified containerPath is a subdir of any
 // Volume's destination.
 func isPathOnVolume(c *Container, containerPath string) bool {
 	cleanedContainerPath := filepath.Clean(containerPath)
 	for _, vol := range c.config.NamedVolumes {
-		if cleanedContainerPath == filepath.Clean(vol.Dest) {
+		cleanedDestination := filepath.Clean(vol.Dest)
+		if cleanedContainerPath == cleanedDestination {
 			return true
 		}
-		for dest := vol.Dest; dest != "/" && dest != "."; dest = filepath.Dir(dest) {
+		if isSubDir(cleanedContainerPath, cleanedDestination) {
+			return true
+		}
+		for dest := cleanedDestination; dest != "/" && dest != "."; dest = filepath.Dir(dest) {
 			if cleanedContainerPath == dest {
 				return true
 			}
@@ -152,15 +166,19 @@ func findBindMount(c *Container, containerPath string) *specs.Mount {
 	return nil
 }
 
-/// isPathOnBindMount returns true if the specified containerPath is a subdir of any
+/// isPathOnMount returns true if the specified containerPath is a subdir of any
 // Mount's destination.
-func isPathOnBindMount(c *Container, containerPath string) bool {
+func isPathOnMount(c *Container, containerPath string) bool {
 	cleanedContainerPath := filepath.Clean(containerPath)
 	for _, m := range c.config.Spec.Mounts {
-		if cleanedContainerPath == filepath.Clean(m.Destination) {
+		cleanedDestination := filepath.Clean(m.Destination)
+		if cleanedContainerPath == cleanedDestination {
 			return true
 		}
-		for dest := m.Destination; dest != "/" && dest != "."; dest = filepath.Dir(dest) {
+		if isSubDir(cleanedContainerPath, cleanedDestination) {
+			return true
+		}
+		for dest := cleanedDestination; dest != "/" && dest != "."; dest = filepath.Dir(dest) {
 			if cleanedContainerPath == dest {
 				return true
 			}

libpod/container_path_resolution_test.go

@@ -0,0 +1,28 @@
+package libpod
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsSubDir(t *testing.T) {
+	assert.True(t, isSubDir("/foo", "/foo"))
+	assert.True(t, isSubDir("/foo/bar", "/foo"))
+	assert.True(t, isSubDir("/foo/bar", "/foo/"))
+	assert.True(t, isSubDir("/foo/bar", "/foo//"))
+	assert.True(t, isSubDir("/foo/bar/", "/foo"))
+	assert.True(t, isSubDir("/foo/bar/baz/", "/foo"))
+	assert.True(t, isSubDir("/foo/bar/baz/", "/foo/bar"))
+	assert.True(t, isSubDir("/foo/bar/baz/", "/foo/bar/"))
+	assert.False(t, isSubDir("/foo/bar/baz/", "/foobar/"))
+	assert.False(t, isSubDir("/foo/bar/baz/../../", "/foobar/"))
+	assert.False(t, isSubDir("/foo/bar/baz/", "../foo/bar"))
+	assert.False(t, isSubDir("/foo/bar/baz/", "../foo/"))
+	assert.False(t, isSubDir("/foo/bar/baz/", "../foo"))
+	assert.False(t, isSubDir("/", ".."))
+	assert.False(t, isSubDir("//", ".."))
+	assert.False(t, isSubDir("//", "../"))
+	assert.False(t, isSubDir("//", "..//"))
+	assert.True(t, isSubDir("/foo/bar/baz/../../", "/foo/"))
+}

test/e2e/run_working_dir_test.go

@@ -46,6 +46,15 @@ var _ = Describe("Podman run", func() {
 		Expect(session).Should(Exit(126))
 	})
 
+	It("podman run a container using a --workdir under a bind mount", func() {
+		volume, err := CreateTempDirInTempDir()
+		Expect(err).To(BeNil())
+
+		session := podmanTest.Podman([]string{"run", "--volume", fmt.Sprintf("%s:/var_ovl/:O", volume), "--workdir", "/var_ovl/log", ALPINE, "true"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+	})
+
 	It("podman run a container on an image with a workdir", func() {
 		dockerfile := fmt.Sprintf(`FROM %s
 RUN  mkdir -p /home/foobar /etc/foobar; chown bin:bin /etc/foobar