From 4181e7297707dd9e8566ebf7ae4da3163a2832ac Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 11 Jan 2026 23:26:51 +0000 Subject: [PATCH] fix: strip markup heartbeat acks --- CHANGELOG.md | 1 + src/auto-reply/heartbeat.test.ts | 32 +++++++++++++++ src/auto-reply/heartbeat.ts | 42 ++++++++++++++++---- src/infra/heartbeat-runner.test.ts | 62 ++++++++++++++++++++++++++++++ 4 files changed, 130 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91563fbaf..cdcc1eda9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ - CLI/Gateway: clarify that `clawdbot gateway status` reports RPC health (connect + RPC) and shows RPC failures separately from connect failures. - CLI/Update: gate progress spinner on stdout TTY and align clean-check step label. (#701) — thanks @bjesuiter. - Telegram: add `/whoami` + `/id` commands to reveal sender id for allowlists; allow `@username` and prefixed ids in `allowFrom` prompts (with stability warning). +- Heartbeat: strip markup-wrapped `HEARTBEAT_OK` so acks don’t leak to external providers (e.g., Telegram). - Control UI: stop auto-writing `telegram.groups["*"]` and warn/confirm before enabling wildcard groups. - WhatsApp: send ack reactions only for handled messages and ignore legacy `messages.ackReaction` (doctor copies to `whatsapp.ackReaction`). (#629) — thanks @pasogott. - Sandbox/Skills: mirror skills into sandbox workspaces for read-only mounts so SKILL.md stays accessible. diff --git a/src/auto-reply/heartbeat.test.ts b/src/auto-reply/heartbeat.test.ts index 9cf47e276..60fa83521 100644 --- a/src/auto-reply/heartbeat.test.ts +++ b/src/auto-reply/heartbeat.test.ts @@ -90,4 +90,36 @@ describe("stripHeartbeatToken", () => { didStrip: false, }); }); + + it("strips HTML-wrapped heartbeat tokens", () => { + expect( + stripHeartbeatToken(`${HEARTBEAT_TOKEN}`, { mode: "heartbeat" }), + ).toEqual({ + shouldSkip: true, + text: "", + didStrip: true, + }); + }); + + it("strips markdown-wrapped heartbeat tokens", () => { + expect( + stripHeartbeatToken(`**${HEARTBEAT_TOKEN}**`, { mode: "heartbeat" }), + ).toEqual({ + shouldSkip: true, + text: "", + didStrip: true, + }); + }); + + it("removes markup-wrapped token and keeps trailing content", () => { + expect( + stripHeartbeatToken(`${HEARTBEAT_TOKEN} all good`, { + mode: "message", + }), + ).toEqual({ + shouldSkip: false, + text: "all good", + didStrip: true, + }); + }); }); diff --git a/src/auto-reply/heartbeat.ts b/src/auto-reply/heartbeat.ts index d76994df5..c5d02576d 100644 --- a/src/auto-reply/heartbeat.ts +++ b/src/auto-reply/heartbeat.ts @@ -52,30 +52,58 @@ export function stripHeartbeatToken( if (!trimmed) return { shouldSkip: true, text: "", didStrip: false }; const mode: StripHeartbeatMode = opts.mode ?? "message"; + const maxAckCharsRaw = opts.maxAckChars; + const parsedAckChars = + typeof maxAckCharsRaw === "string" + ? Number(maxAckCharsRaw) + : maxAckCharsRaw; const maxAckChars = Math.max( 0, - opts.maxAckChars ?? DEFAULT_HEARTBEAT_ACK_MAX_CHARS, + Number.isFinite(parsedAckChars) + ? parsedAckChars + : DEFAULT_HEARTBEAT_ACK_MAX_CHARS, ); - if (!trimmed.includes(HEARTBEAT_TOKEN)) { + // Normalize lightweight markup so HEARTBEAT_OK wrapped in HTML/Markdown + // (e.g., HEARTBEAT_OK or **HEARTBEAT_OK**) still strips. + const stripMarkup = (text: string) => + text + // Drop HTML tags. + .replace(/<[^>]*>/g, " ") + // Decode common nbsp variant. + .replace(/ /gi, " ") + // Remove markdown-ish wrappers at the edges. + .replace(/^[*`~_]+/, "") + .replace(/[*`~_]+$/, ""); + + const trimmedNormalized = stripMarkup(trimmed); + const hasToken = + trimmed.includes(HEARTBEAT_TOKEN) || + trimmedNormalized.includes(HEARTBEAT_TOKEN); + if (!hasToken) { return { shouldSkip: false, text: trimmed, didStrip: false }; } - const stripped = stripTokenAtEdges(trimmed); - if (!stripped.didStrip) { + const strippedOriginal = stripTokenAtEdges(trimmed); + const strippedNormalized = stripTokenAtEdges(trimmedNormalized); + const picked = + strippedOriginal.didStrip && strippedOriginal.text + ? strippedOriginal + : strippedNormalized; + if (!picked.didStrip) { return { shouldSkip: false, text: trimmed, didStrip: false }; } - if (!stripped.text) { + if (!picked.text) { return { shouldSkip: true, text: "", didStrip: true }; } + const rest = picked.text.trim(); if (mode === "heartbeat") { - const rest = stripped.text.trim(); if (rest.length <= maxAckChars) { return { shouldSkip: true, text: "", didStrip: true }; } } - return { shouldSkip: false, text: stripped.text, didStrip: true }; + return { shouldSkip: false, text: rest, didStrip: true }; } diff --git a/src/infra/heartbeat-runner.test.ts b/src/infra/heartbeat-runner.test.ts index a33d4a43d..33bd24159 100644 --- a/src/infra/heartbeat-runner.test.ts +++ b/src/infra/heartbeat-runner.test.ts @@ -2,6 +2,9 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it, vi } from "vitest"; + +// Avoid pulling optional runtime deps during isolated runs. +vi.mock("jiti", () => ({ createJiti: () => () => ({}) })); import { HEARTBEAT_PROMPT } from "../auto-reply/heartbeat.js"; import * as replyModule from "../auto-reply/reply.js"; import type { ClawdbotConfig } from "../config/config.js"; @@ -518,6 +521,65 @@ describe("runHeartbeatOnce", () => { } }); + it("skips delivery for markup-wrapped HEARTBEAT_OK", async () => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-hb-")); + const storePath = path.join(tmpDir, "sessions.json"); + const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); + try { + await fs.writeFile( + storePath, + JSON.stringify( + { + main: { + sessionId: "sid", + updatedAt: Date.now(), + lastProvider: "whatsapp", + lastTo: "+1555", + }, + }, + null, + 2, + ), + ); + + const cfg: ClawdbotConfig = { + agents: { + defaults: { + heartbeat: { + every: "5m", + target: "whatsapp", + to: "+1555", + }, + }, + }, + whatsapp: { allowFrom: ["*"] }, + session: { store: storePath }, + }; + + replySpy.mockResolvedValue({ text: "HEARTBEAT_OK" }); + const sendWhatsApp = vi.fn().mockResolvedValue({ + messageId: "m1", + toJid: "jid", + }); + + await runHeartbeatOnce({ + cfg, + deps: { + sendWhatsApp, + getQueueSize: () => 0, + nowMs: () => 0, + webAuthExists: async () => true, + hasActiveWebListener: () => true, + }, + }); + + expect(sendWhatsApp).not.toHaveBeenCalled(); + } finally { + replySpy.mockRestore(); + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }); + it("skips WhatsApp delivery when not linked or running", async () => { const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-hb-")); const storePath = path.join(tmpDir, "sessions.json");