Skip to content

Commit 180570a

Browse files
committed
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 <shivammittal42006@gmail.com>
1 parent 2b646e7 commit 180570a

File tree

1 file changed

+11
-2
lines changed

1 file changed

+11
-2
lines changed

pkg/bindings/containers/attach.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,15 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar
402402
isTerm = respStruct.ProcessConfig.Tty
403403
}
404404

405+
// Extract and parse detach keys from the exec session
406+
detachKeysInBytes := []byte{}
407+
if respStruct.DetachKeys != "" {
408+
detachKeysInBytes, err = term.ToBytes(respStruct.DetachKeys)
409+
if err != nil {
410+
return fmt.Errorf("invalid detach keys in exec session: %w", err)
411+
}
412+
}
413+
405414
// If we are in TTY mode, we need to set raw mode for the terminal.
406415
// TODO: Share all of this with Attach() for containers.
407416
needTTY := terminalFile != nil && terminal.IsTerminal(int(terminalFile.Fd())) && isTerm
@@ -458,7 +467,7 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar
458467
if options.GetAttachInput() {
459468
go func() {
460469
logrus.Debugf("Copying STDIN to socket")
461-
_, err := detach.Copy(socket, options.InputStream, []byte{})
470+
_, err := detach.Copy(socket, options.InputStream, detachKeysInBytes)
462471
// Ignore "closed network connection" as it occurs when the exec ends, which is expected.
463472
// This avoids noisy logs but does not fix the goroutine leak
464473
// https://github.com/containers/podman/issues/25344
@@ -479,7 +488,7 @@ func ExecStartAndAttach(ctx context.Context, sessionID string, options *ExecStar
479488
return fmt.Errorf("exec session %s has a terminal and must have STDOUT enabled", sessionID)
480489
}
481490
// If not multiplex'ed, read from server and write to stdout
482-
_, err := detach.Copy(options.GetOutputStream(), socket, []byte{})
491+
_, err := detach.Copy(options.GetOutputStream(), socket, detachKeysInBytes)
483492
if err != nil {
484493
return err
485494
}

0 commit comments

Comments
 (0)