From 88cbe2d275d48de3d26d06fe62683ee66a9fc33c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 9 Jan 2026 22:58:11 +0000 Subject: [PATCH] fix: cap pairing requests and suppress outbound pairing replies --- CHANGELOG.md | 1 + docs/gateway/configuration.md | 2 +- docs/gateway/security.md | 2 +- docs/gateway/troubleshooting.md | 18 +++++++++++ docs/providers/whatsapp.md | 6 ++-- docs/start/faq.md | 1 + docs/start/pairing.md | 1 + src/config/schema.ts | 2 +- src/config/types.ts | 3 +- src/pairing/pairing-store.test.ts | 27 ++++++++++++++++ src/pairing/pairing-store.ts | 51 ++++++++++++++++++++++++++++--- src/web/inbound.ts | 8 ++--- src/web/monitor-inbox.test.ts | 11 ++----- 13 files changed, 106 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f9ef3660..8062d27d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Pairing: cap pending DM pairing requests at 3 per provider and avoid pairing replies for outbound DMs. — thanks @steipete - macOS: replace relay smoke test with version check in packaging script. (#615) — thanks @YuriNachos - macOS: avoid clearing Launch at Login during app initialization. (#607) — thanks @wes-davis - Onboarding: skip systemd checks/daemon installs when systemd user services are unavailable; add onboarding flags to skip flow steps and stabilize Docker E2E. (#573) — thanks @steipete diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 799197029..916348e75 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -248,7 +248,7 @@ Controls how WhatsApp direct chats (DMs) are handled: - `"open"`: allow all inbound DMs (**requires** `whatsapp.allowFrom` to include `"*"`) - `"disabled"`: ignore all inbound DMs -Pairing codes expire after 1 hour; the bot only sends a pairing code when a new request is created. +Pairing codes expire after 1 hour; the bot only sends a pairing code when a new request is created. Pending DM pairing requests are capped at **3 per provider** by default. Pairing approvals: - `clawdbot pairing list --provider whatsapp` diff --git a/docs/gateway/security.md b/docs/gateway/security.md index 6a04f38b6..3cd038348 100644 --- a/docs/gateway/security.md +++ b/docs/gateway/security.md @@ -38,7 +38,7 @@ Clawdbot’s stance: All current DM-capable providers support a DM policy (`dmPolicy` or `*.dm.policy`) that gates inbound DMs **before** the message is processed: -- `pairing` (default): unknown senders receive a short pairing code and the bot ignores their message until approved. Codes expire after 1 hour; repeated DMs won’t resend a code until a new request is created. +- `pairing` (default): unknown senders receive a short pairing code and the bot ignores their message until approved. Codes expire after 1 hour; repeated DMs won’t resend a code until a new request is created. Pending requests are capped at **3 per provider** by default. - `allowlist`: unknown senders are blocked (no pairing handshake). - `open`: allow anyone to DM (public). **Requires** the provider allowlist to include `"*"` (explicit opt-in). - `disabled`: ignore inbound DMs entirely. diff --git a/docs/gateway/troubleshooting.md b/docs/gateway/troubleshooting.md index 3b3b35bf3..56e2f4b16 100644 --- a/docs/gateway/troubleshooting.md +++ b/docs/gateway/troubleshooting.md @@ -169,6 +169,24 @@ clawdbot logs --follow tail -f "$(ls -t /tmp/clawdbot/clawdbot-*.log | head -1)" | grep "blocked\\|skip\\|unauthorized" ``` +### Pairing Code Not Arriving + +If `dmPolicy` is `pairing`, unknown senders should receive a code and their message is ignored until approved. + +**Check 1:** Is a pending request already waiting? +```bash +clawdbot pairing list --provider +``` + +Pending DM pairing requests are capped at **3 per provider** by default. If the list is full, new requests won’t generate a code until one is approved or expires. + +**Check 2:** Did the request get created but no reply was sent? +```bash +clawdbot logs --follow | grep "pairing request" +``` + +**Check 3:** Confirm `dmPolicy` isn’t `open`/`allowlist` for that provider. + ### Image + Mention Not Working Known issue: When you send an image with ONLY a mention (no other text), WhatsApp sometimes doesn't include the mention metadata. diff --git a/docs/providers/whatsapp.md b/docs/providers/whatsapp.md index c78182131..9828c75d0 100644 --- a/docs/providers/whatsapp.md +++ b/docs/providers/whatsapp.md @@ -103,9 +103,9 @@ on outbound replies. If you run Clawdbot on your **personal WhatsApp number**, enable `whatsapp.selfChatMode` (see sample above). Behavior: -- Suppresses pairing replies for **outbound DMs** (prevents spamming contacts). +- Outbound DMs never trigger pairing replies (prevents spamming contacts). - Inbound unknown senders still follow `whatsapp.dmPolicy`. -- Self-chat mode avoids auto read receipts and ignores mention JIDs. +- Self-chat mode (allowFrom includes your number) avoids auto read receipts and ignores mention JIDs. - Read receipts sent for non-self-chat DMs. ## Message normalization (what the model sees) @@ -186,7 +186,7 @@ Behavior: ## Config quick map - `whatsapp.dmPolicy` (DM policy: pairing/allowlist/open/disabled). -- `whatsapp.selfChatMode` (same-phone setup; suppress pairing replies for outbound DMs). +- `whatsapp.selfChatMode` (same-phone setup; bot uses your personal WhatsApp number). - `whatsapp.allowFrom` (DM allowlist). - `whatsapp.mediaMaxMb` (inbound media save cap). - `whatsapp.accounts..*` (per-account settings + optional `authDir`). diff --git a/docs/start/faq.md b/docs/start/faq.md index 2fc0c29e9..d2f5695d7 100644 --- a/docs/start/faq.md +++ b/docs/start/faq.md @@ -632,6 +632,7 @@ Treat inbound DMs as untrusted input. Defaults are designed to reduce risk: - Default behavior on DM‑capable providers is **pairing**: - Unknown senders receive a pairing code; the bot does not process their message. - Approve with: `clawdbot pairing approve --provider ` + - Pending requests are capped at **3 per provider**; check `clawdbot pairing list --provider ` if a code didn’t arrive. - Opening DMs publicly requires explicit opt‑in (`dmPolicy: "open"` and allowlist `"*"`). Run `clawdbot doctor` to surface risky DM policies. diff --git a/docs/start/pairing.md b/docs/start/pairing.md index 35cc9088d..79454928a 100644 --- a/docs/start/pairing.md +++ b/docs/start/pairing.md @@ -25,6 +25,7 @@ Default DM policies are documented in: [Security](/gateway/security) Pairing codes: - 8 characters, uppercase, no ambiguous chars (`0O1I`). - **Expire after 1 hour**. The bot only sends the pairing message when a new request is created (roughly once per hour per sender). +- Pending DM pairing requests are capped at **3 per provider** by default; additional requests are ignored until one expires or is approved. ### Approve a sender diff --git a/src/config/schema.ts b/src/config/schema.ts index ec31f91c8..f257012f5 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -205,7 +205,7 @@ const FIELD_HELP: Record = { "whatsapp.dmPolicy": 'Direct message access control ("pairing" recommended). "open" requires whatsapp.allowFrom=["*"].', "whatsapp.selfChatMode": - "Same-phone setup (bot uses your personal WhatsApp number). Suppresses pairing replies for outbound DMs.", + "Same-phone setup (bot uses your personal WhatsApp number).", "signal.dmPolicy": 'Direct message access control ("pairing" recommended). "open" requires signal.allowFrom=["*"].', "imessage.dmPolicy": diff --git a/src/config/types.ts b/src/config/types.ts index 443d5bae7..bbedafe72 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -123,7 +123,6 @@ export type WhatsAppConfig = { dmPolicy?: DmPolicy; /** * Same-phone setup (bot uses your personal WhatsApp number). - * When true, suppress pairing replies for outbound DMs. */ selfChatMode?: boolean; /** Optional allowlist for WhatsApp direct chats (E.164). */ @@ -168,7 +167,7 @@ export type WhatsAppAccountConfig = { authDir?: string; /** Direct message access policy (default: pairing). */ dmPolicy?: DmPolicy; - /** Same-phone setup for this account (suppresses pairing replies for outbound DMs). */ + /** Same-phone setup for this account (bot uses your personal WhatsApp number). */ selfChatMode?: boolean; allowFrom?: string[]; groupAllowFrom?: string[]; diff --git a/src/pairing/pairing-store.test.ts b/src/pairing/pairing-store.test.ts index bbcaac0f6..8bdb24e34 100644 --- a/src/pairing/pairing-store.test.ts +++ b/src/pairing/pairing-store.test.ts @@ -106,4 +106,31 @@ describe("pairing store", () => { } }); }); + + it("caps pending requests at the default limit", async () => { + await withTempStateDir(async () => { + const ids = ["+15550000001", "+15550000002", "+15550000003"]; + for (const id of ids) { + const created = await upsertProviderPairingRequest({ + provider: "whatsapp", + id, + }); + expect(created.created).toBe(true); + } + + const blocked = await upsertProviderPairingRequest({ + provider: "whatsapp", + id: "+15550000004", + }); + expect(blocked.created).toBe(false); + + const list = await listProviderPairingRequests("whatsapp"); + const listIds = list.map((entry) => entry.id); + expect(listIds).toHaveLength(3); + expect(listIds).toContain("+15550000001"); + expect(listIds).toContain("+15550000002"); + expect(listIds).toContain("+15550000003"); + expect(listIds).not.toContain("+15550000004"); + }); + }); }); diff --git a/src/pairing/pairing-store.ts b/src/pairing/pairing-store.ts index 718a7cedd..e41a12ff6 100644 --- a/src/pairing/pairing-store.ts +++ b/src/pairing/pairing-store.ts @@ -10,6 +10,7 @@ import { resolveOAuthDir, resolveStateDir } from "../config/paths.js"; const PAIRING_CODE_LENGTH = 8; const PAIRING_CODE_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"; const PAIRING_PENDING_TTL_MS = 60 * 60 * 1000; +const PAIRING_PENDING_MAX = 3; const PAIRING_STORE_LOCK_OPTIONS = { retries: { retries: 10, @@ -160,6 +161,22 @@ function pruneExpiredRequests(reqs: PairingRequest[], nowMs: number) { return { requests: kept, removed }; } +function resolveLastSeenAt(entry: PairingRequest): number { + return ( + parseTimestamp(entry.lastSeenAt) ?? parseTimestamp(entry.createdAt) ?? 0 + ); +} + +function pruneExcessRequests(reqs: PairingRequest[], maxPending: number) { + if (maxPending <= 0 || reqs.length <= maxPending) { + return { requests: reqs, removed: false }; + } + const sorted = reqs + .slice() + .sort((a, b) => resolveLastSeenAt(a) - resolveLastSeenAt(b)); + return { requests: sorted.slice(-maxPending), removed: true }; +} + function randomCode(): string { // Human-friendly: 8 chars, upper, no ambiguous chars (0O1I). let out = ""; @@ -259,8 +276,13 @@ export async function listProviderPairingRequests( }); const reqs = Array.isArray(value.requests) ? value.requests : []; const nowMs = Date.now(); - const { requests: pruned, removed } = pruneExpiredRequests(reqs, nowMs); - if (removed) { + const { requests: prunedExpired, removed: expiredRemoved } = + pruneExpiredRequests(reqs, nowMs); + const { requests: pruned, removed: cappedRemoved } = pruneExcessRequests( + prunedExpired, + PAIRING_PENDING_MAX, + ); + if (expiredRemoved || cappedRemoved) { await writeJsonFile(filePath, { version: 1, requests: pruned, @@ -309,8 +331,9 @@ export async function upsertProviderPairingRequest(params: { : undefined; let reqs = Array.isArray(value.requests) ? value.requests : []; - const { requests: pruned } = pruneExpiredRequests(reqs, nowMs); - reqs = pruned; + const { requests: prunedExpired, removed: expiredRemoved } = + pruneExpiredRequests(reqs, nowMs); + reqs = prunedExpired; const existingIdx = reqs.findIndex((r) => r.id === id); const existingCodes = new Set( reqs.map((req) => @@ -335,13 +358,31 @@ export async function upsertProviderPairingRequest(params: { meta: meta ?? existing?.meta, }; reqs[existingIdx] = next; + const { requests: capped } = pruneExcessRequests( + reqs, + PAIRING_PENDING_MAX, + ); await writeJsonFile(filePath, { version: 1, - requests: reqs, + requests: capped, } satisfies PairingStore); return { code, created: false }; } + const { requests: capped, removed: cappedRemoved } = pruneExcessRequests( + reqs, + PAIRING_PENDING_MAX, + ); + reqs = capped; + if (PAIRING_PENDING_MAX > 0 && reqs.length >= PAIRING_PENDING_MAX) { + if (expiredRemoved || cappedRemoved) { + await writeJsonFile(filePath, { + version: 1, + requests: reqs, + } satisfies PairingStore); + } + return { code: "", created: false }; + } const code = generateUniqueCode(existingCodes); const next: PairingRequest = { id, diff --git a/src/web/inbound.ts b/src/web/inbound.ts index 2674f68f5..1b112515f 100644 --- a/src/web/inbound.ts +++ b/src/web/inbound.ts @@ -223,8 +223,6 @@ export async function monitorWebInbox(options: { const isSamePhone = from === selfE164; const isSelfChat = isSelfChatMode(selfE164, configuredAllowFrom); const isFromMe = Boolean(msg.key?.fromMe); - const selfChatMode = account.selfChatMode ?? false; - const selfPhoneMode = selfChatMode || isSelfChat; // Pre-compute normalized allowlists for filtering const dmHasWildcard = allowFrom?.includes("*") ?? false; @@ -269,10 +267,8 @@ export async function monitorWebInbox(options: { // DM access control (secure defaults): "pairing" (default) / "allowlist" / "open" / "disabled" if (!group) { - if (isFromMe && !isSamePhone && selfPhoneMode) { - logVerbose( - "Skipping outbound self-phone DM (fromMe); no pairing reply needed.", - ); + if (isFromMe && !isSamePhone) { + logVerbose("Skipping outbound DM (fromMe); no pairing reply needed."); continue; } if (dmPolicy === "disabled") { diff --git a/src/web/monitor-inbox.test.ts b/src/web/monitor-inbox.test.ts index f36a11a0f..285de40ab 100644 --- a/src/web/monitor-inbox.test.ts +++ b/src/web/monitor-inbox.test.ts @@ -1312,7 +1312,7 @@ describe("web monitor inbox", () => { await listener.close(); }); - it("still pairs outbound DMs when same-phone mode is disabled", async () => { + it("skips pairing replies for outbound DMs when same-phone mode is disabled", async () => { mockLoadConfig.mockReturnValue({ whatsapp: { dmPolicy: "pairing", @@ -1347,13 +1347,8 @@ describe("web monitor inbox", () => { await new Promise((resolve) => setImmediate(resolve)); expect(onMessage).not.toHaveBeenCalled(); - expect(upsertPairingRequestMock).toHaveBeenCalledTimes(1); - expect(sock.sendMessage).toHaveBeenCalledWith("999@s.whatsapp.net", { - text: expect.stringContaining("Your WhatsApp phone number: +999"), - }); - expect(sock.sendMessage).toHaveBeenCalledWith("999@s.whatsapp.net", { - text: expect.stringContaining("Pairing code: PAIRCODE"), - }); + expect(upsertPairingRequestMock).not.toHaveBeenCalled(); + expect(sock.sendMessage).not.toHaveBeenCalled(); mockLoadConfig.mockReturnValue({ whatsapp: {