improve user management and webhook validation handling

cmd/bot/main.go

@@ -322,6 +322,7 @@ func run() (runErr error) {
 				contextCache.Cleanup()
 				actionCache.Cleanup()
 				webhookServer.DeliverySeen.Cleanup()
+				clientFactory.Cleanup()
 			}
 		}
 	}()

internal/bot/callbacks/callbacks.go

@@ -753,6 +753,7 @@ func (h *CallbackHandler) HandlePRAction(b *gotgbot.Bot, ctx *ext.Context) error
 	}
 
 	_, _ = ctx.CallbackQuery.Answer(b, &gotgbot.AnswerCallbackQueryOpts{Text: msg, ShowAlert: true})
+	h.ActionCache.Delete(actionID)
 	return nil
 }
 

internal/config/config.go

@@ -34,7 +34,6 @@ func Load() *Config {
 		"MONGODB_URI",
 		"GITHUB_CLIENT_ID",
 		"GITHUB_CLIENT_SECRET",
-		"GITHUB_WEBHOOK_SECRET",
 		"ENCRYPTION_KEY",
 	}
 

internal/github/client.go

@@ -3,13 +3,24 @@ package github
 import (
 	"context"
 	"sync"
+	"time"
 
 	"github.com/google/go-github/v85/github"
 	"golang.org/x/oauth2"
 )
 
+// clientIdleTTL bounds how long an unused authenticated client is kept. The cache is
+// keyed by access token and users re-authenticate over time, so without a TTL it would
+// grow without bound for a long-running process.
+const clientIdleTTL = 30 * time.Minute
+
+type cachedClient struct {
+	client   *github.Client
+	lastUsed time.Time
+}
+
 type ClientFactory struct {
-	clients sync.Map // accessToken -> *github.Client
+	clients sync.Map // accessToken -> *cachedClient
 }
 
 func NewClientFactory() *ClientFactory {
@@ -20,13 +31,26 @@ func NewClientFactory() *ClientFactory {
 // 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)
+	if v, ok := f.clients.Load(accessToken); ok {
+		cc := v.(*cachedClient)
+		cc.lastUsed = time.Now()
+		return cc.client
 	}
 
 	ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: accessToken})
-	tc := oauth2.NewClient(ctx, ts)
-	c := github.NewClient(tc)
-	f.clients.Store(accessToken, c)
+	c := github.NewClient(oauth2.NewClient(ctx, ts))
+	f.clients.Store(accessToken, &cachedClient{client: c, lastUsed: time.Now()})
 	return c
 }
+
+// Cleanup drops clients that have not been used within clientIdleTTL. Safe to call
+// periodically; in-flight requests on an evicted client are unaffected.
+func (f *ClientFactory) Cleanup() {
+	now := time.Now()
+	f.clients.Range(func(key, value any) bool {
+		if now.Sub(value.(*cachedClient).lastUsed) > clientIdleTTL {
+			f.clients.Delete(key)
+		}
+		return true
+	})
+}

internal/github/format.go

@@ -1117,7 +1117,10 @@ func FormatPullRequestReviewThreadEvent(e *github.PullRequestReviewThreadEvent)
 		FormatUser(sender.GetLogin()),
 	)
 
-	return FormatMessageWithButton(msg, "View Thread", e.GetThread().Comments[0].GetHTMLURL())
+	if thread := e.GetThread(); thread != nil && len(thread.Comments) > 0 {
+		return FormatMessageWithButton(msg, "View Thread", thread.Comments[0].GetHTMLURL())
+	}
+	return msg, nil
 }
 
 func FormatPullRequestTargetEvent(e *github.PullRequestTargetEvent) (string, *gotgbot.InlineKeyboardMarkup) {

internal/github/webhooks.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"io"
 	"log"
 	"net/http"
 	"regexp"
@@ -93,7 +92,17 @@ 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)
 
-	payload, err := github.ValidatePayload(r, []byte(s.Config.GitHubWebhookSecret))
+	// Read and validate the body exactly once. ValidatePayloadFromBody consumes
+	// r.Body and returns the payload; it verifies the GitHub HMAC signature only
+	// when a secret (or a signature header) is present, so the dev-mode path with
+	// no secret and no signature still yields the real payload. The previous code
+	// called ValidatePayload (which drains the body) and then tried io.ReadAll on
+	// the now-empty body in the no-secret branch, producing an empty payload.
+	sig := r.Header.Get("X-Hub-Signature-256")
+	if sig == "" {
+		sig = r.Header.Get("X-Hub-Signature")
+	}
+	payload, err := github.ValidatePayloadFromBody(r.Header.Get("Content-Type"), r.Body, sig, []byte(s.Config.GitHubWebhookSecret))
 	if err != nil {
 		var maxBytesErr *http.MaxBytesError
 		if errors.As(err, &maxBytesErr) {
@@ -103,14 +112,12 @@ func (s *WebhookServer) Handler(w http.ResponseWriter, r *http.Request) {
 		}
 
 		if s.Config.GitHubWebhookSecret == "" {
-			log.Printf("Webhook Warning: No GITHUB_WEBHOOK_SECRET configured. Validation skipped. event=%s chat=%d", eventType, chatID)
-			body, _ := io.ReadAll(r.Body)
-			payload = body
+			log.Printf("Webhook REJECTED: signature present but GITHUB_WEBHOOK_SECRET is empty; cannot verify. event=%s delivery=%s chat=%d error=%v", eventType, deliveryID, chatID, err)
 		} else {
 			log.Printf("Webhook REJECTED: Signature mismatch. This means the secret in GitHub doesn't match GITHUB_WEBHOOK_SECRET on Render. event=%s delivery=%s chat=%d error=%v", eventType, deliveryID, chatID, err)
-			http.Error(w, "Invalid signature", http.StatusUnauthorized)
-			return
 		}
+		http.Error(w, "Invalid signature", http.StatusUnauthorized)
+		return
 	}
 
 	event, err := parseWebhookEvent(eventType, payload)
@@ -352,8 +359,9 @@ func (s *WebhookServer) withPRActionButtons(event interface{}, markup *gotgbot.I
 
 func (s *WebhookServer) formatMessage(event interface{}) (msg string, markup *gotgbot.InlineKeyboardMarkup) {
 	defer func() {
-		if recover() != nil {
-			msg, markup = FormatGenericEvent(event)
+		if r := recover(); r != nil {
+			log.Printf("Formatter panic for event %T: %v", event, r)
+			msg, markup = "", nil
 		}
 	}()