fix(telnet): stop Mudlet masking all input for the whole session (#633)

* fix(telnet): stop Mudlet masking all input for the whole session

Mudlet (and other local-echo clients) treat telnet WILL ECHO as a
"mask this field" hint rather than a server-echo request. GoMud asserted
WILL ECHO unconditionally at connect and never withdrew it, so Mudlet
masked every keystroke for the entire session.

Detect the client type at connect via an MNES NEW-ENVIRON probe and
branch the echo behavior:

- Mudlet: baseline WONT ECHO (Mudlet echoes locally); the server no
  longer echoes/masks per character or emits its own newline. Password
  prompts are masked by transiently asserting WILL ECHO and withdrawing
  it (WONT ECHO) once the step validates.
- Raw telnet: unchanged - WILL ECHO baseline with server-side echo and
  mask-character passwords.
- Web client: unchanged - TEXTMASK toggling, no server-side per-char echo.

Detection is synchronous and bounded (200ms per read, 2s overall); a
client that ignores NEW-ENVIRON falls back to non-Mudlet and still logs
in. AI connections skip the probe and keep the historical baseline.

Changes:
- connections.ClientSettings: add IsMudlet / DetectionComplete.
- ConnectionDetails.SetReadDeadline for the bounded probe reads.
- term: NEW-ENVIRON sub-negotiation codes, request/response matchers,
  and a TelnetRequestMNESVars helper.
- TelnetIACHandler: negotiate NEW-ENVIRON and record IsMudlet from
  CLIENT_NAME, plus a unit test for the response parser.
- main.go: detectClientType probe and echo-baseline branch.
- login_prompt_handler: skip server echo/mask and newline for local-echo
  clients; toggle ECHO masking around Mudlet password steps.

* term: alias NEW-ENVIRON codes to existing telnet constants

Address review feedback on PR #633: the NEW-ENVIRON sub-negotiation
codes share byte values with telnet option constants already defined in
this block, so alias them instead of re-declaring literals. Values are
unchanged (IS/VAR=0, SEND/VALUE=1, INFO/ESC=2, USERVAR=3), so behavior
is identical.

* telnet: offer EOR and broaden NEW-ENVIRON detection for Mudlet masking

Real-Mudlet testing showed password masking half-working: Mudlet honored
WILL ECHO (command echo suppressed) but did not put asterisks on the
input line. Aligns the connect/detection sequence with a known-working
downstream fork:

- Offer WILL EOR at connect. Mudlet prefers EOR over GA to delimit
  prompts; with SUPPRESS-GO-AHEAD set and no EOR, Mudlet has no prompt
  anchor for its input-line masking.
- Request ALL NEW-ENVIRON variables (empty SEND) instead of naming
  specific ones - broadly compatible; Mudlet returns CLIENT_NAME among
  them.
- Match the NEW-ENVIRON IS response on the 'IAC SB NEW-ENVIRON IS'
  prefix alone (tolerant of TCP segmentation / trailing bytes), and make
  the parser treat IAC as a name/value terminator so a trailing IAC SE
  never bleeds into the final variable's value.

Author: MorquinDevlar

internal/connections/clientsettings.go

@@ -12,6 +12,14 @@ type ClientSettings struct {
 	// Is MSP enabled?
 	MSPEnabled        bool // Do they accept sound in their client?
 	SendTelnetGoAhead bool // Defaults false, should we send a IAC GA after prompts?
+	// IsMudlet is true when the client identified itself as Mudlet (via the MNES
+	// NEW-ENVIRON CLIENT_NAME variable). Mudlet echoes input locally and treats
+	// the telnet ECHO option purely as a password-masking hint, so it must not
+	// receive server-side echo.
+	IsMudlet bool
+	// DetectionComplete is set once the connect-time client-type probe has
+	// resolved (either a NEW-ENVIRON reply arrived or the probe timed out).
+	DetectionComplete bool
 }
 
 func (c ClientSettings) IsMsp() bool {

internal/connections/connectiondetails.go

@@ -324,6 +324,17 @@ func (cd *ConnectionDetails) Read(p []byte) (n int, err error) {
 	return cd.conn.Read(p)
 }
 
+// SetReadDeadline bounds how long the next Read may block. It only applies to
+// the raw telnet path (net.Conn); SSH and WebSocket connections do not use the
+// connect-time detection probe, so this is a no-op for them. Pass the zero time
+// to clear a previously-set deadline.
+func (cd *ConnectionDetails) SetReadDeadline(t time.Time) error {
+	if cd.conn != nil {
+		return cd.conn.SetReadDeadline(t)
+	}
+	return nil
+}
+
 func (cd *ConnectionDetails) Close() {
 	if cd.heartbeat != nil {
 		cd.heartbeat.stop()

internal/inputhandlers/login_prompt_handler.go

@@ -204,31 +204,41 @@ func CreatePromptHandler(steps []*PromptStep, onComplete CompletionFunc) connect
 				// Handle printable characters
 				//clientInput.Buffer = append(clientInput.Buffer, clientInput.DataIn...)
 
-				// Echo or Mask
-				if currentStep.MaskInput {
-
-					// Cache the mask template string if needed
-					if state.maskTemplate == "" && currentStep.MaskTemplate != "" {
-
-						if maskStr, err := templates.Process(currentStep.MaskTemplate, nil); err != nil {
-							mudlog.Error("Mask template error", "template", currentStep.MaskTemplate, "error", err)
-							state.maskTemplate = "*" // Fallback mask
-						} else {
-							state.maskTemplate = templates.AnsiParse(maskStr)
+				// Local-echo clients (Mudlet, web client) echo input themselves,
+				// so the server must not echo or emit mask characters per byte -
+				// doing so would double every character. Their masking is handled
+				// out-of-band (telnet ECHO toggling for Mudlet, TEXTMASK for the
+				// web client). Only raw telnet relies on this server-side echo.
+				cs := connections.GetClientSettings(clientInput.ConnectionId)
+				isLocalEcho := cs.IsMudlet || connections.IsWebsocket(clientInput.ConnectionId)
+
+				if !isLocalEcho {
+					// Echo or Mask
+					if currentStep.MaskInput {
+
+						// Cache the mask template string if needed
+						if state.maskTemplate == "" && currentStep.MaskTemplate != "" {
+
+							if maskStr, err := templates.Process(currentStep.MaskTemplate, nil); err != nil {
+								mudlog.Error("Mask template error", "template", currentStep.MaskTemplate, "error", err)
+								state.maskTemplate = "*" // Fallback mask
+							} else {
+								state.maskTemplate = templates.AnsiParse(maskStr)
+							}
+
+						} else if state.maskTemplate == "" {
+							state.maskTemplate = "*" // Default fallback if no template specified
 						}
 
-					} else if state.maskTemplate == "" {
-						state.maskTemplate = "*" // Default fallback if no template specified
-					}
+						// Send mask character(s)
+						for i := 0; i < len(clientInput.DataIn); i++ {
+							connections.SendTo([]byte(state.maskTemplate), clientInput.ConnectionId)
+						}
 
-					// Send mask character(s)
-					for i := 0; i < len(clientInput.DataIn); i++ {
-						connections.SendTo([]byte(state.maskTemplate), clientInput.ConnectionId)
+					} else {
+						// Echo input directly
+						connections.SendTo(clientInput.DataIn, clientInput.ConnectionId)
 					}
-
-				} else {
-					// Echo input directly
-					connections.SendTo(clientInput.DataIn, clientInput.ConnectionId)
 				}
 
 			}
@@ -258,7 +268,12 @@ func CreatePromptHandler(steps []*PromptStep, onComplete CompletionFunc) connect
 		}
 
 		// Enter Pressed: Process Input
-		connections.SendTo(term.CRLF, clientInput.ConnectionId) // Echo newline
+		// Mudlet echoes the submitted line (and its newline) locally, so a
+		// server-sent CRLF here would produce a blank line. Raw telnet
+		// (server-side echo) and the web client still need it.
+		if !connections.GetClientSettings(clientInput.ConnectionId).IsMudlet {
+			connections.SendTo(term.CRLF, clientInput.ConnectionId) // Echo newline
+		}
 		submittedInput := strings.TrimSpace(string(clientInput.Buffer))
 		clientInput.Buffer = clientInput.Buffer[:0] // Clear buffer for next input
 		state.maskTemplate = ""                     // Clear cached mask template
@@ -309,6 +324,14 @@ func CreatePromptHandler(steps []*PromptStep, onComplete CompletionFunc) connect
 			mudlog.Debug("Prompt Step Success", "step", currentStep.ID, "value", validatedValue, "connectionId", clientInput.ConnectionId)
 		}
 
+		// A masked Mudlet step just passed: withdraw the telnet ECHO mask so
+		// input is visible again. If the next step is also masked, sendPrompt
+		// re-asserts WILL ECHO; if this was the last step, this restores normal
+		// echo before gameplay begins.
+		if currentStep.MaskInput && connections.GetClientSettings(clientInput.ConnectionId).IsMudlet {
+			connections.SendTo(term.TelnetWONT(term.TELNET_OPT_ECHO), clientInput.ConnectionId)
+		}
+
 		// Advance to Next Step or Complete
 		if advanceAndSendPrompt(state, clientInput) {
 
@@ -367,6 +390,14 @@ func sendPrompt(step *PromptStep, clientInput *connections.ClientInput, results
 		connections.SendTo([]byte(maskCmd), clientInput.ConnectionId)
 	}
 
+	// Mudlet reads telnet WILL ECHO as "mask this field". Assert it for masked
+	// steps (passwords) so Mudlet hides the input; it is withdrawn (WONT ECHO)
+	// once the step validates. Sent before the prompt text so masking is active
+	// the moment the prompt appears.
+	if step.MaskInput && connections.GetClientSettings(clientInput.ConnectionId).IsMudlet {
+		connections.SendTo(term.TelnetWILL(term.TELNET_OPT_ECHO), clientInput.ConnectionId)
+	}
+
 	connections.SendTo([]byte(parsedPrompt), clientInput.ConnectionId)
 }
 

internal/inputhandlers/term_iac.go

@@ -1,6 +1,8 @@
 package inputhandlers
 
 import (
+	"strings"
+
 	"github.com/GoMudEngine/GoMud/internal/configs"
 	"github.com/GoMudEngine/GoMud/internal/connections"
 	"github.com/GoMudEngine/GoMud/internal/mudlog"
@@ -148,6 +150,44 @@ func TelnetIACHandler(clientInput *connections.ClientInput, sharedState map[stri
 			continue
 		}
 
+		//
+		// NEW-ENVIRON / MNES client detection (see handleTelnetConnection in main.go).
+		//
+
+		// Client agreed to NEW-ENVIRON: ask it to send all of its variables.
+		// (Requesting all is more broadly compatible than naming specific vars;
+		// Mudlet replies with CLIENT_NAME among them.)
+		if ok, _ := term.Matches(iacCmd, term.TelnetWillNewEnviron); ok {
+			mudlog.Debug("Received", "type", "IAC (WILL NEW-ENVIRON)")
+			connections.SendTo(term.TelnetNewEnvironSendRequest.BytesWithPayload(nil), clientInput.ConnectionId)
+			continue
+		}
+
+		// Client refused NEW-ENVIRON: it won't identify itself this way, so
+		// treat detection as complete (and not Mudlet).
+		if ok, _ := term.Matches(iacCmd, term.TelnetWontNewEnviron); ok {
+			mudlog.Debug("Received", "type", "IAC (WONT NEW-ENVIRON)")
+
+			cs := connections.GetClientSettings(clientInput.ConnectionId)
+			cs.DetectionComplete = true
+			connections.OverwriteClientSettings(clientInput.ConnectionId, cs)
+
+			continue
+		}
+
+		// Client sent its NEW-ENVIRON variables. Scan for CLIENT_NAME=MUDLET.
+		if ok, payload := term.Matches(iacCmd, term.TelnetNewEnvironResponse); ok {
+			isMudlet := newEnvironIsMudlet(payload)
+			mudlog.Debug("Received", "type", "IAC (NEW-ENVIRON IS)", "isMudlet", isMudlet, "data", term.BytesString(payload))
+
+			cs := connections.GetClientSettings(clientInput.ConnectionId)
+			cs.IsMudlet = isMudlet
+			cs.DetectionComplete = true
+			connections.OverwriteClientSettings(clientInput.ConnectionId, cs)
+
+			continue
+		}
+
 		// Unhanlded IAC command, log it
 		mudlog.Debug("Received", "type", "IAC (Unhandled)", "size", len(clientInput.DataIn), "data", term.TelnetCommandToString(iacCmd))
 
@@ -156,3 +196,52 @@ func TelnetIACHandler(clientInput *connections.ClientInput, sharedState map[stri
 	// We handled it, so don't pass it on
 	return false
 }
+
+// newEnvironIsMudlet walks a NEW-ENVIRON IS payload looking for the MNES
+// CLIENT_NAME variable. It returns true when CLIENT_NAME equals "MUDLET"
+// (case-insensitive). The payload is a sequence of VAR/USERVAR name segments,
+// each optionally followed by a VALUE segment; segments are delimited by the
+// VAR(0)/VALUE(1)/USERVAR(3) control bytes. IAC (255) is also treated as a
+// terminator so a trailing `IAC SE` left in the payload by the lenient matcher
+// never bleeds into the final variable's value.
+func newEnvironIsMudlet(payload []byte) bool {
+	isControl := func(b byte) bool {
+		return b == term.TELNET_NEWENV_VAR || b == term.TELNET_NEWENV_VALUE ||
+			b == term.TELNET_NEWENV_USERVAR || b == term.TELNET_IAC
+	}
+
+	i := 0
+	for i < len(payload) {
+		code := payload[i]
+		i++
+
+		// Only VAR / USERVAR start a named variable.
+		if code != term.TELNET_NEWENV_VAR && code != term.TELNET_NEWENV_USERVAR {
+			continue
+		}
+
+		// Read the variable name up to the next control byte.
+		nameStart := i
+		for i < len(payload) && !isControl(payload[i]) {
+			i++
+		}
+		name := string(payload[nameStart:i])
+
+		// An optional VALUE segment follows.
+		value := ""
+		if i < len(payload) && payload[i] == term.TELNET_NEWENV_VALUE {
+			i++
+			valueStart := i
+			for i < len(payload) && !isControl(payload[i]) {
+				i++
+			}
+			value = string(payload[valueStart:i])
+		}
+
+		if name == "CLIENT_NAME" && strings.EqualFold(value, "MUDLET") {
+			return true
+		}
+	}
+
+	return false
+}

internal/inputhandlers/term_iac_test.go

@@ -0,0 +1,92 @@
+package inputhandlers
+
+import (
+	"testing"
+
+	"github.com/GoMudEngine/GoMud/internal/term"
+)
+
+// newEnvironPayload builds the bytes that appear between `IAC SB NEW-ENVIRON IS`
+// and the trailing `IAC SE` for a sequence of VAR/VALUE pairs.
+func newEnvironPayload(pairs ...[2]string) []byte {
+	out := []byte{}
+	for _, p := range pairs {
+		out = append(out, term.TELNET_NEWENV_VAR)
+		out = append(out, []byte(p[0])...)
+		out = append(out, term.TELNET_NEWENV_VALUE)
+		out = append(out, []byte(p[1])...)
+	}
+	return out
+}
+
+func TestNewEnvironIsMudlet(t *testing.T) {
+	tests := []struct {
+		name    string
+		payload []byte
+		want    bool
+	}{
+		{
+			name:    "mudlet with version",
+			payload: newEnvironPayload([2]string{"CLIENT_NAME", "Mudlet"}, [2]string{"CLIENT_VERSION", "4.17.2"}),
+			want:    true,
+		},
+		{
+			name:    "mudlet uppercase value",
+			payload: newEnvironPayload([2]string{"CLIENT_NAME", "MUDLET"}),
+			want:    true,
+		},
+		{
+			name:    "client name not first",
+			payload: newEnvironPayload([2]string{"CLIENT_VERSION", "4.17.2"}, [2]string{"CLIENT_NAME", "Mudlet"}),
+			want:    true,
+		},
+		{
+			name:    "different client",
+			payload: newEnvironPayload([2]string{"CLIENT_NAME", "TinTin++"}),
+			want:    false,
+		},
+		{
+			name:    "client name without value",
+			payload: append([]byte{term.TELNET_NEWENV_VAR}, []byte("CLIENT_NAME")...),
+			want:    false,
+		},
+		{
+			name:    "empty payload",
+			payload: []byte{},
+			want:    false,
+		},
+		{
+			name:    "uservar segment ignored, var matched",
+			payload: append(append([]byte{term.TELNET_NEWENV_USERVAR}, []byte("FOO")...), newEnvironPayload([2]string{"CLIENT_NAME", "Mudlet"})...),
+			want:    true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := newEnvironIsMudlet(tc.payload); got != tc.want {
+				t.Errorf("newEnvironIsMudlet() = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+// TestNewEnvironResponseMatcher verifies the term matcher extracts the payload
+// that newEnvironIsMudlet expects from a full client response frame, including
+// the trailing IAC SE terminator that the lenient matcher leaves in the payload.
+func TestNewEnvironResponseMatcher(t *testing.T) {
+	// IAC SB NEW-ENVIRON IS <vars> IAC SE
+	frame := append(
+		term.TelnetNewEnvironResponse.BytesWithPayload(newEnvironPayload([2]string{"CLIENT_NAME", "Mudlet"})),
+		term.TELNET_IAC, term.TELNET_SE,
+	)
+
+	ok, payload := term.Matches(frame, term.TelnetNewEnvironResponse)
+	if !ok {
+		t.Fatalf("expected frame to match TelnetNewEnvironResponse")
+	}
+	// The trailing IAC SE remains in payload; the parser must still detect Mudlet.
+	if !newEnvironIsMudlet(payload) {
+		t.Errorf("expected extracted payload to be detected as Mudlet, payload=%v", payload)
+	}
+}

internal/term/telnet.go

@@ -132,6 +132,19 @@ const (
 	TELNET_OPT_PRAGMA_HEARTBEAT  IACByte = 140 // TELOPT PRAGMA HEARTBEAT									RFC:
 	TELNET_OPT_254               IACByte = 254
 	TELNET_OPT_EXTENDED_OPT      IACByte = 255 // Extended-Options-List										RFC: https://www.ietf.org/rfc/rfc861.txt
+
+	// NEW-ENVIRON (option 39) sub-negotiation codes.								RFC: https://www.ietf.org/rfc/rfc1572.txt
+	// Used by the MNES (Mud New-Environ Standard) handshake that we use to detect
+	// Mudlet (and other clients that advertise CLIENT_NAME). Their byte values
+	// coincide with telnet option codes already defined above, so they are aliased
+	// to those constants instead of being re-declared as literals.
+	TELNET_NEWENV_IS      = TELNET_OPT_TXBIN      // 0: Client -> Server: here are my variables
+	TELNET_NEWENV_SEND    = TELNET_OPT_ECHO       // 1: Server -> Client: please send these variables
+	TELNET_NEWENV_INFO    = TELNET_OPT_RECONN     // 2: Client -> Server: a variable changed
+	TELNET_NEWENV_VAR     = TELNET_OPT_TXBIN      // 0: A well-known variable name follows
+	TELNET_NEWENV_VALUE   = TELNET_NEWENV_SEND    // 1: A variable value follows
+	TELNET_NEWENV_ESC     = TELNET_NEWENV_INFO    // 2: Escape the next byte (it is data, not a code)
+	TELNET_NEWENV_USERVAR = TELNET_OPT_SUP_GO_AHD // 3: A user-defined variable name follows
 )
 
 // https://users.cs.cf.ac.uk/Dave.Marshall/Internet/node142.html

internal/term/term.go

@@ -80,6 +80,26 @@ var (
 	// Go Ahead
 	TelnetGoAhead = TerminalCommand{[]byte{TELNET_IAC, TELNET_GA}, []byte{}}
 
+	//
+	// NEW-ENVIRON / MNES (used to detect the client at connect time)
+	//
+	// Ask the client to enable NEW-ENVIRON. A compliant client replies with
+	// TelnetWillNewEnviron (or TelnetWontNewEnviron to refuse).
+	TelnetRequestNewEnviron = TerminalCommand{[]byte{TELNET_IAC, TELNET_DO, TELNET_OPT_NEW_ENV}, []byte{}}
+	// Client agrees / refuses to use NEW-ENVIRON.
+	TelnetWillNewEnviron = TerminalCommand{[]byte{TELNET_IAC, TELNET_WILL, TELNET_OPT_NEW_ENV}, []byte{}}
+	TelnetWontNewEnviron = TerminalCommand{[]byte{TELNET_IAC, TELNET_WONT, TELNET_OPT_NEW_ENV}, []byte{}}
+	// Server -> Client request for variables. Sent with an empty payload to
+	// request ALL of the client's environment variables (broadly compatible;
+	// Mudlet replies with CLIENT_NAME among them).
+	TelnetNewEnvironSendRequest = TerminalCommand{[]byte{TELNET_IAC, TELNET_SB, TELNET_OPT_NEW_ENV, TELNET_NEWENV_SEND}, []byte{TELNET_IAC, TELNET_SE}}
+	// Client -> Server response carrying the variable values. EndChars are left
+	// empty so the match succeeds on the `IAC SB NEW-ENVIRON IS` prefix alone -
+	// the trailing IAC SE is tolerated inside the payload and ignored by the
+	// parser. This is more robust to TCP segmentation and trailing bytes than
+	// requiring an exact terminator.
+	TelnetNewEnvironResponse = TerminalCommand{[]byte{TELNET_IAC, TELNET_SB, TELNET_OPT_NEW_ENV, TELNET_NEWENV_IS}, []byte{}}
+
 	///////////////////////////
 	// ANSI COMMANDS
 	///////////////////////////