mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 02:15:57 +00:00
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.
This commit is contained in:
1 parent
d2bd270968
commit
7bde3f2f7c
11 files changed
+151
-37
No files matched your search
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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",
|
||||
}))
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
Reference in new issue
Block a user