From e770dddbf5e3084c939760c50ca84c1adee9c4c4 Mon Sep 17 00:00:00 2001
From: Dragos Vingarzan <vingarzan@gmail.com>
Date: Tue, 20 Aug 2024 18:25:39 +0200
Subject: [PATCH] x/term: disabling auto-completion around GetPassword()

Triggering the completion during password input might cause some
unintended behavior around handling of TAB, or maybe the auto-completion
functionality would review the secret input. Hence simply
disabling/re-enabling it around the t.readLine call.

Fixes #72736

Change-Id: I64270e8570086247247466afb2536b2581d6af25
Reviewed-on: https://go-review.googlesource.com/c/term/+/607115
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
---
 terminal.go      |  9 +++++++++
 terminal_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/terminal.go b/terminal.go
index f636667..14f8947 100644
--- a/terminal.go
+++ b/terminal.go
@@ -44,6 +44,8 @@ type Terminal struct {
 	// bytes, as an index into |line|). If it returns ok=false, the key
 	// press is processed normally. Otherwise it returns a replacement line
 	// and the new cursor position.
+	//
+	// This will be disabled during ReadPassword.
 	AutoCompleteCallback func(line string, pos int, key rune) (newLine string, newPos int, ok bool)
 
 	// Escape contains a pointer to the escape codes for this terminal.
@@ -692,6 +694,8 @@ func (t *Terminal) Write(buf []byte) (n int, err error) {
 
 // ReadPassword temporarily changes the prompt and reads a password, without
 // echo, from the terminal.
+//
+// The AutoCompleteCallback is disabled during this call.
 func (t *Terminal) ReadPassword(prompt string) (line string, err error) {
 	t.lock.Lock()
 	defer t.lock.Unlock()
@@ -699,6 +703,11 @@ func (t *Terminal) ReadPassword(prompt string) (line string, err error) {
 	oldPrompt := t.prompt
 	t.prompt = []rune(prompt)
 	t.echo = false
+	oldAutoCompleteCallback := t.AutoCompleteCallback
+	t.AutoCompleteCallback = nil
+	defer func() {
+		t.AutoCompleteCallback = oldAutoCompleteCallback
+	}()
 
 	line, err = t.readLine()
 
diff --git a/terminal_test.go b/terminal_test.go
index d5c1794..29dd874 100644
--- a/terminal_test.go
+++ b/terminal_test.go
@@ -396,6 +396,32 @@ func TestReadPasswordLineEnd(t *testing.T) {
 	}
 }
 
+func MockAutoCompleteCallback(line string, pos int, key rune) (newLine string, newPos int, ok bool) {
+	return "not-good", pos, true
+}
+
+func TestReadPasswordDisabledAutoCompleteCallback(t *testing.T) {
+	input := "testgood\ranother line\r"
+	expectedPassword := "testgood"
+	terminal := NewTerminal(
+		&MockTerminal{
+			toSend:       []byte(input),
+			bytesPerRead: 1,
+		},
+		"prompt")
+	terminal.AutoCompleteCallback = MockAutoCompleteCallback
+	password, err := terminal.ReadPassword("Password: ")
+	if err != nil {
+		t.Fatalf("failed to read password: %v", err)
+	}
+	if password != expectedPassword {
+		t.Fatalf("failed to read password, got %q", password)
+	}
+	if terminal.AutoCompleteCallback == nil {
+		t.Fatalf("AutoCompleteCallback should not be nil after ReadPassword")
+	}
+}
+
 func TestMakeRawState(t *testing.T) {
 	fd := int(os.Stdout.Fd())
 	if !IsTerminal(fd) {