fix(tgbot): escape template parameters once in I18nBot

Every bot message is sent HTML-parsed while the intended markup lives
in the translation templates, yet the values filled into them were
escaped only at some call sites. Remarks and e-mails, which a node
chooses for the inbounds it reports, reached the scheduled report and
the client info unescaped and turned into markup, and the reports that
escaped their whole text instead showed the templates' own tags.

I18nBot now escapes every parameter value, the call sites drop their
own escaping and the whole-report escapes, the two values concatenated
outside a template are escaped where they are written, and callback
answers, which are plain text, are unescaped again.
This commit is contained in:
MHSanaei
2026-11-01 08:58:43 +01:00
parent a9191cb468
commit fac9009398
10 changed files with 88 additions and 172 deletions
@@ -43,7 +43,7 @@ func (t *Tgbot) screenClientIps(chatID int64, email string) {
if err != nil || len(ips) == 0 {
body += t.I18nBot("tgbot.noIpRecord")
} else {
body += t.I18nBot("tgbot.messages.ips", "IPs=="+html.EscapeString(formatIPLog(ips)))
body += t.I18nBot("tgbot.messages.ips", "IPs=="+formatIPLog(ips))
}
body += t.I18nBot("tgbot.messages.refreshedOn", "Time=="+time.Now().Format("2006-01-02 15:04:05"))
rows := [][]telego.InlineKeyboardButton{
@@ -158,9 +158,9 @@ func (t *Tgbot) screenClientLinks(chatID int64, email string) {
return
}
body := t.clientScreenBody(email, "")
body += t.I18nBot("tgbot.messages.subURL", "URL=="+html.EscapeString(subURL))
body += t.I18nBot("tgbot.messages.subURL", "URL=="+subURL)
if subJSON != "" {
body += t.I18nBot("tgbot.messages.jsonURL", "URL=="+html.EscapeString(subJSON))
body += t.I18nBot("tgbot.messages.jsonURL", "URL=="+subJSON)
}
body += t.I18nBot("tgbot.messages.refreshedOn", "Time=="+time.Now().Format("2006-01-02 15:04:05"))
rows := [][]telego.InlineKeyboardButton{
@@ -213,7 +213,7 @@ func (t *Tgbot) screenClientQR(chatID int64, email string) {
return
}
body := t.clientScreenBody(email, t.I18nBot("tgbot.answers.qrCodeForClient", "Email=="+email))
body += t.I18nBot("tgbot.messages.subURL", "URL=="+html.EscapeString(subURL))
body += t.I18nBot("tgbot.messages.subURL", "URL=="+subURL)
rows := [][]telego.InlineKeyboardButton{
tu.InlineKeyboardRow(t.btn("tgbot.buttons.individualLinks", t.encodeQuery("client_individual_links "+email))),
t.backRow(),
+1 -1
View File
@@ -235,7 +235,7 @@ func (t *Tgbot) screenReport(chatID int64) {
tu.InlineKeyboardRow(t.btn("tgbot.buttons.refresh", cbReport)),
t.backRow(),
}
t.renderScreen(chatID, t.newScreen("reports", html.EscapeString(report.String()), rows...))
t.renderScreen(chatID, t.newScreen("reports", report.String(), rows...))
}
// screenCommands explains the classic commands; kept because muscle memory is
+13 -2
View File
@@ -5,6 +5,7 @@ import (
"crypto/rand"
"embed"
"fmt"
"html"
"math/big"
"net"
"net/url"
@@ -249,9 +250,19 @@ func (t *Tgbot) NewTgbot() *Tgbot {
return new(Tgbot)
}
// I18nBot retrieves a localized message for the bot interface.
// I18nBot retrieves a localized message for the bot interface. Every message is sent
// HTML-parsed, so param values (remarks, e-mails, errors) are escaped here.
func (t *Tgbot) I18nBot(name string, params ...string) string {
return locale.I18n(locale.Bot, name, params...)
escaped := make([]string, len(params))
for i, param := range params {
key, value, found := strings.Cut(param, "==")
if !found {
escaped[i] = param
continue
}
escaped[i] = key + "==" + html.EscapeString(value)
}
return locale.I18n(locale.Bot, name, escaped...)
}
// GetHashStorage returns the hash storage instance for callback queries.
+1 -1
View File
@@ -321,7 +321,7 @@ func (t *Tgbot) clientInfoMsg(
output := ""
output += t.I18nBot("tgbot.messages.email", "Email=="+traffic.Email)
if attachIds, err := t.clientService.GetInboundIdsForEmail(nil, traffic.Email); err == nil && len(attachIds) > 0 {
output += fmt.Sprintf("🔗 Inbounds: %s\r\n", t.describeAttachedInbounds(attachIds))
output += fmt.Sprintf("🔗 Inbounds: %s\r\n", html.EscapeString(t.describeAttachedInbounds(attachIds)))
}
if printEnabled {
output += t.I18nBot("tgbot.messages.enabled", "Enable=="+enabled)
+3 -1
View File
@@ -11,6 +11,7 @@ import (
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
"github.com/mhsanaei/3x-ui/v3/internal/logger"
"github.com/mhsanaei/3x-ui/v3/internal/util/common"
"github.com/mhsanaei/3x-ui/v3/internal/web/locale"
"github.com/mymmrac/telego"
)
@@ -197,7 +198,8 @@ func (t *Tgbot) allowInviteAttempt(from *telego.User) bool {
inviteAttemptsMu.Unlock()
if count == inviteAttemptLimit+1 {
t.SendMsgToTgbotAdmins(t.I18nBot("tgbot.messages.inviteRateLimitedAdmin",
// locale.I18n directly: User is a ready-made mention that I18nBot would escape.
t.SendMsgToTgbotAdmins(locale.I18n(locale.Bot, "tgbot.messages.inviteRateLimitedAdmin",
"User=="+tgUserMention(from),
"ID=="+strconv.FormatInt(from.ID, 10),
"Limit=="+strconv.Itoa(inviteAttemptLimit)))
+2 -1
View File
@@ -2,6 +2,7 @@ package tgbot
import (
"fmt"
"html"
"net"
"os"
"strconv"
@@ -292,7 +293,7 @@ func (t *Tgbot) notifyExhausted() {
output.WriteString(":\r\n")
for _, traffic := range disabledClients {
output.WriteString(" ")
output.WriteString(traffic.Email)
output.WriteString(html.EscapeString(traffic.Email))
}
output.WriteString("\r\n")
}
@@ -2,7 +2,6 @@ package tgbot
import (
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@ -11,18 +10,13 @@ import (
"strings"
"sync"
"testing"
"time"
"github.com/mymmrac/telego"
"github.com/nicksnyder/go-i18n/v2/i18n"
"golang.org/x/text/language"
"gorm.io/gorm"
"github.com/mhsanaei/3x-ui/v3/internal/database"
"github.com/mhsanaei/3x-ui/v3/internal/database/dbtest"
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
"github.com/mhsanaei/3x-ui/v3/internal/web/locale"
"github.com/mhsanaei/3x-ui/v3/internal/web/service"
"github.com/mhsanaei/3x-ui/v3/internal/xray"
)
@@ -173,157 +167,29 @@ func TestResetAllTrafficsAnswersWithNoClients(t *testing.T) {
}
}
// Regression test: the report leaves as one HTML-parsed message, so a remark
// holding "<" must reach Telegram escaped instead of dropping the whole page.
func TestTrafficUsageReportEscapesHtml(t *testing.T) {
mock, sent := captureReportServer(t)
swapTestBot(t, mock.URL)
defer mock.Close()
// Every bot message is HTML-parsed, and a node chooses the remark and e-mails the master
// stores for its inbound: filled into a template unescaped, they became markup.
func TestClientInfoEscapesNodeValues(t *testing.T) {
tb := initReportDB(t)
seedReportClients(t, "DE <fast>", []string{"a@x"})
tb.answerCallback(&telego.CallbackQuery{
ID: "q1",
From: telego.User{ID: 1},
Data: "get_sorted_traffic_usage_report",
Message: &telego.Message{Chat: telego.Chat{ID: 1}},
}, true) // admin
got := sent()
if len(got) != 1 {
t.Fatalf("messages = %d, want 1", len(got))
draftLocalizer(t, &i18n.Message{ID: "tgbot.messages.email", Other: "📧 Email: {{ .Email }}\r\n"})
remark := `<a href="https://evil.example/">Panel session expired</a>`
email := "<b>zz</b>"
seedReportClients(t, remark, []string{email})
var traffic xray.ClientTraffic
if err := database.GetDB().Where("email = ?", email).First(&traffic).Error; err != nil {
t.Fatalf("read traffic: %v", err)
}
if strings.Contains(got[0].body(), "<fast>") {
t.Errorf("report text = %q, want the remark escaped", got[0].body())
}
}
func TestNotifyExhaustedLooksUpEachIDOnce(t *testing.T) {
for _, outcome := range []string{"empty", "failure", "exhausted", "expiry", "admin", "zero", "disabled-inbound"} {
t.Run(outcome, func(t *testing.T) {
tb := initReportDB(t)
clientInfoLocalizer(t)
mock, sent := captureReportServer(t)
swapTestBot(t, mock.URL)
t.Cleanup(mock.Close)
clients := []model.Client{
{Email: "notify-a", ID: "uuid-a", TgID: 6886, Enable: true},
{Email: "notify-b", ID: "uuid-b", TgID: 6886, Enable: true},
}
if outcome == "zero" {
for i := range clients {
clients[i].TgID = 0
}
}
if outcome == "admin" {
tgBotMutex.Lock()
previous := adminIds
adminIds = append(append([]int64(nil), adminIds...), 6886)
tgBotMutex.Unlock()
t.Cleanup(func() {
tgBotMutex.Lock()
adminIds = previous
tgBotMutex.Unlock()
})
}
db := database.GetDB()
for i := range 2 {
inbound := &model.Inbound{UserId: 1, Tag: fmt.Sprintf("notify-%d", i), Port: 41890 + i, Enable: true, Protocol: model.VLESS, Settings: `{"clients":[{"email":"notify-a","id":"uuid-a","enable":true,"tgId":6886},{"email":"notify-b","id":"uuid-b","enable":true,"tgId":6886}]}`}
if outcome == "disabled-inbound" {
inbound.Enable = false
}
if err := db.Create(inbound).Error; err != nil {
t.Fatal(err)
}
if err := (&service.ClientService{}).SyncInbound(nil, inbound.Id, clients); err != nil {
t.Fatal(err)
}
// Keep the inbound eligible even when the Telegram account has no traffic.
traffic := &xray.ClientTraffic{Email: fmt.Sprintf("unrelated-%d", i), InboundId: inbound.Id}
if outcome == "exhausted" || outcome == "expiry" {
traffic.Email = clients[i].Email
traffic.Enable = true
traffic.Total = 1
traffic.Up = 2
if outcome == "expiry" {
traffic.Total = 0
traffic.ExpiryTime = time.Now().Add(-time.Hour).UnixMilli()
}
}
if err := db.Create(traffic).Error; err != nil {
t.Fatal(err)
}
}
lookups := 0
cb := "test:notify_lookup"
if err := db.Callback().Query().Before("gorm:query").Register(cb, func(tx *gorm.DB) {
if tx.Statement.Table == "clients" && strings.Contains(fmt.Sprint(tx.Statement.Clauses["WHERE"]), "tg_id") {
lookups++
if outcome == "failure" {
tx.AddError(errors.New("injected Telegram lookup failure"))
}
}
}); err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = db.Callback().Query().Remove(cb) })
tb.notifyExhausted()
wantLookups := 1
if outcome == "admin" || outcome == "zero" || outcome == "disabled-inbound" {
wantLookups = 0
}
if lookups != wantLookups {
t.Fatalf("lookups = %d, want %d after %s", lookups, wantLookups, outcome)
}
got := sent()
if outcome == "exhausted" || outcome == "expiry" {
if len(got) != 1 || !strings.Contains(got[0].Text, "notify-a") || !strings.Contains(got[0].Text, "notify-b") {
t.Fatalf("want one notification containing both clients, got %+v", got)
}
} else if len(got) != 0 {
t.Fatalf("unexpected notifications: %+v", got)
}
})
}
}
out := tb.clientInfoMsg(&traffic, true, false, false, true, true, false)
func TestServerUsageInfoShowsUnavailableUptime(t *testing.T) {
bundle := i18n.NewBundle(language.English)
_ = bundle.AddMessages(language.English, &i18n.Message{ID: "tgbot.messages.serverUpTime", Other: "Uptime: {{.UpTime}} {{.Unit}}\n"}, &i18n.Message{ID: "tgbot.days", Other: "days"})
oldLocale := locale.SwapBotLocalizer(i18n.NewLocalizer(bundle, "en"))
t.Cleanup(func() { locale.SwapBotLocalizer(oldLocale) })
statusCache.mutex.Lock()
oldStatus, oldTime := statusCache.data, statusCache.timestamp
statusCache.mutex.Unlock()
serverStatsCache.mutex.Lock()
oldStats, oldStatsTime := serverStatsCache.data, serverStatsCache.timestamp
serverStatsCache.mutex.Unlock()
t.Cleanup(func() {
statusCache.mutex.Lock()
statusCache.data, statusCache.timestamp = oldStatus, oldTime
statusCache.mutex.Unlock()
serverStatsCache.mutex.Lock()
serverStatsCache.data, serverStatsCache.timestamp = oldStats, oldStatsTime
serverStatsCache.mutex.Unlock()
})
for _, unavailable := range []bool{true, false} {
t.Run(map[bool]string{true: "unavailable", false: "valid zero"}[unavailable], func(t *testing.T) {
bot := &Tgbot{}
status := &service.Status{Loads: []float64{0, 0, 0}}
if unavailable {
status.UnavailableMetrics = []string{"uptime"}
}
bot.setCachedStatus(status)
bot.setCachedServerStats("")
info := bot.prepareServerUsageInfo()
want := "Uptime: 0 days"
if unavailable {
want = "Uptime: —"
}
if !strings.Contains(info, want) {
t.Fatalf("report did not contain %q: %s", want, info)
}
})
for _, raw := range []string{"<a href=", "<b>zz</b>"} {
if strings.Contains(out, raw) {
t.Errorf("client info = %q, want %q escaped", out, raw)
}
}
for _, want := range []string{"📧 Email: &lt;b&gt;zz&lt;/b&gt;", "&lt;a href="} {
if !strings.Contains(out, want) {
t.Errorf("client info = %q, want it to contain %q", out, want)
}
}
}
+5 -7
View File
@@ -2,7 +2,6 @@ package tgbot
import (
"context"
"html"
"slices"
"strconv"
"strings"
@@ -1030,17 +1029,17 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
t.openOwnClientScreen(chatId, callbackQuery.From.ID, callbackQuery.Data)
case "add_client_ch_default_email":
userStateMgr.set(actor, "awaiting_email")
t.wizardPrompt(chatId, draft, t.I18nBot("tgbot.messages.email_prompt", "ClientEmail=="+html.EscapeString(draft.email)))
t.wizardPrompt(chatId, draft, t.I18nBot("tgbot.messages.email_prompt", "ClientEmail=="+draft.email))
case "add_client_ch_default_comment":
userStateMgr.set(actor, "awaiting_comment")
t.wizardPrompt(chatId, draft, t.I18nBot("tgbot.messages.comment_prompt", "ClientComment=="+html.EscapeString(draft.comment)))
t.wizardPrompt(chatId, draft, t.I18nBot("tgbot.messages.comment_prompt", "ClientComment=="+draft.comment))
case "add_client_ch_default_tg_id":
userStateMgr.set(actor, "awaiting_tg_id")
current := draft.tgID
if current == "" {
current = "—"
}
t.wizardPrompt(chatId, draft, t.I18nBot("tgbot.messages.tgid_prompt", "ClientTGID=="+html.EscapeString(current)))
t.wizardPrompt(chatId, draft, t.I18nBot("tgbot.messages.tgid_prompt", "ClientTGID=="+current))
case "add_client_ch_default_traffic":
inlineKeyboard := tu.InlineKeyboard(
tu.InlineKeyboardRow(
@@ -1192,9 +1191,8 @@ func (t *Tgbot) answerCallback(callbackQuery *telego.CallbackQuery, isAdmin bool
}
report.WriteString(t.I18nBot("tgbot.messages.FinishProcess"))
// Escaped whole (a stray "<" makes Telegram reject a page) and paged:
// past ~80 clients the report exceeds one message.
t.sendPagedNotice(chatId, html.EscapeString(report.String()))
// Paged: past ~80 clients the report exceeds one message.
t.sendPagedNotice(chatId, report.String())
// The confirmation has served its purpose: leaving it on screen left the
// only button being the one just pressed.
t.screenHome(chatId, callbackQuery.From.ID)
+3 -1
View File
@@ -3,6 +3,7 @@ package tgbot
import (
"context"
"fmt"
"html"
"strings"
"time"
@@ -149,9 +150,10 @@ func (t *Tgbot) SendMsgToTgbotAdmins(msg string, replyMarkup ...telego.ReplyMark
// sendCallbackAnswerTgBot answers a callback query with a message.
func (t *Tgbot) sendCallbackAnswerTgBot(id string, message string) {
// Callback answers are plain text, so the HTML entities I18nBot adds are undone.
params := telego.AnswerCallbackQueryParams{
CallbackQueryID: id,
Text: message,
Text: html.UnescapeString(message),
}
if err := liveBot().AnswerCallbackQuery(context.Background(), &params); err != nil {
logger.Warning(err)
@@ -6,12 +6,14 @@ import (
"net/http"
"net/http/httptest"
"strings"
"sync"
"testing"
"github.com/mhsanaei/3x-ui/v3/internal/logger"
"github.com/mymmrac/telego"
tu "github.com/mymmrac/telego/telegoutil"
"github.com/nicksnyder/go-i18n/v2/i18n"
)
func TestIsTelegramNotModifiedError(t *testing.T) {
@@ -136,3 +138,37 @@ func TestPageMessageSplitsLinkListWithoutBlankLines(t *testing.T) {
t.Errorf("pages contain %d links, want 50", links)
}
}
// Regression test: callback answers are shown as plain text, so the entities
// I18nBot escapes for HTML-parsed messages must not surface in them.
func TestCallbackAnswerIsPlainText(t *testing.T) {
var mu sync.Mutex
var texts []string
mock := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/bot"+testBotToken+"/answerCallbackQuery" {
w.WriteHeader(http.StatusNotFound)
return
}
var body struct {
Text string `json:"text"`
}
_ = json.NewDecoder(r.Body).Decode(&body)
mu.Lock()
texts = append(texts, body.Text)
mu.Unlock()
w.Header().Set("Content-Type", "application/json")
_ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": true})
}))
defer mock.Close()
swapTestBot(t, mock.URL)
draftLocalizer(t, &i18n.Message{ID: "tgbot.answers.canceled", Other: "❌ {{ .Email }}: Operation canceled."})
tb := &Tgbot{}
tb.sendCallbackAnswerTgBot("q1", tb.I18nBot("tgbot.answers.canceled", "Email==a&b<c>"))
mu.Lock()
defer mu.Unlock()
if len(texts) != 1 || texts[0] != "❌ a&b<c>: Operation canceled." {
t.Errorf("answerCallbackQuery texts = %q, want the e-mail shown verbatim", texts)
}
}