Skip to content

Commit 9922240

Browse files
authored
feat: enable masking password inputs instead of blocking echo (coder#17469)
Closes coder#17059
1 parent 614a7d0 commit 9922240

File tree

4 files changed

+116
-26
lines changed

4 files changed

+116
-26
lines changed

cli/cliui/prompt.go

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,29 @@
11
package cliui
22

33
import (
4+
"bufio"
45
"bytes"
56
"encoding/json"
67
"fmt"
78
"io"
89
"os"
910
"os/signal"
1011
"strings"
12+
"unicode"
1113

12-
"github.com/bgentry/speakeasy"
1314
"github.com/mattn/go-isatty"
1415
"golang.org/x/xerrors"
1516

17+
"github.com/coder/coder/v2/pty"
1618
"github.com/coder/pretty"
1719
"github.com/coder/serpent"
1820
)
1921

2022
// PromptOptions supply a set of options to the prompt.
2123
type PromptOptions struct {
22-
Text string
23-
Default string
24+
Text string
25+
Default string
26+
// When true, the input will be masked with asterisks.
2427
Secret bool
2528
IsConfirm bool
2629
Validate func(string) error
@@ -88,14 +91,13 @@ func Prompt(inv *serpent.Invocation, opts PromptOptions) (string, error) {
8891
var line string
8992
var err error
9093

94+
signal.Notify(interrupt, os.Interrupt)
95+
defer signal.Stop(interrupt)
96+
9197
inFile, isInputFile := inv.Stdin.(*os.File)
9298
if opts.Secret && isInputFile && isatty.IsTerminal(inFile.Fd()) {
93-
// we don't install a signal handler here because speakeasy has its own
94-
line, err = speakeasy.Ask("")
99+
line, err = readSecretInput(inFile, inv.Stdout)
95100
} else {
96-
signal.Notify(interrupt, os.Interrupt)
97-
defer signal.Stop(interrupt)
98-
99101
line, err = readUntil(inv.Stdin, '\n')
100102

101103
// Check if the first line beings with JSON object or array chars.
@@ -204,3 +206,58 @@ func readUntil(r io.Reader, delim byte) (string, error) {
204206
}
205207
}
206208
}
209+
210+
// readSecretInput reads secret input from the terminal rune-by-rune,
211+
// masking each character with an asterisk.
212+
func readSecretInput(f *os.File, w io.Writer) (string, error) {
213+
// Put terminal into raw mode (no echo, no line buffering).
214+
oldState, err := pty.MakeInputRaw(f.Fd())
215+
if err != nil {
216+
return "", err
217+
}
218+
defer func() {
219+
_ = pty.RestoreTerminal(f.Fd(), oldState)
220+
}()
221+
222+
reader := bufio.NewReader(f)
223+
var runes []rune
224+
225+
for {
226+
r, _, err := reader.ReadRune()
227+
if err != nil {
228+
return "", err
229+
}
230+
231+
switch {
232+
case r == '\r' || r == '\n':
233+
// Finish on Enter
234+
if _, err := fmt.Fprint(w, "\r\n"); err != nil {
235+
return "", err
236+
}
237+
return string(runes), nil
238+
239+
case r == 3:
240+
// Ctrl+C
241+
return "", ErrCanceled
242+
243+
case r == 127 || r == '\b':
244+
// Backspace/Delete: remove last rune
245+
if len(runes) > 0 {
246+
// Erase the last '*' on the screen
247+
if _, err := fmt.Fprint(w, "\b \b"); err != nil {
248+
return "", err
249+
}
250+
runes = runes[:len(runes)-1]
251+
}
252+
253+
default:
254+
// Only mask printable, non-control runes
255+
if !unicode.IsControl(r) {
256+
runes = append(runes, r)
257+
if _, err := fmt.Fprint(w, "*"); err != nil {
258+
return "", err
259+
}
260+
}
261+
}
262+
}
263+
}

cli/cliui/prompt_test.go

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ import (
66
"io"
77
"os"
88
"os/exec"
9+
"runtime"
910
"testing"
1011

1112
"github.com/stretchr/testify/assert"
1213
"github.com/stretchr/testify/require"
1314
"golang.org/x/xerrors"
1415

1516
"github.com/coder/coder/v2/cli/cliui"
16-
"github.com/coder/coder/v2/pty"
1717
"github.com/coder/coder/v2/pty/ptytest"
1818
"github.com/coder/coder/v2/testutil"
1919
"github.com/coder/serpent"
@@ -181,6 +181,48 @@ func TestPrompt(t *testing.T) {
181181
resp := testutil.TryReceive(ctx, t, doneChan)
182182
require.Equal(t, "valid", resp)
183183
})
184+
185+
t.Run("MaskedSecret", func(t *testing.T) {
186+
t.Parallel()
187+
ctx := testutil.Context(t, testutil.WaitShort)
188+
ptty := ptytest.New(t)
189+
doneChan := make(chan string)
190+
go func() {
191+
resp, err := newPrompt(ctx, ptty, cliui.PromptOptions{
192+
Text: "Password:",
193+
Secret: true,
194+
}, nil)
195+
assert.NoError(t, err)
196+
doneChan <- resp
197+
}()
198+
ptty.ExpectMatch("Password: ")
199+
200+
ptty.WriteLine("test")
201+
202+
resp := testutil.TryReceive(ctx, t, doneChan)
203+
require.Equal(t, "test", resp)
204+
})
205+
206+
t.Run("UTF8Password", func(t *testing.T) {
207+
t.Parallel()
208+
ctx := testutil.Context(t, testutil.WaitShort)
209+
ptty := ptytest.New(t)
210+
doneChan := make(chan string)
211+
go func() {
212+
resp, err := newPrompt(ctx, ptty, cliui.PromptOptions{
213+
Text: "Password:",
214+
Secret: true,
215+
}, nil)
216+
assert.NoError(t, err)
217+
doneChan <- resp
218+
}()
219+
ptty.ExpectMatch("Password: ")
220+
221+
ptty.WriteLine("和製漢字")
222+
223+
resp := testutil.TryReceive(ctx, t, doneChan)
224+
require.Equal(t, "和製漢字", resp)
225+
})
184226
}
185227

186228
func newPrompt(ctx context.Context, ptty *ptytest.PTY, opts cliui.PromptOptions, invOpt func(inv *serpent.Invocation)) (string, error) {
@@ -209,13 +251,12 @@ func TestPasswordTerminalState(t *testing.T) {
209251
passwordHelper()
210252
return
211253
}
254+
if runtime.GOOS == "windows" {
255+
t.Skip("Skipping on windows. PTY doesn't read ptty.Write correctly.")
256+
}
212257
t.Parallel()
213258

214259
ptty := ptytest.New(t)
215-
ptyWithFlags, ok := ptty.PTY.(pty.WithFlags)
216-
if !ok {
217-
t.Skip("unable to check PTY local echo on this platform")
218-
}
219260

220261
cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") //nolint:gosec
221262
cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1")
@@ -229,21 +270,16 @@ func TestPasswordTerminalState(t *testing.T) {
229270
defer process.Kill()
230271

231272
ptty.ExpectMatch("Password: ")
232-
233-
require.Eventually(t, func() bool {
234-
echo, err := ptyWithFlags.EchoEnabled()
235-
return err == nil && !echo
236-
}, testutil.WaitShort, testutil.IntervalMedium, "echo is on while reading password")
273+
ptty.Write('t')
274+
ptty.Write('e')
275+
ptty.Write('s')
276+
ptty.Write('t')
277+
ptty.ExpectMatch("****")
237278

238279
err = process.Signal(os.Interrupt)
239280
require.NoError(t, err)
240281
_, err = process.Wait()
241282
require.NoError(t, err)
242-
243-
require.Eventually(t, func() bool {
244-
echo, err := ptyWithFlags.EchoEnabled()
245-
return err == nil && echo
246-
}, testutil.WaitShort, testutil.IntervalMedium, "echo is off after reading password")
247283
}
248284

249285
// nolint:unused

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ require (
8383
github.com/armon/circbuf v0.0.0-20190214190532-5111143e8da2
8484
github.com/awalterschulze/gographviz v2.0.3+incompatible
8585
github.com/aws/smithy-go v1.22.3
86-
github.com/bgentry/speakeasy v0.2.0
8786
github.com/bramvdbogaerde/go-scp v1.5.0
8887
github.com/briandowns/spinner v1.23.0
8988
github.com/cakturk/go-netstat v0.0.0-20200220111822-e5b49efee7a5

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -815,8 +815,6 @@ github.com/bep/tmc v0.5.1/go.mod h1:tGYHN8fS85aJPhDLgXETVKp+PR382OvFi2+q2GkGsq0=
815815
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d h1:xDfNPAt8lFiC1UJrqV3uuy861HCTo708pDMbjHHdCas=
816816
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d/go.mod h1:6QX/PXZ00z/TKoufEY6K/a0k6AhaJrQKdFe6OfVXsa4=
817817
github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
818-
github.com/bgentry/speakeasy v0.2.0 h1:tgObeVOf8WAvtuAX6DhJ4xks4CFNwPDZiqzGqIHE51E=
819-
github.com/bgentry/speakeasy v0.2.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
820818
github.com/bmatcuk/doublestar/v4 v4.8.1 h1:54Bopc5c2cAvhLRAzqOGCYHYyhcDHsFF4wWIR5wKP38=
821819
github.com/bmatcuk/doublestar/v4 v4.8.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
822820
github.com/bool64/shared v0.1.5 h1:fp3eUhBsrSjNCQPcSdQqZxxh9bBwrYiZ+zOKFkM0/2E=

0 commit comments

Comments
 (0)