fix(exec): honour DetachKeys in remote exec and fall back to system default

Use a *string field for DetachKeys in ExecCreateConfig so the JSON decoder
can distinguish an absent field (nil) from an explicitly-empty one ("").
Previously both mapped to the same empty string, causing DetachKeys:""
to be silently ignored.

When DetachKeys is absent from the request, store the system default from
runtimeConfig.Engine.DetachKeys so that exec inspect returns a meaningful
value rather than an empty string.

Adds unit tests for the JSON decoding and API tests covering all three
scenarios: no DetachKeys, empty DetachKeys, and a non-empty value.

Fixes: #28307
Signed-off-by: Devesh B <98201065+DeveshB-1@users.noreply.github.com>

Author: DeveshB-1

pkg/api/handlers/compat/exec.go

@@ -46,8 +46,9 @@ func ExecCreateHandler(w http.ResponseWriter, r *http.Request) {
 	libpodConfig.AttachStdin = input.AttachStdin
 	libpodConfig.AttachStderr = input.AttachStderr
 	libpodConfig.AttachStdout = input.AttachStdout
-	if input.DetachKeys != "" {
-		libpodConfig.DetachKeys = &input.DetachKeys
+	// DetachKeys is a *string so nil means "not provided" and "" means "disable detach".
+	if input.DetachKeys != nil {
+		libpodConfig.DetachKeys = input.DetachKeys
 	}
 	libpodConfig.Environment = make(map[string]string)
 	for _, envStr := range input.Env {
@@ -81,6 +82,12 @@ func ExecCreateHandler(w http.ResponseWriter, r *http.Request) {
 	}
 	libpodConfig.ExitCommand = exitCommandArgs
 
+	// When DetachKeys was not provided, store the system default so that
+	// exec inspect returns a meaningful value rather than an empty string.
+	if libpodConfig.DetachKeys == nil {
+		libpodConfig.DetachKeys = &runtimeConfig.Engine.DetachKeys
+	}
+
 	// Run the exit command after 5 minutes, to mimic Docker's exec cleanup
 	// behavior.
 	libpodConfig.ExitCommandDelay = runtimeConfig.Engine.ExitCommandDelay

pkg/api/handlers/exec_create_config_test.go

@@ -0,0 +1,65 @@
+package handlers_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/containers/podman/v6/pkg/api/handlers"
+)
+
+// TestExecCreateConfigDetachKeys verifies that DetachKeys *string correctly
+// distinguishes "field absent" (nil) from "explicitly set to empty string" (*"").
+// This is the regression test for the bug where DetachKeys:"" was silently
+// ignored because the zero value of string is indistinguishable from absent.
+func TestExecCreateConfigDetachKeys(t *testing.T) {
+	tests := []struct {
+		name    string
+		body    string
+		wantNil bool
+		wantVal string
+	}{
+		{
+			name:    "empty body - DetachKeys should be nil",
+			body:    `{}`,
+			wantNil: true,
+		},
+		{
+			name:    "field absent - DetachKeys should be nil",
+			body:    `{"AttachStdout":true}`,
+			wantNil: true,
+		},
+		{
+			name:    "field set to empty string - DetachKeys should be non-nil empty",
+			body:    `{"DetachKeys":""}`,
+			wantNil: false,
+			wantVal: "",
+		},
+		{
+			name:    "field set to value - DetachKeys should be non-nil with value",
+			body:    `{"DetachKeys":"ctrl-p,ctrl-q"}`,
+			wantNil: false,
+			wantVal: "ctrl-p,ctrl-q",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var cfg handlers.ExecCreateConfig
+			if err := json.Unmarshal([]byte(tt.body), &cfg); err != nil {
+				t.Fatalf("unmarshal: %v", err)
+			}
+			if tt.wantNil {
+				if cfg.DetachKeys != nil {
+					t.Errorf("expected DetachKeys to be nil, got %q", *cfg.DetachKeys)
+				}
+			} else {
+				if cfg.DetachKeys == nil {
+					t.Fatal("expected DetachKeys to be non-nil")
+				}
+				if *cfg.DetachKeys != tt.wantVal {
+					t.Errorf("DetachKeys = %q, want %q", *cfg.DetachKeys, tt.wantVal)
+				}
+			}
+		})
+	}
+}

pkg/api/handlers/types.go

@@ -243,6 +243,9 @@ type HistoryResponse struct {
 
 type ExecCreateConfig struct {
 	dockerContainer.ExecCreateRequest
+	// DetachKeys shadows the embedded string field so the JSON decoder
+	// can distinguish "field absent" (nil) from "explicitly set to empty" (*"").
+	DetachKeys *string `json:"DetachKeys"`
 }
 
 type ExecStartConfig struct {

pkg/domain/infra/tunnel/containers.go

@@ -614,7 +614,7 @@ func makeExecConfig(options entities.ExecOptions) *handlers.ExecCreateConfig {
 	createConfig.AttachStdin = options.Interactive
 	createConfig.AttachStdout = true
 	createConfig.AttachStderr = true
-	createConfig.DetachKeys = options.DetachKeys
+	createConfig.DetachKeys = &options.DetachKeys
 	createConfig.Env = env
 	createConfig.WorkingDir = options.WorkDir
 	createConfig.Cmd = options.Cmd

test/apiv2/20-containers.at

@@ -913,4 +913,39 @@ t POST containers/create?platform=linux/amd64 \
 t POST containers/create?platform=linux/aarch64 \
   Image=$IMAGE \
   404
+
+# 28405: exec create must honour DetachKeys:"" (explicitly disable detach).
+# Previously the JSON decoder could not distinguish an absent field from an
+# explicitly-empty one, so DetachKeys:"" was silently ignored.
+podman run -d --name exec-detach-keys-test $IMAGE top
+
+# Without DetachKeys the system default must be preserved (non-empty).
+t POST containers/exec-detach-keys-test/exec \
+  Cmd='["true"]' \
+  201 \
+  .Id~[0-9a-f]\\{64\\}
+eid=$(jq -r '.Id' <<<"$output")
+t GET exec/$eid/json 200 .DetachKeys
+
+# Explicitly empty DetachKeys disables detach - exec create must accept it.
+t POST containers/exec-detach-keys-test/exec \
+  Cmd='["true"]' \
+  DetachKeys='' \
+  201 \
+  .Id~[0-9a-f]\\{64\\}
+eid=$(jq -r '.Id' <<<"$output")
+t GET exec/$eid/json 200 .DetachKeys=''
+
+# A non-empty DetachKeys must be stored and returned by inspect.
+t POST containers/exec-detach-keys-test/exec \
+  Cmd='["true"]' \
+  DetachKeys='ctrl-c' \
+  201 \
+  .Id~[0-9a-f]\\{64\\}
+eid=$(jq -r '.Id' <<<"$output")
+t GET exec/$eid/json 200 .DetachKeys='ctrl-c'
+
+podman rm -f exec-detach-keys-test
+
+
 podman rmi -f $IMAGE