From 654f63e8f83f1b9c3525ef2cde7aaf39b065bfbd Mon Sep 17 00:00:00 2001 From: Shawn <118158941+kevinWangSheng@users.noreply.github.com> Date: Mon, 2 Mar 2026 09:11:22 +0800 Subject: [PATCH] fix(signal): prevent sentTranscript sync messages from bypassing loop protection (#31093) * fix(signal): prevent sentTranscript sync messages from bypassing loop protection Issue: #31084 On daemon restart, sentTranscript sync messages could bypass loop protection because the syncMessage check happened before the sender validation. This reorganizes the checks to: 1. First resolve the sender (phone or UUID) 2. Check if the message is from our own account (both phone and UUID) 3. Only skip sync messages from other sources after confirming not own account This ensures that sync messages from the own account are properly filtered to prevent self-reply loops, while still allowing messages synced from other devices to be processed. Added optional accountUuid config field for UUID-based account identification. * fix(signal): cover UUID-only own-message loop protection * build: regenerate host env security policy swift --------- Co-authored-by: Kevin Wang Co-authored-by: Peter Steinberger --- src/config/types.signal.ts | 2 ++ src/signal/monitor.ts | 1 + .../event-handler.inbound-contract.test.ts | 29 +++++++++++++++++++ src/signal/monitor/event-handler.ts | 24 ++++++++++----- src/signal/monitor/event-handler.types.ts | 1 + 5 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/config/types.signal.ts b/src/config/types.signal.ts index cf45fa340..cc31ea869 100644 --- a/src/config/types.signal.ts +++ b/src/config/types.signal.ts @@ -6,6 +6,8 @@ export type SignalReactionLevel = "off" | "ack" | "minimal" | "extensive"; export type SignalAccountConfig = CommonChannelMessagingConfig & { /** Optional explicit E.164 account for signal-cli. */ account?: string; + /** Optional account UUID for signal-cli (used for loop protection). */ + accountUuid?: string; /** Optional full base URL for signal-cli HTTP daemon. */ httpUrl?: string; /** HTTP host for signal-cli daemon (default 127.0.0.1). */ diff --git a/src/signal/monitor.ts b/src/signal/monitor.ts index d874fea11..13812593c 100644 --- a/src/signal/monitor.ts +++ b/src/signal/monitor.ts @@ -423,6 +423,7 @@ export async function monitorSignalProvider(opts: MonitorSignalOpts = {}): Promi cfg, baseUrl, account, + accountUuid: accountInfo.config.accountUuid, accountId: accountInfo.accountId, blockStreaming: accountInfo.config.blockStreaming, historyLimit, diff --git a/src/signal/monitor/event-handler.inbound-contract.test.ts b/src/signal/monitor/event-handler.inbound-contract.test.ts index ecb5c270b..c47db1b57 100644 --- a/src/signal/monitor/event-handler.inbound-contract.test.ts +++ b/src/signal/monitor/event-handler.inbound-contract.test.ts @@ -172,4 +172,33 @@ describe("signal createSignalEventHandler inbound contract", () => { expect(capture.ctx).toBeTruthy(); expect(capture.ctx?.CommandAuthorized).toBe(false); }); + + it("drops own UUID inbound messages when only accountUuid is configured", async () => { + const ownUuid = "123e4567-e89b-12d3-a456-426614174000"; + const handler = createSignalEventHandler( + createBaseSignalEventHandlerDeps({ + cfg: { + messages: { inbound: { debounceMs: 0 } }, + channels: { signal: { dmPolicy: "open", allowFrom: ["*"], accountUuid: ownUuid } }, + }, + account: undefined, + accountUuid: ownUuid, + historyLimit: 0, + }), + ); + + await handler( + createSignalReceiveEvent({ + sourceNumber: null, + sourceUuid: ownUuid, + dataMessage: { + message: "self message", + attachments: [], + }, + }), + ); + + expect(capture.ctx).toBeUndefined(); + expect(dispatchInboundMessageMock).not.toHaveBeenCalled(); + }); }); diff --git a/src/signal/monitor/event-handler.ts b/src/signal/monitor/event-handler.ts index 583328f8a..a3e51323b 100644 --- a/src/signal/monitor/event-handler.ts +++ b/src/signal/monitor/event-handler.ts @@ -420,18 +420,28 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { if (!envelope) { return; } - if (envelope.syncMessage) { - return; - } + // Check for syncMessage (e.g., sentTranscript from other devices) + // We need to check if it's from our own account to prevent self-reply loops const sender = resolveSignalSender(envelope); if (!sender) { return; } - if (deps.account && sender.kind === "phone") { - if (sender.e164 === normalizeE164(deps.account)) { - return; - } + + // Check if the message is from our own account to prevent loop/self-reply + // This handles both phone number and UUID based identification + const normalizedAccount = deps.account ? normalizeE164(deps.account) : undefined; + const isOwnMessage = + (sender.kind === "phone" && normalizedAccount != null && sender.e164 === normalizedAccount) || + (sender.kind === "uuid" && deps.accountUuid != null && sender.raw === deps.accountUuid); + if (isOwnMessage) { + return; + } + + // For non-own sync messages (e.g., messages synced from other devices), + // we could process them but for now we skip to be conservative + if (envelope.syncMessage) { + return; } const dataMessage = envelope.dataMessage ?? envelope.editMessage?.dataMessage; diff --git a/src/signal/monitor/event-handler.types.ts b/src/signal/monitor/event-handler.types.ts index 480e7ad49..a7f3c6b1d 100644 --- a/src/signal/monitor/event-handler.types.ts +++ b/src/signal/monitor/event-handler.types.ts @@ -72,6 +72,7 @@ export type SignalEventHandlerDeps = { cfg: OpenClawConfig; baseUrl: string; account?: string; + accountUuid?: string; accountId: string; blockStreaming?: boolean; historyLimit: number;