fix: prevent partial login failures from becoming shell prompts

docs/login-stream.md

@@ -37,6 +37,25 @@ password. Custom `PasswordChangePrompt` and `FailurePrompt` still replace their
 respective defaults; their narrative matches obey the same completion rules.
 Other custom prompts remain full matches against the current normalized line.
 
+An incomplete line that matches a failure or password-change narrative is
+**pending**, not eligible for another prompt interpretation. For example,
+`Invalid user>` cannot become a successful shell prompt just because its CRLF
+arrives in the next read. Keep its text and login state unchanged until the
+line completes, reaches EOF/timeout, or grows into a non-diagnostic banner.
+This precedence applies to custom rules as well and prevents credential or
+continuation writes from overlapping patterns.
+
+The 512-byte limit is only the budget for unfinished prompts. A completed
+CRLF/LF line may use the logical-line budget (64 KiB), so longer custom prompts
+remain supported without increasing repeated partial-line scanning. Default
+shell prompts still have their separate 128-byte limit.
+
+Default shell detection recognizes a structured bracketed Unix prompt such as
+`[admin@login ~]$` or `[admin@router /var/log/error]#` before applying the banner
+word filter. User, host, and path components are not narrative labels. Plain
+labels such as `[Warning]#` and `[ERROR]%` remain excluded. Explicit failure
+and password-change rules retain precedence over shell detection.
+
 Overlong logical lines are ignored until a line reset or ending. They are not
 converted into a sliding suffix that could accidentally look like a prompt.
 
@@ -80,3 +99,9 @@ UTF-8 and ANSI fragments, long banners, environment subnegotiation logs, and
 cleanup writes. Fuzz targets check diagnostic-banner chunking and oversized
 subnegotiation recovery. `BenchmarkLoginLongBanner` measures the 1 KiB, 4 KiB,
 and 64 KiB banner paths without adding timing assertions to unit tests.
+
+Prompt regression tests additionally cover failure messages ending in `>`,
+separated CR/LF, overlapping custom rules, EOF/timeout, completed prompts above
+512 bytes, the 64 KiB line limit, and bracketed Unix prompt components. The
+`FuzzFailurePromptChunking` target checks that fragmenting an explicit failure
+never produces a usable client or extra authentication writes.

login.go

@@ -38,6 +38,11 @@ var (
 	defaultMarkedBracketShellPrompt = regexp.MustCompile(
 		`^\[[^\[\]\r\n]{1,126}\][#$%]$`,
 	)
+	// An explicit user@host structure disambiguates Unix prompt components
+	// from banner labels, even when the host or directory is named "error".
+	defaultUnixBracketShellPrompt = regexp.MustCompile(
+		`^\[[\pL\pN_.\-]+@[\pL\pN_.:\-]+(?:[ \t]+[^\[\]\r\n]+)?\][#$%]$`,
+	)
 	defaultTokenShellPrompt = regexp.MustCompile(
 		`^(?:[\pL\pN_.:@/\\~()\-]+(?:[ \t]+[\pL\pN_.:@/\\~()\-]+){0,2})?\s*[#$>%]$`,
 	)
@@ -178,15 +183,28 @@ func (m *loginMachine) messageResult(line []byte, trace *tracer) loginResult {
 }
 
 func (m *loginMachine) handleLine(writer io.Writer, line []byte, trace *tracer) (loginResult, error) {
+	// Completed lines are bounded by the logical-line limit, not the smaller
+	// partial-prompt budget. Custom prompts may legitimately exceed 512 bytes.
+	if len(line) > m.config.maxLoginBuffer {
+		return loginPending, nil
+	}
+	// A diagnostic can contain a legacy-encoded username. Do not discard
+	// an otherwise recognizable failure or password-change message just
+	// because that field is not UTF-8; a later shell prompt must not win.
 	if result := m.messageResult(line, trace); result != loginPending {
 		return result, nil
 	}
+	// Only interactive actions and success require valid UTF-8. Incomplete
+	// lines remain guarded by handlePartialLine until a real boundary.
+	if !utf8.Valid(line) {
+		return loginPending, nil
+	}
 	return m.handlePrompt(writer, line, trace)
 }
 
 // Interactive prompts normally have no newline. Check them only at the end of
 // the currently available chunk, never after every character of a banner.
-func (m *loginMachine) handlePrompt(writer io.Writer, line []byte, trace *tracer) (loginResult, error) {
+func (m *loginMachine) handlePartialLine(writer io.Writer, line []byte, trace *tracer) (loginResult, error) {
 	if len(line) > maxPartialPromptBytes || !utf8.Valid(line) {
 		return loginPending, nil
 	}
@@ -195,12 +213,31 @@ func (m *loginMachine) handlePrompt(writer io.Writer, line []byte, trace *tracer
 		return loginPending, nil
 	}
 
-	// Require input punctuation for an incomplete password-change prompt.
-	// "New password policy: ..." must not match the prefix "New password".
-	if (bytes.HasSuffix(line, []byte(":")) || bytes.HasSuffix(line, []byte("："))) &&
-		fullLineMatch(m.patterns.passwordChange, line) {
-		trace.printf("login password-change input prompt matched phase=%s", m.phase)
-		return loginPasswordChangeRequired, nil
+	if fullLineMatch(m.patterns.passwordChange, line) {
+		// Input prompts can be acted on immediately; narrative messages must
+		// wait for a real line ending, EOF, or the overall login timeout.
+		if bytes.HasSuffix(line, []byte(":")) || bytes.HasSuffix(line, []byte("：")) {
+			trace.printf("login password-change input prompt matched phase=%s", m.phase)
+			return loginPasswordChangeRequired, nil
+		}
+		return loginPending, nil
+	}
+	// Pending does not mean safe to treat as a prompt. For example, "Invalid
+	// user>" also resembles a shell prompt. Keep the entire text and state
+	// until it becomes a complete diagnostic or a longer, harmless banner.
+	// Apply this before *any* prompt action, including custom prompt rules.
+	if fullLineMatch(m.patterns.failure, line) {
+		return loginPending, nil
+	}
+	return m.handlePrompt(writer, line, trace)
+}
+
+// handlePrompt runs only after the caller has validated the appropriate line
+// limit and given diagnostic messages precedence over interactive actions.
+func (m *loginMachine) handlePrompt(writer io.Writer, line []byte, trace *tracer) (loginResult, error) {
+	line = bytes.TrimSpace(line)
+	if len(line) == 0 {
+		return loginPending, nil
 	}
 	if fullLineMatch(m.patterns.continuePrompt, line) {
 		if m.continueResponses >= maxContinueResponses {
@@ -261,6 +298,9 @@ func defaultShellPromptMatches(line []byte) bool {
 	if len(line) == 0 || len(line) > maxDefaultPromptBytes {
 		return false
 	}
+	if defaultUnixBracketShellPrompt.Match(line) {
+		return true
+	}
 	if isBannerLikeShellPrompt(string(line)) {
 		return false
 	}
@@ -376,7 +416,7 @@ func (m *loginMachine) consumeChunk(writer io.Writer, text *loginText, chunk []b
 		}
 	}
 
-	result, err := m.handlePrompt(writer, text.current(), trace)
+	result, err := m.handlePartialLine(writer, text.current(), trace)
 	ending := ""
 	if result == loginSuccess && text.pendingCR {
 		ending = "\r"

login_encoding_test.go

@@ -0,0 +1,136 @@
+package tclientlib
+
+import (
+	"bytes"
+	"errors"
+	"io"
+	"regexp"
+	"testing"
+	"time"
+)
+
+func TestLoginEncodingDiagnosticsBeforeUTF8Validation(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		message string
+		config  ClientConfig
+		want    error
+	}{
+		{"latin1-failure", "Login failed for user Andr\xe9.", ClientConfig{}, ErrLoginFailed},
+		{"malformed-failure", "Login failed for user Andr\xf0\x28\x8c\x28.", ClientConfig{}, ErrLoginFailed},
+		{"latin1-password-expiry", "The password for Andr\xe9 has expired.", ClientConfig{}, ErrPasswordChangeRequired},
+		{"custom-failure", "Rejected Andr\xe9>", ClientConfig{FailurePrompt: regexp.MustCompile(`^Rejected .+>$`)}, ErrLoginFailed},
+		{"custom-password-expiry", "Rotate Andr\xe9>", ClientConfig{PasswordChangePrompt: regexp.MustCompile(`^Rotate .+>$`)}, ErrPasswordChangeRequired},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			for _, ending := range []string{"\r\n", "\n", ""} {
+				wire := []byte("Password:\r\n" + tc.message + ending)
+				if ending != "" {
+					// Discarding a diagnostic must not let this later prompt win.
+					wire = append(wire, []byte("router#\r\nTAIL")...)
+				}
+				for split := 0; split <= len(wire); split++ {
+					conn := &streamTestConn{chunks: [][]byte{wire[:split], wire[split:]}, lastError: io.EOF}
+					config := tc.config
+					config.Password, config.Timeout = "secret", time.Second
+					client, err := NewClientConn(conn, &config)
+					if client != nil {
+						_ = client.Close()
+					}
+					if client != nil || !errors.Is(err, tc.want) {
+						t.Fatalf("ending=%q split=%d: client=%t err=%v, want %v", ending, split, client != nil, err, tc.want)
+					}
+					if !conn.closed.Load() || conn.writes.String() != "secret\r\n" {
+						t.Fatalf("ending=%q split=%d: closed=%t writes=%q", ending, split, conn.closed.Load(), conn.writes.String())
+					}
+				}
+			}
+		})
+	}
+}
+
+func TestLoginEncodingInvalidPromptsCannotTriggerActions(t *testing.T) {
+	for _, kind := range []string{"username", "password", "continue", "shell"} {
+		t.Run(kind, func(t *testing.T) {
+			config, err := prepareConfig(&ClientConfig{User: "admin", Password: "secret"})
+			if err != nil {
+				t.Fatal(err)
+			}
+			pattern := regexp.MustCompile(`^.+$`)
+			switch kind {
+			case "username":
+				config.UsernamePrompt = pattern
+			case "password":
+				config.PasswordPrompt = pattern
+			case "continue":
+				config.ContinuePrompt = pattern
+			case "shell":
+				config.ShellPrompt = pattern
+			}
+			machine := newLoginMachine(config)
+			var sent bytes.Buffer
+			for _, complete := range []bool{false, true} {
+				line := []byte("device\xe9>")
+				var result loginResult
+				if complete {
+					result, err = machine.handleLine(&sent, line, nil)
+				} else {
+					result, err = machine.handlePartialLine(&sent, line, nil)
+				}
+				if err != nil || result != loginPending || sent.Len() != 0 ||
+					machine.phase != loginAwaitPrompt || machine.continueResponses != 0 {
+					t.Fatalf("complete=%t: result=%v err=%v writes=%q phase=%v", complete, result, err, sent.Bytes(), machine.phase)
+				}
+			}
+		})
+	}
+}
+
+func TestLoginEncodingNonDiagnosticBannerAndUTF8Prompt(t *testing.T) {
+	// Encoding errors alone are not authentication errors. The valid prompt
+	// contains a multibyte character that can also be split across reads.
+	wire := []byte("Device Andr\xe9 ready\r\nPassword for André:\r\n[admin@login ~]$\r\nTAIL")
+	for split := 0; split <= len(wire); split++ {
+		conn := &streamTestConn{chunks: [][]byte{wire[:split], wire[split:]}, lastError: io.EOF}
+		client, err := NewClientConn(conn, &ClientConfig{Password: "secret", Timeout: time.Second})
+		if err != nil {
+			t.Fatalf("split=%d: %v", split, err)
+		}
+		tail, readErr := io.ReadAll(client)
+		_ = client.Close()
+		if readErr != nil || string(tail) != "\r\nTAIL" || conn.writes.String() != "secret\r\n" {
+			t.Fatalf("split=%d: tail=%q writes=%q err=%v", split, tail, conn.writes.String(), readErr)
+		}
+	}
+}
+
+func FuzzLoginDiagnosticEncoding(f *testing.F) {
+	f.Add(uint8(0), uint16(1), []byte{0xe9})
+	f.Add(uint8(1), uint16(4096), []byte{0xc0, 0x80})
+	f.Fuzz(func(t *testing.T, choice uint8, size uint16, raw []byte) {
+		if len(raw) > 32 {
+			raw = raw[:32]
+		}
+		// Keep the diagnostic structure intact while varying invalid
+		// UTF-8 continuation bytes. Exclude IAC and terminal control bytes.
+		name := []byte("Andr")
+		for _, value := range raw {
+			name = append(name, 0x80+value%0x40)
+		}
+		message := "Login failed for user " + string(name) + "."
+		want := ErrLoginFailed
+		if choice%2 != 0 {
+			message = "The password for " + string(name) + " has expired."
+			want = ErrPasswordChangeRequired
+		}
+		wire := []byte("Password:\r\n" + message + "\r\nrouter#")
+		conn := &streamTestConn{chunks: streamChunks(wire, int(size)%4096+1), lastError: io.EOF}
+		client, err := NewClientConn(conn, &ClientConfig{Password: "secret", Timeout: time.Second})
+		if client != nil {
+			_ = client.Close()
+		}
+		if client != nil || !errors.Is(err, want) || conn.writes.String() != "secret\r\n" {
+			t.Fatalf("client=%t err=%v want=%v writes=%q", client != nil, err, want, conn.writes.String())
+		}
+	})
+}