diff --git a/internal/web/service/tgbot/screen_client_views.go b/internal/web/service/tgbot/screen_client_views.go index 3e82b1a01..9a2136a2d 100644 --- a/internal/web/service/tgbot/screen_client_views.go +++ b/internal/web/service/tgbot/screen_client_views.go @@ -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(), diff --git a/internal/web/service/tgbot/screen_views.go b/internal/web/service/tgbot/screen_views.go index 3303ffa11..d21871b3a 100644 --- a/internal/web/service/tgbot/screen_views.go +++ b/internal/web/service/tgbot/screen_views.go @@ -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 diff --git a/internal/web/service/tgbot/tgbot.go b/internal/web/service/tgbot/tgbot.go index 374cf5034..e3065395e 100644 --- a/internal/web/service/tgbot/tgbot.go +++ b/internal/web/service/tgbot/tgbot.go @@ -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. diff --git a/internal/web/service/tgbot/tgbot_client.go b/internal/web/service/tgbot/tgbot_client.go index 5e9868781..e11e2ce77 100644 --- a/internal/web/service/tgbot/tgbot_client.go +++ b/internal/web/service/tgbot/tgbot_client.go @@ -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) diff --git a/internal/web/service/tgbot/tgbot_invite.go b/internal/web/service/tgbot/tgbot_invite.go index 35f1bf3f4..ac1ff8474 100644 --- a/internal/web/service/tgbot/tgbot_invite.go +++ b/internal/web/service/tgbot/tgbot_invite.go @@ -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))) diff --git a/internal/web/service/tgbot/tgbot_report.go b/internal/web/service/tgbot/tgbot_report.go index c1da45bbc..dd7a65c3f 100644 --- a/internal/web/service/tgbot/tgbot_report.go +++ b/internal/web/service/tgbot/tgbot_report.go @@ -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") } diff --git a/internal/web/service/tgbot/tgbot_report_batching_test.go b/internal/web/service/tgbot/tgbot_report_batching_test.go index ba0166ba9..96bb1f03d 100644 --- a/internal/web/service/tgbot/tgbot_report_batching_test.go +++ b/internal/web/service/tgbot/tgbot_report_batching_test.go @@ -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 ", []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 := `Panel session expired` + email := "zz" + 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(), "") { - 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{"zz"} { + if strings.Contains(out, raw) { + t.Errorf("client info = %q, want %q escaped", out, raw) + } + } + for _, want := range []string{"📧 Email: <b>zz</b>", "<a href="} { + if !strings.Contains(out, want) { + t.Errorf("client info = %q, want it to contain %q", out, want) + } } } diff --git a/internal/web/service/tgbot/tgbot_router.go b/internal/web/service/tgbot/tgbot_router.go index 107fdb896..e883401bb 100644 --- a/internal/web/service/tgbot/tgbot_router.go +++ b/internal/web/service/tgbot/tgbot_router.go @@ -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) diff --git a/internal/web/service/tgbot/tgbot_send.go b/internal/web/service/tgbot/tgbot_send.go index e134f1c18..8db98c51d 100644 --- a/internal/web/service/tgbot/tgbot_send.go +++ b/internal/web/service/tgbot/tgbot_send.go @@ -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(), ¶ms); err != nil { logger.Warning(err) diff --git a/internal/web/service/tgbot/tgbot_send_test.go b/internal/web/service/tgbot/tgbot_send_test.go index 958ffccf6..b86391725 100644 --- a/internal/web/service/tgbot/tgbot_send_test.go +++ b/internal/web/service/tgbot/tgbot_send_test.go @@ -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")) + + mu.Lock() + defer mu.Unlock() + if len(texts) != 1 || texts[0] != "❌ a&b: Operation canceled." { + t.Errorf("answerCallbackQuery texts = %q, want the e-mail shown verbatim", texts) + } +}