diff --git a/cmd/bot/main.go b/cmd/bot/main.go index ae3711f..ddc4c9f 100644 --- a/cmd/bot/main.go +++ b/cmd/bot/main.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "html" "github-webhook/internal/bot/middleware" "log" "log/slog" @@ -42,6 +43,9 @@ func main() { func run() (runErr error) { cfg := config.Load() + if cfg.GitHubWebhookSecret == "" { + log.Printf("WARNING: GITHUB_WEBHOOK_SECRET is empty — incoming webhook signature verification is DISABLED. Only use this for local development.") + } database, err := db.Connect(cfg) if err != nil { return fmt.Errorf("connect to DB: %w", err) @@ -122,6 +126,11 @@ func run() (runErr error) { dispatcher.AddHandler(handlers.NewCallback(callbackquery.Prefix("act:"), cbHandler.HandlePRAction)) mux := http.NewServeMux() + mux.HandleFunc("/healthz", func(writer http.ResponseWriter, request *http.Request) { + writer.Header().Set("Content-Type", "text/plain") + writer.WriteHeader(http.StatusOK) + _, _ = writer.Write([]byte("ok")) + }) mux.HandleFunc("/", func(writer http.ResponseWriter, request *http.Request) { html := fmt.Sprintf(` @@ -155,42 +164,49 @@ func run() (runErr error) { } oauthStateCache.Delete(state) - requestCtx, cancel := context.WithTimeout(r.Context(), 15*time.Second) - defer cancel() - token, err := oauth.ExchangeCode(requestCtx, code) - if err != nil { - http.Error(w, "Failed to exchange code", http.StatusInternalServerError) - return - } + // Do the (slow) GitHub exchange + DB write in the background so we return a + // fast 200 to the browser. The request context is cancelled when we return, + // so the goroutine uses its own context with a generous timeout. + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() - encToken, err := utils.Encrypt(token.AccessToken, cfg.EncryptionKey) - if err != nil { - http.Error(w, "Encryption failed", http.StatusInternalServerError) - return - } + token, err := oauth.ExchangeCode(ctx, code) + if err != nil { + log.Printf("OAuth exchange failed for %d: %v", telegramID, err) + return + } - ghClient := clientFactory.GetUserClient(requestCtx, token.AccessToken) - u, _, err := ghClient.Users.Get(requestCtx, "") - if err != nil { - http.Error(w, "Failed to fetch user", http.StatusInternalServerError) - return - } + encToken, err := utils.Encrypt(token.AccessToken, cfg.EncryptionKey) + if err != nil { + log.Printf("OAuth encrypt failed for %d: %v", telegramID, err) + return + } - user := &models.User{ - ID: telegramID, - GitHubUserID: u.GetID(), - GitHubUsername: u.GetLogin(), - EncryptedOAuthToken: encToken, - } - if err := database.UpsertUser(requestCtx, user); err != nil { - http.Error(w, "DB Error", http.StatusInternalServerError) - return - } + ghClient := clientFactory.GetUserClient(ctx, token.AccessToken) + u, _, err := ghClient.Users.Get(ctx, "") + if err != nil { + log.Printf("OAuth fetch user failed for %d: %v", telegramID, err) + return + } - _, _ = b.SendMessage(telegramID, fmt.Sprintf("✅ GitHub account %s connected successfully!", u.GetLogin()), &gotgbot.SendMessageOpts{ParseMode: "HTML"}) + user := &models.User{ + ID: telegramID, + GitHubUserID: u.GetID(), + GitHubUsername: u.GetLogin(), + EncryptedOAuthToken: encToken, + } + if err := database.UpsertUser(ctx, user); err != nil { + log.Printf("OAuth DB upsert failed for %d: %v", telegramID, err) + _, _ = b.SendMessage(telegramID, "⚠️ Connected to GitHub but failed to save your token. Please run /connect again.", &gotgbot.SendMessageOpts{ParseMode: "HTML"}) + return + } - html := fmt.Sprintf(` + _, _ = b.SendMessage(telegramID, fmt.Sprintf("✅ GitHub account %s connected successfully!", html.EscapeString(u.GetLogin())), &gotgbot.SendMessageOpts{ParseMode: "HTML"}) + }() + + htmlBody := fmt.Sprintf(` Connected @@ -205,7 +221,7 @@ func run() (runErr error) { `, b.User.Username, b.User.Username) w.Header().Set("Content-Type", "text/html") - _, _ = w.Write([]byte(html)) + _, _ = w.Write([]byte(htmlBody)) }) listener, err := net.Listen("tcp", ":"+cfg.Port) @@ -291,6 +307,25 @@ func run() (runErr error) { signalCtx, stopSignals := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stopSignals() + // Periodically sweep TTL caches so they do not grow unbounded (entries otherwise + // only evict on re-access, and most are never read again). + go func() { + ticker := time.NewTicker(10 * time.Minute) + defer ticker.Stop() + for { + select { + case <-signalCtx.Done(): + return + case <-ticker.C: + database.ChatReposCache.Cleanup() + oauthStateCache.Cleanup() + contextCache.Cleanup() + actionCache.Cleanup() + webhookServer.DeliverySeen.Cleanup() + } + } + }() + select { case <-signalCtx.Done(): log.Printf("Shutdown signal received") diff --git a/internal/bot/callbacks/callbacks.go b/internal/bot/callbacks/callbacks.go index 604784a..aa8b12e 100644 --- a/internal/bot/callbacks/callbacks.go +++ b/internal/bot/callbacks/callbacks.go @@ -36,7 +36,7 @@ func (h *CallbackHandler) getClient(b *gotgbot.Bot, ctx *ext.Context) (*gh.Clien client, err := github.GetClientForUser(context.Background(), h.DB, h.ClientFactory, ctx.EffectiveUser.Id, h.EncryptionKey) if err != nil { msg := "Auth error." - if err.Error() == "unauthorized" { + if errors.Is(err, github.ErrUnauthorized) { msg = "Please /connect to GitHub first." } _, _ = ctx.CallbackQuery.Answer(b, &gotgbot.AnswerCallbackQueryOpts{Text: msg, ShowAlert: true}) @@ -95,55 +95,6 @@ func cbAddRepoID(repoID int64) string { return cb(cbPrefixSettings, cbAddRepo, "id", strconv.FormatInt(repoID, 10)) } -func repoButtonText(name string) string { - const max = 42 - if len(name) <= max { - return name - } - return name[:max-1] + "…" -} - -func repoPageNav(page int, resp *gh.Response) []gotgbot.InlineKeyboardButton { - var navRow []gotgbot.InlineKeyboardButton - - if resp.FirstPage != 0 && resp.PrevPage != 0 { - navRow = append(navRow, ui.Callback("<", cbAddRepoPage(resp.PrevPage), - ui.WithStyle(ui.StylePrimary), - ui.WithCustomEmojiEnv(ui.IconPrevious), - )) - } - - startPage := page - 1 - if startPage < 1 { - startPage = 1 - } - - endPage := page + 1 - if resp.LastPage != 0 && endPage > resp.LastPage { - endPage = resp.LastPage - } - if resp.LastPage == 0 && resp.NextPage != 0 { - endPage = resp.NextPage - } - - for i := startPage; i <= endPage; i++ { - text := strconv.Itoa(i) - if i == page { - text = "· " + text + " ·" - } - navRow = append(navRow, ui.Callback(text, cbAddRepoPage(i), ui.WithStyle(ui.StylePrimary))) - } - - if resp.NextPage != 0 { - navRow = append(navRow, ui.Callback(">", cbAddRepoPage(resp.NextPage), - ui.WithStyle(ui.StylePrimary), - ui.WithCustomEmojiEnv(ui.IconNext), - )) - } - - return navRow -} - func (h *CallbackHandler) HandleSettings(b *gotgbot.Bot, ctx *ext.Context) error { if ctx.EffectiveChat.Type != gotgbot.ChatTypePrivate && !utils.IsAdmin(b, ctx.EffectiveChat.Id, ctx.EffectiveUser.Id) { _, _ = ctx.CallbackQuery.Answer(b, &gotgbot.AnswerCallbackQueryOpts{Text: "Only admins can change settings", ShowAlert: true}) @@ -372,7 +323,7 @@ func (h *CallbackHandler) handleStopNotifications(b *gotgbot.Bot, ctx *ext.Conte if l.WebhookID != 0 { client, err := github.GetClientForUser(context.Background(), h.DB, h.ClientFactory, ctx.EffectiveUser.Id, h.EncryptionKey) if err != nil { - if err.Error() == "unauthorized" { + if errors.Is(err, github.ErrUnauthorized) { warning = "\n\nWarning: your GitHub account is not connected, so the GitHub webhook could not be removed automatically." } else { warning = "\n\nWarning: your GitHub token could not be decrypted, so the GitHub webhook was not removed automatically." @@ -486,7 +437,7 @@ func (h *CallbackHandler) handlePresets(b *gotgbot.Bot, ctx *ext.Context, l *mod func (h *CallbackHandler) showIndividualEvents(b *gotgbot.Bot, ctx *ext.Context, l *models.RepoLink, page int) error { client, err := github.GetClientForUser(context.Background(), h.DB, h.ClientFactory, ctx.EffectiveUser.Id, h.EncryptionKey) if err != nil { - if err.Error() == "unauthorized" { + if errors.Is(err, github.ErrUnauthorized) { _, _, _ = ctx.EffectiveMessage.EditText(b, "Error: You must be connected to GitHub to view/edit settings.", nil) } else { _, _, _ = ctx.EffectiveMessage.EditText(b, "Auth error. Please reconnect.", nil) @@ -583,7 +534,7 @@ func (h *CallbackHandler) showRepoList(b *gotgbot.Bot, ctx *ext.Context) error { var kb [][]gotgbot.InlineKeyboardButton for _, l := range links { kb = append(kb, []gotgbot.InlineKeyboardButton{ - ui.Callback(repoButtonText(l.RepoFullName), cbRepo(cbRepoMenu, &l), + ui.Callback(ui.CompactButtonText(l.RepoFullName), cbRepo(cbRepoMenu, &l), ui.WithStyle(ui.StylePrimary), ui.WithCustomEmojiEnv(ui.IconSettings), ), @@ -619,14 +570,14 @@ func (h *CallbackHandler) handleRepoPage(b *gotgbot.Bot, ctx *ext.Context, page var kb [][]gotgbot.InlineKeyboardButton for _, repo := range repos { kb = append(kb, []gotgbot.InlineKeyboardButton{ - ui.Callback(repoButtonText(repo.GetFullName()), cbAddRepoID(repo.GetID()), + ui.Callback(ui.CompactButtonText(repo.GetFullName()), cbAddRepoID(repo.GetID()), ui.WithStyle(ui.StylePrimary), ui.WithCustomEmojiEnv(ui.IconAdd), ), }) } - if navRow := repoPageNav(page, resp); len(navRow) > 0 { + if navRow := ui.RepoPageNav(page, resp, cbAddRepoPage); len(navRow) > 0 { kb = append(kb, navRow) } diff --git a/internal/bot/commands/commands.go b/internal/bot/commands/commands.go index 8f8f820..e96d31d 100644 --- a/internal/bot/commands/commands.go +++ b/internal/bot/commands/commands.go @@ -140,55 +140,6 @@ func repoSettingsCallback(link models.RepoLink) string { return settingsCallback("c", "r", linkID) } -func compactButtonText(name string) string { - const max = 42 - if len(name) <= max { - return name - } - return name[:max-1] + "…" -} - -func repoPageKeyboardNav(page int, resp *github.Response) []gotgbot.InlineKeyboardButton { - var navRow []gotgbot.InlineKeyboardButton - - if resp.FirstPage != 0 && resp.PrevPage != 0 { - navRow = append(navRow, ui.Callback("<", addRepoPageCallback(resp.PrevPage), - ui.WithStyle(ui.StylePrimary), - ui.WithCustomEmojiEnv(ui.IconPrevious), - )) - } - - startPage := page - 1 - if startPage < 1 { - startPage = 1 - } - - endPage := page + 1 - if resp.LastPage != 0 && endPage > resp.LastPage { - endPage = resp.LastPage - } - if resp.LastPage == 0 && resp.NextPage != 0 { - endPage = resp.NextPage - } - - for i := startPage; i <= endPage; i++ { - text := strconv.Itoa(i) - if i == page { - text = "· " + text + " ·" - } - navRow = append(navRow, ui.Callback(text, addRepoPageCallback(i), ui.WithStyle(ui.StylePrimary))) - } - - if resp.NextPage != 0 { - navRow = append(navRow, ui.Callback(">", addRepoPageCallback(resp.NextPage), - ui.WithStyle(ui.StylePrimary), - ui.WithCustomEmojiEnv(ui.IconNext), - )) - } - - return navRow -} - func (h *CommandHandler) AddRepo(b *gotgbot.Bot, ctx *ext.Context) error { if err := requireAdminOrPrivate(b, ctx, "Only admins can add repositories."); err != nil { return err @@ -202,7 +153,7 @@ func (h *CommandHandler) AddRepo(b *gotgbot.Bot, ctx *ext.Context) error { repoFullName := args[1] client, err := gh.GetClientForUser(context.Background(), h.DB, h.ClientFactory, ctx.EffectiveUser.Id, h.EncryptionKey) if err != nil { - if err.Error() == "unauthorized" { + if errors.Is(err, gh.ErrUnauthorized) { url, urlErr := h.loginURLForUser(ctx.EffectiveUser.Id) if urlErr != nil { return urlErr @@ -316,7 +267,7 @@ func (h *CommandHandler) listUserRepos(b *gotgbot.Bot, ctx *ext.Context) error { func (h *CommandHandler) sendRepoList(b *gotgbot.Bot, ctx *ext.Context, page int) error { client, err := gh.GetClientForUser(context.Background(), h.DB, h.ClientFactory, ctx.EffectiveUser.Id, h.EncryptionKey) if err != nil { - if err.Error() == "unauthorized" { + if errors.Is(err, gh.ErrUnauthorized) { _, _ = ctx.EffectiveMessage.Reply(b, "Please /connect your GitHub account first to list repositories.", nil) } else { _, _ = ctx.EffectiveMessage.Reply(b, "Auth error. Reconnect via /connect", nil) @@ -346,14 +297,14 @@ func (h *CommandHandler) sendRepoList(b *gotgbot.Bot, ctx *ext.Context, page int var kb [][]gotgbot.InlineKeyboardButton for _, repo := range repos { kb = append(kb, []gotgbot.InlineKeyboardButton{ - ui.Callback(compactButtonText(repo.GetFullName()), addRepoIDCallback(repo.GetID()), + ui.Callback(ui.CompactButtonText(repo.GetFullName()), addRepoIDCallback(repo.GetID()), ui.WithStyle(ui.StylePrimary), ui.WithCustomEmojiEnv(ui.IconAdd), ), }) } - if navRow := repoPageKeyboardNav(page, resp); len(navRow) > 0 { + if navRow := ui.RepoPageNav(page, resp, addRepoPageCallback); len(navRow) > 0 { kb = append(kb, navRow) } @@ -381,7 +332,7 @@ func (h *CommandHandler) Settings(b *gotgbot.Bot, ctx *ext.Context) error { var kb [][]gotgbot.InlineKeyboardButton for _, l := range links { kb = append(kb, []gotgbot.InlineKeyboardButton{ - ui.Callback(compactButtonText(l.RepoFullName), repoSettingsCallback(l), + ui.Callback(ui.CompactButtonText(l.RepoFullName), repoSettingsCallback(l), ui.WithStyle(ui.StylePrimary), ui.WithCustomEmojiEnv(ui.IconSettings), ), @@ -417,7 +368,7 @@ func (h *CommandHandler) RemoveRepo(b *gotgbot.Bot, ctx *ext.Context) error { if link.WebhookID != 0 { client, err := gh.GetClientForUser(context.Background(), h.DB, h.ClientFactory, ctx.EffectiveUser.Id, h.EncryptionKey) if err != nil { - if err.Error() == "unauthorized" { + if errors.Is(err, gh.ErrUnauthorized) { webhookStatusMsg = "\n\n⚠️ Warning: You are not connected to GitHub. The webhook could not be removed from the repository settings. Please remove it manually." } else { webhookStatusMsg = "\n\n⚠️ Warning: Could not decrypt your access token. Webhook not removed from GitHub." @@ -654,7 +605,7 @@ func (h *CommandHandler) handleIssueAction(b *gotgbot.Bot, ctx *ext.Context, sta func (h *CommandHandler) getAuthenticatedClient(b *gotgbot.Bot, ctx *ext.Context) (*github.Client, error) { client, err := gh.GetClientForUser(context.Background(), h.DB, h.ClientFactory, ctx.EffectiveUser.Id, h.EncryptionKey) if err != nil { - if err.Error() == "unauthorized" { + if errors.Is(err, gh.ErrUnauthorized) { url, urlErr := h.loginURLForUser(ctx.EffectiveUser.Id) if urlErr != nil { return nil, urlErr diff --git a/internal/bot/commands/reply_handler.go b/internal/bot/commands/reply_handler.go index d5dbe8d..85259e1 100644 --- a/internal/bot/commands/reply_handler.go +++ b/internal/bot/commands/reply_handler.go @@ -2,6 +2,7 @@ package commands import ( "context" + "errors" "fmt" "strings" @@ -50,7 +51,7 @@ func (h *ReplyHandler) HandleReply(b *gotgbot.Bot, ctx *ext.Context) error { client, err := github.GetClientForUser(context.Background(), h.DB, h.ClientFactory, ctx.EffectiveUser.Id, h.EncryptionKey) if err != nil { - if err.Error() == "unauthorized" { + if errors.Is(err, github.ErrUnauthorized) { _, _ = msg.Reply(b, "Please /connect your GitHub account in a private chat before replying to GitHub items.", nil) } else { _, _ = msg.Reply(b, "Auth error. Reconnect via /connect", nil) diff --git a/internal/bot/middleware/middleware.go b/internal/bot/middleware/middleware.go index 3d33a9b..5c98201 100644 --- a/internal/bot/middleware/middleware.go +++ b/internal/bot/middleware/middleware.go @@ -2,7 +2,9 @@ package middleware import ( "context" + "time" + "github-webhook/internal/cache" "github-webhook/internal/db" "github-webhook/internal/models" @@ -10,6 +12,10 @@ import ( "github.com/PaulSonOfLars/gotgbot/v2/ext" ) +// chatUpsertSeen debounces chat upserts so we write to the DB at most once per chat +// every 10 minutes instead of on every incoming update. +var chatUpsertSeen = cache.New[int64, struct{}]() + func TrackUserAndChat(database *db.DB) func(b *gotgbot.Bot, ctx *ext.Context) error { return func(b *gotgbot.Bot, ctx *ext.Context) error { if ctx.EffectiveChat != nil { @@ -23,6 +29,11 @@ func TrackUserAndChat(database *db.DB) func(b *gotgbot.Bot, ctx *ext.Context) er dbChat.Title = ctx.EffectiveChat.Username } + if _, seen := chatUpsertSeen.Get(ctx.EffectiveChat.Id); seen { + return nil + } + chatUpsertSeen.Set(ctx.EffectiveChat.Id, struct{}{}, 10*time.Minute) + go func() { _ = database.UpsertChat(context.Background(), dbChat) }() diff --git a/internal/bot/ui/buttons.go b/internal/bot/ui/buttons.go index cca2ed5..a74ad4f 100644 --- a/internal/bot/ui/buttons.go +++ b/internal/bot/ui/buttons.go @@ -3,9 +3,11 @@ package ui import ( "log" "os" + "strconv" "strings" "github.com/PaulSonOfLars/gotgbot/v2" + gh "github.com/google/go-github/v85/github" ) const ( @@ -85,3 +87,56 @@ func validateCallback(data string) { log.Printf("Telegram callback_data exceeds 64 bytes: length=%d data=%q", len(data), data) } } + +// RepoPageNav builds the previous / numbered / next pagination row for the add-repo +// repository picker. pageData maps a page number to its callback data string; commands +// and callbacks pass their own builder so ui stays unaware of the callback protocol. +func RepoPageNav(page int, resp *gh.Response, pageData func(int) string) []gotgbot.InlineKeyboardButton { + var navRow []gotgbot.InlineKeyboardButton + + if resp.FirstPage != 0 && resp.PrevPage != 0 { + navRow = append(navRow, Callback("<", pageData(resp.PrevPage), + WithStyle(StylePrimary), + WithCustomEmojiEnv(IconPrevious), + )) + } + + startPage := page - 1 + if startPage < 1 { + startPage = 1 + } + + endPage := page + 1 + if resp.LastPage != 0 && endPage > resp.LastPage { + endPage = resp.LastPage + } + if resp.LastPage == 0 && resp.NextPage != 0 { + endPage = resp.NextPage + } + + for i := startPage; i <= endPage; i++ { + text := strconv.Itoa(i) + if i == page { + text = "· " + text + " ·" + } + navRow = append(navRow, Callback(text, pageData(i), WithStyle(StylePrimary))) + } + + if resp.NextPage != 0 { + navRow = append(navRow, Callback(">", pageData(resp.NextPage), + WithStyle(StylePrimary), + WithCustomEmojiEnv(IconNext), + )) + } + + return navRow +} + +// CompactButtonText truncates a button label to Telegram-friendly length with an ellipsis. +func CompactButtonText(name string) string { + const max = 42 + if len(name) <= max { + return name + } + return name[:max-1] + "…" +} diff --git a/internal/db/db.go b/internal/db/db.go index 77a51aa..d66a6d4 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -83,7 +83,15 @@ func (d *DB) GetUserByTelegramID(ctx context.Context, telegramID int64) (*models func (d *DB) UpsertUser(ctx context.Context, user *models.User) error { opts := options.UpdateOne().SetUpsert(true) filter := bson.M{"_id": user.ID} - update := bson.M{ + _, err := d.Users.UpdateOne(ctx, filter, buildUserUpsert(user), opts) + return err +} + +// buildUserUpsert returns the update document for UpsertUser. Mutable fields go under +// $set; the immutable _id goes under $setOnInsert so re-upserts of existing users never +// attempt to modify _id (which MongoDB rejects). +func buildUserUpsert(user *models.User) bson.M { + return bson.M{ "$set": bson.M{ "github_user_id": user.GitHubUserID, "github_username": user.GitHubUsername, @@ -94,8 +102,6 @@ func (d *DB) UpsertUser(ctx context.Context, user *models.User) error { "_id": user.ID, }, } - _, err := d.Users.UpdateOne(ctx, filter, update, opts) - return err } func (d *DB) ClearUserToken(ctx context.Context, userID int64) error { diff --git a/internal/db/db_test.go b/internal/db/db_test.go new file mode 100644 index 0000000..58d7c57 --- /dev/null +++ b/internal/db/db_test.go @@ -0,0 +1,49 @@ +package db + +import ( + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" + + "github-webhook/internal/models" +) + +// TestBuildUserUpsertShape locks the UpsertUser BSON shape without requiring a live +// MongoDB: _id must live under $setOnInsert (never $set, which MongoDB rejects for +// existing documents) and the document must be valid BSON. +func TestBuildUserUpsertShape(t *testing.T) { + update := buildUserUpsert(&models.User{ + ID: 42, + GitHubUserID: 7, + GitHubUsername: "alice", + EncryptedOAuthToken: "enc", + Scopes: []string{"repo"}, + }) + + if _, ok := update["$set"]; !ok { + t.Fatal("update missing $set") + } + if _, ok := update["$setOnInsert"]; !ok { + t.Fatal("update missing $setOnInsert") + } + + set := update["$set"].(bson.M) + if _, ok := set["_id"]; ok { + t.Fatal("_id must not appear under $set (immutable field)") + } + + setOnInsert := update["$setOnInsert"].(bson.M) + id, ok := setOnInsert["_id"] + if !ok || id != int64(42) { + t.Fatalf("expected _id=42 under $setOnInsert, got %v (present=%v)", id, ok) + } + + data, err := bson.Marshal(update) + if err != nil { + t.Fatalf("update is not valid BSON: %v", err) + } + var round bson.M + if err := bson.Unmarshal(data, &round); err != nil { + t.Fatalf("bson round-trip failed: %v", err) + } +} diff --git a/internal/github/auth_helper.go b/internal/github/auth_helper.go index 94727d0..a283214 100644 --- a/internal/github/auth_helper.go +++ b/internal/github/auth_helper.go @@ -2,6 +2,7 @@ package github import ( "context" + "errors" "fmt" "github-webhook/internal/db" @@ -10,12 +11,16 @@ import ( "github.com/google/go-github/v85/github" ) +// ErrUnauthorized is returned by GetClientForUser when the user has no linked GitHub +// account or no decrypted OAuth token. Callers should use errors.Is to detect it. +var ErrUnauthorized = errors.New("unauthorized") + // GetClientForUser retrieves the user's OAuth token from the database, decrypts it, // and returns an authenticated GitHub client. func GetClientForUser(ctx context.Context, database *db.DB, factory *ClientFactory, userID int64, encryptionKey string) (*github.Client, error) { user, err := database.GetUserByTelegramID(ctx, userID) if err != nil || user.EncryptedOAuthToken == "" { - return nil, fmt.Errorf("unauthorized") + return nil, ErrUnauthorized } token, err := utils.Decrypt(user.EncryptedOAuthToken, encryptionKey) diff --git a/internal/github/client.go b/internal/github/client.go index 0b5a434..6ce150a 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -2,21 +2,31 @@ package github import ( "context" + "sync" "github.com/google/go-github/v85/github" "golang.org/x/oauth2" ) type ClientFactory struct { + clients sync.Map // accessToken -> *github.Client } func NewClientFactory() *ClientFactory { return &ClientFactory{} } -// GetUserClient returns a GitHub client authenticated as a specific User (via OAuth token) +// GetUserClient returns a GitHub client authenticated as a specific User (via OAuth token). +// Clients are cached per access token to reuse the underlying TCP connection and avoid +// rebuilding the oauth2 transport on every call. func (f *ClientFactory) GetUserClient(ctx context.Context, accessToken string) *github.Client { + if c, ok := f.clients.Load(accessToken); ok { + return c.(*github.Client) + } + ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: accessToken}) tc := oauth2.NewClient(ctx, ts) - return github.NewClient(tc) + c := github.NewClient(tc) + f.clients.Store(accessToken, c) + return c } diff --git a/internal/github/markdown_utils.go b/internal/github/markdown_utils.go index dd5577f..561b45b 100644 --- a/internal/github/markdown_utils.go +++ b/internal/github/markdown_utils.go @@ -68,6 +68,8 @@ func EscapeMarkdownV2URL(text string) string { } // FormatTextWithMarkdown preserves Markdown links and code blocks while escaping other special characters. +// Link anchors are also escaped: an anchor such as "foo.bar" would otherwise break Telegram's +// MarkdownV2 parser (unescaped ".") and force a plain-text fallback that drops all formatting. func FormatTextWithMarkdown(text string) string { emailRe := regexp.MustCompile(`<[^> ]+@[^> ]+>`) var emails []string @@ -95,12 +97,30 @@ func FormatTextWithMarkdown(text string) string { for i, original := range originals { placeholder := fmt.Sprintf("___PLACEHOLDER_%d___", i) escapedPlaceholder := EscapeMarkdownV2(placeholder) - escapedBody = strings.Replace(escapedBody, escapedPlaceholder, original, 1) + escapedBody = strings.Replace(escapedBody, escapedPlaceholder, restoreProtectedSegment(original), 1) } return escapedBody } +// restoreProtectedSegment returns a MarkdownV2-safe form of a protected segment. Links get their +// anchor and URL escaped; code spans/fenced blocks are returned verbatim (their contents are literal). +func restoreProtectedSegment(segment string) string { + if text, url, ok := parseMarkdownLink(segment); ok { + return fmt.Sprintf("[%s](%s)", EscapeMarkdownV2(text), EscapeMarkdownV2URL(url)) + } + return segment +} + +func parseMarkdownLink(segment string) (text, url string, ok bool) { + linkRe := regexp.MustCompile(`^\[(.+)\]\((.+)\)$`) + m := linkRe.FindStringSubmatch(segment) + if m == nil { + return "", "", false + } + return m[1], m[2], true +} + func FormatReleaseBody(body string) string { formattedText := FormatTextWithMarkdown(body) lines := strings.Split(formattedText, "\n") diff --git a/internal/github/markdown_utils_test.go b/internal/github/markdown_utils_test.go index 7c24205..595dabe 100644 --- a/internal/github/markdown_utils_test.go +++ b/internal/github/markdown_utils_test.go @@ -159,3 +159,29 @@ func TestFormatReleaseBody(t *testing.T) { } }) } + +// An unescaped link anchor (e.g. a "." in "release.v2") makes Telegram's MarkdownV2 +// parser reject the whole message, forcing a plain-text fallback that drops all styling. +func TestFormatTextWithMarkdownEscapesLinkAnchor(t *testing.T) { + in := "See [release.v2.1](https://github.com/o/r/releases/tag/v2.1) for details." + want := "See [release\\.v2\\.1](https://github.com/o/r/releases/tag/v2.1) for details\\." + if got := FormatTextWithMarkdown(in); got != want { + t.Fatalf("FormatTextWithMarkdown() = %q, want %q", got, want) + } +} + +func TestFormatTextWithMarkdownKeepsCodeVerbatim(t *testing.T) { + in := "Run `go test ./...` and check `a.b` inline." + want := "Run `go test ./...` and check `a.b` inline\\." + if got := FormatTextWithMarkdown(in); got != want { + t.Fatalf("code spans should pass through unchanged, got %q", got) + } +} + +func TestFormatTextWithMarkdownEscapesBodySpecials(t *testing.T) { + in := "Fix a.b and call me (soon)! Use ~strike~." + want := "Fix a\\.b and call me \\(soon\\)\\! Use \\~strike\\~\\." + if got := FormatTextWithMarkdown(in); got != want { + t.Fatalf("FormatTextWithMarkdown() = %q, want %q", got, want) + } +} diff --git a/internal/github/webhooks.go b/internal/github/webhooks.go index 9ef3227..7f63982 100644 --- a/internal/github/webhooks.go +++ b/internal/github/webhooks.go @@ -38,6 +38,7 @@ type WebhookServer struct { Bot *gotgbot.Bot ContextCache *cache.Cache[string, models.MessageContext] // Key: "chat_id:message_id" ActionCache *cache.Cache[string, models.PRActionContext] // Key: UUID + DeliverySeen *cache.Cache[string, struct{}] // Key: X-GitHub-Delivery (idempotency) Wg sync.WaitGroup } @@ -48,6 +49,7 @@ func NewWebhookServer(cfg *config.Config, database *db.DB, bot *gotgbot.Bot, ctx Bot: bot, ContextCache: ctxCache, ActionCache: actionCache, + DeliverySeen: cache.New[string, struct{}](), } } @@ -58,6 +60,9 @@ func (s *WebhookServer) Handler(w http.ResponseWriter, r *http.Request) { return } + r.Body = http.MaxBytesReader(w, r.Body, maxWebhookPayloadBytes) + defer r.Body.Close() + receivedAt := time.Now() var chatID int64 eventType := github.WebHookType(r) @@ -88,9 +93,6 @@ func (s *WebhookServer) Handler(w http.ResponseWriter, r *http.Request) { log.Printf("Webhook received event=%s delivery=%s hook_id=%s chat=%d remote=%s", eventType, deliveryID, hookIDHeader, chatID, r.RemoteAddr) - r.Body = http.MaxBytesReader(w, r.Body, maxWebhookPayloadBytes) - defer r.Body.Close() - payload, err := github.ValidatePayload(r, []byte(s.Config.GitHubWebhookSecret)) if err != nil { var maxBytesErr *http.MaxBytesError @@ -123,6 +125,15 @@ func (s *WebhookServer) Handler(w http.ResponseWriter, r *http.Request) { hookID, _ = strconv.ParseInt(idStr, 10, 64) } + if deliveryID != "" { + if _, seen := s.DeliverySeen.Get(deliveryID); seen { + log.Printf("Webhook duplicate delivery ignored event=%s delivery=%s chat=%d", eventType, deliveryID, chatID) + w.WriteHeader(http.StatusOK) + return + } + s.DeliverySeen.Set(deliveryID, struct{}{}, 10*time.Minute) + } + s.Wg.Add(1) go func() { defer s.Wg.Done() @@ -161,6 +172,13 @@ func (s *WebhookServer) processEvent(event interface{}, chatID int64, hookID int msg = normalizeMessage(msg) + // Telegram rejects messages over 4096 runes. The plain-text fallback already + // truncates; cap here too so a long formatted event is not lost outright. + const maxTelegramText = 4096 + if runes := []rune(msg); len(runes) > maxTelegramText { + msg = string(runes[:maxTelegramText-1]) + "…" + } + var threadID int64 if hookID != 0 { ctx, cancel := context.WithTimeout(context.Background(), webhookDBTimeout)