From 180570abdc4c252147b39e4952d6de6fdd019ba0 Mon Sep 17 00:00:00 2001 From: shiavm006 Date: Thu, 6 Nov 2025 11:01:37 +0530 Subject: [PATCH] Fix remote exec ignoring --detach-keys option The remote client completely ignores the --detach-keys option for exec sessions. The ExecStartAndAttach() function in pkg/bindings/containers/attach.go hardcodes empty detach keys ([]byte{}) instead of using the detach keys stored in the exec session configuration. This prevents users from detaching from exec sessions using custom key sequences, making the --detach-keys option ineffective. This commit fixes the issue by: 1. Extracting detach keys from the exec session after inspection 2. Parsing them using term.ToBytes() (unless empty string) 3. Using the parsed keys in both detach.Copy() calls The fix maintains backward compatibility: - Exec sessions without detach keys -> empty string -> []byte{} (unchanged) - Empty detach keys (--detach-keys="") -> empty string -> []byte{} (correct) - Custom detach keys -> parsed and used (new functionality) Related to PR #25083 and issue #25089 which track comprehensive detach key functionality. A proper interactive test for detach behavior exists in test/system/450-interactive.bats but is currently skipped for remote exec pending additional infrastructure work. Signed-off-by: shiavm006 --- pkg/bindings/containers/attach.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/bindings/containers/attach.go b/pkg/bindings/containers/attach.go index 4b3c7d7fbe..361b037435 100644 --- a/pkg/bindings/containers/attach.go +++ b/pkg/bindings/containers/attach.go @@ -402,6 +402,15 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar isTerm = respStruct.ProcessConfig.Tty } + // Extract and parse detach keys from the exec session + detachKeysInBytes := []byte{} + if respStruct.DetachKeys != "" { + detachKeysInBytes, err = term.ToBytes(respStruct.DetachKeys) + if err != nil { + return fmt.Errorf("invalid detach keys in exec session: %w", err) + } + } + // If we are in TTY mode, we need to set raw mode for the terminal. // TODO: Share all of this with Attach() for containers. needTTY := terminalFile != nil && terminal.IsTerminal(int(terminalFile.Fd())) && isTerm @@ -458,7 +467,7 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar if options.GetAttachInput() { go func() { logrus.Debugf("Copying STDIN to socket") - _, err := detach.Copy(socket, options.InputStream, []byte{}) + _, err := detach.Copy(socket, options.InputStream, detachKeysInBytes) // Ignore "closed network connection" as it occurs when the exec ends, which is expected. // This avoids noisy logs but does not fix the goroutine leak // https://github.com/containers/podman/issues/25344 @@ -479,7 +488,7 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar return fmt.Errorf("exec session %s has a terminal and must have STDOUT enabled", sessionID) } // If not multiplex'ed, read from server and write to stdout - _, err := detach.Copy(options.GetOutputStream(), socket, []byte{}) + _, err := detach.Copy(options.GetOutputStream(), socket, detachKeysInBytes) if err != nil { return err }