Merge pull request #18492 from daw1012345/main

Ensure the consistent setting of the HOME env variable on container start

Author: openshift-merge-robot

libpod/container_internal_common.go

@@ -459,15 +459,9 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc
 		g.AddMount(overlayMount)
 	}
 
-	hasHomeSet := false
-	for _, s := range c.config.Spec.Process.Env {
-		if strings.HasPrefix(s, "HOME=") {
-			hasHomeSet = true
-			break
-		}
-	}
-	if !hasHomeSet && execUser.Home != "" {
-		c.config.Spec.Process.Env = append(c.config.Spec.Process.Env, fmt.Sprintf("HOME=%s", execUser.Home))
+	err = c.setHomeEnvIfNeeded()
+	if err != nil {
+		return nil, nil, err
 	}
 
 	if c.config.User != "" {
@@ -2432,6 +2426,41 @@ func (c *Container) generateCurrentUserPasswdEntry() (string, int, int, error) {
 	return pwd, uid, rootless.GetRootlessGID(), nil
 }
 
+// Sets the HOME env. variable with precedence: existing home env. variable, execUser home
+func (c *Container) setHomeEnvIfNeeded() error {
+	getExecUserHome := func() (string, error) {
+		overrides := c.getUserOverrides()
+		execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, c.config.User, overrides)
+		if err != nil {
+			if cutil.StringInSlice(c.config.User, c.config.HostUsers) {
+				execUser, err = lookupHostUser(c.config.User)
+			}
+
+			if err != nil {
+				return "", err
+			}
+		}
+
+		return execUser.Home, nil
+	}
+
+	// Ensure HOME is not already set in Env
+	home := ""
+	for _, s := range c.config.Spec.Process.Env {
+		if strings.HasPrefix(s, "HOME=") {
+			return nil
+		}
+	}
+
+	home, err := getExecUserHome()
+	if err != nil {
+		return err
+	}
+
+	c.config.Spec.Process.Env = append(c.config.Spec.Process.Env, fmt.Sprintf("HOME=%s", home))
+	return nil
+}
+
 func (c *Container) userPasswdEntry(u *user.User) (string, error) {
 	// Look up the user to see if it exists in the container image.
 	_, err := lookup.GetUser(c.state.Mountpoint, u.Username)
@@ -2464,17 +2493,7 @@ func (c *Container) userPasswdEntry(u *user.User) (string, error) {
 			}
 		}
 	}
-	// Set HOME environment if not already set
-	hasHomeSet := false
-	for _, s := range c.config.Spec.Process.Env {
-		if strings.HasPrefix(s, "HOME=") {
-			hasHomeSet = true
-			break
-		}
-	}
-	if !hasHomeSet {
-		c.config.Spec.Process.Env = append(c.config.Spec.Process.Env, fmt.Sprintf("HOME=%s", homeDir))
-	}
+
 	if c.config.PasswdEntry != "" {
 		return c.passwdEntry(u.Username, u.Uid, u.Gid, u.Name, homeDir), nil
 	}

test/e2e/start_test.go

@@ -249,4 +249,89 @@ var _ = Describe("Podman start", func() {
 		Expect(session1).Should(Exit(0))
 		Expect(session1.OutputToString()).To(BeEquivalentTo(cid2))
 	})
+
+	It("podman start container does not set HOME to home of caller", func() {
+		home, err := os.UserHomeDir()
+		Expect(err).ToNot(HaveOccurred())
+		session := podmanTest.Podman([]string{"create", "--userns", "keep-id", "--user", "bin:bin", "--volume", fmt.Sprintf("%s:%s:ro", home, home), ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		cid := session.OutputToString()
+
+		session = podmanTest.Podman([]string{"start", cid})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"inspect", cid, "--format", "{{.Config.Env}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		env := session.OutputToString()
+		Expect(env).To(ContainSubstring("HOME"))
+		Expect(env).ToNot(ContainSubstring(fmt.Sprintf("HOME=%s", home)))
+
+		session = podmanTest.Podman([]string{"restart", cid})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"inspect", cid, "--format", "{{.Config.Env}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		env = session.OutputToString()
+		Expect(env).To(ContainSubstring("HOME"))
+		Expect(env).ToNot(ContainSubstring(fmt.Sprintf("HOME=%s", home)))
+	})
+
+	It("podman start container sets HOME to home of execUser", func() {
+		session := podmanTest.Podman([]string{"create", "--userns", "keep-id", "--user", "bin:bin", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		cid := session.OutputToString()
+
+		session = podmanTest.Podman([]string{"start", cid})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"inspect", cid, "--format", "{{.Config.Env}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		env := session.OutputToString()
+		Expect(env).To(ContainSubstring("HOME=/bin"))
+
+		session = podmanTest.Podman([]string{"restart", cid})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"inspect", cid, "--format", "{{.Config.Env}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		env = session.OutputToString()
+		Expect(env).To(ContainSubstring("HOME=/bin"))
+	})
+
+	It("podman start container retains the HOME env if present", func() {
+		session := podmanTest.Podman([]string{"create", "--userns", "keep-id", "--user", "bin:bin", "--env=HOME=/env/is/respected", ALPINE, "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		cid := session.OutputToString()
+
+		session = podmanTest.Podman([]string{"start", cid})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"inspect", cid, "--format", "{{.Config.Env}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		env := session.OutputToString()
+		Expect(env).To(ContainSubstring("HOME=/env/is/respected"))
+
+		session = podmanTest.Podman([]string{"restart", cid})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+
+		session = podmanTest.Podman([]string{"inspect", cid, "--format", "{{.Config.Env}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		env = session.OutputToString()
+		Expect(env).To(ContainSubstring("HOME=/env/is/respected"))
+	})
 })