From 78066d4e1942c4ba71f03068b8f710dbca61555c Mon Sep 17 00:00:00 2001 From: David Wahler Date: Mon, 16 May 2022 19:56:15 +0000 Subject: [PATCH 01/13] add failing unit test for terminal echo state when reading password --- cli/cliui/prompt_unix_test.go | 63 +++++++++++++++++++++++++++++++++++ pty/pty.go | 4 +++ pty/pty_other.go | 4 +++ pty/pty_windows.go | 5 +++ 4 files changed, 76 insertions(+) create mode 100644 cli/cliui/prompt_unix_test.go diff --git a/cli/cliui/prompt_unix_test.go b/cli/cliui/prompt_unix_test.go new file mode 100644 index 0000000000000..6f6e29bd33a56 --- /dev/null +++ b/cli/cliui/prompt_unix_test.go @@ -0,0 +1,63 @@ +//go:build linux || darwin + +package cliui_test + +import ( + "context" + "os" + "os/exec" + "testing" + "time" + + "github.com/coder/coder/cli/cliui" + "github.com/coder/coder/pty/ptytest" + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" +) + +func TestPasswordTerminalState(t *testing.T) { + if os.Getenv("TEST_SUBPROCESS") == "1" { + passwordHelper() + return + } + t.Parallel() + + ptty := ptytest.New(t) + + cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") + cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1") + cmd.Stdin = ptty.PTY.PTYFile() + cmd.Stdout = ptty.PTY.PTYFile() + cmd.Stderr = os.Stderr + err := cmd.Start() + require.NoError(t, err) + defer cmd.Process.Kill() + + ptty.ExpectMatch("Password: ") + time.Sleep(10 * time.Millisecond) // wait for child process to turn off echo and start reading input + + termios, err := unix.IoctlGetTermios(int(ptty.PTY.PTYFile().Fd()), unix.TCGETS) + require.NoError(t, err) + require.Zero(t, termios.Lflag&unix.ECHO, "echo is on while reading password") + + cmd.Process.Signal(os.Interrupt) + _, err = cmd.Process.Wait() + require.NoError(t, err) + + termios, err = unix.IoctlGetTermios(int(ptty.PTY.PTYFile().Fd()), unix.TCGETS) + require.NoError(t, err) + require.NotZero(t, termios.Lflag&unix.ECHO, "echo is off after reading password") +} + +func passwordHelper() { + cmd := &cobra.Command{ + Run: func(cmd *cobra.Command, args []string) { + cliui.Prompt(cmd, cliui.PromptOptions{ + Text: "Password:", + Secret: true, + }) + }, + } + cmd.ExecuteContext(context.Background()) +} diff --git a/pty/pty.go b/pty/pty.go index 00eb7d33ea4c1..df3458d60d57e 100644 --- a/pty/pty.go +++ b/pty/pty.go @@ -2,6 +2,7 @@ package pty import ( "io" + "os" ) // PTY is a minimal interface for interacting with a TTY. @@ -26,6 +27,9 @@ type PTY interface { // Resize sets the size of the PTY. Resize(height uint16, width uint16) error + + // The file descriptor of the underlying PTY, if available + PTYFile() *os.File } // New constructs a new Pty. diff --git a/pty/pty_other.go b/pty/pty_other.go index b826bd3a3398f..5a0f53d6bc1e9 100644 --- a/pty/pty_other.go +++ b/pty/pty_other.go @@ -66,3 +66,7 @@ func (p *otherPty) Close() error { } return nil } + +func (p *otherPty) PTYFile() *os.File { + return p.pty +} diff --git a/pty/pty_windows.go b/pty/pty_windows.go index 854ecfe36eeda..349b446fdac87 100644 --- a/pty/pty_windows.go +++ b/pty/pty_windows.go @@ -112,3 +112,8 @@ func (p *ptyWindows) Close() error { return nil } + +func (p *ptyWindows) PTYFile() *os.File { + // not supported on Windows(?) + return nil +} From a3dee263ffedb247c6490d7b26f1db888bbcc65e Mon Sep 17 00:00:00 2001 From: David Wahler Date: Mon, 16 May 2022 20:57:01 +0000 Subject: [PATCH 02/13] fixup! add explanatory comment --- cli/cliui/prompt_unix_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/cliui/prompt_unix_test.go b/cli/cliui/prompt_unix_test.go index 6f6e29bd33a56..e6274b456f8cb 100644 --- a/cli/cliui/prompt_unix_test.go +++ b/cli/cliui/prompt_unix_test.go @@ -27,6 +27,7 @@ func TestPasswordTerminalState(t *testing.T) { cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1") + // connect the child process's stdio to the PTY directly, not via a pipe cmd.Stdin = ptty.PTY.PTYFile() cmd.Stdout = ptty.PTY.PTYFile() cmd.Stderr = os.Stderr From 6b78f455a0329240a13f4143a9e95025b6f26097 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Mon, 16 May 2022 20:57:51 +0000 Subject: [PATCH 03/13] don't try to handle SIGINT in cliui.Prompt --- cli/cliui/prompt.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/cli/cliui/prompt.go b/cli/cliui/prompt.go index 3e4c0689da162..0c0953b9db4d6 100644 --- a/cli/cliui/prompt.go +++ b/cli/cliui/prompt.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "os" - "os/signal" "strings" "github.com/bgentry/speakeasy" @@ -34,8 +33,6 @@ func Prompt(cmd *cobra.Command, opts PromptOptions) (string, error) { _, _ = fmt.Fprint(cmd.OutOrStdout(), Styles.Placeholder.Render("("+opts.Default+") ")) } interrupt := make(chan os.Signal, 1) - signal.Notify(interrupt, os.Interrupt) - defer signal.Stop(interrupt) errCh := make(chan error, 1) lineCh := make(chan string) From 8d3ee6a4d6ded62d35de1ddfd8771308fc8cd725 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Mon, 16 May 2022 23:36:33 +0000 Subject: [PATCH 04/13] Skip signal handler for passwords, but keep it for other prompts --- cli/cliui/prompt.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cli/cliui/prompt.go b/cli/cliui/prompt.go index 0c0953b9db4d6..ac39404e27d3f 100644 --- a/cli/cliui/prompt.go +++ b/cli/cliui/prompt.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "os" + "os/signal" "strings" "github.com/bgentry/speakeasy" @@ -42,8 +43,12 @@ func Prompt(cmd *cobra.Command, opts PromptOptions) (string, error) { inFile, isInputFile := cmd.InOrStdin().(*os.File) if opts.Secret && isInputFile && isatty.IsTerminal(inFile.Fd()) { + // we don't install a signal handler here because speakeasy has its own line, err = speakeasy.Ask("") } else { + signal.Notify(interrupt, os.Interrupt) + defer signal.Stop(interrupt) + reader := bufio.NewReader(cmd.InOrStdin()) line, err = reader.ReadString('\n') From 57a198f1101079913541e5250d38b49ea29161d2 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Tue, 17 May 2022 00:05:30 +0000 Subject: [PATCH 05/13] fix lint errors --- cli/cliui/prompt_unix_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cli/cliui/prompt_unix_test.go b/cli/cliui/prompt_unix_test.go index e6274b456f8cb..c4beb4638ae52 100644 --- a/cli/cliui/prompt_unix_test.go +++ b/cli/cliui/prompt_unix_test.go @@ -9,11 +9,12 @@ import ( "testing" "time" - "github.com/coder/coder/cli/cliui" - "github.com/coder/coder/pty/ptytest" "github.com/spf13/cobra" "github.com/stretchr/testify/require" "golang.org/x/sys/unix" + + "github.com/coder/coder/cli/cliui" + "github.com/coder/coder/pty/ptytest" ) func TestPasswordTerminalState(t *testing.T) { @@ -25,7 +26,7 @@ func TestPasswordTerminalState(t *testing.T) { ptty := ptytest.New(t) - cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") + cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") //nolint:gosec cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1") // connect the child process's stdio to the PTY directly, not via a pipe cmd.Stdin = ptty.PTY.PTYFile() @@ -42,7 +43,8 @@ func TestPasswordTerminalState(t *testing.T) { require.NoError(t, err) require.Zero(t, termios.Lflag&unix.ECHO, "echo is on while reading password") - cmd.Process.Signal(os.Interrupt) + err = cmd.Process.Signal(os.Interrupt) + require.NoError(t, err) _, err = cmd.Process.Wait() require.NoError(t, err) From 1dd8ccfc2bcbfb42b621a8a0257f716c1a5c3176 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Tue, 17 May 2022 00:31:09 +0000 Subject: [PATCH 06/13] only run password prompt test on Linux --- cli/cliui/{prompt_unix_test.go => prompt_linux_test.go} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename cli/cliui/{prompt_unix_test.go => prompt_linux_test.go} (98%) diff --git a/cli/cliui/prompt_unix_test.go b/cli/cliui/prompt_linux_test.go similarity index 98% rename from cli/cliui/prompt_unix_test.go rename to cli/cliui/prompt_linux_test.go index c4beb4638ae52..0086bc0963383 100644 --- a/cli/cliui/prompt_unix_test.go +++ b/cli/cliui/prompt_linux_test.go @@ -1,4 +1,4 @@ -//go:build linux || darwin +//go:build linux package cliui_test From fa51354b9c4772acc151f27e38e8299343da9f08 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Tue, 17 May 2022 20:47:27 +0000 Subject: [PATCH 07/13] get rid of pty.PTYFile() accessor that isn't implementable on Windows --- cli/cliui/prompt_linux_test.go | 15 ++++++++------- pty/pty.go | 24 ++++++++++++++++-------- pty/pty_other.go | 13 ++++--------- pty/pty_windows.go | 9 ++------- 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/cli/cliui/prompt_linux_test.go b/cli/cliui/prompt_linux_test.go index 0086bc0963383..f227cac90ffc9 100644 --- a/cli/cliui/prompt_linux_test.go +++ b/cli/cliui/prompt_linux_test.go @@ -29,26 +29,27 @@ func TestPasswordTerminalState(t *testing.T) { cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") //nolint:gosec cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1") // connect the child process's stdio to the PTY directly, not via a pipe - cmd.Stdin = ptty.PTY.PTYFile() - cmd.Stdout = ptty.PTY.PTYFile() + cmd.Stdin = ptty.Input().Reader + cmd.Stdout = ptty.Output().Writer cmd.Stderr = os.Stderr err := cmd.Start() require.NoError(t, err) - defer cmd.Process.Kill() + process := cmd.Process + defer process.Kill() ptty.ExpectMatch("Password: ") time.Sleep(10 * time.Millisecond) // wait for child process to turn off echo and start reading input - termios, err := unix.IoctlGetTermios(int(ptty.PTY.PTYFile().Fd()), unix.TCGETS) + termios, err := unix.IoctlGetTermios(int(ptty.Input().Reader.Fd()), unix.TCGETS) require.NoError(t, err) require.Zero(t, termios.Lflag&unix.ECHO, "echo is on while reading password") - err = cmd.Process.Signal(os.Interrupt) + err = process.Signal(os.Interrupt) require.NoError(t, err) - _, err = cmd.Process.Wait() + _, err = process.Wait() require.NoError(t, err) - termios, err = unix.IoctlGetTermios(int(ptty.PTY.PTYFile().Fd()), unix.TCGETS) + termios, err = unix.IoctlGetTermios(int(ptty.Input().Reader.Fd()), unix.TCGETS) require.NoError(t, err) require.NotZero(t, termios.Lflag&unix.ECHO, "echo is off after reading password") } diff --git a/pty/pty.go b/pty/pty.go index df3458d60d57e..f4f2999d1f019 100644 --- a/pty/pty.go +++ b/pty/pty.go @@ -15,7 +15,7 @@ type PTY interface { // uses the output stream for writing. // // The same stream could be read to validate output. - Output() io.ReadWriter + Output() ReadWriter // Input handles TTY input. // @@ -23,13 +23,10 @@ type PTY interface { // uses the PTY input for reading. // // The same stream would be used to provide user input: pty.Input().Write(...) - Input() io.ReadWriter + Input() ReadWriter // Resize sets the size of the PTY. Resize(height uint16, width uint16) error - - // The file descriptor of the underlying PTY, if available - PTYFile() *os.File } // New constructs a new Pty. @@ -37,7 +34,18 @@ func New() (PTY, error) { return newPty() } -type readWriter struct { - io.Reader - io.Writer +// ReadWriter is an implementation of io.ReadWriter that wraps two separate +// underlying file descriptors, one for reading and one for writing, and allows +// them to be accessed separately. +type ReadWriter struct { + Reader *os.File + Writer *os.File +} + +func (rw ReadWriter) Read(p []byte) (int, error) { + return rw.Reader.Read(p) +} + +func (rw ReadWriter) Write(p []byte) (int, error) { + return rw.Writer.Write(p) } diff --git a/pty/pty_other.go b/pty/pty_other.go index 5a0f53d6bc1e9..d6e21d4d3ffe1 100644 --- a/pty/pty_other.go +++ b/pty/pty_other.go @@ -4,7 +4,6 @@ package pty import ( - "io" "os" "sync" @@ -28,15 +27,15 @@ type otherPty struct { pty, tty *os.File } -func (p *otherPty) Input() io.ReadWriter { - return readWriter{ +func (p *otherPty) Input() ReadWriter { + return ReadWriter{ Reader: p.tty, Writer: p.pty, } } -func (p *otherPty) Output() io.ReadWriter { - return readWriter{ +func (p *otherPty) Output() ReadWriter { + return ReadWriter{ Reader: p.pty, Writer: p.tty, } @@ -66,7 +65,3 @@ func (p *otherPty) Close() error { } return nil } - -func (p *otherPty) PTYFile() *os.File { - return p.pty -} diff --git a/pty/pty_windows.go b/pty/pty_windows.go index 349b446fdac87..0d6691b2e7481 100644 --- a/pty/pty_windows.go +++ b/pty/pty_windows.go @@ -68,14 +68,14 @@ type ptyWindows struct { } func (p *ptyWindows) Output() io.ReadWriter { - return readWriter{ + return ReadWriter{ Reader: p.outputRead, Writer: p.outputWrite, } } func (p *ptyWindows) Input() io.ReadWriter { - return readWriter{ + return ReadWriter{ Reader: p.inputRead, Writer: p.inputWrite, } @@ -112,8 +112,3 @@ func (p *ptyWindows) Close() error { return nil } - -func (p *ptyWindows) PTYFile() *os.File { - // not supported on Windows(?) - return nil -} From cc81c7ed6437620be9d82fdae73ea680c06d2c96 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Tue, 17 May 2022 20:54:43 +0000 Subject: [PATCH 08/13] bump password prompt test delay to 100ms --- cli/cliui/prompt_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/cliui/prompt_linux_test.go b/cli/cliui/prompt_linux_test.go index f227cac90ffc9..7cce84d837285 100644 --- a/cli/cliui/prompt_linux_test.go +++ b/cli/cliui/prompt_linux_test.go @@ -38,7 +38,7 @@ func TestPasswordTerminalState(t *testing.T) { defer process.Kill() ptty.ExpectMatch("Password: ") - time.Sleep(10 * time.Millisecond) // wait for child process to turn off echo and start reading input + time.Sleep(100 * time.Millisecond) // wait for child process to turn off echo and start reading input termios, err := unix.IoctlGetTermios(int(ptty.Input().Reader.Fd()), unix.TCGETS) require.NoError(t, err) From 687194fe1513c71884c32614e046ac3293b1de65 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Tue, 17 May 2022 21:14:46 +0000 Subject: [PATCH 09/13] make TestPasswordTerminalState cross-platform --- cli/cliui/prompt_linux_test.go | 67 ---------------------------------- cli/cliui/prompt_test.go | 58 +++++++++++++++++++++++++++++ pty/pty.go | 9 +++++ pty/pty_linux.go | 13 +++++++ pty/pty_windows.go | 9 +++++ 5 files changed, 89 insertions(+), 67 deletions(-) delete mode 100644 cli/cliui/prompt_linux_test.go create mode 100644 pty/pty_linux.go diff --git a/cli/cliui/prompt_linux_test.go b/cli/cliui/prompt_linux_test.go deleted file mode 100644 index 7cce84d837285..0000000000000 --- a/cli/cliui/prompt_linux_test.go +++ /dev/null @@ -1,67 +0,0 @@ -//go:build linux - -package cliui_test - -import ( - "context" - "os" - "os/exec" - "testing" - "time" - - "github.com/spf13/cobra" - "github.com/stretchr/testify/require" - "golang.org/x/sys/unix" - - "github.com/coder/coder/cli/cliui" - "github.com/coder/coder/pty/ptytest" -) - -func TestPasswordTerminalState(t *testing.T) { - if os.Getenv("TEST_SUBPROCESS") == "1" { - passwordHelper() - return - } - t.Parallel() - - ptty := ptytest.New(t) - - cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") //nolint:gosec - cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1") - // connect the child process's stdio to the PTY directly, not via a pipe - cmd.Stdin = ptty.Input().Reader - cmd.Stdout = ptty.Output().Writer - cmd.Stderr = os.Stderr - err := cmd.Start() - require.NoError(t, err) - process := cmd.Process - defer process.Kill() - - ptty.ExpectMatch("Password: ") - time.Sleep(100 * time.Millisecond) // wait for child process to turn off echo and start reading input - - termios, err := unix.IoctlGetTermios(int(ptty.Input().Reader.Fd()), unix.TCGETS) - require.NoError(t, err) - require.Zero(t, termios.Lflag&unix.ECHO, "echo is on while reading password") - - err = process.Signal(os.Interrupt) - require.NoError(t, err) - _, err = process.Wait() - require.NoError(t, err) - - termios, err = unix.IoctlGetTermios(int(ptty.Input().Reader.Fd()), unix.TCGETS) - require.NoError(t, err) - require.NotZero(t, termios.Lflag&unix.ECHO, "echo is off after reading password") -} - -func passwordHelper() { - cmd := &cobra.Command{ - Run: func(cmd *cobra.Command, args []string) { - cliui.Prompt(cmd, cliui.PromptOptions{ - Text: "Password:", - Secret: true, - }) - }, - } - cmd.ExecuteContext(context.Background()) -} diff --git a/cli/cliui/prompt_test.go b/cli/cliui/prompt_test.go index dc14925cc16bc..4cc756780aa62 100644 --- a/cli/cliui/prompt_test.go +++ b/cli/cliui/prompt_test.go @@ -2,12 +2,16 @@ package cliui_test import ( "context" + "os" + "os/exec" "testing" + "time" "github.com/spf13/cobra" "github.com/stretchr/testify/require" "github.com/coder/coder/cli/cliui" + "github.com/coder/coder/pty" "github.com/coder/coder/pty/ptytest" ) @@ -110,3 +114,57 @@ func newPrompt(ptty *ptytest.PTY, opts cliui.PromptOptions) (string, error) { cmd.SetIn(ptty.Input()) return value, cmd.ExecuteContext(context.Background()) } + +func TestPasswordTerminalState(t *testing.T) { + if os.Getenv("TEST_SUBPROCESS") == "1" { + passwordHelper() + return + } + t.Parallel() + + ptty := ptytest.New(t) + ptyWithFlags, ok := ptty.PTY.(pty.PTYWithFlags) + if !ok { + t.Skip("unable to check PTY local echo on this platform") + } + + cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") //nolint:gosec + cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1") + // connect the child process's stdio to the PTY directly, not via a pipe + cmd.Stdin = ptty.Input().Reader + cmd.Stdout = ptty.Output().Writer + cmd.Stderr = os.Stderr + err := cmd.Start() + require.NoError(t, err) + process := cmd.Process + defer process.Kill() + + ptty.ExpectMatch("Password: ") + time.Sleep(100 * time.Millisecond) // wait for child process to turn off echo and start reading input + + echo, err := ptyWithFlags.EchoEnabled() + require.NoError(t, err) + require.False(t, echo, "echo is on while reading password") + + err = process.Signal(os.Interrupt) + require.NoError(t, err) + _, err = process.Wait() + require.NoError(t, err) + + echo, err = ptyWithFlags.EchoEnabled() + require.NoError(t, err) + require.True(t, echo, "echo is off after reading password") +} + +// Platform-specific function to get the +func passwordHelper() { + cmd := &cobra.Command{ + Run: func(cmd *cobra.Command, args []string) { + cliui.Prompt(cmd, cliui.PromptOptions{ + Text: "Password:", + Secret: true, + }) + }, + } + cmd.ExecuteContext(context.Background()) +} diff --git a/pty/pty.go b/pty/pty.go index f4f2999d1f019..5e938f4922302 100644 --- a/pty/pty.go +++ b/pty/pty.go @@ -29,6 +29,15 @@ type PTY interface { Resize(height uint16, width uint16) error } +// PTYWithFlags represents a PTY whose flags can be inspected, in particular +// to determine whether local echo is enabled. +type PTYWithFlags interface { + PTY + + // EchoEnabled determines whether local echo is currently enabled for this terminal. + EchoEnabled() (bool, error) +} + // New constructs a new Pty. func New() (PTY, error) { return newPty() diff --git a/pty/pty_linux.go b/pty/pty_linux.go new file mode 100644 index 0000000000000..b18d801c228e8 --- /dev/null +++ b/pty/pty_linux.go @@ -0,0 +1,13 @@ +// go:build linux + +package pty + +import "golang.org/x/sys/unix" + +func (p *otherPty) EchoEnabled() (bool, error) { + termios, err := unix.IoctlGetTermios(int(p.pty.Fd()), unix.TCGETS) + if err != nil { + return false, err + } + return (termios.Lflag & unix.ECHO) != 0, nil +} diff --git a/pty/pty_windows.go b/pty/pty_windows.go index 0d6691b2e7481..3be5c974b84be 100644 --- a/pty/pty_windows.go +++ b/pty/pty_windows.go @@ -112,3 +112,12 @@ func (p *ptyWindows) Close() error { return nil } + +func (p *ptyWindows) EchoEnabled() (bool, error) { + var state uint32 + err := windows.GetConsoleMode(p.handle, &state) + if err != nil { + return err + } + return (state & windows.ENABLE_ECHO_INPUT) != 0 +} From dfa133cbb7140067ea228e55297a1cf7ba351014 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Tue, 17 May 2022 21:41:20 +0000 Subject: [PATCH 10/13] hopefully fix Windows build --- cli/cliui/prompt_test.go | 2 +- pty/pty.go | 4 ++-- pty/pty_windows.go | 11 +++++------ 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/cli/cliui/prompt_test.go b/cli/cliui/prompt_test.go index 4cc756780aa62..5d9929fdc1efa 100644 --- a/cli/cliui/prompt_test.go +++ b/cli/cliui/prompt_test.go @@ -123,7 +123,7 @@ func TestPasswordTerminalState(t *testing.T) { t.Parallel() ptty := ptytest.New(t) - ptyWithFlags, ok := ptty.PTY.(pty.PTYWithFlags) + ptyWithFlags, ok := ptty.PTY.(pty.WithFlags) if !ok { t.Skip("unable to check PTY local echo on this platform") } diff --git a/pty/pty.go b/pty/pty.go index 5e938f4922302..7a8fe6c99edb6 100644 --- a/pty/pty.go +++ b/pty/pty.go @@ -29,9 +29,9 @@ type PTY interface { Resize(height uint16, width uint16) error } -// PTYWithFlags represents a PTY whose flags can be inspected, in particular +// WithFlags represents a PTY whose flags can be inspected, in particular // to determine whether local echo is enabled. -type PTYWithFlags interface { +type WithFlags interface { PTY // EchoEnabled determines whether local echo is currently enabled for this terminal. diff --git a/pty/pty_windows.go b/pty/pty_windows.go index 3be5c974b84be..f1b4452be09a2 100644 --- a/pty/pty_windows.go +++ b/pty/pty_windows.go @@ -4,7 +4,6 @@ package pty import ( - "io" "os" "sync" "unsafe" @@ -67,14 +66,14 @@ type ptyWindows struct { closed bool } -func (p *ptyWindows) Output() io.ReadWriter { +func (p *ptyWindows) Output() ReadWriter { return ReadWriter{ Reader: p.outputRead, Writer: p.outputWrite, } } -func (p *ptyWindows) Input() io.ReadWriter { +func (p *ptyWindows) Input() ReadWriter { return ReadWriter{ Reader: p.inputRead, Writer: p.inputWrite, @@ -115,9 +114,9 @@ func (p *ptyWindows) Close() error { func (p *ptyWindows) EchoEnabled() (bool, error) { var state uint32 - err := windows.GetConsoleMode(p.handle, &state) + err := windows.GetConsoleMode(p.console, &state) if err != nil { - return err + return false, err } - return (state & windows.ENABLE_ECHO_INPUT) != 0 + return (state & windows.ENABLE_ECHO_INPUT) != 0, nil } From f2dce8b896f0d81af2bde3d31864266d6a7dedb1 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Tue, 17 May 2022 22:27:40 +0000 Subject: [PATCH 11/13] try again to fix Windows build --- pty/pty_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pty/pty_windows.go b/pty/pty_windows.go index f1b4452be09a2..1b1427c0ee277 100644 --- a/pty/pty_windows.go +++ b/pty/pty_windows.go @@ -114,7 +114,7 @@ func (p *ptyWindows) Close() error { func (p *ptyWindows) EchoEnabled() (bool, error) { var state uint32 - err := windows.GetConsoleMode(p.console, &state) + err := windows.GetConsoleMode(windows.Handle(p.inputRead.Fd()), &state) if err != nil { return false, err } From 4ee7ffd74a9f8b43aab252aa7b0ce47ff977fa9b Mon Sep 17 00:00:00 2001 From: David Wahler Date: Tue, 17 May 2022 22:37:19 +0000 Subject: [PATCH 12/13] remove support for EchoEnabled() on Windows --- pty/pty_windows.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pty/pty_windows.go b/pty/pty_windows.go index 1b1427c0ee277..93e58c4405772 100644 --- a/pty/pty_windows.go +++ b/pty/pty_windows.go @@ -111,12 +111,3 @@ func (p *ptyWindows) Close() error { return nil } - -func (p *ptyWindows) EchoEnabled() (bool, error) { - var state uint32 - err := windows.GetConsoleMode(windows.Handle(p.inputRead.Fd()), &state) - if err != nil { - return false, err - } - return (state & windows.ENABLE_ECHO_INPUT) != 0, nil -} From c59b6ef16dfa0920af4b659bc74fc3c65aa75795 Mon Sep 17 00:00:00 2001 From: David Wahler Date: Wed, 18 May 2022 15:08:50 +0000 Subject: [PATCH 13/13] fixup! remove errant comment --- cli/cliui/prompt_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/cliui/prompt_test.go b/cli/cliui/prompt_test.go index 5d9929fdc1efa..1926349c2d1fc 100644 --- a/cli/cliui/prompt_test.go +++ b/cli/cliui/prompt_test.go @@ -156,7 +156,6 @@ func TestPasswordTerminalState(t *testing.T) { require.True(t, echo, "echo is off after reading password") } -// Platform-specific function to get the func passwordHelper() { cmd := &cobra.Command{ Run: func(cmd *cobra.Command, args []string) {