Handle WORKDIR with dangling target

When the WORKDIR is a symlink and the target directory does not exist, RUN
commands would fail.  If the target doesn't exist, create the directory in
the container.  This also converts the code to use SecureJoin so that if
a symlink is in the middle of a filepath, it will be converted correctly.

Fixes: #1569

Tip of the hat to @karelyatin for the initial fix

Signed-off-by: TomSweeneyRedHat <[email protected]>

Closes: #1576
Approved by: rhatdan

imagebuildah/build.go

@@ -27,6 +27,7 @@ import (
 	"github.com/containers/image/types"
 	"github.com/containers/storage"
 	"github.com/containers/storage/pkg/archive"
+	"github.com/cyphar/filepath-securejoin"
 	docker "github.com/fsouza/go-dockerclient"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/runtime-spec/specs-go"
@@ -480,22 +481,22 @@ func (s *StageExecutor) volumeCacheRestore() error {
 }
 
 // Copy copies data into the working tree.  The "Download" field is how
-// imagebuilder tells us the instruction was "ADD" and not "COPY".
+// imagebuilder tells us the instruction was "ADD" and not "COPY"
 func (s *StageExecutor) Copy(excludes []string, copies ...imagebuilder.Copy) error {
 	for _, copy := range copies {
-		// If the file exists, check to see if it's a symlink.
-		// If it is a symlink, convert to it's target otherwise
-		// the symlink will be overwritten.
-		fileDest, _ := os.Lstat(filepath.Join(s.mountPoint, copy.Dest))
-		if fileDest != nil {
-			if fileDest.Mode()&os.ModeSymlink != 0 {
-				if symLink, err := resolveSymlink(s.mountPoint, copy.Dest); err == nil {
-					copy.Dest = symLink
-				} else {
-					return errors.Wrapf(err, "error reading symbolic link to %q", copy.Dest)
-				}
-			}
+		// Check the file and see if part of it is a symlink.
+		// Convert it to the target if so.  To be ultrasafe
+		// do the same for the mountpoint.
+		secureMountPoint, err := securejoin.SecureJoin("", s.mountPoint)
+		finalPath, err := securejoin.SecureJoin(secureMountPoint, copy.Dest)
+		if err != nil {
+			return errors.Wrapf(err, "error resolving symlinks for copy destination %s", copy.Dest)
+		}
+		if !strings.HasPrefix(finalPath, secureMountPoint) {
+			return errors.Wrapf(err, "error resolving copy destination %s", copy.Dest)
 		}
+		copy.Dest = strings.TrimPrefix(finalPath, secureMountPoint)
+
 		if copy.Download {
 			logrus.Debugf("ADD %#v, %#v", excludes, copy)
 		} else {
@@ -1819,8 +1820,8 @@ func (b *Executor) deleteSuccessfulIntermediateCtrs() error {
 }
 
 func (s *StageExecutor) EnsureContainerPath(path string) error {
-	targetPath := filepath.Join(s.mountPoint, path)
-	_, err := os.Lstat(targetPath)
+	targetPath, err := securejoin.SecureJoin(s.mountPoint, path)
+	_, err = os.Lstat(targetPath)
 	if err != nil && os.IsNotExist(err) {
 		err = os.MkdirAll(targetPath, 0755)
 	}

tests/bud.bats

@@ -1198,7 +1198,48 @@ load helpers
   run_buildah --debug=false run ${ctr} ls -alF /tempest
   expect_output --substring "/tempest -> /var/lib/tempest/"
 
-  run_buildah --debug=false run ${ctr} ls -alF /etc/notareal.conf 
+  run_buildah --debug=false run ${ctr} ls -alF /etc/notareal.conf
+  expect_output --substring "\-rw\-rw\-r\-\-"
+}
+
+@test "bud WORKDIR isa symlink no target dir" {
+  target=alpine-image
+  ctr=alpine-ctr
+  run_buildah --debug=false bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f Dockerfile-2 ${TESTSDIR}/bud/workdir-symlink
+  expect_output --substring "STEP 2: RUN ln -sf "
+
+  run_buildah --debug=false from --signature-policy ${TESTSDIR}/policy.json --name=${ctr} ${target}
+  expect_output --substring ${ctr}
+
+  run_buildah --debug=false run ${ctr} ls -alF /tempest
+  expect_output --substring "/tempest -> /var/lib/tempest/"
+
+  run_buildah --debug=false run ${ctr} ls /tempest
+  expect_output --substring "Dockerfile-2"
+
+  run_buildah --debug=false run ${ctr} ls -alF /etc/notareal.conf
+  expect_output --substring "\-rw\-rw\-r\-\-"
+}
+
+@test "bud WORKDIR isa symlink no target dir and follow on dir" {
+  target=alpine-image
+  ctr=alpine-ctr
+  run_buildah --debug=false bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f Dockerfile-3 ${TESTSDIR}/bud/workdir-symlink
+  expect_output --substring "STEP 2: RUN ln -sf "
+
+  run_buildah --debug=false from --signature-policy ${TESTSDIR}/policy.json --name=${ctr} ${target}
+  expect_output --substring ${ctr}
+
+  run_buildah --debug=false run ${ctr} ls -alF /tempest
+  expect_output --substring "/tempest -> /var/lib/tempest/"
+
+  run_buildah --debug=false run ${ctr} ls /tempest
+  expect_output --substring "Dockerfile-3"
+
+  run_buildah --debug=false run ${ctr} ls /tempest/lowerdir
+  expect_output --substring "Dockerfile-3"
+
+  run_buildah --debug=false run ${ctr} ls -alF /etc/notareal.conf
   expect_output --substring "\-rw\-rw\-r\-\-"
 }
 

tests/bud/workdir-symlink/Dockerfile-2

@@ -0,0 +1,7 @@
+# No directory created for the target of the symlink
+FROM alpine
+RUN ln -sf /var/lib/tempest /tempest
+WORKDIR /tempest
+RUN touch /etc/notareal.conf
+RUN chmod 664 /etc/notareal.conf
+COPY Dockerfile-2 ./Dockerfile-2

tests/bud/workdir-symlink/Dockerfile-3

@@ -0,0 +1,10 @@
+# No directory created for the target of the symlink
+FROM alpine
+RUN ln -sf /var/lib/tempest /tempest
+WORKDIR /tempest/lowerdir
+RUN touch /etc/notareal.conf
+RUN chmod 664 /etc/notareal.conf
+RUN mkdir -p /tempest/lowerdir
+COPY Dockerfile-3 ./Dockerfile-3
+COPY Dockerfile-3 /tempest/Dockerfile-3
+COPY Dockerfile-3 /tempest/lowerdir/Dockerfile-3

vendor.conf

@@ -4,6 +4,7 @@ github.com/BurntSushi/toml v0.2.0
 github.com/containerd/continuity 004b46473808b3e7a4a3049c20e4376c91eb966d
 github.com/containernetworking/cni v0.7.0-rc2
 github.com/containers/image 9467ac9cfd92c545aa389f22f27e552de053c0f2
+github.com/cyphar/filepath-securejoin v0.2.1
 github.com/vbauerster/mpb v3.3.4
 github.com/mattn/go-isatty v0.0.4
 github.com/VividCortex/ewma v1.1.1