coder · mafredri · Sep 12, 2022 · Aug 11, 2022 · Sep 12, 2022 · Sep 12, 2022
diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -31,6 +31,7 @@
     "gonet",
     "gossh",
     "gsyslog",
+    "GTTY",
     "hashicorp",
     "hclsyntax",
     "httpapi",
@@ -67,6 +68,7 @@
     "ntqry",
     "OIDC",
     "oneof",
+    "opty",
     "paralleltest",
     "parameterscopeid",
     "pqtype",
@@ -76,6 +78,7 @@
     "provisionerd",
     "provisionersdk",
     "ptty",
+    "ptys",
     "ptytest",
     "reconfig",
     "retrier",
@@ -87,6 +90,7 @@
     "sourcemapped",
     "Srcs",
     "stretchr",
+    "STTY",
     "stuntest",
     "tailbroker",
     "tailcfg",
@@ -105,6 +109,7 @@
     "tfjson",
     "tfplan",
     "tfstate",
+    "tios",
     "tparallel",
     "trimprefix",
     "tsdial",

diff --git a/agent/agent.go b/agent/agent.go
@@ -374,7 +374,7 @@ func (a *agent) runStartupScript(ctx context.Context, script string) error {
 		return nil
 	}
 
-	writer, err := os.OpenFile(filepath.Join(os.TempDir(), "coder-startup-script.log"), os.O_CREATE|os.O_RDWR, 0600)
+	writer, err := os.OpenFile(filepath.Join(os.TempDir(), "coder-startup-script.log"), os.O_CREATE|os.O_RDWR, 0o600)
 	if err != nil {
 		return xerrors.Errorf("open startup script log file: %w", err)
 	}
@@ -537,6 +537,8 @@ func (a *agent) init(ctx context.Context) {
 		},
 		SubsystemHandlers: map[string]ssh.SubsystemHandler{
 			"sftp": func(session ssh.Session) {
+				session.DisablePTYEmulation()
+
 				server, err := sftp.NewServer(session)
 				if err != nil {
 					a.logger.Debug(session.Context(), "initialize sftp server", slog.Error(err))
@@ -661,7 +663,8 @@ func (a *agent) createCommand(ctx context.Context, rawCommand string, env []stri
 }
 
 func (a *agent) handleSSHSession(session ssh.Session) (retErr error) {
-	cmd, err := a.createCommand(session.Context(), session.RawCommand(), session.Environ())
+	ctx := session.Context()
+	cmd, err := a.createCommand(ctx, session.RawCommand(), session.Environ())
 	if err != nil {
 		return err
 	}
@@ -678,32 +681,34 @@ func (a *agent) handleSSHSession(session ssh.Session) (retErr error) {
 
 	sshPty, windowSize, isPty := session.Pty()
 	if isPty {
+		// Disable minimal PTY emulation set by gliderlabs/ssh (NL-to-CRNL).
+		// See https://github.com/coder/coder/issues/3371.
+		session.DisablePTYEmulation()
+
 		cmd.Env = append(cmd.Env, fmt.Sprintf("TERM=%s", sshPty.Term))
 
 		// The pty package sets `SSH_TTY` on supported platforms.
-		ptty, process, err := pty.Start(cmd)
+		ptty, process, err := pty.Start(cmd, pty.WithPTYOption(
+			pty.WithSSHRequest(sshPty),
+			pty.WithLogger(slog.Stdlib(ctx, a.logger, slog.LevelInfo)),
+		))
 		if err != nil {
 			return xerrors.Errorf("start command: %w", err)
 		}
 		defer func() {
 			closeErr := ptty.Close()
 			if closeErr != nil {
-				a.logger.Warn(context.Background(), "failed to close tty",
-					slog.Error(closeErr))
+				a.logger.Warn(ctx, "failed to close tty", slog.Error(closeErr))
 				if retErr == nil {
 					retErr = closeErr
 				}
 			}
 		}()
-		err = ptty.Resize(uint16(sshPty.Window.Height), uint16(sshPty.Window.Width))
-		if err != nil {
-			return xerrors.Errorf("resize ptty: %w", err)
-		}
 		go func() {
 			for win := range windowSize {
 				resizeErr := ptty.Resize(uint16(win.Height), uint16(win.Width))
 				if resizeErr != nil {
-					a.logger.Warn(context.Background(), "failed to resize tty", slog.Error(resizeErr))
+					a.logger.Warn(ctx, "failed to resize tty", slog.Error(resizeErr))
 				}
 			}
 		}()
@@ -718,8 +723,7 @@ func (a *agent) handleSSHSession(session ssh.Session) (retErr error) {
 		// ExitErrors just mean the command we run returned a non-zero exit code, which is normal
 		// and not something to be concerned about.  But, if it's something else, we should log it.
 		if err != nil && !xerrors.As(err, &exitErr) {
-			a.logger.Warn(context.Background(), "wait error",
-				slog.Error(err))
+			a.logger.Warn(ctx, "wait error", slog.Error(err))
 		}
 		return err
 	}

diff --git a/cli/ssh.go b/cli/ssh.go
@@ -195,6 +195,13 @@ func ssh() *cobra.Command {
 			// shutdown of services.
 			defer cancel()
 
+			if validOut {
+				// Set initial window size.
+				width, height, err := term.GetSize(int(stdoutFile.Fd()))
+				if err == nil {
+					_ = sshSession.WindowChange(height, width)
+				}
+			}
 			err = sshSession.Wait()
 			if err != nil {
 				// If the connection drops unexpectedly, we get an ExitMissingError but no other

diff --git a/go.mod b/go.mod
@@ -51,6 +51,15 @@ replace github.com/tcnksm/go-httpstat => github.com/kylecarbs/go-httpstat v0.0.0
 // https://github.com/tailscale/tailscale/compare/main...coder:tailscale:main
 replace tailscale.com => github.com/coder/tailscale v1.1.1-0.20220907193453-fb5ba5ab658d
 
+// Switch to our fork that imports fixes from http://github.com/tailscale/ssh.
+// See: https://github.com/coder/coder/issues/3371
+//
+// Note that http://github.com/tailscale/ssh has been merged into the Tailscale
+// repo as tailscale.com/tempfork/gliderlabs/ssh, however, we can't replace the
+// subpath and it includes changes to golang.org/x/crypto/ssh as well which
+// makes importing it directly a bit messy.
+replace github.com/gliderlabs/ssh => github.com/coder/ssh v0.0.0-20220811105153-fcea99919338
+
 require (
 	cdr.dev/slog v1.4.2-0.20220525200111-18dce5c2cd5f
 	cloud.google.com/go/compute v1.7.0
@@ -126,6 +135,7 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.8.0
 	github.com/tabbed/pqtype v0.1.1
+	github.com/u-root/u-root v0.9.0
 	github.com/unrolled/secure v1.12.0
 	go.mozilla.org/pkcs7 v0.0.0-20200128120323-432b2356ecb1
 	go.opentelemetry.io/otel v1.8.0

diff --git a/go.sum b/go.sum
@@ -352,6 +352,8 @@ github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1 h1:UqBrPWSYvRI2s5RtOu
 github.com/coder/glog v1.0.1-0.20220322161911-7365fe7f2cd1/go.mod h1:EWib/APOK0SL3dFbYqvxE3UYd8E6s1ouQ7iEp/0LWV4=
 github.com/coder/retry v1.3.0 h1:5lAAwt/2Cm6lVmnfBY7sOMXcBOwcwJhmV5QGSELIVWY=
 github.com/coder/retry v1.3.0/go.mod h1:tXuRgZgWjUnU5LZPT4lJh4ew2elUhexhlnXzrJWdyFY=
+github.com/coder/ssh v0.0.0-20220811105153-fcea99919338 h1:tN5GKFT68YLVzJoA8AHuiMNJ0qlhoD3pGN3JY9gxSko=
+github.com/coder/ssh v0.0.0-20220811105153-fcea99919338/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914=
 github.com/coder/tailscale v1.1.1-0.20220907193453-fb5ba5ab658d h1:IQ8wJn8MfDS+sesYPpn3EDAyvoGMxFvyyE9uWtcfU6w=
 github.com/coder/tailscale v1.1.1-0.20220907193453-fb5ba5ab658d/go.mod h1:MO+tWkQp2YIF3KBnnej/mQvgYccRS5Xk/IrEpZ4Z3BU=
 github.com/coder/wireguard-go/tun/netstack v0.0.0-20220823170024-a78136eb0cab h1:9yEvRWXXfyKzXu8AqywCi+tFZAoqCy4wVcsXwuvZNMc=
@@ -638,9 +640,6 @@ github.com/gin-gonic/gin v1.7.0 h1:jGB9xAJQ12AIGNB4HguylppmDK1Am9ppF7XnGXXJuoU=
 github.com/gin-gonic/gin v1.7.0/go.mod h1:jD2toBW3GZUr5UMcdrwQA10I7RuaFOl/SGeDjXkfUtY=
 github.com/github/fakeca v0.1.0 h1:Km/MVOFvclqxPM9dZBC4+QE564nU4gz4iZ0D9pMw28I=
 github.com/github/fakeca v0.1.0/go.mod h1:+bormgoGMMuamOscx7N91aOuUST7wdaJ2rNjeohylyo=
-github.com/gliderlabs/ssh v0.2.2/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0=
-github.com/gliderlabs/ssh v0.3.4 h1:+AXBtim7MTKaLVPgvE+3mhewYRawNLTd+jEEz/wExZw=
-github.com/gliderlabs/ssh v0.3.4/go.mod h1:ZSS+CUoKHDrqVakTfTWUlKSr9MtMFkC4UvtQKD7O914=
 github.com/go-chi/chi v1.5.4 h1:QHdzF2szwjqVV4wmByUnTcsbIg7UGaQ0tPF2t5GcAIs=
 github.com/go-chi/chi v1.5.4/go.mod h1:uaf8YgoFazUOkPBG7fxPftUylNumIev9awIWOENIuEg=
 github.com/go-chi/chi/v5 v5.0.7 h1:rDTPXLDHGATaeHvVlLcR4Qe0zftYethFucbjVQ1PxU8=
@@ -1810,6 +1809,8 @@ github.com/tomarrell/wrapcheck/v2 v2.4.0/go.mod h1:68bQ/eJg55BROaRTbMjC7vuhL2Ogf
 github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce/go.mod h1:o8v6yHRoik09Xen7gje4m9ERNah1d1PPsVq1VEx9vE4=
 github.com/tommy-muehle/go-mnd/v2 v2.4.0/go.mod h1:WsUAkMJMYww6l/ufffCD3m+P7LEvr8TnZn9lwVDlgzw=
 github.com/tv42/httpunix v0.0.0-20191220191345-2ba4b9c3382c/go.mod h1:hzIxponao9Kjc7aWznkXaL4U4TWaDSs8zcsY4Ka08nM=
+github.com/u-root/u-root v0.9.0 h1:1dpUzrE0FyKrNEjxpKFOkyveuV1f3T0Ko5CQg4gTkCg=
+github.com/u-root/u-root v0.9.0/go.mod h1:ewc9w6JF1ayZCVC9Y5wsrUiCBw3nMmPC3QItvrEwmew=
 github.com/u-root/uio v0.0.0-20210528114334-82958018845c/go.mod h1:LpEX5FO/cB+WF4TYGY1V5qktpaZLkKkSegbr0V4eYXA=
 github.com/u-root/uio v0.0.0-20220204230159-dac05f7d2cb4 h1:hl6sK6aFgTLISijk6xIzeqnPzQcsLqqvL6vEfTPinME=
 github.com/u-root/uio v0.0.0-20220204230159-dac05f7d2cb4/go.mod h1:LpEX5FO/cB+WF4TYGY1V5qktpaZLkKkSegbr0V4eYXA=

diff --git a/pty/pty.go b/pty/pty.go
@@ -2,13 +2,19 @@ package pty
 
 import (
 	"io"
+	"log"
 	"os"
+
+	"github.com/gliderlabs/ssh"
 )
 
 // PTY is a minimal interface for interacting with a TTY.
 type PTY interface {
 	io.Closer
 
+	// Name of the TTY. Example on Linux would be "/dev/pts/1".
+	Name() string
+
 	// Output handles TTY output.
 	//
 	// cmd.SetOutput(pty.Output()) would be used to specify a command
@@ -35,7 +41,6 @@ type PTY interface {
 // to Wait() on a process, this abstraction provides a goroutine-safe interface for interacting with
 // the process.
 type Process interface {
-
 	// Wait for the command to complete.  Returned error is as for exec.Cmd.Wait()
 	Wait() error
 
@@ -52,9 +57,33 @@ type WithFlags interface {
 	EchoEnabled() (bool, error)
 }
 
+// Options represents a an option for a PTY.
+type Option func(*ptyOptions)
+
+type ptyOptions struct {
+	logger *log.Logger
+	sshReq *ssh.Pty
+}
+
+// WithSSHRequest applies the ssh.Pty request to the PTY.
+//
+// Only partially supported on Windows (e.g. window size).
+func WithSSHRequest(req ssh.Pty) Option {
+	return func(opts *ptyOptions) {
+		opts.sshReq = &req
+	}
+}
+
+// WithLogger sets a logger for logging errors.
+func WithLogger(logger *log.Logger) Option {
+	return func(opts *ptyOptions) {
+		opts.logger = logger
+	}
+}
+
 // New constructs a new Pty.
-func New() (PTY, error) {
-	return newPty()
+func New(opts ...Option) (PTY, error) {
+	return newPty(opts...)
 }
 
 // ReadWriter is an implementation of io.ReadWriter that wraps two separate

diff --git a/pty/pty_linux.go b/pty/pty_linux.go
@@ -2,12 +2,23 @@
 
 package pty
 
-import "golang.org/x/sys/unix"
+import (
+	"github.com/u-root/u-root/pkg/termios"
+	"golang.org/x/sys/unix"
+)
 
-func (p *otherPty) EchoEnabled() (bool, error) {
-	termios, err := unix.IoctlGetTermios(int(p.pty.Fd()), unix.TCGETS)
+func (p *otherPty) EchoEnabled() (echo bool, err error) {
+	err = p.control(p.pty, func(fd uintptr) error {
+		t, err := termios.GetTermios(fd)
+		if err != nil {
+			return err
+		}
+
+		echo = (t.Lflag & unix.ECHO) != 0
+		return nil
+	})
 	if err != nil {
 		return false, err
 	}
-	return (termios.Lflag & unix.ECHO) != 0, nil
+	return echo, nil
 }