fix(tgbot): require client ownership for non-admin link callbacks (#6489)

A non-admin tapping a subscription, individual-links or QR-links button was
served whatever email that button carried. Those keyboards outlive the chat
they were sent to (group chats, forwarded cards, a client whose tgId was
later revoked), so the email in the callback data cannot authorise itself.
The lookup the self-service usage command already performs now decides
whether the callback is served.

The gate also has to read the email at all: encodeQuery replaces any
callback payload past 64 chars with a hash, so for a client whose email is
long enough the non-admin path saw a bare hash and dropped the tap without a
word. The raw data is now decoded before the gate, which is the same decode
the admin path already performs, and an email the caller cannot prove is
answered with the generic error instead of silence.

internal/web/service/tgbot/tgbot_client.go

@@ -537,6 +537,21 @@ func (t *Tgbot) clientInfoMsg(
 	return output
 }
 
+// clientOwnedByTgUser reports whether email belongs to a client bound to this
+// Telegram account, the same list the self-service usage command reads.
+func (t *Tgbot) clientOwnedByTgUser(tgUserID int64, email string) bool {
+	traffics, err := t.inboundService.GetClientTrafficTgBot(tgUserID)
+	if err != nil {
+		return false
+	}
+	for _, traffic := range traffics {
+		if traffic.Email == email {
+			return true
+		}
+	}
+	return false
+}
+
 // getClientUsage retrieves and sends client usage information to the chat.
 func (t *Tgbot) getClientUsage(chatId int64, tgUserID int64, email ...string) {
 	traffics, err := t.inboundService.GetClientTrafficTgBot(tgUserID)

internal/web/service/tgbot/tgbot_client_links_authz_test.go

@@ -0,0 +1,125 @@
+package tgbot
+
+import (
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database"
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+	"github.com/mhsanaei/3x-ui/v3/internal/web/global"
+	"github.com/mhsanaei/3x-ui/v3/internal/xray"
+
+	"github.com/mymmrac/telego"
+)
+
+const (
+	ownerTgID = int64(4242)
+	ownerMail = "owner@x"
+)
+
+// newLinksCallbackTgbot seeds one inbound whose settings bind email to
+// ownerTgID, the traffic row the ownership lookup joins on, and a mocked API.
+func newLinksCallbackTgbot(t *testing.T, email string) (*Tgbot, func(string) int) {
+	t.Helper()
+	mock, calls := staleButtonServer(t, map[string]any{
+		"answerCallbackQuery": map[string]any{"ok": true, "result": true},
+		"sendMessage": map[string]any{"ok": true, "result": map[string]any{
+			"message_id": 1,
+			"date":       0,
+			"chat":       map[string]any{"id": ownerTgID, "type": "private"},
+		}},
+	})
+	swapTestBot(t, mock.URL)
+	t.Cleanup(mock.Close)
+
+	if err := database.InitDB(filepath.Join(t.TempDir(), "x-ui.db")); err != nil {
+		t.Fatalf("InitDB: %v", err)
+	}
+	t.Cleanup(func() { _ = database.CloseDB() })
+
+	inbound := &model.Inbound{
+		UserId:   1,
+		Remark:   "in",
+		Port:     443,
+		Protocol: model.VLESS,
+		Enable:   true,
+		Settings: `{"clients":[{"email":"` + email + `","tgId":4242,"subId":"sub-owned"}]}`,
+	}
+	if err := database.GetDB().Create(inbound).Error; err != nil {
+		t.Fatalf("seed inbound: %v", err)
+	}
+	if err := database.GetDB().Create(&xray.ClientTraffic{
+		InboundId: inbound.Id,
+		Email:     email,
+		Enable:    true,
+	}).Error; err != nil {
+		t.Fatalf("seed traffic: %v", err)
+	}
+
+	origRunning := isRunning
+	t.Cleanup(func() { isRunning = origRunning })
+	isRunning = true
+
+	return &Tgbot{}, calls
+}
+
+func tapClientLinks(t *testing.T, tb *Tgbot, tgUserID int64, data string) {
+	t.Helper()
+	tb.answerCallback(&telego.CallbackQuery{
+		ID:      "q1",
+		From:    telego.User{ID: tgUserID},
+		Data:    data,
+		Message: &telego.Message{Chat: telego.Chat{ID: tgUserID}},
+	}, false)
+}
+
+// Regression test: a non-admin tapping a link callback carrying another
+// client's email must be refused; without the ownership check it is served.
+func TestClientLinkCallbackRefusesForeignClient(t *testing.T) {
+	tb, calls := newLinksCallbackTgbot(t, ownerMail)
+
+	tapClientLinks(t, tb, ownerTgID, "client_sub_links someone-else@x")
+
+	if n := calls("sendMessage"); n != 0 {
+		t.Errorf("sendMessage calls = %d, want 0: a non-admin received a foreign client's links", n)
+	}
+	if n := calls("answerCallbackQuery"); n != 1 {
+		t.Errorf("answerCallbackQuery calls = %d, want 1: the refused tap must be answered", n)
+	}
+}
+
+// The same guard must not lock the owner out of their own links.
+func TestClientLinkCallbackServesOwnClient(t *testing.T) {
+	tb, calls := newLinksCallbackTgbot(t, ownerMail)
+
+	tapClientLinks(t, tb, ownerTgID, "client_sub_links "+ownerMail)
+
+	if n := calls("sendMessage"); n != 1 {
+		t.Errorf("sendMessage calls = %d, want 1: the owner must still get its links", n)
+	}
+	if n := calls("answerCallbackQuery"); n != 0 {
+		t.Errorf("answerCallbackQuery calls = %d, want 0: an allowed tap is not refused", n)
+	}
+}
+
+// Regression test: a payload past 64 chars arrives as its hash, so an email long
+// enough to be hashed must still be decoded and served to its owner.
+func TestHashedLinkCallbackServesOwnClient(t *testing.T) {
+	const longMail = "[email protected]"
+	tb, calls := newLinksCallbackTgbot(t, longMail)
+
+	origStorage := hashStorage
+	hashStorage = global.NewHashStorage(20 * time.Minute)
+	t.Cleanup(func() { hashStorage = origStorage })
+
+	data := tb.encodeQuery("client_sub_links " + longMail)
+	if data == "client_sub_links "+longMail {
+		t.Fatalf("encodeQuery left %q unhashed; the test needs a hashed payload", data)
+	}
+	tapClientLinks(t, tb, ownerTgID, data)
+
+	if n := calls("sendMessage"); n != 1 {
+		t.Errorf("sendMessage calls = %d, want 1: the owner's hashed button must still be served", n)
+	}
+}

internal/web/service/tgbot/tgbot_router.go

@@ -922,8 +922,15 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 		}
 	}
 
-	if !isAdmin && !isClientSelfCallback(callbackQuery.Data) {
-		return
+	if !isAdmin {
+		// encodeQuery hashes any payload past 64 chars, so a long email's button
+		// must be decoded before the gate can see which client it names.
+		if decoded, err := t.decodeQuery(callbackQuery.Data); err == nil {
+			callbackQuery.Data = decoded
+		}
+		if !isClientSelfCallback(callbackQuery.Data) {
+			return
+		}
 	}
 
 	switch callbackQuery.Data {
@@ -1302,20 +1309,23 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
 
 		}
 	default:
-		if after, ok := strings.CutPrefix(callbackQuery.Data, "client_sub_links "); ok {
-			email := after
-			t.sendClientSubLinks(chatId, email)
+		action, email, ok := splitClientLinkCallback(callbackQuery.Data)
+		if !ok {
 			return
 		}
-		if after, ok := strings.CutPrefix(callbackQuery.Data, "client_individual_links "); ok {
-			email := after
-			t.sendClientIndividualLinks(chatId, email)
+		// The keyboard outlives the chat it was sent to, so the email in it
+		// cannot authorise itself: a non-admin only reaches their own clients.
+		if !isAdmin && !t.clientOwnedByTgUser(callbackQuery.From.ID, email) {
+			t.sendCallbackAnswerTgBot(callbackQuery.ID, t.I18nBot("tgbot.answers.errorOperation"))
 			return
 		}
-		if after, ok := strings.CutPrefix(callbackQuery.Data, "client_qr_links "); ok {
-			email := after
+		switch action {
+		case "client_sub_links":
+			t.sendClientSubLinks(chatId, email)
+		case "client_individual_links":
+			t.sendClientIndividualLinks(chatId, email)
+		case "client_qr_links":
 			t.sendClientQRLinks(chatId, email)
-			return
 		}
 	}
 }
@@ -1325,16 +1335,25 @@ func checkAdmin(tgId int64) bool {
 	return slices.Contains(adminIds, tgId)
 }
 
-// isClientSelfCallback reports whether a callback is one of the per-user client
-// actions that resolve their own data from the caller's Telegram id, and so are
-// safe to run for a non-admin. Every other callback is admin-only (default-deny).
+// isClientSelfCallback reports whether a callback is per-user rather than
+// admin-only; the caller still has to prove the client is its own.
 func isClientSelfCallback(data string) bool {
 	switch data {
 	case "client_traffic", "client_commands", "client_sub_links",
 		"client_individual_links", "client_qr_links":
 		return true
 	}
-	return strings.HasPrefix(data, "client_sub_links ") ||
-		strings.HasPrefix(data, "client_individual_links ") ||
-		strings.HasPrefix(data, "client_qr_links ")
+	_, _, ok := splitClientLinkCallback(data)
+	return ok
+}
+
+// splitClientLinkCallback splits "<action> <email>" for the per-client link
+// callbacks; ok is false for every other data.
+func splitClientLinkCallback(data string) (action, email string, ok bool) {
+	for _, candidate := range []string{"client_sub_links", "client_individual_links", "client_qr_links"} {
+		if rest, found := strings.CutPrefix(data, candidate+" "); found && rest != "" {
+			return candidate, rest, true
+		}
+	}
+	return "", "", false
 }