From c1fadd09c3e870a8a47bd29310b92cfb577c9252 Mon Sep 17 00:00:00 2001
From: Houmgaor <contact@houmgaor.com>
Date: Tue, 24 Feb 2026 13:57:58 +0100
Subject: [PATCH] fix(commands): validate argument parsing in chat commands

KeyQuest set, Rights, and Teleport commands silently used zero values
when given malformed arguments (bad hex, non-integer coords). Now they
send the existing i18n error messages back to the player instead.
---
 docs/improvements.md                      |  6 +++++-
 server/channelserver/handlers_commands.go | 26 ++++++++++++++++++-----
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/docs/improvements.md b/docs/improvements.md
index a33beb535..80beedfca 100644
--- a/docs/improvements.md
+++ b/docs/improvements.md
@@ -71,7 +71,11 @@ From `technical-debt.md`: `model_character.go:88,101,113` has `TODO: fix bookshe
 
 **Fix:** On parse error, send a chat message back to the player explaining the expected format, then return early. Each command's branch already has access to the session for sending messages.
 
-**Status:** Pending.
+**Status:** **Done.** All 4 sites now validate parse results and send the existing i18n error messages:
+
+- `hex.DecodeString` (KeyQuest set) — sends kqf.set.error on invalid hex
+- `strconv.Atoi` (Rights) — sends rights.error on non-integer
+- `strconv.ParseInt` x/y (Teleport) — sends teleport.error on non-integer coords
 
 ---
 
diff --git a/server/channelserver/handlers_commands.go b/server/channelserver/handlers_commands.go
index 98beceab6..2dda089ba 100644
--- a/server/channelserver/handlers_commands.go
+++ b/server/channelserver/handlers_commands.go
@@ -237,7 +237,11 @@ func parseChatCommand(s *Session, command string) {
 						sendServerChatMessage(s, fmt.Sprintf(s.server.i18n.commands.kqf.get, s.kqf))
 					case "set":
 						if len(args) > 2 && len(args[2]) == 16 {
-							hexd, _ := hex.DecodeString(args[2])
+							hexd, err := hex.DecodeString(args[2])
+							if err != nil {
+								sendServerChatMessage(s, fmt.Sprintf(s.server.i18n.commands.kqf.set.error, commands["KeyQuest"].Prefix))
+								return
+							}
 							s.kqf = hexd
 							s.kqfOverride = true
 							sendServerChatMessage(s, s.server.i18n.commands.kqf.set.success)
@@ -253,8 +257,12 @@ func parseChatCommand(s *Session, command string) {
 	case commands["Rights"].Prefix:
 		if commands["Rights"].Enabled || s.isOp() {
 			if len(args) > 1 {
-				v, _ := strconv.Atoi(args[1])
-				err := s.server.userRepo.SetRights(s.userID, uint32(v))
+				v, err := strconv.Atoi(args[1])
+				if err != nil {
+					sendServerChatMessage(s, fmt.Sprintf(s.server.i18n.commands.rights.error, commands["Rights"].Prefix))
+					return
+				}
+				err = s.server.userRepo.SetRights(s.userID, uint32(v))
 				if err == nil {
 					sendServerChatMessage(s, fmt.Sprintf(s.server.i18n.commands.rights.success, v))
 				} else {
@@ -365,8 +373,16 @@ func parseChatCommand(s *Session, command string) {
 	case commands["Teleport"].Prefix:
 		if commands["Teleport"].Enabled || s.isOp() {
 			if len(args) > 2 {
-				x, _ := strconv.ParseInt(args[1], 10, 16)
-				y, _ := strconv.ParseInt(args[2], 10, 16)
+				x, err := strconv.ParseInt(args[1], 10, 16)
+				if err != nil {
+					sendServerChatMessage(s, fmt.Sprintf(s.server.i18n.commands.teleport.error, commands["Teleport"].Prefix))
+					return
+				}
+				y, err := strconv.ParseInt(args[2], 10, 16)
+				if err != nil {
+					sendServerChatMessage(s, fmt.Sprintf(s.server.i18n.commands.teleport.error, commands["Teleport"].Prefix))
+					return
+				}
 				payload := byteframe.NewByteFrame()
 				payload.SetLE()
 				payload.WriteUint8(2)        // SetState type(position == 2)