Merge pull request #4711 from rata/runc-1.3-env-fixes

[1.3]  Override HOME if its set to the empty string #4704

libcontainer/env.go

@@ -17,13 +17,16 @@ import (
 //     contains no \0 (nil) bytes;
 //   - removes any duplicates (keeping only the last value for each key)
 //   - sets PATH for the current process, if found in the list;
-//   - adds HOME to returned environment, if not found in the list.
+//   - adds HOME to returned environment, if not found in the list,
+//     or the value is empty.
 //
 // Returns the prepared environment.
 func prepareEnv(env []string, uid int) ([]string, error) {
 	if env == nil {
 		return nil, nil
 	}
+	var homeIsSet bool
+
 	// Deduplication code based on dedupEnv from Go 1.22 os/exec.
 
 	// Construct the output in reverse order, to preserve the
@@ -40,6 +43,7 @@ func prepareEnv(env []string, uid int) ([]string, error) {
 			return nil, errors.New("invalid environment variable: name cannot be empty")
 		}
 		key := kv[:i]
+		val := kv[i+1:]
 		if saw[key] { // Duplicate.
 			continue
 		}
@@ -49,17 +53,25 @@ func prepareEnv(env []string, uid int) ([]string, error) {
 		}
 		if key == "PATH" {
 			// Needs to be set as it is used for binary lookup.
-			if err := os.Setenv("PATH", kv[i+1:]); err != nil {
+			if err := os.Setenv("PATH", val); err != nil {
 				return nil, err
 			}
 		}
+		if key == "HOME" {
+			if val != "" {
+				homeIsSet = true
+			} else {
+				// Don't add empty HOME to the environment, we will override it later.
+				continue
+			}
+		}
 		out = append(out, kv)
 	}
 	// Restore the original order.
 	slices.Reverse(out)
 
 	// If HOME is not found in env, get it from container's /etc/passwd and add.
-	if !saw["HOME"] {
+	if !homeIsSet {
 		home, err := getUserHome(uid)
 		if err != nil {
 			// For backward compatibility, don't return an error, but merely log it.

libcontainer/env_test.go

@@ -37,6 +37,18 @@ func TestPrepareEnv(t *testing.T) {
 			env:     []string{"TERM=vt100", "HOME=/home/one", "HOME=/home/two", "TERM=xterm", "HOME=/home/three", "FOO=bar"},
 			wantEnv: []string{"TERM=xterm", "HOME=/home/three", "FOO=bar"},
 		},
+		{
+			env:     []string{"HOME=", "HOME=/foo"},
+			wantEnv: []string{"HOME=/foo"},
+		},
+		{
+			env:     []string{"HOME="},
+			wantEnv: []string{home},
+		},
+		{
+			env:     []string{"HOME=/foo", "HOME="},
+			wantEnv: []string{home},
+		},
 	}
 
 	for _, tc := range tests {

tests/integration/env.bats

@@ -0,0 +1,97 @@
+#!/usr/bin/env bats
+# shellcheck disable=SC2016
+# This disables the check for shell variables inside single quotes
+# We do that all the time in this file, as we are testing env vars.
+
+load helpers
+
+function setup() {
+	setup_busybox
+}
+
+function teardown() {
+	teardown_bundle
+}
+
+# Several of these tests are inspired on regressions caught by Docker, besides other tests that
+# check the behavior we already had in runc:
+# https://github.com/moby/moby/blob/843e51459f14ebc964d349eba1013dc8a3e9d52e/integration-cli/docker_cli_links_test.go#L197-L204
+# https://github.com/moby/moby/blob/843e51459f14ebc964d349eba1013dc8a3e9d52e/integration-cli/docker_cli_run_test.go#L822-L843
+#
+
+@test "non-empty HOME env is used" {
+	update_config ' .process.env += ["HOME=/override"]'
+	update_config ' .process.args += ["-c", "echo $HOME"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+	[[ "${lines[0]}" == '/override' ]]
+}
+
+@test "empty HOME env var is overridden" {
+	update_config ' .process.env += ["HOME="]'
+	update_config ' .process.args += ["-c", "echo $HOME"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+	[[ "${lines[0]}" == '/root' ]]
+}
+
+@test "empty HOME env var is overridden with multiple overrides" {
+	update_config ' .process.env += ["HOME=/override", "HOME="]'
+	update_config ' .process.args += ["-c", "echo $HOME"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+	[[ "${lines[0]}" == '/root' ]]
+}
+
+@test "env var HOME is set only once" {
+	# env will show if an env var is set multiple times.
+	update_config ' .process.args = ["env"]'
+	update_config ' .process.env = ["HOME=", "PATH=/usr/bin:/bin"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+
+	# There should be 2 words/env-vars: HOME and PATH.
+	[ "$(wc -w <<<"$output")" -eq 2 ]
+}
+
+@test "env var override is set only once" {
+	# env will show if an env var is set multiple times.
+	update_config ' .process.args = ["env"]'
+	update_config ' .process.env = ["ONE=two", "ONE=", "PATH=/usr/bin:/bin"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+
+	# There should be 3 words/env-vars: ONE, PATH and HOME.
+	[ "$(wc -w <<<"$output")" -eq 3 ]
+}
+
+@test "env var override" {
+	update_config ' .process.env += ["ONE=two", "ONE=three"]'
+	update_config ' .process.args += ["-c", "echo ONE=\"$ONE\""]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+	[[ "${lines[0]}" == "ONE=three" ]]
+}
+
+@test "env var with new-line is honored" {
+	update_config ' .process.env = ["NEW_LINE_ENV=\n", "PATH=/usr/bin:/bin"]'
+	update_config ' .process.args = ["env"]'
+
+	runc run test_busybox
+	[ "$status" -eq 0 ]
+
+	# There should be 4 lines
+	# NEW_LINE is a \n and when printed, it takes another line:
+	# 1. HOME=...
+	# 2. PATH=...
+	# 3. NEW_LINE_ENV=
+	# 4.
+	#
+	[ "$(wc -l <<<"$output")" -eq 4 ]
+}