From 7b1be7f17723c89a636a65bb6f06456bfb8e5156 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 3 Dec 2025 16:57:15 +0100 Subject: [PATCH 1/2] libpod: fix workdir MkdirAll() all check MkdirAll can fail with EEXIST when the path is a symlink and the target doesn't exist. As such we should ignore the error. Note there is something fundemantal wrong here with the path access as it is following the symlink to the host, however it is only for a stat() so it is not an security issue here. Fixes: 637c264e2e ("fix issues found by nilness") Signed-off-by: Paul Holzinger --- libpod/container_internal_common.go | 3 +++ test/e2e/run_working_dir_test.go | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index acaa76ef3a..606f279077 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -935,6 +935,9 @@ func (c *Container) resolveWorkDir() error { return fmt.Errorf("detecting workdir %q on container %s: %w", workdir, c.ID(), err) } if err := os.MkdirAll(resolvedWorkdir, 0o755); err != nil { + if errors.Is(err, fs.ErrExist) { + return nil + } return fmt.Errorf("creating container %s workdir: %w", c.ID(), err) } diff --git a/test/e2e/run_working_dir_test.go b/test/e2e/run_working_dir_test.go index 91c1f6bf28..ac7f2b7a10 100644 --- a/test/e2e/run_working_dir_test.go +++ b/test/e2e/run_working_dir_test.go @@ -56,4 +56,14 @@ WORKDIR /etc/foobar`, ALPINE) Expect(session).Should(ExitCleanly()) Expect(session.OutputToString()).To(Equal("/home/foobar")) }) + + It("podman run on an image with a symlinked workdir", func() { + dockerfile := fmt.Sprintf(`FROM %s +RUN mkdir /A && ln -s /A /B +WORKDIR /B`, ALPINE) + podmanTest.BuildImage(dockerfile, "test", "false") + + session := podmanTest.PodmanExitCleanly("run", "test", "pwd") + Expect(session.OutputToString()).To(Equal("/A")) + }) }) From d18e44e9abb3bf5b7294aa70806e1368fdddfdd0 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 3 Dec 2025 19:15:24 +0100 Subject: [PATCH 2/2] libpod: simplify resolveWorkDir() The code checks for isPathOnVolume and isPathOnMount so we can just use the SecureJoin here directly to check for path existance. Then instead of walking symlinks and trying to guess if they are on a mount just assume if it is a link (path is different from the normal joined one) then don't error out early and let the OCI runtime deal with it. The runtime does produce a less readable error but it still fails and we have much less fragile code. Signed-off-by: Paul Holzinger --- libpod/container_internal_common.go | 59 +++++------------------------ test/e2e/run_working_dir_test.go | 24 +++++++++++- 2 files changed, 33 insertions(+), 50 deletions(-) diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 606f279077..577b29749a 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -847,51 +847,6 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc return g.Config, cleanupFunc, nil } -// isWorkDirSymlink returns true if resolved workdir is symlink or a chain of symlinks, -// and final resolved target is present either on volume, mount or inside of container -// otherwise it returns false. Following function is meant for internal use only and -// can change at any point of time. -func (c *Container) isWorkDirSymlink(resolvedPath string) bool { - // We cannot create workdir since explicit --workdir is - // set in config but workdir could also be a symlink. - // If it's a symlink, check if the resolved target is present in the container. - // If so, that's a valid use case: return nil. - - maxSymLinks := 0 - // Linux only supports a chain of 40 links. - // Reference: https://github.com/torvalds/linux/blob/master/include/linux/namei.h#L13 - for maxSymLinks <= 40 { - resolvedSymlink, err := os.Readlink(resolvedPath) - if err != nil { - // End sym-link resolution loop. - break - } - if resolvedSymlink != "" { - _, resolvedSymlinkWorkdir, _, err := c.resolvePath(c.state.Mountpoint, resolvedSymlink) - if isPathOnVolume(c, resolvedSymlinkWorkdir) || isPathOnMount(c, resolvedSymlinkWorkdir) { - // Resolved symlink exists on external volume or mount - return true - } - if err != nil { - // Could not resolve path so end sym-link resolution loop. - break - } - if resolvedSymlinkWorkdir != "" { - resolvedPath = resolvedSymlinkWorkdir - err := fileutils.Exists(resolvedSymlinkWorkdir) - if err == nil { - // Symlink resolved successfully and resolved path exists on container, - // this is a valid use-case so return nil. - logrus.Debugf("Workdir is a symlink with target to %q and resolved symlink exists on container", resolvedSymlink) - return true - } - } - } - maxSymLinks++ - } - return false -} - // resolveWorkDir resolves the container's workdir and, depending on the // configuration, will create it, or error out if it does not exist. // Note that the container must be mounted before. @@ -906,7 +861,7 @@ func (c *Container) resolveWorkDir() error { return nil } - _, resolvedWorkdir, _, err := c.resolvePath(c.state.Mountpoint, workdir) + resolvedWorkdir, err := securejoin.SecureJoin(c.state.Mountpoint, workdir) if err != nil { return err } @@ -923,11 +878,17 @@ func (c *Container) resolveWorkDir() error { // No need to create it (e.g., `--workdir=/foo`), so let's make sure // the path exists on the container. if errors.Is(err, os.ErrNotExist) { - // If resolved Workdir path gets marked as a valid symlink, - // return nil cause this is valid use-case. - if c.isWorkDirSymlink(resolvedWorkdir) { + // Check if path is a symlink, securejoin resolves and follows the links + // so the path will be different from the normal join if it is one. + if resolvedWorkdir != filepath.Join(c.state.Mountpoint, workdir) { + // Path must be a symlink to non existing directory. + // It could point to mounts that are only created later so that make + // an assumption here and let's just continue and let the oci runtime + // do its job. return nil } + // If they are the same we know there is no symlink/relative path involved. + // We can return a nicer error message without having to go through the OCI runtime. return fmt.Errorf("workdir %q does not exist on container %s", workdir, c.ID()) } // This might be a serious error (e.g., permission), so diff --git a/test/e2e/run_working_dir_test.go b/test/e2e/run_working_dir_test.go index ac7f2b7a10..76ce78ab47 100644 --- a/test/e2e/run_working_dir_test.go +++ b/test/e2e/run_working_dir_test.go @@ -10,6 +10,7 @@ import ( . "github.com/containers/podman/v6/test/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/types" ) var _ = Describe("Podman run", func() { @@ -59,11 +60,32 @@ WORKDIR /etc/foobar`, ALPINE) It("podman run on an image with a symlinked workdir", func() { dockerfile := fmt.Sprintf(`FROM %s -RUN mkdir /A && ln -s /A /B +RUN mkdir /A && ln -s /A /B && ln -s /vol/test /link WORKDIR /B`, ALPINE) podmanTest.BuildImage(dockerfile, "test", "false") session := podmanTest.PodmanExitCleanly("run", "test", "pwd") Expect(session.OutputToString()).To(Equal("/A")) + + path := filepath.Join(podmanTest.TempDir, "test") + err := os.Mkdir(path, 0o755) + Expect(err).ToNot(HaveOccurred()) + + session = podmanTest.PodmanExitCleanly("run", "--workdir=/link", "--volume", podmanTest.TempDir+":/vol", "test", "pwd") + Expect(session.OutputToString()).To(Equal("/vol/test")) + + // This will fail in the runtime since the target doesn't exists + session = podmanTest.Podman([]string{"run", "--workdir=/link", "test", "pwd"}) + session.WaitWithDefaultTimeout() + var matcher types.GomegaMatcher + if filepath.Base(podmanTest.OCIRuntime) == "crun" { + matcher = ExitWithError(127, "chdir to `/link`: No such file or directory") + } else if filepath.Base(podmanTest.OCIRuntime) == "runc" { + matcher = ExitWithError(126, "mkdir /link: file exists") + } else { + // unknown runtime, just check it failed + matcher = Not(ExitCleanly()) + } + Expect(session).Should(matcher) }) })