mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-19 12:16:28 +00:00
fix(security): harden pairing auth — fail-closed, rate-limit, TTL
- Change WS pairing check from fail-open to fail-closed on DB error (router.go: previously granted RoleOperator on any IsPaired() error) - Add "browser" to InternalChannels so it's properly excluded from outbound dispatch without ad-hoc helpers - Rate-limit browser.pairing.status endpoint to prevent sender_id enumeration (reuses server RateLimiter via PairingMethods injection) - Add expires_at column to paired_devices with 30-day TTL for defense-in-depth; IsPaired() now checks expiry, ListPaired() prunes - Add confidence_score column to team_tasks, team_messages, team_task_comments - Bump RequiredSchemaVersion to 21
This commit is contained in:
1 parent
8d6729e959
commit
0b5124a8f1
8 files changed
+43
-11
No files matched your search
@@ -29,7 +29,7 @@ func registerAllMethods(server *gateway.Server, agents *agent.Router, sessStore
|
||||
|
||||
// Phase 2: Pairing (store created externally, shared with channel manager).
|
||||
// OnApprove callback is set later by the caller after channel manager is created.
|
||||
pairingMethods := methods.NewPairingMethods(pairingStore, msgBus)
|
||||
pairingMethods := methods.NewPairingMethods(pairingStore, msgBus, server.RateLimiter())
|
||||
pairingMethods.Register(router)
|
||||
|
||||
// Phase 2: Usage (queries SessionStore for real token data)
|
||||
|
||||
@@ -19,10 +19,12 @@ import (
|
||||
)
|
||||
|
||||
// InternalChannels are system channels excluded from outbound dispatch.
|
||||
// "browser" uses WebSocket directly — no outbound channel routing needed.
|
||||
var InternalChannels = map[string]bool{
|
||||
"cli": true,
|
||||
"system": true,
|
||||
"subagent": true,
|
||||
"browser": true,
|
||||
}
|
||||
|
||||
// IsInternalChannel checks if a channel name is internal.
|
||||
|
||||
@@ -23,10 +23,11 @@ type PairingMethods struct {
|
||||
msgBus *bus.MessageBus
|
||||
onApprove PairingApproveCallback
|
||||
broadcaster func(protocol.EventFrame)
|
||||
rateLimiter *gateway.RateLimiter
|
||||
}
|
||||
|
||||
func NewPairingMethods(service store.PairingStore, msgBus *bus.MessageBus) *PairingMethods {
|
||||
return &PairingMethods{service: service, msgBus: msgBus}
|
||||
func NewPairingMethods(service store.PairingStore, msgBus *bus.MessageBus, rateLimiter *gateway.RateLimiter) *PairingMethods {
|
||||
return &PairingMethods{service: service, msgBus: msgBus, rateLimiter: rateLimiter}
|
||||
}
|
||||
|
||||
// SetOnApprove sets a callback that fires after a pairing is approved.
|
||||
@@ -203,6 +204,11 @@ func (m *PairingMethods) handleRevoke(ctx context.Context, client *gateway.Clien
|
||||
// handleBrowserPairingStatus lets a pending browser client check if its pairing code has been approved.
|
||||
// Called by unauthenticated clients during the browser pairing flow.
|
||||
func (m *PairingMethods) handleBrowserPairingStatus(ctx context.Context, client *gateway.Client, req *protocol.RequestFrame) {
|
||||
// Rate-limit unauthenticated polling to prevent sender_id enumeration.
|
||||
if m.rateLimiter != nil && m.rateLimiter.Enabled() && !m.rateLimiter.Allow("pairing:"+client.RemoteAddr()) {
|
||||
client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrResourceExhausted, "rate limited"))
|
||||
return
|
||||
}
|
||||
locale := store.LocaleFromContext(ctx)
|
||||
var params struct {
|
||||
SenderID string `json:"sender_id"`
|
||||
|
||||
@@ -142,9 +142,13 @@ func (r *MethodRouter) handleConnect(ctx context.Context, client *Client, req *p
|
||||
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)",
|
||||
slog.Warn("security.pairing_check_failed",
|
||||
"sender_id", params.SenderID, "error", pairErr)
|
||||
paired = true
|
||||
// Fail-closed: deny access on DB error instead of granting operator role.
|
||||
locale := i18n.Normalize(client.locale)
|
||||
client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInternal,
|
||||
i18n.T(locale, i18n.MsgInternalError, pairErr.Error())))
|
||||
return
|
||||
}
|
||||
if paired {
|
||||
client.role = permissions.RoleOperator
|
||||
|
||||
@@ -16,6 +16,7 @@ const (
|
||||
codeAlphabet = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"
|
||||
codeLength = 8
|
||||
codeTTL = 60 * time.Minute
|
||||
pairedDeviceTTL = 30 * 24 * time.Hour // 30 days
|
||||
maxPendingPerAccount = 3
|
||||
)
|
||||
|
||||
@@ -90,12 +91,13 @@ func (s *PGPairingStore) ApprovePairing(code, approvedBy string) (*store.PairedD
|
||||
// Remove from pending
|
||||
s.db.Exec("DELETE FROM pairing_requests WHERE id = $1", reqID)
|
||||
|
||||
// Add to paired
|
||||
// Add to paired (with expiry for defense-in-depth)
|
||||
now := time.Now()
|
||||
expiresAt := now.Add(pairedDeviceTTL)
|
||||
_, err = s.db.Exec(
|
||||
`INSERT INTO paired_devices (id, sender_id, channel, chat_id, paired_by, paired_at, metadata)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7)`,
|
||||
uuid.Must(uuid.NewV7()), senderID, channel, chatID, approvedBy, now, metaJSON,
|
||||
`INSERT INTO paired_devices (id, sender_id, channel, chat_id, paired_by, paired_at, metadata, expires_at)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`,
|
||||
uuid.Must(uuid.NewV7()), senderID, channel, chatID, approvedBy, now, metaJSON, expiresAt,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("create paired device: %w", err)
|
||||
@@ -142,7 +144,10 @@ func (s *PGPairingStore) RevokePairing(senderID, channel string) error {
|
||||
|
||||
func (s *PGPairingStore) IsPaired(senderID, channel string) (bool, error) {
|
||||
var count int64
|
||||
err := s.db.QueryRow("SELECT COUNT(*) FROM paired_devices WHERE sender_id = $1 AND channel = $2", senderID, channel).Scan(&count)
|
||||
err := s.db.QueryRow(
|
||||
"SELECT COUNT(*) FROM paired_devices WHERE sender_id = $1 AND channel = $2 AND (expires_at IS NULL OR expires_at > NOW())",
|
||||
senderID, channel,
|
||||
).Scan(&count)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("pairing check query: %w", err)
|
||||
}
|
||||
@@ -182,6 +187,9 @@ func (s *PGPairingStore) ListPending() []store.PairingRequestData {
|
||||
}
|
||||
|
||||
func (s *PGPairingStore) ListPaired() []store.PairedDeviceData {
|
||||
// Prune expired paired devices
|
||||
s.db.Exec("DELETE FROM paired_devices WHERE expires_at IS NOT NULL AND expires_at < NOW()")
|
||||
|
||||
rows, err := s.db.Query("SELECT sender_id, channel, chat_id, paired_by, paired_at, COALESCE(metadata, '{}') FROM paired_devices ORDER BY paired_at DESC")
|
||||
if err != nil {
|
||||
return nil
|
||||
|
||||
@@ -2,4 +2,4 @@ package upgrade
|
||||
|
||||
// RequiredSchemaVersion is the schema migration version this binary requires.
|
||||
// Bump this whenever adding a new SQL migration file.
|
||||
const RequiredSchemaVersion uint = 20
|
||||
const RequiredSchemaVersion uint = 21
|
||||
@@ -0,0 +1,4 @@
|
||||
ALTER TABLE paired_devices DROP COLUMN IF EXISTS expires_at;
|
||||
ALTER TABLE team_tasks DROP COLUMN IF EXISTS confidence_score;
|
||||
ALTER TABLE team_messages DROP COLUMN IF EXISTS confidence_score;
|
||||
ALTER TABLE team_task_comments DROP COLUMN IF EXISTS confidence_score;
|
||||
@@ -0,0 +1,8 @@
|
||||
-- Add expiry to paired devices for defense-in-depth.
|
||||
-- NULL means no expiry (backward compat for existing rows).
|
||||
ALTER TABLE paired_devices ADD COLUMN IF NOT EXISTS expires_at TIMESTAMPTZ;
|
||||
|
||||
-- Add confidence_score to team tables for agent self-assessment.
|
||||
ALTER TABLE team_tasks ADD COLUMN IF NOT EXISTS confidence_score FLOAT;
|
||||
ALTER TABLE team_messages ADD COLUMN IF NOT EXISTS confidence_score FLOAT;
|
||||
ALTER TABLE team_task_comments ADD COLUMN IF NOT EXISTS confidence_score FLOAT;
|
||||
Reference in new issue
Block a user