From 2d715405182f67937c152d4cd74b282a2e6ca786 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <gscrivan@redhat.com>
Date: Thu, 30 Jul 2020 22:59:45 +0200
Subject: volumes: do not recurse when chowning

keep the file ownership when chowning and honor the user namespace
mappings.

Closes: https://github.com/containers/podman/issues/7130

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

<MH: Fixed conflicts from cherry pick>

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
---
 libpod/container_internal.go |  35 ++++++-----
 test/system/070-build.bats   | 134 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 156 insertions(+), 13 deletions(-)

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index e277a88c5..675311461 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -1521,9 +1521,6 @@ func (c *Container) chownVolume(volumeName string) error {
 		return errors.Wrapf(err, "error retrieving named volume %s for container %s", volumeName, c.ID())
 	}
 
-	uid := int(c.config.Spec.Process.User.UID)
-	gid := int(c.config.Spec.Process.User.GID)
-
 	vol.lock.Lock()
 	defer vol.lock.Unlock()
 
@@ -1534,22 +1531,34 @@ func (c *Container) chownVolume(volumeName string) error {
 
 	if vol.state.NeedsChown {
 		vol.state.NeedsChown = false
+
+		uid := int(c.config.Spec.Process.User.UID)
+		gid := int(c.config.Spec.Process.User.GID)
+
+		if c.config.IDMappings.UIDMap != nil {
+			p := idtools.IDPair{
+				UID: uid,
+				GID: gid,
+			}
+			mappings := idtools.NewIDMappingsFromMaps(c.config.IDMappings.UIDMap, c.config.IDMappings.GIDMap)
+			newPair, err := mappings.ToHost(p)
+			if err != nil {
+				return errors.Wrapf(err, "error mapping user %d:%d", uid, gid)
+			}
+			uid = newPair.UID
+			gid = newPair.GID
+		}
+
 		vol.state.UIDChowned = uid
 		vol.state.GIDChowned = gid
 
 		if err := vol.save(); err != nil {
 			return err
 		}
-		err := filepath.Walk(vol.MountPoint(), func(path string, info os.FileInfo, err error) error {
-			if err != nil {
-				return err
-			}
-			if err := os.Lchown(path, uid, gid); err != nil {
-				return err
-			}
-			return nil
-		})
-		if err != nil {
+
+		mountPoint := vol.MountPoint()
+
+		if err := os.Lchown(mountPoint, uid, gid); err != nil {
 			return err
 		}
 	}
diff --git a/test/system/070-build.bats b/test/system/070-build.bats
index 7d6660270..6879b956f 100644
--- a/test/system/070-build.bats
+++ b/test/system/070-build.bats
@@ -98,6 +98,140 @@ EOF
     is "$output" ".*error building at STEP .*: source can't be a URL for COPY"
 }
 
+@test "podman build - workdir, cmd, env, label" {
+    tmpdir=$PODMAN_TMPDIR/build-test
+    mkdir -p $tmpdir
+
+    # Random workdir, and multiple random strings to verify command & env
+    workdir=/$(random_string 10)
+    s_echo=$(random_string 15)
+    s_env1=$(random_string 20)
+    s_env2=$(random_string 25)
+    s_env3=$(random_string 30)
+    s_env4=$(random_string 40)
+
+    # Label name: make sure it begins with a letter! jq barfs if you
+    # try to ask it for '.foo.<N>xyz', i.e. any string beginning with digit
+    label_name=l$(random_string 8)
+    label_value=$(random_string 12)
+
+    # Command to run on container startup with no args
+    cat >$tmpdir/mycmd <<EOF
+#!/bin/sh
+PATH=/usr/bin:/bin
+pwd
+echo "\$1"
+printenv | grep MYENV | sort | sed -e 's/^MYENV.=//'
+EOF
+
+    # For overridding with --env-file
+    cat >$PODMAN_TMPDIR/env-file <<EOF
+MYENV3=$s_env3
+http_proxy=http-proxy-in-env-file
+https_proxy=https-proxy-in-env-file
+EOF
+
+    cat >$tmpdir/Containerfile <<EOF
+FROM $IMAGE
+LABEL $label_name=$label_value
+RUN mkdir $workdir
+WORKDIR $workdir
+
+# Test for #7094 - chowning of invalid symlinks
+RUN mkdir -p /a/b/c
+RUN ln -s /no/such/nonesuch /a/b/c/badsymlink
+RUN ln -s /bin/mydefaultcmd /a/b/c/goodsymlink
+RUN touch /a/b/c/myfile
+RUN chown -h 1:2 /a/b/c/badsymlink /a/b/c/goodsymlink && chown -h 4:5 /a/b/c/myfile
+VOLUME /a/b/c
+
+# Test for environment passing and override
+ENV MYENV1=$s_env1
+ENV MYENV2 this-should-be-overridden-by-env-host
+ENV MYENV3 this-should-be-overridden-by-env-file
+ENV MYENV4 this-should-be-overridden-by-cmdline
+ENV http_proxy http-proxy-in-image
+ENV ftp_proxy  ftp-proxy-in-image
+ADD mycmd /bin/mydefaultcmd
+RUN chmod 755 /bin/mydefaultcmd
+RUN chown 2:3 /bin/mydefaultcmd
+CMD ["/bin/mydefaultcmd","$s_echo"]
+EOF
+
+    # cd to the dir, so we test relative paths (important for podman-remote)
+    cd $PODMAN_TMPDIR
+    run_podman build -t build_test -f build-test/Containerfile build-test
+
+    # Run without args - should run the above script. Verify its output.
+    export MYENV2="$s_env2"
+    export MYENV3="env-file-should-override-env-host!"
+    run_podman run --rm \
+               --env-file=$PODMAN_TMPDIR/env-file \
+               --env-host \
+               -e MYENV4="$s_env4" \
+               build_test
+    is "${lines[0]}" "$workdir" "container default command: pwd"
+    is "${lines[1]}" "$s_echo"  "container default command: output from echo"
+    is "${lines[2]}" "$s_env1"  "container default command: env1"
+    is "${lines[3]}" "$s_env2"  "container default command: env2"
+    is "${lines[4]}" "$s_env3"  "container default command: env3 (from envfile)"
+    is "${lines[5]}" "$s_env4"  "container default command: env4 (from cmdline)"
+
+    # Proxies - environment should override container, but not env-file
+    http_proxy=http-proxy-from-env  ftp_proxy=ftp-proxy-from-env \
+              run_podman run --rm --env-file=$PODMAN_TMPDIR/env-file \
+              build_test \
+              printenv http_proxy https_proxy ftp_proxy
+    is "${lines[0]}" "http-proxy-in-env-file"  "env-file overrides env"
+    is "${lines[1]}" "https-proxy-in-env-file" "env-file sets proxy var"
+    is "${lines[2]}" "ftp-proxy-from-env"      "ftp-proxy is passed through"
+
+    # test that workdir is set for command-line commands also
+    run_podman run --rm build_test pwd
+    is "$output" "$workdir" "pwd command in container"
+
+    # Confirm that 'podman inspect' shows the expected values
+    # FIXME: can we rely on .Env[0] being PATH, and the rest being in order??
+    run_podman image inspect build_test
+    tests="
+Env[1]             | MYENV1=$s_env1
+Env[2]             | MYENV2=this-should-be-overridden-by-env-host
+Env[3]             | MYENV3=this-should-be-overridden-by-env-file
+Env[4]             | MYENV4=this-should-be-overridden-by-cmdline
+Cmd[0]             | /bin/mydefaultcmd
+Cmd[1]             | $s_echo
+WorkingDir         | $workdir
+Labels.$label_name | $label_value
+"
+
+    parse_table "$tests" | while read field expect; do
+        actual=$(jq -r ".[0].Config.$field" <<<"$output")
+        dprint "# actual=<$actual> expect=<$expect}>"
+        is "$actual" "$expect" "jq .Config.$field"
+    done
+
+    # Bad symlink in volume. Prior to #7094, well, we wouldn't actually
+    # get here because any 'podman run' on a volume that had symlinks,
+    # be they dangling or valid, would barf with
+    #    Error: chown <mountpath>/_data/symlink: ENOENT
+    run_podman run --rm build_test stat -c'%u:%g:%N' /a/b/c/badsymlink
+    is "$output" "1:2:'/a/b/c/badsymlink' -> '/no/such/nonesuch'" \
+       "bad symlink to nonexistent file is chowned and preserved"
+
+    run_podman run --rm build_test stat -c'%u:%g:%N' /a/b/c/goodsymlink
+    is "$output" "1:2:'/a/b/c/goodsymlink' -> '/bin/mydefaultcmd'" \
+       "good symlink to existing file is chowned and preserved"
+
+    run_podman run --rm build_test stat -c'%u:%g' /bin/mydefaultcmd
+    is "$output" "2:3" "target of symlink is not chowned"
+
+    run_podman run --rm build_test stat -c'%u:%g:%N' /a/b/c/myfile
+    is "$output" "4:5:/a/b/c/myfile" "file in volume is chowned"
+
+    # Clean up
+    run_podman rmi -f build_test
+}
+
 @test "podman build - stdin test" {
     if is_remote && is_rootless; then
         skip "unreliable with podman-remote and rootless; #2972"
-- 
cgit v1.2.3-54-g00ecf