From 0514425bac51146ef7012954997cede4a6f7fb7a Mon Sep 17 00:00:00 2001
From: Kyle McLaren <mclaren@fly.io>
Date: Sat, 27 Jun 2026 08:12:24 +0000
Subject: [PATCH] fix: don't allocate a remote PTY for piped stdin in machine
 run --shell (#4536)

`fly machine run --shell` hardcoded allocPTY=true when opening the SSH
session, so a remote pseudo-terminal was requested even for a fixed
`--command` with piped, non-TTY stdin. The remote terminal line
discipline then echoed the piped bytes back, racily exposing secrets.

Mirror the gating used by `fly ssh console`: only allocate a PTY for an
interactive shell (empty --command). Also harden ssh/io.go so a PTY is
never requested unless stdin is an actual terminal, regardless of the
caller's AllocPTY value, and add a unit test covering the decision.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 internal/command/machine/run.go |  7 +++++-
 ssh/io.go                       | 24 ++++++++++++++------
 ssh/io_test.go                  | 40 +++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 8 deletions(-)
 create mode 100644 ssh/io_test.go

diff --git a/internal/command/machine/run.go b/internal/command/machine/run.go
index c8716064f3..ac1e0661d2 100644
--- a/internal/command/machine/run.go
+++ b/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))
 		}
diff --git a/ssh/io.go b/ssh/io.go
index d56d459ddb..50f0abdd99 100644
--- a/ssh/io.go
+++ b/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
diff --git a/ssh/io_test.go b/ssh/io_test.go
new file mode 100644
index 0000000000..7d5c10d205
--- /dev/null
+++ b/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")
+	}
+}