From d8d1ebc0bc3db45f0ac65b32908f343f5b17ab4b Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Sun, 28 Jun 2026 09:58:16 +0700 Subject: [PATCH] feat(telegram): switch to long polling, remove webhook path Replace webhook-based updates with long-polling GetUpdates loop. Eliminates external HTTP endpoint requirement and simplifies self-hosted deployments. Removes webhook.go and associated webhook routing from server. --- internal/server/router.go | 30 +++--- internal/telegram/client.go | 22 +++-- internal/telegram/webhook.go | 149 ----------------------------- internal/telegram/webhook_test.go | 154 ------------------------------ 4 files changed, 30 insertions(+), 325 deletions(-) delete mode 100644 internal/telegram/webhook.go delete mode 100644 internal/telegram/webhook_test.go diff --git a/internal/server/router.go b/internal/server/router.go index 1f31bf9..1f82e85 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -13,7 +13,6 @@ import ( "github.com/tiennm99/miti99bot/internal/log" "github.com/tiennm99/miti99bot/internal/modules" - "github.com/tiennm99/miti99bot/internal/telegram" ) // cronNameRe limits cron path segments to a safe alphabet so log injection via @@ -21,35 +20,34 @@ import ( // router boundary). Same shape as Telegram command names. var cronNameRe = regexp.MustCompile(`^[a-z0-9_]{1,32}$`) -// cronAuthHeader is the shared-secret header EventBridge Scheduler attaches when -// invoking /cron/{name}. +// cronAuthHeader is the shared-secret header a caller attaches when invoking +// /cron/{name} for a manual trigger. const cronAuthHeader = "X-Cron-Token" // Config wires the router's runtime dependencies. type Config struct { - Bot *bot.Bot - Registry *modules.Registry - WebhookSecret string + Bot *bot.Bot + Registry *modules.Registry - // CronSecret protects /cron/{name} against unauthenticated calls; EventBridge - // Scheduler attaches it as the X-Cron-Token header. Empty means /cron/{name} - // is fully disabled (404). + // CronSecret protects /cron/{name} against unauthenticated calls; a caller + // attaches it as the X-Cron-Token header. Empty means /cron/{name} is fully + // disabled (404) — the default on self-host, where the in-process scheduler + // (internal/cron) is the sole cron trigger. CronSecret string } // New builds the application's HTTP handler. Routes: // -// GET / → health -// POST /webhook → Telegram update intake (constant-time secret check) -// POST /cron/{name} → EventBridge Scheduler entry (shared-secret check) +// GET / → health (Coolify container monitor; not publicly routed) +// POST /cron/{name} → optional manual cron trigger (shared-secret check) // -// Anything else is 404. All routes pass through LogRequests so every -// request emits a structured `req` log line (CloudWatch Logs consumes them -// for 5xx-rate alerts and per-route latency). +// There is no /webhook route: Telegram updates arrive via long polling +// (cmd/server runs b.Start), so the bot needs no public inbound ingress. +// Anything else is 404. All routes pass through LogRequests so every request +// emits a structured `req` log line. func New(cfg Config) http.Handler { mux := http.NewServeMux() mux.Handle("/", HealthHandler()) - mux.Handle("/webhook", telegram.WebhookHandler(cfg.Bot, cfg.WebhookSecret)) mux.Handle("/cron/", cronHandler(cfg.Registry, cfg.CronSecret)) return LogRequests(mux) } diff --git a/internal/telegram/client.go b/internal/telegram/client.go index b1b0397..a4e2cc3 100644 --- a/internal/telegram/client.go +++ b/internal/telegram/client.go @@ -4,19 +4,29 @@ import ( "github.com/go-telegram/bot" ) -// NewBot constructs a Telegram bot configured for webhook mode: +// pollingAllowedUpdates restricts getUpdates to the update kinds the modules +// actually handle (text commands + inline-keyboard callbacks), matching the +// allowed_updates the old webhook registration set. Anything else (channel +// posts, edited messages, etc.) is dropped server-side by Telegram. +var pollingAllowedUpdates = bot.AllowedUpdates{"message", "callback_query"} + +// NewBot constructs a Telegram bot for long-polling mode (the sole transport +// on self-host — b.Start runs the getUpdates loop in cmd/server): // -// - WithSkipGetMe: avoid a 5s blocking call to Telegram during cold start. -// Token validity surfaces on the first outgoing API call instead. -// - WithNotAsyncHandlers: handlers run synchronously inside the dispatcher's -// goroutine. The webhook handler can rely on r.Context() staying live for -// the duration of dispatch, which a goroutine-spawning default would break. +// - WithSkipGetMe: avoid a blocking GetMe call at startup. Token validity +// surfaces on the first outgoing API call instead. +// - WithNotAsyncHandlers: handlers run synchronously inside the dispatch +// goroutine. Module handlers take their own ctx (not r.Context()), so this +// is safe; it also bounds in-flight work to one update at a time, which +// suits the single-replica polling deployment. +// - WithAllowedUpdates: only request the update kinds the bot handles. // // Callers may pass extra options that override these defaults. func NewBot(token string, opts ...bot.Option) (*bot.Bot, error) { defaults := []bot.Option{ bot.WithSkipGetMe(), bot.WithNotAsyncHandlers(), + bot.WithAllowedUpdates(pollingAllowedUpdates), } return bot.New(token, append(defaults, opts...)...) } diff --git a/internal/telegram/webhook.go b/internal/telegram/webhook.go deleted file mode 100644 index 536316b..0000000 --- a/internal/telegram/webhook.go +++ /dev/null @@ -1,149 +0,0 @@ -package telegram - -import ( - "context" - "crypto/subtle" - "encoding/json" - "errors" - "net/http" - "runtime/debug" - "time" - "unicode/utf8" - - "github.com/go-telegram/bot" - "github.com/go-telegram/bot/models" - - "github.com/tiennm99/miti99bot/internal/log" -) - -// secretTokenHeader is the case-insensitive HTTP header Telegram sets when it -// POSTs an update to the webhook. It must equal the value passed to setWebhook. -// See: https://core.telegram.org/bots/api#setwebhook -// #nosec G101 — header name, not credential value -const secretTokenHeader = "X-Telegram-Bot-Api-Secret-Token" - -// maxWebhookBody bounds inbound JSON. Telegram updates are well under 100 KiB -// even with media; 1 MiB is a defensive ceiling against malformed clients. -const maxWebhookBody = 1 << 20 - -// handlerTimeout caps a single Telegram update handler. Telegram retries after -// 60s of no 2xx; 10s leaves headroom for outbound API calls inside handlers -// without holding a Lambda instance long enough to block other updates. -const handlerTimeout = 10 * time.Second - -// WebhookHandler returns an http.HandlerFunc that validates Telegram's secret -// token (constant-time) and dispatches the update synchronously to the bot. -// -// Dispatch is synchronous because the bot is constructed with -// bot.WithNotAsyncHandlers — handlers run inside this goroutine, so r.Context() -// stays live and bounded by handlerTimeout. -// -// secret must be non-empty; main is responsible for failing-fast at startup. -func WebhookHandler(b *bot.Bot, secret string) http.HandlerFunc { - secretBytes := []byte(secret) - return func(w http.ResponseWriter, r *http.Request) { - // Rejection paths use bare status codes (no response body) so internet - // scanners hitting the public Function URL can't fingerprint this as a - // Telegram webhook from the response text. CloudWatch metric filters - // still see the distinct status codes (401 / 405 / 413 / 400), and the - // structured log lines below carry the *reason* for operator triage. - if r.Method != http.MethodPost { - log.Warn("webhook rejected", "reason", "method", "method", r.Method) - w.WriteHeader(http.StatusMethodNotAllowed) - return - } - got := []byte(r.Header.Get(secretTokenHeader)) - if subtle.ConstantTimeCompare(got, secretBytes) != 1 { - log.Warn("webhook rejected", "reason", "secret_mismatch") - w.WriteHeader(http.StatusUnauthorized) - return - } - - r.Body = http.MaxBytesReader(w, r.Body, maxWebhookBody) - var update models.Update - if err := json.NewDecoder(r.Body).Decode(&update); err != nil { - // MaxBytesReader returns *http.MaxBytesError when the cap is hit; - // surface 413 distinctly so Telegram (and ops dashboards) can - // distinguish "body too big" from generic malformed JSON. - var maxBytesErr *http.MaxBytesError - if errors.As(err, &maxBytesErr) { - log.Warn("webhook rejected", "reason", "body_too_large") - w.WriteHeader(http.StatusRequestEntityTooLarge) - return - } - log.Warn("webhook rejected", "reason", "bad_json", "err", err) - w.WriteHeader(http.StatusBadRequest) - return - } - - logDispatch(&update) - - ctx, cancel := context.WithTimeout(r.Context(), handlerTimeout) - defer cancel() - // Recover panics so a buggy handler does not propagate up to the - // http.Server (which would close the response mid-write and trigger - // Telegram's 24-hour retry loop on the same poisoned update). - panicked := false - func() { - defer func() { - if rec := recover(); rec != nil { - panicked = true - log.Error("webhook handler panic", - "panic", rec, - "stack", string(debug.Stack())) - } - }() - b.ProcessUpdate(ctx, &update) - }() - // Suppress the trailing 200 if a panic occurred: a poisoned handler - // may have already written headers/body, and a second WriteHeader - // here emits `superfluous response.WriteHeader` noise. The - // LogRequests middleware will mark this as 500 from its own recover - // path; we just stay quiet here. - if !panicked { - w.WriteHeader(http.StatusOK) - } - } -} - -// dispatchTextPreview caps message text in dispatch logs so chatty media -// captions or long DM threads don't bloat CloudWatch / drive up cost. -const dispatchTextPreview = 64 - -// truncateRunes returns the longest prefix of s whose UTF-8 byte length is -// <= maxBytes AND that ends on a rune boundary. Byte-slicing alone would -// split a multi-byte rune (Vietnamese, emoji, CJK), producing invalid UTF-8 -// in the log line that downstream JSON encoders replace with U+FFFD. -func truncateRunes(s string, maxBytes int) string { - if len(s) <= maxBytes { - return s - } - cut := maxBytes - for cut > 0 && !utf8.RuneStart(s[cut]) { - cut-- - } - return s[:cut] -} - -// logDispatch emits a single structured line per inbound update so the -// CloudWatch trail has chat type + command text without resorting to -// the library's pointer-printing debug mode. Cheap (no allocation when -// the message is short) and fires once per webhook hit. -func logDispatch(u *models.Update) { - if u == nil || u.Message == nil { - return - } - text := u.Message.Text - if text == "" { - text = u.Message.Caption - } - if len(text) > dispatchTextPreview { - text = truncateRunes(text, dispatchTextPreview) + "…" - } - log.Info("dispatch", - "update_id", u.ID, - "chat_id", u.Message.Chat.ID, - "chat_type", string(u.Message.Chat.Type), - "text", text, - ) -} diff --git a/internal/telegram/webhook_test.go b/internal/telegram/webhook_test.go deleted file mode 100644 index 864b643..0000000 --- a/internal/telegram/webhook_test.go +++ /dev/null @@ -1,154 +0,0 @@ -package telegram - -import ( - "bytes" - "context" - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/go-telegram/bot" - "github.com/go-telegram/bot/models" -) - -const testSecret = "super-secret-token" - -// validUpdate is a minimal Telegram update payload that decodes cleanly. The -// bot has no handlers registered so ProcessUpdate is a no-op match. -const validUpdate = `{"update_id": 1}` - -func mustBot(t *testing.T) *bot.Bot { - t.Helper() - b, err := NewBot("TEST:TOKEN") - if err != nil { - t.Fatalf("NewBot: %v", err) - } - return b -} - -func TestWebhookHandler_RejectsNonPost(t *testing.T) { - h := WebhookHandler(mustBot(t), testSecret) - req := httptest.NewRequest(http.MethodGet, "/webhook", nil) - rec := httptest.NewRecorder() - h(rec, req) - if rec.Code != http.StatusMethodNotAllowed { - t.Errorf("status = %d, want 405", rec.Code) - } -} - -func TestWebhookHandler_RejectsMissingSecret(t *testing.T) { - h := WebhookHandler(mustBot(t), testSecret) - req := httptest.NewRequest(http.MethodPost, "/webhook", strings.NewReader(validUpdate)) - rec := httptest.NewRecorder() - h(rec, req) - if rec.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", rec.Code) - } -} - -func TestWebhookHandler_RejectsWrongSecret(t *testing.T) { - h := WebhookHandler(mustBot(t), testSecret) - req := httptest.NewRequest(http.MethodPost, "/webhook", strings.NewReader(validUpdate)) - req.Header.Set(secretTokenHeader, "wrong") - rec := httptest.NewRecorder() - h(rec, req) - if rec.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", rec.Code) - } -} - -func TestWebhookHandler_RejectsWrongSecretSamePrefix(t *testing.T) { - // Locks the constant-time compare: a value sharing a prefix must still - // 401, not silently succeed. - h := WebhookHandler(mustBot(t), testSecret) - req := httptest.NewRequest(http.MethodPost, "/webhook", strings.NewReader(validUpdate)) - req.Header.Set(secretTokenHeader, testSecret[:len(testSecret)-1]+"X") - rec := httptest.NewRecorder() - h(rec, req) - if rec.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", rec.Code) - } -} - -func TestWebhookHandler_RejectsMalformedJSON(t *testing.T) { - h := WebhookHandler(mustBot(t), testSecret) - req := httptest.NewRequest(http.MethodPost, "/webhook", strings.NewReader("not-json")) - req.Header.Set(secretTokenHeader, testSecret) - rec := httptest.NewRecorder() - h(rec, req) - if rec.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", rec.Code) - } -} - -func TestWebhookHandler_RejectsOversizedBody(t *testing.T) { - h := WebhookHandler(mustBot(t), testSecret) - // Valid-prefixed JSON so the decoder doesn't bail on the first byte; the - // long string field forces a read past maxWebhookBody, triggering - // *http.MaxBytesError. Plain "aaaa…" without the JSON wrapper would fail - // at byte 1 with a SyntaxError and never exercise the cap. - body := bytes.Buffer{} - body.WriteString(`{"update_id":1,"message":{"text":"`) - body.Write(bytes.Repeat([]byte("a"), maxWebhookBody+1)) - body.WriteString(`"}}`) - req := httptest.NewRequest(http.MethodPost, "/webhook", &body) - req.Header.Set(secretTokenHeader, testSecret) - rec := httptest.NewRecorder() - h(rec, req) - if rec.Code != http.StatusRequestEntityTooLarge { - t.Errorf("status = %d, want 413", rec.Code) - } -} - -func TestWebhookHandler_AcceptsValidUpdate(t *testing.T) { - h := WebhookHandler(mustBot(t), testSecret) - req := httptest.NewRequest(http.MethodPost, "/webhook", strings.NewReader(validUpdate)) - req.Header.Set(secretTokenHeader, testSecret) - rec := httptest.NewRecorder() - h(rec, req) - if rec.Code != http.StatusOK { - t.Errorf("status = %d, want 200", rec.Code) - } -} - -func TestTruncateRunes_KeepsUTF8Valid(t *testing.T) { - // Single-byte (ASCII): output must equal a byte slice when boundary aligns. - if got := truncateRunes("hello world", 5); got != "hello" { - t.Errorf("ascii: got %q, want %q", got, "hello") - } - // Multi-byte (Vietnamese): max=5 bytes, "ầ" is 3 bytes ("\xe1\xba\xa7"). - // "h" (1) + "ầ" (3) = 4 bytes; next rune would push to 7. truncate at 5 - // would land mid-rune; the helper must walk back to byte 4 so the slice - // ends on a rune boundary and the result decodes cleanly. - if got := truncateRunes("hầuhầuhầu", 5); got != "hầu" { - t.Errorf("vietnamese: got %q (len %d), want %q (len %d)", got, len(got), "hầu", len("hầu")) - } - // Length-below-cap path: pass through unchanged. - if got := truncateRunes("abc", 10); got != "abc" { - t.Errorf("short: got %q, want %q", got, "abc") - } -} - -// panicUpdate matches the panicHandler registered below by /panic command. -const panicUpdate = `{"update_id":2,"message":{"message_id":1,"date":1,"chat":{"id":1,"type":"private"},"from":{"id":1,"is_bot":false,"first_name":"x"},"text":"/panic","entities":[{"type":"bot_command","offset":0,"length":6}]}}` - -func TestWebhookHandler_RecoversPanicAndReturns200(t *testing.T) { - // A panicking handler must NOT propagate to the http.Server (would close - // the response mid-write and trigger Telegram's 24-hour retry storm on the - // same poisoned update). Recovery returns 200; Telegram does not retry. - b := mustBot(t) - b.RegisterHandler(bot.HandlerTypeMessageText, "panic", bot.MatchTypeCommand, - func(ctx context.Context, _ *bot.Bot, _ *models.Update) { - panic("boom") - }) - - h := WebhookHandler(b, testSecret) - req := httptest.NewRequest(http.MethodPost, "/webhook", strings.NewReader(panicUpdate)) - req.Header.Set(secretTokenHeader, testSecret) - rec := httptest.NewRecorder() - h(rec, req) - if rec.Code != http.StatusOK { - t.Errorf("status = %d, want 200 after recover", rec.Code) - } -}