From bb7fc094ec373403ee5e48dcdbb974ffdd58a2bb Mon Sep 17 00:00:00 2001
From: awdemos <awdemos@users.noreply.github.com>
Date: Thu, 27 Aug 2026 06:28:29 -0500
Subject: [PATCH] tailcat_ssh: avoid PTY session hang on slow/interactive
 shells

runWithPTY previously blocked on io.Copy(sess, ptmx) before calling
cmd.Wait(). If the shell did not exit immediately, the function would
never reach cmd.Wait(), and the deferred ptmx.Close never ran. This
could hang the whole SSH session.

Restructure the PTY path so cmd.Wait() and the stdout copy run
concurrently. When the command exits first, cancel the window-resize
goroutine and drain the remaining stdout. When the client disconnects
first, kill the shell and wait for it to exit.

Also close the pty slave fd after cmd.Start and set tty=nil so the
deferred cleanup does not double-close it.

Updates tailscale/tailcat (adversarial audit).
---
 tailcat_ssh.go | 88 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 71 insertions(+), 17 deletions(-)

diff --git a/tailcat_ssh.go b/tailcat_ssh.go
index d6bb2906e..9bedc19f5 100644
--- a/tailcat_ssh.go
+++ b/tailcat_ssh.go
@@ -6,6 +6,7 @@
 package tailcat
 
 import (
+	"context"
 	"crypto/ed25519"
 	"crypto/rand"
 	"crypto/x509"
@@ -169,35 +170,88 @@ func runWithPTY(sess ssh.Session, cmd *exec.Cmd, ptyReq ssh.Pty, winCh <-chan ss
 		sess.Exit(1)
 		return
 	}
-	tty.Close() // child owns the tty now
-
-	// Handle window size changes. The goroutine runs until gliderssh
-	// closes winCh, which happens only once the whole session channel
-	// shuts down, after this function has returned and closed ptmx. It
-	// therefore gets its own duplicated file descriptor rather than
-	// racing the deferred ptmx.Close (and whatever reuses that fd).
+	// Child owns its own fd to the slave; close the parent's copy now so
+	// the slave gets EOF once the child exits, and so we don't double-close
+	// it in the deferred cleanup.
+	tty.Close()
+	tty = nil
+
+	// Handle window size changes. We keep a duplicated fd to the pty
+	// master so the window goroutine can outlive this function without
+	// racing the deferred ptmx.Close. The goroutine is cancelled once the
+	// command exits so the duplicated fd is released and the pty master
+	// gets EOF, unblocking the stdout copy.
+	winchCtx, winchCancel := context.WithCancel(context.Background())
+	var winchWg sync.WaitGroup
 	if winchFd, err := unix.Dup(int(ptmx.Fd())); err == nil {
+		winchWg.Add(1)
 		go func() {
+			defer winchWg.Done()
 			defer unix.Close(winchFd)
-			for win := range winCh {
-				unix.IoctlSetWinsize(winchFd, syscall.TIOCSWINSZ, &unix.Winsize{
-					Row:    uint16(win.Height),
-					Col:    uint16(win.Width),
-					Xpixel: uint16(win.WidthPixels),
-					Ypixel: uint16(win.HeightPixels),
-				})
+			for {
+				select {
+				case win, ok := <-winCh:
+					if !ok {
+						return
+					}
+					unix.IoctlSetWinsize(winchFd, syscall.TIOCSWINSZ, &unix.Winsize{
+						Row:    uint16(win.Height),
+						Col:    uint16(win.Width),
+						Xpixel: uint16(win.WidthPixels),
+						Ypixel: uint16(win.HeightPixels),
+					})
+				case <-winchCtx.Done():
+					return
+				}
 			}
 		}()
 	}
+	defer func() {
+		winchCancel()
+		winchWg.Wait()
+	}()
 
 	// I/O: session ↔ pty master.
 	go func() {
 		io.Copy(ptmx, sess) // stdin
 	}()
-	io.Copy(sess, ptmx) // stdout (blocks until pty closes)
 
-	if err := cmd.Wait(); err != nil {
-		sess.Exit(exitCode(err))
+	// Wait for the command to finish or for the stdout copy to finish
+	// (e.g. the client disconnected). In the common case the command
+	// exits first; we then cancel the window goroutine so its duplicated
+	// pty-master fd is closed, allowing the stdout copy to drain EOF and
+	// return. If the client goes away first, kill the shell.
+	cmdDone := make(chan error, 1)
+	go func() {
+		cmdDone <- cmd.Wait()
+	}()
+
+	stdoutDone := make(chan struct{})
+	go func() {
+		defer close(stdoutDone)
+		io.Copy(sess, ptmx) // stdout
+	}()
+
+	var waitErr error
+	select {
+	case waitErr = <-cmdDone:
+		// Command exited. Release the window goroutine's fd so the
+		// stdout copy gets EOF and returns.
+		winchCancel()
+		winchWg.Wait()
+		<-stdoutDone
+	case <-stdoutDone:
+		// Client/session went away before the command exited.
+		winchCancel()
+		winchWg.Wait()
+		if cmd.Process != nil {
+			cmd.Process.Kill()
+		}
+		waitErr = <-cmdDone
+	}
+
+	if waitErr != nil {
+		sess.Exit(exitCode(waitErr))
 		return
 	}
 	sess.Exit(0)