From b72bb11629de9e6d10bd1ad52a7e9d0693359709 Mon Sep 17 00:00:00 2001
From: Daniel J Walsh <dwalsh@redhat.com>
Date: Wed, 18 Oct 2023 10:21:30 -0400
Subject: [PATCH] Add TERM iff TERM not defined in container when podman exec
 -t

Fixes: https://github.com/containers/podman/issues/20334

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
---
 libpod/container.go                |  8 ++++++
 pkg/api/handlers/compat/exec.go    |  5 ++++
 pkg/domain/infra/abi/containers.go |  4 +++
 pkg/util/utils.go                  | 17 +++++++++++++
 test/system/075-exec.bats          | 41 ++++++++++++++++++++++++++++++
 5 files changed, 75 insertions(+)

diff --git a/libpod/container.go b/libpod/container.go
index f13ea55b4a..7c54a1519b 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -713,6 +713,14 @@ func (c *Container) LinuxResources() *spec.LinuxResources {
 	return nil
 }
 
+// Env returns the default environment variables defined for the container
+func (c *Container) Env() []string {
+	if c.config.Spec != nil && c.config.Spec.Process != nil {
+		return c.config.Spec.Process.Env
+	}
+	return nil
+}
+
 // State Accessors
 // Require locking
 
diff --git a/pkg/api/handlers/compat/exec.go b/pkg/api/handlers/compat/exec.go
index 17f199a8b3..0a5d075356 100644
--- a/pkg/api/handlers/compat/exec.go
+++ b/pkg/api/handlers/compat/exec.go
@@ -16,6 +16,7 @@ import (
 	api "github.com/containers/podman/v4/pkg/api/types"
 	"github.com/containers/podman/v4/pkg/domain/entities"
 	"github.com/containers/podman/v4/pkg/specgenutil"
+	"github.com/containers/podman/v4/pkg/util"
 	"github.com/gorilla/mux"
 	"github.com/sirupsen/logrus"
 )
@@ -59,6 +60,10 @@ func ExecCreateHandler(w http.ResponseWriter, r *http.Request) {
 	libpodConfig.Privileged = input.Privileged
 	libpodConfig.User = input.User
 
+	if input.Tty {
+		util.ExecAddTERM(ctr.Env(), libpodConfig.Environment)
+	}
+
 	// Make our exit command
 	storageConfig := runtime.StorageConfig()
 	runtimeConfig, err := runtime.GetConfig()
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index 2d14981671..9cdf212924 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/pkg/domain/infra/abi/containers.go
@@ -867,6 +867,10 @@ func (ic *ContainerEngine) ContainerExec(ctx context.Context, nameOrID string, o
 	}
 	ctr := containers[0]
 
+	if options.Tty {
+		util.ExecAddTERM(ctr.Env(), options.Envs)
+	}
+
 	execConfig, err := makeExecConfig(options, ic.Libpod)
 	if err != nil {
 		return ec, err
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index ec3fc67541..679110df2f 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -1219,3 +1219,20 @@ func ConvertTimeout(timeout int) uint {
 	}
 	return uint(timeout)
 }
+
+// ExecAddTERM when container does not have a TERM environment variable and
+// caller wants a tty, then leak the existing TERM environment into
+// the container.
+func ExecAddTERM(existingEnv []string, execEnvs map[string]string) {
+	if _, ok := execEnvs["TERM"]; ok {
+		return
+	}
+
+	for _, val := range existingEnv {
+		if strings.HasPrefix(val, "TERM=") {
+			return
+		}
+	}
+
+	execEnvs["TERM"] = "xterm"
+}
diff --git a/test/system/075-exec.bats b/test/system/075-exec.bats
index 6b16723b34..7e21dd3a03 100644
--- a/test/system/075-exec.bats
+++ b/test/system/075-exec.bats
@@ -166,6 +166,47 @@ load helpers
     run_podman rm -f -t0 $cid
 }
 
+@test "podman exec --tty" {
+    # Outer loops: different variations on the RUN container
+    for run_opt_t in "" "-t"; do
+        for run_term_env in "" "explicit_RUN_term"; do
+            local run_opt_env=
+            if [[ -n "$run_term_env" ]]; then
+                run_opt_env="--env=TERM=$run_term_env"
+            fi
+            run_podman run -d $run_opt_t $run_opt_env --name test $IMAGE top
+
+            # Inner loops: different variations on EXEC
+            for exec_opt_t in "" "-t"; do
+                for exec_term_env in "" "explicit_EXEC_term"; do
+                    # What to expect.
+                    local expected=
+                    # if -t is set anywhere, either run or exec, go with xterm
+                    if [[ -n "$run_opt_t$exec_opt_t" ]]; then
+                        expected="xterm"
+                    fi
+                    # ...unless overridden by explicit --env
+                    if [[ -n "$run_term_env$exec_term_env" ]]; then
+                        # (exec overrides run)
+                        expected="${exec_term_env:-$run_term_env}"
+                    fi
+
+                    local exec_opt_env=
+                    if [[ -n "$exec_term_env" ]]; then
+                        exec_opt_env="--env=TERM=$exec_term_env"
+                    fi
+
+                    local desc="run $run_opt_t $run_opt_env, exec $exec_opt_t $exec_opt_env"
+                    TERM=exec-term run_podman exec $exec_opt_t $exec_opt_env test sh -c 'echo -n $TERM'
+                    assert "$output" = "$expected" "$desc"
+                done
+            done
+
+            run_podman rm -f -t0 test
+        done
+    done
+}
+
 @test "podman exec - does not leak session IDs on invalid command" {
     skip_if_remote "FIXME FIXME FIXME: this should work on remote, but does not"
     run_podman run -d $IMAGE top