8000 bug: Don't try to handle SIGINT when prompting for passwords by dwahler · Pull Request #1498 · coder/coder · GitHub
[go: up one dir, main page]

Skip to content

bug: Don't try to handle SIGINT when prompting for passwords #1498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
May 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions cli/cliui/prompt.go
defer signal.Stop(interrupt)
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,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)

errCh := make(chan error, 1)
lineCh := make(chan string)
Expand All @@ -45,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')

Expand Down
57 changes: 57 additions & 0 deletions cli/cliui/prompt_test.go
10000
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@ package cliui_test

import (
"context"
"os& 8000 quot;
"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"
)

Expand Down Expand Up @@ -110,3 +114,56 @@ 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.WithFlags)
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")
}

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())
}
31 changes: 26 additions & 5 deletions pty/pty.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package pty

import (
"io"
"os"
)

// PTY is a minimal interface for interacting with a TTY.
Expand All @@ -14,26 +15,46 @@ 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.
//
// cmd.SetInput(pty.Input()) would be used to specify a command
// 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
}

// WithFlags represents a PTY whose flags can be inspected, in particular
// to determine whether local echo is enabled.
type WithFlags 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()
}

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)
}
13 changes: 13 additions & 0 deletions
Original file line number Diff line number Diff line change
@@ -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
}
9 changes: 4 additions & 5 deletions pty/pty_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package pty

import (
"io"
"os"
"sync"

Expand All @@ -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,
}
Expand Down
9 changes: 4 additions & 5 deletions pty/pty_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package pty

import (
"io"
"os"
"sync"
"unsafe"
Expand Down Expand Up @@ -67,15 +66,15 @@ type ptyWindows struct {
closed bool
}

func (p *ptyWindows) Output() io.ReadWriter {
return readWriter{
func (p *ptyWindows) Output() ReadWriter {
return ReadWriter{
Reader: p.outputRead,
Writer: p.outputWrite,
}
}

func (p *ptyWindows) Input() io.ReadWriter {
return readWriter{
func (p *ptyWindows) Input() ReadWriter {
return ReadWriter{
Reader: p.inputRead,
Writer: p.inputWrite,
}
Expand Down
0