diff --git a/CHANGELOG.md b/CHANGELOG.md index 2da1ad55c..374778c2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ - Groups: `whatsapp.groups`, `telegram.groups`, and `imessage.groups` now act as allowlists when set. Add `"*"` to keep allow-all behavior. ### Fixes +- Pairing: generate DM pairing codes with CSPRNG, expire pending codes after 1 hour, and avoid re-sending codes for already pending requests. +- Pairing: lock + atomically write pairing stores with 0600 perms and stop logging pairing codes in provider logs. - Tools: add Telegram/WhatsApp reaction tools (with per-provider gating). Thanks @zats for PR #353. - Tools: unify reaction removal semantics across Discord/Slack/Telegram/WhatsApp and allow WhatsApp reaction routing across accounts. - Gateway/CLI: add daemon runtime selection (Node recommended; Bun optional) and document WhatsApp/Baileys Bun WebSocket instability on reconnect. diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 0fb9f769d..b3dbef3b4 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -195,6 +195,8 @@ 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 approvals: - `clawdbot pairing list --provider whatsapp` - `clawdbot pairing approve --provider whatsapp ` diff --git a/docs/gateway/security.md b/docs/gateway/security.md index 2dbf7e47e..cbe943d32 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. +- `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. - `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/providers/discord.md b/docs/providers/discord.md index 1f8dcbcee..a68882569 100644 --- a/docs/providers/discord.md +++ b/docs/providers/discord.md @@ -23,7 +23,7 @@ Status: ready for DM and guild text channels via the official Discord bot gatewa - If you prefer env vars, still add `discord: { enabled: true }` to `~/.clawdbot/clawdbot.json` and set `DISCORD_BOT_TOKEN`. 5. Direct chats: use `user:` (or a `<@id>` mention) when delivering; all turns land in the shared `main` session. 6. Guild channels: use `channel:` for delivery. Mentions are required by default and can be set per guild or per channel. -7. Direct chats: secure by default via `discord.dm.policy` (default: `"pairing"`). Unknown senders get a pairing code; approve via `clawdbot pairing approve --provider discord `. +7. Direct chats: secure by default via `discord.dm.policy` (default: `"pairing"`). Unknown senders get a pairing code (expires after 1 hour); approve via `clawdbot pairing approve --provider discord `. - To keep old “open to anyone” behavior: set `discord.dm.policy="open"` and `discord.dm.allowFrom=["*"]`. - To hard-allowlist: set `discord.dm.policy="allowlist"` and list senders in `discord.dm.allowFrom`. - To ignore all DMs: set `discord.dm.enabled=false` or `discord.dm.policy="disabled"`. diff --git a/docs/providers/imessage.md b/docs/providers/imessage.md index c7c6d9469..3f031d5f1 100644 --- a/docs/providers/imessage.md +++ b/docs/providers/imessage.md @@ -39,7 +39,7 @@ Example: ## Access control (DMs + groups) DMs: - Default: `imessage.dmPolicy = "pairing"`. -- Unknown senders receive a pairing code; messages are ignored until approved. +- Unknown senders receive a pairing code; messages are ignored until approved (codes expire after 1 hour). - Approve via: - `clawdbot pairing list --provider imessage` - `clawdbot pairing approve --provider imessage ` diff --git a/docs/providers/signal.md b/docs/providers/signal.md index 6677d90c6..e83994e61 100644 --- a/docs/providers/signal.md +++ b/docs/providers/signal.md @@ -42,7 +42,7 @@ Example: ## Access control (DMs + groups) DMs: - Default: `signal.dmPolicy = "pairing"`. -- Unknown senders receive a pairing code; messages are ignored until approved. +- Unknown senders receive a pairing code; messages are ignored until approved (codes expire after 1 hour). - Approve via: - `clawdbot pairing list --provider signal` - `clawdbot pairing approve --provider signal ` diff --git a/docs/providers/slack.md b/docs/providers/slack.md index d8031920d..f897e534f 100644 --- a/docs/providers/slack.md +++ b/docs/providers/slack.md @@ -195,7 +195,7 @@ Ack reactions are controlled globally via `messages.ackReaction` + - Full command list + config: [Slash commands](/tools/slash-commands) ## DM security (pairing) -- Default: `slack.dm.policy="pairing"` — unknown DM senders get a pairing code. +- Default: `slack.dm.policy="pairing"` — unknown DM senders get a pairing code (expires after 1 hour). - Approve via: `clawdbot pairing approve --provider slack `. - To allow anyone: set `slack.dm.policy="open"` and `slack.dm.allowFrom=["*"]`. diff --git a/docs/providers/telegram.md b/docs/providers/telegram.md index 0999c09bb..b68cc54fc 100644 --- a/docs/providers/telegram.md +++ b/docs/providers/telegram.md @@ -45,7 +45,7 @@ Telegram forum topics include a `message_thread_id` per message. Clawdbot: - Exposes `MessageThreadId` + `IsForum` in template context for routing/templating. ## Access control (DMs + groups) -- Default: `telegram.dmPolicy = "pairing"`. Unknown senders receive a pairing code; messages are ignored until approved. +- Default: `telegram.dmPolicy = "pairing"`. Unknown senders receive a pairing code; messages are ignored until approved (codes expire after 1 hour). - Approve via: - `clawdbot pairing list --provider telegram` - `clawdbot pairing approve --provider telegram ` diff --git a/docs/providers/whatsapp.md b/docs/providers/whatsapp.md index d9d7bb11d..c8f3dcd8b 100644 --- a/docs/providers/whatsapp.md +++ b/docs/providers/whatsapp.md @@ -51,7 +51,7 @@ WhatsApp requires a real mobile number for verification. VoIP and virtual number - Status/broadcast chats are ignored. - Direct chats use E.164; groups use group JID. - **DM policy**: `whatsapp.dmPolicy` controls direct chat access (default: `pairing`). - - Pairing: unknown senders get a pairing code (approve via `clawdbot pairing approve --provider whatsapp `). + - Pairing: unknown senders get a pairing code (approve via `clawdbot pairing approve --provider whatsapp `; codes expire after 1 hour). - Open: requires `whatsapp.allowFrom` to include `"*"`. - Self messages are always allowed; “self-chat mode” still requires `whatsapp.allowFrom` to include your own number. - **Group policy**: `whatsapp.groupPolicy` controls group handling (`open|disabled|allowlist`). diff --git a/docs/start/pairing.md b/docs/start/pairing.md index 398bc9a6a..949e5ed80 100644 --- a/docs/start/pairing.md +++ b/docs/start/pairing.md @@ -22,6 +22,10 @@ When a provider is configured with DM policy `pairing`, unknown senders get a sh 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). + ### Approve a sender ```bash diff --git a/src/discord/monitor.ts b/src/discord/monitor.ts index 6b088fb3f..3fb3285ea 100644 --- a/src/discord/monitor.ts +++ b/src/discord/monitor.ts @@ -412,7 +412,7 @@ export function createDiscordMessageHandler(params: { if (!permitted) { commandAuthorized = false; if (dmPolicy === "pairing") { - const { code } = await upsertProviderPairingRequest({ + const { code, created } = await upsertProviderPairingRequest({ provider: "discord", id: author.id, meta: { @@ -420,26 +420,28 @@ export function createDiscordMessageHandler(params: { name: author.username ?? undefined, }, }); - logVerbose( - `discord pairing request sender=${author.id} tag=${formatDiscordUserTag(author)} code=${code}`, - ); - try { - await sendMessageDiscord( - `user:${author.id}`, - [ - "Clawdbot: access not configured.", - "", - `Pairing code: ${code}`, - "", - "Ask the bot owner to approve with:", - "clawdbot pairing approve --provider discord ", - ].join("\n"), - { token, rest: client.rest }, - ); - } catch (err) { + if (created) { logVerbose( - `discord pairing reply failed for ${author.id}: ${String(err)}`, + `discord pairing request sender=${author.id} tag=${formatDiscordUserTag(author)}`, ); + try { + await sendMessageDiscord( + `user:${author.id}`, + [ + "Clawdbot: access not configured.", + "", + `Pairing code: ${code}`, + "", + "Ask the bot owner to approve with:", + "clawdbot pairing approve --provider discord ", + ].join("\n"), + { token, rest: client.rest }, + ); + } catch (err) { + logVerbose( + `discord pairing reply failed for ${author.id}: ${String(err)}`, + ); + } } } else { logVerbose( @@ -1107,7 +1109,7 @@ function createDiscordNativeCommand(params: { if (!permitted) { commandAuthorized = false; if (dmPolicy === "pairing") { - const { code } = await upsertProviderPairingRequest({ + const { code, created } = await upsertProviderPairingRequest({ provider: "discord", id: user.id, meta: { @@ -1115,17 +1117,19 @@ function createDiscordNativeCommand(params: { name: user.username ?? undefined, }, }); - await interaction.reply({ - content: [ - "Clawdbot: access not configured.", - "", - `Pairing code: ${code}`, - "", - "Ask the bot owner to approve with:", - "clawdbot pairing approve --provider discord ", - ].join("\n"), - ephemeral: true, - }); + if (created) { + await interaction.reply({ + content: [ + "Clawdbot: access not configured.", + "", + `Pairing code: ${code}`, + "", + "Ask the bot owner to approve with:", + "clawdbot pairing approve --provider discord ", + ].join("\n"), + ephemeral: true, + }); + } } else { await interaction.reply({ content: "You are not authorized to use this command.", diff --git a/src/imessage/monitor.ts b/src/imessage/monitor.ts index 7a191f22d..7db1ac381 100644 --- a/src/imessage/monitor.ts +++ b/src/imessage/monitor.ts @@ -230,7 +230,7 @@ export async function monitorIMessageProvider( if (!dmAuthorized) { if (dmPolicy === "pairing") { const senderId = normalizeIMessageHandle(sender); - const { code } = await upsertProviderPairingRequest({ + const { code, created } = await upsertProviderPairingRequest({ provider: "imessage", id: senderId, meta: { @@ -238,30 +238,30 @@ export async function monitorIMessageProvider( chatId: chatId ? String(chatId) : undefined, }, }); - logVerbose( - `imessage pairing request sender=${senderId} code=${code}`, - ); - try { - await sendMessageIMessage( - sender, - [ - "Clawdbot: access not configured.", - "", - `Pairing code: ${code}`, - "", - "Ask the bot owner to approve with:", - "clawdbot pairing approve --provider imessage ", - ].join("\n"), - { - client, - maxBytes: mediaMaxBytes, - ...(chatId ? { chatId } : {}), - }, - ); - } catch (err) { - logVerbose( - `imessage pairing reply failed for ${senderId}: ${String(err)}`, - ); + if (created) { + logVerbose(`imessage pairing request sender=${senderId}`); + try { + await sendMessageIMessage( + sender, + [ + "Clawdbot: access not configured.", + "", + `Pairing code: ${code}`, + "", + "Ask the bot owner to approve with:", + "clawdbot pairing approve --provider imessage ", + ].join("\n"), + { + client, + maxBytes: mediaMaxBytes, + ...(chatId ? { chatId } : {}), + }, + ); + } catch (err) { + logVerbose( + `imessage pairing reply failed for ${senderId}: ${String(err)}`, + ); + } } } else { logVerbose( diff --git a/src/pairing/pairing-store.test.ts b/src/pairing/pairing-store.test.ts new file mode 100644 index 000000000..bbcaac0f6 --- /dev/null +++ b/src/pairing/pairing-store.test.ts @@ -0,0 +1,109 @@ +import crypto from "node:crypto"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + +import { describe, expect, it, vi } from "vitest"; + +import { resolveOAuthDir } from "../config/paths.js"; +import { + listProviderPairingRequests, + upsertProviderPairingRequest, +} from "./pairing-store.js"; + +async function withTempStateDir(fn: (stateDir: string) => Promise) { + const previous = process.env.CLAWDBOT_STATE_DIR; + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-pairing-")); + process.env.CLAWDBOT_STATE_DIR = dir; + try { + return await fn(dir); + } finally { + if (previous === undefined) delete process.env.CLAWDBOT_STATE_DIR; + else process.env.CLAWDBOT_STATE_DIR = previous; + await fs.rm(dir, { recursive: true, force: true }); + } +} + +describe("pairing store", () => { + it("reuses pending code and reports created=false", async () => { + await withTempStateDir(async () => { + const first = await upsertProviderPairingRequest({ + provider: "discord", + id: "u1", + }); + const second = await upsertProviderPairingRequest({ + provider: "discord", + id: "u1", + }); + expect(first.created).toBe(true); + expect(second.created).toBe(false); + expect(second.code).toBe(first.code); + + const list = await listProviderPairingRequests("discord"); + expect(list).toHaveLength(1); + expect(list[0]?.code).toBe(first.code); + }); + }); + + it("expires pending requests after TTL", async () => { + await withTempStateDir(async (stateDir) => { + const created = await upsertProviderPairingRequest({ + provider: "signal", + id: "+15550001111", + }); + expect(created.created).toBe(true); + + const oauthDir = resolveOAuthDir(process.env, stateDir); + const filePath = path.join(oauthDir, "signal-pairing.json"); + const raw = await fs.readFile(filePath, "utf8"); + const parsed = JSON.parse(raw) as { + requests?: Array>; + }; + const expiredAt = new Date(Date.now() - 2 * 60 * 60 * 1000).toISOString(); + const requests = (parsed.requests ?? []).map((entry) => ({ + ...entry, + createdAt: expiredAt, + lastSeenAt: expiredAt, + })); + await fs.writeFile( + filePath, + `${JSON.stringify({ version: 1, requests }, null, 2)}\n`, + "utf8", + ); + + const list = await listProviderPairingRequests("signal"); + expect(list).toHaveLength(0); + + const next = await upsertProviderPairingRequest({ + provider: "signal", + id: "+15550001111", + }); + expect(next.created).toBe(true); + }); + }); + + it("regenerates when a generated code collides", async () => { + await withTempStateDir(async () => { + const spy = vi.spyOn(crypto, "randomInt"); + try { + spy.mockReturnValue(0); + const first = await upsertProviderPairingRequest({ + provider: "telegram", + id: "123", + }); + expect(first.code).toBe("AAAAAAAA"); + + const sequence = Array(8).fill(0).concat(Array(8).fill(1)); + let idx = 0; + spy.mockImplementation(() => sequence[idx++] ?? 1); + const second = await upsertProviderPairingRequest({ + provider: "telegram", + id: "456", + }); + expect(second.code).toBe("BBBBBBBB"); + } finally { + spy.mockRestore(); + } + }); + }); +}); diff --git a/src/pairing/pairing-store.ts b/src/pairing/pairing-store.ts index 02f31e345..f7428467b 100644 --- a/src/pairing/pairing-store.ts +++ b/src/pairing/pairing-store.ts @@ -1,9 +1,26 @@ +import crypto from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import lockfile from "proper-lockfile"; + 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_STORE_LOCK_OPTIONS = { + retries: { + retries: 10, + factor: 2, + minTimeout: 100, + maxTimeout: 10_000, + randomize: true, + }, + stale: 30_000, +} as const; + export type PairingProvider = | "telegram" | "signal" @@ -74,24 +91,92 @@ async function readJsonFile( } async function writeJsonFile(filePath: string, value: unknown): Promise { - await fs.promises.mkdir(path.dirname(filePath), { recursive: true }); - await fs.promises.writeFile( - filePath, - `${JSON.stringify(value, null, 2)}\n`, - "utf-8", + const dir = path.dirname(filePath); + await fs.promises.mkdir(dir, { recursive: true, mode: 0o700 }); + const tmp = path.join( + dir, + `${path.basename(filePath)}.${crypto.randomUUID()}.tmp`, ); + await fs.promises.writeFile(tmp, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf-8", + }); + await fs.promises.chmod(tmp, 0o600); + await fs.promises.rename(tmp, filePath); +} + +async function ensureJsonFile(filePath: string, fallback: unknown) { + try { + await fs.promises.access(filePath); + } catch { + await writeJsonFile(filePath, fallback); + } +} + +async function withFileLock( + filePath: string, + fallback: unknown, + fn: () => Promise, +): Promise { + await ensureJsonFile(filePath, fallback); + let release: (() => Promise) | undefined; + try { + release = await lockfile.lock(filePath, PAIRING_STORE_LOCK_OPTIONS); + return await fn(); + } finally { + if (release) { + try { + await release(); + } catch { + // ignore unlock errors + } + } + } +} + +function parseTimestamp(value: string | undefined): number | null { + if (!value) return null; + const parsed = Date.parse(value); + if (!Number.isFinite(parsed)) return null; + return parsed; +} + +function isExpired(entry: PairingRequest, nowMs: number): boolean { + const createdAt = parseTimestamp(entry.createdAt); + if (!createdAt) return true; + return nowMs - createdAt > PAIRING_PENDING_TTL_MS; +} + +function pruneExpiredRequests(reqs: PairingRequest[], nowMs: number) { + const kept: PairingRequest[] = []; + let removed = false; + for (const req of reqs) { + if (isExpired(req, nowMs)) { + removed = true; + continue; + } + kept.push(req); + } + return { requests: kept, removed }; } function randomCode(): string { // Human-friendly: 8 chars, upper, no ambiguous chars (0O1I). - const alphabet = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"; let out = ""; - for (let i = 0; i < 8; i++) { - out += alphabet[Math.floor(Math.random() * alphabet.length)]; + for (let i = 0; i < PAIRING_CODE_LENGTH; i++) { + const idx = crypto.randomInt(0, PAIRING_CODE_ALPHABET.length); + out += PAIRING_CODE_ALPHABET[idx]; } return out; } +function generateUniqueCode(existing: Set): string { + for (let attempt = 0; attempt < 500; attempt += 1) { + const code = randomCode(); + if (!existing.has(code)) return code; + } + throw new Error("failed to generate unique pairing code"); +} + function normalizeId(value: string | number): string { return String(value).trim(); } @@ -129,26 +214,32 @@ export async function addProviderAllowFromStoreEntry(params: { }): Promise<{ changed: boolean; allowFrom: string[] }> { const env = params.env ?? process.env; const filePath = resolveAllowFromPath(params.provider, env); - const { value } = await readJsonFile(filePath, { - version: 1, - allowFrom: [], - }); - const current = (Array.isArray(value.allowFrom) ? value.allowFrom : []) - .map((v) => normalizeAllowEntry(params.provider, String(v))) - .filter(Boolean); - const normalized = normalizeAllowEntry( - params.provider, - normalizeId(params.entry), + return await withFileLock( + filePath, + { version: 1, allowFrom: [] } satisfies AllowFromStore, + async () => { + const { value } = await readJsonFile(filePath, { + version: 1, + allowFrom: [], + }); + const current = (Array.isArray(value.allowFrom) ? value.allowFrom : []) + .map((v) => normalizeAllowEntry(params.provider, String(v))) + .filter(Boolean); + const normalized = normalizeAllowEntry( + params.provider, + normalizeId(params.entry), + ); + if (!normalized) return { changed: false, allowFrom: current }; + if (current.includes(normalized)) + return { changed: false, allowFrom: current }; + const next = [...current, normalized]; + await writeJsonFile(filePath, { + version: 1, + allowFrom: next, + } satisfies AllowFromStore); + return { changed: true, allowFrom: next }; + }, ); - if (!normalized) return { changed: false, allowFrom: current }; - if (current.includes(normalized)) - return { changed: false, allowFrom: current }; - const next = [...current, normalized]; - await writeJsonFile(filePath, { - version: 1, - allowFrom: next, - } satisfies AllowFromStore); - return { changed: true, allowFrom: next }; } export async function listProviderPairingRequests( @@ -156,21 +247,35 @@ export async function listProviderPairingRequests( env: NodeJS.ProcessEnv = process.env, ): Promise { const filePath = resolvePairingPath(provider, env); - const { value } = await readJsonFile(filePath, { - version: 1, - requests: [], - }); - const reqs = Array.isArray(value.requests) ? value.requests : []; - return reqs - .filter( - (r) => - r && - typeof r.id === "string" && - typeof r.code === "string" && - typeof r.createdAt === "string", - ) - .slice() - .sort((a, b) => a.createdAt.localeCompare(b.createdAt)); + return await withFileLock( + filePath, + { version: 1, requests: [] } satisfies PairingStore, + async () => { + const { value } = await readJsonFile(filePath, { + version: 1, + requests: [], + }); + const reqs = Array.isArray(value.requests) ? value.requests : []; + const nowMs = Date.now(); + const { requests: pruned, removed } = pruneExpiredRequests(reqs, nowMs); + if (removed) { + await writeJsonFile(filePath, { + version: 1, + requests: pruned, + } satisfies PairingStore); + } + return pruned + .filter( + (r) => + r && + typeof r.id === "string" && + typeof r.code === "string" && + typeof r.createdAt === "string", + ) + .slice() + .sort((a, b) => a.createdAt.localeCompare(b.createdAt)); + }, + ); } export async function upsertProviderPairingRequest(params: { @@ -181,56 +286,75 @@ export async function upsertProviderPairingRequest(params: { }): Promise<{ code: string; created: boolean }> { const env = params.env ?? process.env; const filePath = resolvePairingPath(params.provider, env); - const { value } = await readJsonFile(filePath, { - version: 1, - requests: [], - }); - const now = new Date().toISOString(); - const id = normalizeId(params.id); - const meta = - params.meta && typeof params.meta === "object" - ? Object.fromEntries( - Object.entries(params.meta) - .map(([k, v]) => [k, String(v ?? "").trim()] as const) - .filter(([_, v]) => Boolean(v)), - ) - : undefined; + return await withFileLock( + filePath, + { version: 1, requests: [] } satisfies PairingStore, + async () => { + const { value } = await readJsonFile(filePath, { + version: 1, + requests: [], + }); + const now = new Date().toISOString(); + const nowMs = Date.now(); + const id = normalizeId(params.id); + const meta = + params.meta && typeof params.meta === "object" + ? Object.fromEntries( + Object.entries(params.meta) + .map(([k, v]) => [k, String(v ?? "").trim()] as const) + .filter(([_, v]) => Boolean(v)), + ) + : undefined; - const reqs = Array.isArray(value.requests) ? value.requests : []; - const existingIdx = reqs.findIndex((r) => r.id === id); - if (existingIdx >= 0) { - const existing = reqs[existingIdx]; - const existingCode = - existing && typeof existing.code === "string" ? existing.code.trim() : ""; - const code = existingCode || randomCode(); - const next: PairingRequest = { - id, - code, - createdAt: existing?.createdAt ?? now, - lastSeenAt: now, - meta: meta ?? existing?.meta, - }; - reqs[existingIdx] = next; - await writeJsonFile(filePath, { - version: 1, - requests: reqs, - } satisfies PairingStore); - return { code, created: false }; - } + let reqs = Array.isArray(value.requests) ? value.requests : []; + const { requests: pruned } = pruneExpiredRequests(reqs, nowMs); + reqs = pruned; + const existingIdx = reqs.findIndex((r) => r.id === id); + const existingCodes = new Set( + reqs.map((req) => + String(req.code ?? "") + .trim() + .toUpperCase(), + ), + ); - const code = randomCode(); - const next: PairingRequest = { - id, - code, - createdAt: now, - lastSeenAt: now, - ...(meta ? { meta } : {}), - }; - await writeJsonFile(filePath, { - version: 1, - requests: [...reqs, next], - } satisfies PairingStore); - return { code, created: true }; + if (existingIdx >= 0) { + const existing = reqs[existingIdx]; + const existingCode = + existing && typeof existing.code === "string" + ? existing.code.trim() + : ""; + const code = existingCode || generateUniqueCode(existingCodes); + const next: PairingRequest = { + id, + code, + createdAt: existing?.createdAt ?? now, + lastSeenAt: now, + meta: meta ?? existing?.meta, + }; + reqs[existingIdx] = next; + await writeJsonFile(filePath, { + version: 1, + requests: reqs, + } satisfies PairingStore); + return { code, created: false }; + } + + const code = generateUniqueCode(existingCodes); + const next: PairingRequest = { + id, + code, + createdAt: now, + lastSeenAt: now, + ...(meta ? { meta } : {}), + }; + await writeJsonFile(filePath, { + version: 1, + requests: [...reqs, next], + } satisfies PairingStore); + return { code, created: true }; + }, + ); } export async function approveProviderPairingCode(params: { @@ -243,26 +367,42 @@ export async function approveProviderPairingCode(params: { if (!code) return null; const filePath = resolvePairingPath(params.provider, env); - const { value } = await readJsonFile(filePath, { - version: 1, - requests: [], - }); - const reqs = Array.isArray(value.requests) ? value.requests : []; - const idx = reqs.findIndex( - (r) => String(r.code ?? "").toUpperCase() === code, + return await withFileLock( + filePath, + { version: 1, requests: [] } satisfies PairingStore, + async () => { + const { value } = await readJsonFile(filePath, { + version: 1, + requests: [], + }); + const reqs = Array.isArray(value.requests) ? value.requests : []; + const nowMs = Date.now(); + const { requests: pruned, removed } = pruneExpiredRequests(reqs, nowMs); + const idx = pruned.findIndex( + (r) => String(r.code ?? "").toUpperCase() === code, + ); + if (idx < 0) { + if (removed) { + await writeJsonFile(filePath, { + version: 1, + requests: pruned, + } satisfies PairingStore); + } + return null; + } + const entry = pruned[idx]; + if (!entry) return null; + pruned.splice(idx, 1); + await writeJsonFile(filePath, { + version: 1, + requests: pruned, + } satisfies PairingStore); + await addProviderAllowFromStoreEntry({ + provider: params.provider, + entry: entry.id, + env, + }); + return { id: entry.id, entry }; + }, ); - if (idx < 0) return null; - const entry = reqs[idx]; - if (!entry) return null; - reqs.splice(idx, 1); - await writeJsonFile(filePath, { - version: 1, - requests: reqs, - } satisfies PairingStore); - await addProviderAllowFromStoreEntry({ - provider: params.provider, - entry: entry.id, - env, - }); - return { id: entry.id, entry }; } diff --git a/src/signal/monitor.tool-result.test.ts b/src/signal/monitor.tool-result.test.ts index 3bd3ed130..ccf01fc4c 100644 --- a/src/signal/monitor.tool-result.test.ts +++ b/src/signal/monitor.tool-result.test.ts @@ -144,4 +144,47 @@ describe("monitorSignalProvider tool results", () => { "Pairing code: PAIRCODE", ); }); + + it("does not resend pairing code when a request is already pending", async () => { + config = { + ...config, + signal: { autoStart: false, dmPolicy: "pairing", allowFrom: [] }, + }; + upsertPairingRequestMock + .mockResolvedValueOnce({ code: "PAIRCODE", created: true }) + .mockResolvedValueOnce({ code: "PAIRCODE", created: false }); + + streamMock.mockImplementation(async ({ onEvent }) => { + const payload = { + envelope: { + sourceNumber: "+15550001111", + sourceName: "Ada", + timestamp: 1, + dataMessage: { + message: "hello", + }, + }, + }; + await onEvent({ + event: "receive", + data: JSON.stringify(payload), + }); + await onEvent({ + event: "receive", + data: JSON.stringify({ + ...payload, + envelope: { ...payload.envelope, timestamp: 2 }, + }), + }); + }); + + await monitorSignalProvider({ + autoStart: false, + baseUrl: "http://127.0.0.1:8080", + }); + + await flush(); + + expect(sendMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/signal/monitor.ts b/src/signal/monitor.ts index 8f785f164..7c1873a2c 100644 --- a/src/signal/monitor.ts +++ b/src/signal/monitor.ts @@ -336,33 +336,33 @@ export async function monitorSignalProvider( if (!dmAllowed) { if (dmPolicy === "pairing") { const senderId = normalizeE164(sender); - const { code } = await upsertProviderPairingRequest({ + const { code, created } = await upsertProviderPairingRequest({ provider: "signal", id: senderId, meta: { name: envelope.sourceName ?? undefined, }, }); - logVerbose( - `signal pairing request sender=${senderId} code=${code}`, - ); - try { - await sendMessageSignal( - senderId, - [ - "Clawdbot: access not configured.", - "", - `Pairing code: ${code}`, - "", - "Ask the bot owner to approve with:", - "clawdbot pairing approve --provider signal ", - ].join("\n"), - { baseUrl, account, maxBytes: mediaMaxBytes }, - ); - } catch (err) { - logVerbose( - `signal pairing reply failed for ${senderId}: ${String(err)}`, - ); + if (created) { + logVerbose(`signal pairing request sender=${senderId}`); + try { + await sendMessageSignal( + senderId, + [ + "Clawdbot: access not configured.", + "", + `Pairing code: ${code}`, + "", + "Ask the bot owner to approve with:", + "clawdbot pairing approve --provider signal ", + ].join("\n"), + { baseUrl, account, maxBytes: mediaMaxBytes }, + ); + } catch (err) { + logVerbose( + `signal pairing reply failed for ${senderId}: ${String(err)}`, + ); + } } } else { logVerbose( diff --git a/src/slack/monitor.tool-result.test.ts b/src/slack/monitor.tool-result.test.ts index 8042c3744..55ab49cf1 100644 --- a/src/slack/monitor.tool-result.test.ts +++ b/src/slack/monitor.tool-result.test.ts @@ -399,4 +399,43 @@ describe("monitorSlackProvider tool results", () => { "Pairing code: PAIRCODE", ); }); + + it("does not resend pairing code when a request is already pending", async () => { + config = { + ...config, + slack: { dm: { enabled: true, policy: "pairing", allowFrom: [] } }, + }; + upsertPairingRequestMock + .mockResolvedValueOnce({ code: "PAIRCODE", created: true }) + .mockResolvedValueOnce({ code: "PAIRCODE", created: false }); + + const controller = new AbortController(); + const run = monitorSlackProvider({ + botToken: "bot-token", + appToken: "app-token", + abortSignal: controller.signal, + }); + + await waitForEvent("message"); + const handler = getSlackHandlers()?.get("message"); + if (!handler) throw new Error("Slack message handler not registered"); + + const baseEvent = { + type: "message", + user: "U1", + text: "hello", + ts: "123", + channel: "C1", + channel_type: "im", + }; + + await handler({ event: baseEvent }); + await handler({ event: { ...baseEvent, ts: "124", text: "hello again" } }); + + await flush(); + controller.abort(); + await run; + + expect(sendMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/slack/monitor.ts b/src/slack/monitor.ts index 2f855fbb1..33581db13 100644 --- a/src/slack/monitor.ts +++ b/src/slack/monitor.ts @@ -653,31 +653,33 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { if (dmPolicy === "pairing") { const sender = await resolveUserName(message.user); const senderName = sender?.name ?? undefined; - const { code } = await upsertProviderPairingRequest({ + const { code, created } = await upsertProviderPairingRequest({ provider: "slack", id: message.user, meta: { name: senderName }, }); - logVerbose( - `slack pairing request sender=${message.user} name=${senderName ?? "unknown"} code=${code}`, - ); - try { - await sendMessageSlack( - message.channel, - [ - "Clawdbot: access not configured.", - "", - `Pairing code: ${code}`, - "", - "Ask the bot owner to approve with:", - "clawdbot pairing approve --provider slack ", - ].join("\n"), - { token: botToken, client: app.client }, - ); - } catch (err) { + if (created) { logVerbose( - `slack pairing reply failed for ${message.user}: ${String(err)}`, + `slack pairing request sender=${message.user} name=${senderName ?? "unknown"}`, ); + try { + await sendMessageSlack( + message.channel, + [ + "Clawdbot: access not configured.", + "", + `Pairing code: ${code}`, + "", + "Ask the bot owner to approve with:", + "clawdbot pairing approve --provider slack ", + ].join("\n"), + { token: botToken, client: app.client }, + ); + } catch (err) { + logVerbose( + `slack pairing reply failed for ${message.user}: ${String(err)}`, + ); + } } } else { logVerbose( @@ -1468,22 +1470,24 @@ export async function monitorSlackProvider(opts: MonitorSlackOpts = {}) { }); if (!permitted) { if (dmPolicy === "pairing") { - const { code } = await upsertProviderPairingRequest({ + const { code, created } = await upsertProviderPairingRequest({ provider: "slack", id: command.user_id, meta: { name: senderName }, }); - await respond({ - text: [ - "Clawdbot: access not configured.", - "", - `Pairing code: ${code}`, - "", - "Ask the bot owner to approve with:", - "clawdbot pairing approve --provider slack ", - ].join("\n"), - response_type: "ephemeral", - }); + if (created) { + await respond({ + text: [ + "Clawdbot: access not configured.", + "", + `Pairing code: ${code}`, + "", + "Ask the bot owner to approve with:", + "clawdbot pairing approve --provider slack ", + ].join("\n"), + response_type: "ephemeral", + }); + } } else { await respond({ text: "You are not authorized to use this command.", diff --git a/src/telegram/bot.test.ts b/src/telegram/bot.test.ts index 5663dc61e..18152a772 100644 --- a/src/telegram/bot.test.ts +++ b/src/telegram/bot.test.ts @@ -193,6 +193,47 @@ describe("createTelegramBot", () => { expect(String(sendMessageSpy.mock.calls[0]?.[1])).toContain("PAIRME12"); }); + it("does not resend pairing code when a request is already pending", async () => { + onSpy.mockReset(); + sendMessageSpy.mockReset(); + const replySpy = replyModule.__replySpy as unknown as ReturnType< + typeof vi.fn + >; + replySpy.mockReset(); + + loadConfig.mockReturnValue({ telegram: { dmPolicy: "pairing" } }); + readTelegramAllowFromStore.mockResolvedValue([]); + upsertTelegramPairingRequest + .mockResolvedValueOnce({ code: "PAIRME12", created: true }) + .mockResolvedValueOnce({ code: "PAIRME12", created: false }); + + createTelegramBot({ token: "tok" }); + const handler = onSpy.mock.calls[0][1] as ( + ctx: Record, + ) => Promise; + + const message = { + chat: { id: 1234, type: "private" }, + text: "hello", + date: 1736380800, + from: { id: 999, username: "random" }, + }; + + await handler({ + message, + me: { username: "clawdbot_bot" }, + getFile: async () => ({ download: async () => new Uint8Array() }), + }); + await handler({ + message: { ...message, text: "hello again" }, + me: { username: "clawdbot_bot" }, + getFile: async () => ({ download: async () => new Uint8Array() }), + }); + + expect(replySpy).not.toHaveBeenCalled(); + expect(sendMessageSpy).toHaveBeenCalledTimes(1); + }); + it("triggers typing cue via onReplyStart", async () => { onSpy.mockReset(); sendChatActionSpy.mockReset(); diff --git a/src/telegram/bot.ts b/src/telegram/bot.ts index ef6b9b279..e6f49faaf 100644 --- a/src/telegram/bot.ts +++ b/src/telegram/bot.ts @@ -247,33 +247,34 @@ export function createTelegramBot(opts: TelegramBotOptions) { username?: string; } | undefined; - const { code } = await upsertTelegramPairingRequest({ + const { code, created } = await upsertTelegramPairingRequest({ chatId: candidate, username: from?.username, firstName: from?.first_name, lastName: from?.last_name, }); - logger.info( - { - chatId: candidate, - username: from?.username, - firstName: from?.first_name, - lastName: from?.last_name, - code, - }, - "telegram pairing request", - ); - await bot.api.sendMessage( - chatId, - [ - "Clawdbot: access not configured.", - "", - `Pairing code: ${code}`, - "", - "Ask the bot owner to approve with:", - "clawdbot telegram pairing approve ", - ].join("\n"), - ); + if (created) { + logger.info( + { + chatId: candidate, + username: from?.username, + firstName: from?.first_name, + lastName: from?.last_name, + }, + "telegram pairing request", + ); + await bot.api.sendMessage( + chatId, + [ + "Clawdbot: access not configured.", + "", + `Pairing code: ${code}`, + "", + "Ask the bot owner to approve with:", + "clawdbot telegram pairing approve ", + ].join("\n"), + ); + } } catch (err) { logVerbose( `telegram pairing reply failed for chat ${chatId}: ${String(err)}`, diff --git a/src/web/inbound.ts b/src/web/inbound.ts index bb2229726..7c73fb7b1 100644 --- a/src/web/inbound.ts +++ b/src/web/inbound.ts @@ -258,31 +258,33 @@ export async function monitorWebInbox(options: { normalizedAllowFrom.includes(candidate)); if (!allowed) { if (dmPolicy === "pairing") { - const { code } = await upsertProviderPairingRequest({ + const { code, created } = await upsertProviderPairingRequest({ provider: "whatsapp", id: candidate, meta: { name: (msg.pushName ?? "").trim() || undefined, }, }); - logVerbose( - `whatsapp pairing request sender=${candidate} name=${msg.pushName ?? "unknown"} code=${code}`, - ); - try { - await sock.sendMessage(remoteJid, { - text: [ - "Clawdbot: access not configured.", - "", - `Pairing code: ${code}`, - "", - "Ask the bot owner to approve with:", - "clawdbot pairing approve --provider whatsapp ", - ].join("\n"), - }); - } catch (err) { + if (created) { logVerbose( - `whatsapp pairing reply failed for ${candidate}: ${String(err)}`, + `whatsapp pairing request sender=${candidate} name=${msg.pushName ?? "unknown"}`, ); + try { + await sock.sendMessage(remoteJid, { + text: [ + "Clawdbot: access not configured.", + "", + `Pairing code: ${code}`, + "", + "Ask the bot owner to approve with:", + "clawdbot pairing approve --provider whatsapp ", + ].join("\n"), + }); + } catch (err) { + logVerbose( + `whatsapp pairing reply failed for ${candidate}: ${String(err)}`, + ); + } } } else { logVerbose( diff --git a/src/web/monitor-inbox.test.ts b/src/web/monitor-inbox.test.ts index bd9946b4b..6abc8e6fa 100644 --- a/src/web/monitor-inbox.test.ts +++ b/src/web/monitor-inbox.test.ts @@ -1005,6 +1005,9 @@ describe("web monitor inbox", () => { it("locks down when no config is present (pairing for unknown senders)", async () => { // No config file => locked-down defaults apply (pairing for unknown senders) mockLoadConfig.mockReturnValue({}); + upsertPairingRequestMock + .mockResolvedValueOnce({ code: "PAIRCODE", created: true }) + .mockResolvedValueOnce({ code: "PAIRCODE", created: false }); const onMessage = vi.fn(); const listener = await monitorWebInbox({ verbose: false, onMessage }); @@ -1034,6 +1037,26 @@ describe("web monitor inbox", () => { text: expect.stringContaining("Pairing code: PAIRCODE"), }); + const upsertBlockedAgain = { + type: "notify", + messages: [ + { + key: { + id: "no-config-1b", + fromMe: false, + remoteJid: "999@s.whatsapp.net", + }, + message: { conversation: "ping again" }, + messageTimestamp: 1_700_000_002, + }, + ], + }; + + sock.ev.emit("messages.upsert", upsertBlockedAgain); + await new Promise((resolve) => setImmediate(resolve)); + expect(onMessage).not.toHaveBeenCalled(); + expect(sock.sendMessage).toHaveBeenCalledTimes(1); + // Message from self should be allowed const upsertSelf = { type: "notify",