diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index acaa76ef3a..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 @@ -935,6 +896,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..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() { @@ -56,4 +57,35 @@ 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 && 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) + }) })