diff --git a/libcontainer/env.go b/libcontainer/env.go index fe1abd500..71b2f69d0 100644 --- a/libcontainer/env.go +++ b/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. diff --git a/libcontainer/env_test.go b/libcontainer/env_test.go index c91747167..18dc52e88 100644 --- a/libcontainer/env_test.go +++ b/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 { diff --git a/tests/integration/env.bats b/tests/integration/env.bats new file mode 100644 index 000000000..cb984942e --- /dev/null +++ b/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 ] +}