fix: don't allocate a remote PTY for piped stdin in machine run --shell (#4536)#4947
Draft
kylemclaren wants to merge 1 commit into
Draft
fix: don't allocate a remote PTY for piped stdin in machine run --shell (#4536)#4947kylemclaren wants to merge 1 commit into
kylemclaren wants to merge 1 commit into
Conversation
…ll (#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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4536.
fly machine run --shell --command <cmd>always requested a remote PTY. When stdin is piped (non-TTY), the remote terminal's line discipline echoes that piped input back to the local terminal — which can leak secrets, e.g.echo TOKEN=… | fly machine run … --shell.A remote PTY is now requested only when one was asked for and stdin is an actual terminal (the same gating
fly ssh consoleuses); with a fixed--command, no PTY is allocated.Testing: added unit tests for the PTY-gating helpers (
wantPTY,getFd) inssh/; they don't build/pass onmaster(the gating didn't exist) and pass with the fix.go test ./ssh/... ./internal/command/machine/...is green. The stray echo itself is a non-deterministic race, so the tests cover the gating logic rather than a flaky behavioral repro.✅ Live-verified against a real org (over WireGuard; ephemeral machines auto-destroyed):
echo TEST=123 | fly machine run debian --shell --command "echo test"leakedTEST=123to stdout on 8/8 runs.testprinted). The fix is structural — it refuses a remote PTY when stdin isn't a terminal.🤖 Generated with Claude Code