From 7bde3f2f7ccd0450cb9d32b9619db27708114313 Mon Sep 17 00:00:00 2001 From: viettranx Date: Fri, 13 Mar 2026 17:29:28 +0700 Subject: [PATCH] fix(pairing): handle transient DB errors in IsPaired to prevent spurious pair requests IsPaired() silently returned false on DB query failures (connection timeout, pool exhaustion), causing the system to treat already-paired users/groups as unpaired and create spurious pairing requests. Changed IsPaired signature to return (bool, error). All 13 callers across all channels (Telegram, Discord, Slack, Feishu, WhatsApp, Zalo) and gateway (browser WS, pairing method) now fail-open on DB errors: log a warning and assume paired, avoiding false rejections. --- internal/channels/discord/handler.go | 23 ++++++++++++++---- internal/channels/feishu/bot_policy.go | 24 +++++++++++++++---- internal/channels/slack/handlers_mention.go | 26 +++++++++++++++++---- internal/channels/telegram/handlers.go | 18 ++++++++++++-- internal/channels/whatsapp/whatsapp.go | 23 ++++++++++++++---- internal/channels/zalo/personal/policy.go | 21 +++++++++++++++-- internal/channels/zalo/zalo.go | 9 ++++++- internal/gateway/methods/pairing.go | 7 +++++- internal/gateway/router.go | 26 ++++++++++++++------- internal/store/pairing_store.go | 2 +- internal/store/pg/pairing.go | 9 ++++--- 11 files changed, 151 insertions(+), 37 deletions(-) diff --git a/internal/channels/discord/handler.go b/internal/channels/discord/handler.go index c0a60dbe..cba40d34 100644 --- a/internal/channels/discord/handler.go +++ b/internal/channels/discord/handler.go @@ -272,9 +272,17 @@ func (c *Channel) checkGroupPolicy(senderID, channelID string) bool { return true } groupSenderID := fmt.Sprintf("group:%s", channelID) - if c.pairingService != nil && c.pairingService.IsPaired(groupSenderID, c.Name()) { - c.approvedGroups.Store(channelID, true) - return true + if c.pairingService != nil { + paired, err := c.pairingService.IsPaired(groupSenderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "group_sender", groupSenderID, "channel", c.Name(), "error", err) + paired = true + } + if paired { + c.approvedGroups.Store(channelID, true) + return true + } } c.sendPairingReply(groupSenderID, channelID) return false @@ -305,7 +313,14 @@ func (c *Channel) checkDMPolicy(senderID, channelID string) bool { default: // "pairing" paired := false if c.pairingService != nil { - paired = c.pairingService.IsPaired(senderID, c.Name()) + p, err := c.pairingService.IsPaired(senderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "sender_id", senderID, "channel", c.Name(), "error", err) + paired = true + } else { + paired = p + } } inAllowList := c.HasAllowList() && c.IsAllowed(senderID) diff --git a/internal/channels/feishu/bot_policy.go b/internal/channels/feishu/bot_policy.go index 8070501e..a4862195 100644 --- a/internal/channels/feishu/bot_policy.go +++ b/internal/channels/feishu/bot_policy.go @@ -84,11 +84,18 @@ func (c *Channel) checkGroupPolicy(senderID, chatID string) bool { return true } groupSenderID := fmt.Sprintf("group:%s", chatID) - if c.pairingService != nil && c.pairingService.IsPaired(groupSenderID, c.Name()) { - c.approvedGroups.Store(chatID, true) - return true + if c.pairingService != nil { + paired, err := c.pairingService.IsPaired(groupSenderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "group_sender", groupSenderID, "channel", c.Name(), "error", err) + paired = true + } + if paired { + c.approvedGroups.Store(chatID, true) + return true + } } - c.sendPairingReply(groupSenderID, chatID) return false default: // "open" @@ -117,7 +124,14 @@ func (c *Channel) checkDMPolicy(senderID, chatID string) bool { default: // "pairing" paired := false if c.pairingService != nil { - paired = c.pairingService.IsPaired(senderID, c.Name()) + p, err := c.pairingService.IsPaired(senderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "sender_id", senderID, "channel", c.Name(), "error", err) + paired = true + } else { + paired = p + } } inAllowList := c.HasAllowList() && c.IsAllowed(senderID) diff --git a/internal/channels/slack/handlers_mention.go b/internal/channels/slack/handlers_mention.go index 3fa594c4..2a0ba6f7 100644 --- a/internal/channels/slack/handlers_mention.go +++ b/internal/channels/slack/handlers_mention.go @@ -129,8 +129,16 @@ func (c *Channel) checkDMPolicy(senderID, channelID string) bool { case "allowlist": return c.HasAllowList() && c.IsAllowed(senderID) default: // "pairing" - if c.pairingService != nil && c.pairingService.IsPaired(senderID, c.Name()) { - return true + if c.pairingService != nil { + paired, err := c.pairingService.IsPaired(senderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "sender_id", senderID, "channel", c.Name(), "error", err) + return true + } + if paired { + return true + } } if c.HasAllowList() && c.IsAllowed(senderID) { return true @@ -163,9 +171,17 @@ func (c *Channel) checkGroupPolicy(senderID, channelID string) bool { return true } groupSenderID := fmt.Sprintf("group:%s", channelID) - if c.pairingService != nil && c.pairingService.IsPaired(groupSenderID, c.Name()) { - c.approvedGroups.Store(channelID, true) - return true + if c.pairingService != nil { + paired, err := c.pairingService.IsPaired(groupSenderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "group_sender", groupSenderID, "channel", c.Name(), "error", err) + paired = true + } + if paired { + c.approvedGroups.Store(channelID, true) + return true + } } c.sendPairingReply(groupSenderID, channelID) return false diff --git a/internal/channels/telegram/handlers.go b/internal/channels/telegram/handlers.go index 28c15a65..1dd159c7 100644 --- a/internal/channels/telegram/handlers.go +++ b/internal/channels/telegram/handlers.go @@ -148,7 +148,15 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) { default: // "pairing" or unknown → secure default paired := false if c.pairingService != nil { - paired = c.pairingService.IsPaired(userID, c.Name()) || c.pairingService.IsPaired(senderID, c.Name()) + p1, err1 := c.pairingService.IsPaired(userID, c.Name()) + p2, err2 := c.pairingService.IsPaired(senderID, c.Name()) + if err1 != nil || err2 != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "user_id", userID, "channel", c.Name(), "err1", err1, "err2", err2) + paired = true + } else { + paired = p1 || p2 + } } inAllowList := c.HasAllowList() && (c.IsAllowed(userID) || c.IsAllowed(senderID)) @@ -267,7 +275,13 @@ func (c *Channel) handleMessage(ctx context.Context, update telego.Update) { if isGroup && topicCfg.groupPolicy == "pairing" && c.pairingService != nil { if _, cached := c.approvedGroups.Load(chatIDStr); !cached { groupSenderID := fmt.Sprintf("group:%d", chatID) - if c.pairingService.IsPaired(groupSenderID, c.Name()) { + paired, err := c.pairingService.IsPaired(groupSenderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "group_sender", groupSenderID, "channel", c.Name(), "error", err) + paired = true + } + if paired { c.approvedGroups.Store(chatIDStr, true) } else { c.sendGroupPairingReply(ctx, chatID, chatIDStr, groupSenderID, localKey, messageThreadID, message.Chat.Title) diff --git a/internal/channels/whatsapp/whatsapp.go b/internal/channels/whatsapp/whatsapp.go index d42353ed..981298e3 100644 --- a/internal/channels/whatsapp/whatsapp.go +++ b/internal/channels/whatsapp/whatsapp.go @@ -290,9 +290,17 @@ func (c *Channel) checkGroupPolicy(senderID, chatID string) bool { return true } groupSenderID := fmt.Sprintf("group:%s", chatID) - if c.pairingService != nil && c.pairingService.IsPaired(groupSenderID, c.Name()) { - c.approvedGroups.Store(chatID, true) - return true + if c.pairingService != nil { + paired, err := c.pairingService.IsPaired(groupSenderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "group_sender", groupSenderID, "channel", c.Name(), "error", err) + paired = true + } + if paired { + c.approvedGroups.Store(chatID, true) + return true + } } c.sendPairingReply(groupSenderID, chatID) return false @@ -323,7 +331,14 @@ func (c *Channel) checkDMPolicy(senderID, chatID string) bool { default: // "pairing" paired := false if c.pairingService != nil { - paired = c.pairingService.IsPaired(senderID, c.Name()) + p, err := c.pairingService.IsPaired(senderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "sender_id", senderID, "channel", c.Name(), "error", err) + paired = true + } else { + paired = p + } } inAllowList := c.HasAllowList() && c.IsAllowed(senderID) diff --git a/internal/channels/zalo/personal/policy.go b/internal/channels/zalo/personal/policy.go index f4f645bb..6dcf31db 100644 --- a/internal/channels/zalo/personal/policy.go +++ b/internal/channels/zalo/personal/policy.go @@ -37,7 +37,14 @@ func (c *Channel) checkDMPolicy(senderID, chatID string) bool { default: // "pairing" paired := false if c.pairingService != nil { - paired = c.pairingService.IsPaired(senderID, c.Name()) + p, err := c.pairingService.IsPaired(senderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "sender_id", senderID, "channel", c.Name(), "error", err) + paired = true + } else { + paired = p + } } inAllowList := c.HasAllowList() && c.IsAllowed(senderID) @@ -115,7 +122,17 @@ func (c *Channel) checkGroupPolicy(senderID, groupID string) bool { // pass — already approved } else { groupSenderID := fmt.Sprintf("group:%s", groupID) - if c.pairingService != nil && c.pairingService.IsPaired(groupSenderID, c.Name()) { + paired := false + if c.pairingService != nil { + p, err := c.pairingService.IsPaired(groupSenderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "group_sender", groupSenderID, "channel", c.Name(), "error", err) + p = true + } + paired = p + } + if paired { c.approvedGroups.Store(groupID, true) } else { c.sendPairingReply(groupSenderID, groupID) diff --git a/internal/channels/zalo/zalo.go b/internal/channels/zalo/zalo.go index 68af4b55..2c5ce04a 100644 --- a/internal/channels/zalo/zalo.go +++ b/internal/channels/zalo/zalo.go @@ -290,7 +290,14 @@ func (c *Channel) checkDMPolicy(senderID, chatID string) bool { // Check if already paired or in allowlist paired := false if c.pairingService != nil { - paired = c.pairingService.IsPaired(senderID, c.Name()) + p, err := c.pairingService.IsPaired(senderID, c.Name()) + if err != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "sender_id", senderID, "channel", c.Name(), "error", err) + paired = true + } else { + paired = p + } } inAllowList := c.HasAllowList() && c.IsAllowed(senderID) diff --git a/internal/gateway/methods/pairing.go b/internal/gateway/methods/pairing.go index 157921c2..99c9bf90 100644 --- a/internal/gateway/methods/pairing.go +++ b/internal/gateway/methods/pairing.go @@ -3,6 +3,7 @@ package methods import ( "context" "encoding/json" + "log/slog" "github.com/nextlevelbuilder/goclaw/internal/bus" "github.com/nextlevelbuilder/goclaw/internal/gateway" @@ -215,7 +216,11 @@ func (m *PairingMethods) handleBrowserPairingStatus(ctx context.Context, client return } - if m.service.IsPaired(params.SenderID, "browser") { + paired, pairErr := m.service.IsPaired(params.SenderID, "browser") + if pairErr != nil { + slog.Warn("security.pairing_check_failed", "sender_id", params.SenderID, "error", pairErr) + } + if paired { client.SendResponse(protocol.NewOKResponse(req.ID, map[string]any{ "status": "approved", })) diff --git a/internal/gateway/router.go b/internal/gateway/router.go index a64b3fd5..a6546a8d 100644 --- a/internal/gateway/router.go +++ b/internal/gateway/router.go @@ -121,15 +121,23 @@ func (r *MethodRouter) handleConnect(ctx context.Context, client *Client, req *p ps := r.server.pairingService // Path 3a: Reconnecting with a previously-paired sender_id - if ps != nil && params.SenderID != "" && ps.IsPaired(params.SenderID, "browser") { - client.role = permissions.RoleOperator - client.authenticated = true - client.userID = params.UserID - client.pairedSenderID = params.SenderID - client.pairedChannel = "browser" - slog.Info("browser pairing authenticated", "sender_id", params.SenderID, "client", client.id) - r.sendConnectResponse(client, req.ID) - return + if ps != nil && params.SenderID != "" { + paired, pairErr := ps.IsPaired(params.SenderID, "browser") + if pairErr != nil { + slog.Warn("security.pairing_check_failed, assuming paired (fail-open)", + "sender_id", params.SenderID, "error", pairErr) + paired = true + } + if paired { + client.role = permissions.RoleOperator + client.authenticated = true + client.userID = params.UserID + client.pairedSenderID = params.SenderID + client.pairedChannel = "browser" + slog.Info("browser pairing authenticated", "sender_id", params.SenderID, "client", client.id) + r.sendConnectResponse(client, req.ID) + return + } } // Path 3b: No token, no valid pairing → initiate browser pairing (if service available) diff --git a/internal/store/pairing_store.go b/internal/store/pairing_store.go index 082ff432..6f824628 100644 --- a/internal/store/pairing_store.go +++ b/internal/store/pairing_store.go @@ -28,7 +28,7 @@ type PairingStore interface { ApprovePairing(code, approvedBy string) (*PairedDeviceData, error) DenyPairing(code string) error RevokePairing(senderID, channel string) error - IsPaired(senderID, channel string) bool + IsPaired(senderID, channel string) (bool, error) ListPending() []PairingRequestData ListPaired() []PairedDeviceData } diff --git a/internal/store/pg/pairing.go b/internal/store/pg/pairing.go index 8db43930..cb6ea6fe 100644 --- a/internal/store/pg/pairing.go +++ b/internal/store/pg/pairing.go @@ -140,10 +140,13 @@ func (s *PGPairingStore) RevokePairing(senderID, channel string) error { return nil } -func (s *PGPairingStore) IsPaired(senderID, channel string) bool { +func (s *PGPairingStore) IsPaired(senderID, channel string) (bool, error) { var count int64 - s.db.QueryRow("SELECT COUNT(*) FROM paired_devices WHERE sender_id = $1 AND channel = $2", senderID, channel).Scan(&count) - return count > 0 + err := s.db.QueryRow("SELECT COUNT(*) FROM paired_devices WHERE sender_id = $1 AND channel = $2", senderID, channel).Scan(&count) + if err != nil { + return false, fmt.Errorf("pairing check query: %w", err) + } + return count > 0, nil } func (s *PGPairingStore) ListPending() []store.PairingRequestData {