fix: don't allocate a remote PTY for piped stdin in machine run --shell (#4536)

internal/command/machine/run.go

@@ -501,7 +501,12 @@ func runMachineRun(ctx context.Context) error {
 			return err
 		}
 
-		err = ssh.Console(ctx, sshClient, flag.GetString(ctx, "command"), true, "")
+		// Only allocate a remote PTY for an interactive shell. With a fixed
+		// --command the input comes from stdin (often piped/non-TTY), and a
+		// PTY would echo that piped input back to the terminal, leaking
+		// secrets (issue #4536). Mirror the gating used by `fly ssh console`.
+		cmd := flag.GetString(ctx, "command")
+		err = ssh.Console(ctx, sshClient, cmd, cmd == "", "")
 		if destroy {
 			err = soManyErrors("console", err, "destroy machine", Destroy(ctx, app.Name, machine, true))
 		}

ssh/io.go

@@ -47,18 +47,28 @@ func getFd(reader io.Reader) (fd int, ok bool) {
 	return fd, term.IsTerminal(fd)
 }
 
+// wantPTY reports whether a remote pseudo-terminal should be requested for the
+// session. A PTY is only useful for an interactive session, so we require both
+// that the caller asked for one (allocPTY) and that stdin is an actual
+// terminal. Requesting a PTY for piped, non-terminal stdin makes the remote
+// terminal line discipline echo the piped input back, which can leak secrets
+// (issue #4536).
+func wantPTY(allocPTY, stdinIsTerminal bool) bool {
+	return allocPTY && stdinIsTerminal
+}
+
 func (s *SessionIO) attach(ctx context.Context, sess *ssh.Session, cmd string) error {
 
-	if s.AllocPTY {
+	fd, stdinIsTerminal := getFd(s.Stdin)
+
+	if wantPTY(s.AllocPTY, stdinIsTerminal) {
 		width, height := DefaultWidth, DefaultHeight
 
-		if fd, ok := getFd(s.Stdin); ok {
-			state, err := term.MakeRaw(fd)
-			if err != nil {
-				return err
-			}
-			defer term.Restore(fd, state)
+		state, err := term.MakeRaw(fd)
+		if err != nil {
+			return err
 		}
+		defer term.Restore(fd, state)
 
 		if w, h, err := s.getAndWatchSize(ctx, sess); err == nil {
 			width, height = w, h

ssh/io_test.go

@@ -0,0 +1,40 @@
+package ssh
+
+import (
+	"bytes"
+	"testing"
+)
+
+func TestWantPTY(t *testing.T) {
+	cases := []struct {
+		name            string
+		allocPTY        bool
+		stdinIsTerminal bool
+		want            bool
+	}{
+		{"interactive terminal", true, true, true},
+		// Regression test for issue #4536: a caller may ask for a PTY
+		// (AllocPTY) while stdin is piped/non-interactive. Allocating a
+		// remote PTY in that case makes the terminal line discipline echo
+		// the piped bytes back, leaking secrets. We must not request a PTY.
+		{"alloc requested but stdin piped", true, false, false},
+		{"no alloc, terminal", false, true, false},
+		{"no alloc, piped", false, false, false},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := wantPTY(tc.allocPTY, tc.stdinIsTerminal); got != tc.want {
+				t.Fatalf("wantPTY(%v, %v) = %v, want %v", tc.allocPTY, tc.stdinIsTerminal, got, tc.want)
+			}
+		})
+	}
+}
+
+// Piped, non-terminal stdin (the scenario in issue #4536) must never be
+// detected as a terminal, so wantPTY collapses to false for it.
+func TestGetFdNonTerminal(t *testing.T) {
+	if _, ok := getFd(bytes.NewBufferString("TEST=123\n")); ok {
+		t.Fatal("getFd reported a bytes.Buffer as a terminal")
+	}
+}