From 92dc61d5edb1b5ce85f7e4563d400cc861a28359 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 14 Sep 2022 13:03:56 +0200 Subject: [PATCH 1/2] libpod: rename function the function checks if a path is under any mount, not just bind mounts. Signed-off-by: Giuseppe Scrivano --- libpod/container_internal_common.go | 4 ++-- libpod/container_path_resolution.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index c7f59aba59..9c4a3bb678 100644 --- a/libpod/container_internal_common.go +++ b/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 } diff --git a/libpod/container_path_resolution.go b/libpod/container_path_resolution.go index 35622d6230..eddfd361e2 100644 --- a/libpod/container_path_resolution.go +++ b/libpod/container_path_resolution.go @@ -152,9 +152,9 @@ 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) { From 14e5d1c15da82f7eb315c320765aeca69f4b58af Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 14 Sep 2022 13:01:43 +0200 Subject: [PATCH 2/2] libpod: fix lookup for subpath in volumes a subdirectory that is below a mount destination is detected as a subpath. Closes: https://github.com/containers/podman/issues/15789 Signed-off-by: Giuseppe Scrivano --- libpod/container_path_resolution.go | 26 ++++++++++++++++++---- libpod/container_path_resolution_test.go | 28 ++++++++++++++++++++++++ test/e2e/run_working_dir_test.go | 9 ++++++++ 3 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 libpod/container_path_resolution_test.go diff --git a/libpod/container_path_resolution.go b/libpod/container_path_resolution.go index eddfd361e2..cd86df5409 100644 --- a/libpod/container_path_resolution.go +++ b/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 } @@ -157,10 +171,14 @@ func findBindMount(c *Container, containerPath string) *specs.Mount { 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 + } + if isSubDir(cleanedContainerPath, cleanedDestination) { return true } - for dest := m.Destination; dest != "/" && dest != "."; dest = filepath.Dir(dest) { + for dest := cleanedDestination; dest != "/" && dest != "."; dest = filepath.Dir(dest) { if cleanedContainerPath == dest { return true } diff --git a/libpod/container_path_resolution_test.go b/libpod/container_path_resolution_test.go new file mode 100644 index 0000000000..f906c752d2 --- /dev/null +++ b/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/")) +} diff --git a/test/e2e/run_working_dir_test.go b/test/e2e/run_working_dir_test.go index ff91a420f6..84792481f8 100644 --- a/test/e2e/run_working_dir_test.go +++ b/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