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 <pholzing@redhat.com>

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

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)
 	})
 })